On Wed, Jun 22, 2016 at 5:20 PM, Prakash, Prashanth <pprakash@xxxxxxxxxxxxxx> wrote: > Hi Rafael, > > On 6/21/2016 5:50 PM, Rafael J. Wysocki wrote: >> On Wednesday, June 15, 2016 02:05:05 PM Prakash, Prashanth wrote: >>> Hi Rafael, >>> >>> Any inputs on this patch? >> Does it actually work? > Yes, it works :) >> It looks like sybus_evaluate_ost() schedules itself with a delay in an >> endless loop and the poweroff will happen anyway without waiting. >> >> I guess the idea is that acpi_sybus_notify() will schedule a delayed work >> doing the sybus_evaluate_ost() and that will do the orderly_poweroff() thing, >> but that's not what the patch is doing. > > The specification requires us to start graceful shutdown and then keep evaluating _OST > method every 10 seconds with a status code of 0x81 to indicate that shutdown is in > progress. So add a comment like this, so people don't have to wonder. Also I'm not sure if the delayed_work_pending() really does what you expect it to do. And why to you need a delayed work instead of just a work that will do "evaluate _OST; wait" in an endless loop? > Since we need not trigger a graceful shutdown every time we evaluate _OST method. I > am calling orderly_poweroff only when we get the initial request via Notify. That's OK. >> That "sybus" naming looks sort of lame BTW. What's wrong with using "sb" >> instead? > Sure, I will change the naming to use sb instead of sybus. >> >> Besides actypes.h is an ACPICA file and patches updating it have to go via >> upstream ACPICA. > I can split this into 2 patches, so that they can be merged independently. Would that work? I guess so, unless the ACPICA maintainers have objections to the ACPICA one. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html