Re: [PATCH 4/4] x86/platform/atom: Check state of Punit managed devices on s2idle

[Hans de Goede <hdegoede@redhat.com>]

Hi Andy,

On 1/2/24 01:07, Andy Shevchenko wrote:
> On Sun, Dec 31, 2023 at 6:33 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> From: Johannes Stezenbach <js@sig21.net>
>>
>> This is a port of "pm: Add pm suspend debug notifier for North IPs"
>> from the latte-l-oss branch of:
>> from https://github.com/MiCode/Xiaomi_Kernel_OpenSource latte-l-oss
>>
>> With the new acpi_s2idle_dev_ops and acpi_register_lps0_dev()
>> functionality this can now finally be ported to the mainline kernel
>> without requiring adding non-upstreamable hooks into the cpu_idle
>> driver mechanism.
>>
>> This adds a check that all hardware blocks in the North complex
>> (controlled by Punit) are in a state that allows the SoC to enter S0i3
>> and prints an error message for any device in D0.
> 
> ...
> 
>>  static void punit_dbgfs_register(struct punit_device *punit_device)
>>  {
>> +       punit_dev = punit_device;
> 
> This is not the correct (semantically) place for this.
> 
> Instead, optionally introduce a local variable in the
> punit_atom_debug_init() and assign the global one there. Also it seems
> that you may move this global variable under ifdeffery (and hence its
> assignment) and have less stale bytes in the object file. (With this
> said, it seems that local variables are plausible to have.)

Thank you for the reviews. I agree with all your review remarks
and I'll submit a v2 series addressing all of them soon.

Regards,

Hans