Re: Request for testing an alternate branch

On Wed, Dec 11, 2013 at 1:26 PM, John Baldwin <jhb_at_freebsd.org> wrote:
> On Thursday, December 05, 2013 1:21:13 am Justin Hibbits wrote:
>> I've been working on the projects/pmac_pmu branch for some time now to
>> add suspend/resume as well as CPU speed change for certain PowerPC
>> machines, about a year since I created the branch, and now it's stable
>> enough that I want to merge it into HEAD, hence this request. However,
>> it does touch several drivers, turning them into "early drivers", such
>> that they can be initialized, and suspended and resumed at a different
>> time.  Saying that, I do need testing from other architectures, to make
>> sure I haven't broken anything.
>>
>> The technical details:
>>
>> To get proper ordering, I've extended the bus_generic_suspend() and
>> bus_generic_resume() to do multiple passes.  Devices which cannot be
>> enabled or disabled at the current pass level would return an EAGAIN.
>> This could possibly cause problems, since it's an addition to an
>> existing API rather than a new API to run along side it, so it needs a
>> great deal of testing.  It works fine on PowerPC, but I don't have any
>> i386/amd64 or sparc64 hardware to test it on, so would like others who
>> do to test it.  I don't think that it would impact x86 at all (testing
>> is obviously required), because the nexus is not an EARLY_DRIVER_MODULE,
>> so all devices would be handled at the same pass.  But, I do know the
>> sparc64 has an EARLY_DRIVER_MODULE() nexus, so that will likely be
>> impacted.
>
> I have patches to change many x86 drivers to use EARLY_DRIVER_MODULE()
> FWIW.
>
> Also, I'm still not a fan of the EAGAIN approach.  I'd rather have a method
> in bus_if.m to suspend or resume a single device and to track that a device
> is suspended or resumed via a device_t flag or some such. (I think I had
> suggested this previously as it would also allow us to have a tool to
> suspend/resume individual drivers at runtime apart from a full suspend/resume
> request).
>
> --
> John Baldwin

Understood.  You had mentioned something along those lines before.  Is
it safe/sane to partially merge a branch into HEAD?  If so, I can
merge only the changes necessary for PMU cpufreq, and work on the
suspend/resume separately.  I put them together because most of the
low level code involved is the same between them.

I do like your idea of a device_t flag, but I'm not sure what the best
way to go with that would be.  I do already use a device_t flag to
determine if the device is suspended, but only bus_generic_* access
that in this patch.

Would a better way be:
* root_suspend instead of suspending its children, instead traverses
and suspends each descendent in reverse order.
 * While doing this, insert each device upon successful suspend into a list.
* For root_resume(), traverse the list back, and suspend each device
in the reverse order.

With this, add a new method, called device_suspend_child() to suspend
a specific child if it hasn't already been suspended.
 * This could require modifying the PCI driver to move the device
child suspend/resume into those functions, instead of doing that logic
in the device_suspend/device_resume itself.

I guess, in short, I'm asking:  Is it fine if I merge only the code
necessary for this cpufreq?  That would require making other changes
to this before merging in, to isolate that code, but it's very doable.

- Justin