Re: [PATCH AUTOSEL 6.17] pmdomain: thead: create auxiliary device for rebooting

[Icenowy Zheng <uwu@icenowy.me>]

在 2025-10-09星期四的 11:55 -0400，Sasha Levin写道：
> From: Icenowy Zheng <uwu@icenowy.me>
> 
> [ Upstream commit 64581f41f4c4aa1845edeee6bb0c8f2a7103d9aa ]

This commit seems to be not backportable -- it depends on the new
driver for the auxiliary device.

I suggest not to backport it.

> 
> The reboot / power off operations require communication with the AON
> firmware too.
> 
> As the driver is already present, create an auxiliary device with
> name
> "reboot" to match that driver, and pass the AON channel by using
> platform_data.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> 
> LLM Generated explanations, may be completely bogus:
> 
> YES
> 
> - User-visible bugfix: Without this, TH1520 systems cannot register
>   platform reboot/poweroff handlers via the AON firmware, so
>   reboot/poweroff won’t work even though the AON protocol and the
> reboot
>   driver exist. This commit wires them up by instantiating the
> auxiliary
>   device that the reboot driver matches on.
> - Small and contained: Adds a tiny helper and one call in the TH1520
> PM
>   domain driver; no architectural changes.
> - Specific code changes
>   - Adds `th1520_pd_reboot_init()` which creates an auxiliary device
>     named `reboot` and passes the AON channel through `platform_data`
> so
>     the reboot driver can use it: drivers/pmdomain/thead/th1520-pm-
>     domains.c:176
>     - `adev = devm_auxiliary_device_create(dev, "reboot", aon_chan);`
>   - Hooks it into probe after setting up PM domains and the optional
> GPU
>     pwrseq: drivers/pmdomain/thead/th1520-pm-domains.c:250
>     - `ret = th1520_pd_reboot_init(dev, aon_chan);`
>   - Error path is correctly routed back to the provider cleanup.
> - Correct driver pairing: The created device name matches the
> existing
>   driver’s ID table
>   - Reboot driver expects `th1520_pm_domains.reboot` and consumes
>     `adev->dev.platform_data` as the `th1520_aon_chan` to issue AON
> RPCs
>     for poweroff/restart: drivers/power/reset/th1520-aon-reboot.c:51
> and
>     drivers/power/reset/th1520-aon-reboot.c:82
> - Risk and scope:
>   - TH1520-specific; no impact on other platforms.
>   - Only instantiates an auxiliary device; safe if the reboot driver
>     isn’t present.
>   - No ABI/uAPI changes.
> - Important follow-up fix to include: The original change used
>   `PTR_ERR_OR_ZERO(adev)` with `devm_auxiliary_device_create()`,
> which
>   returns NULL on failure (not an error pointer). That was fixed by
>   “pmdomain: thead: Fix error pointer vs NULL bug in
>   th1520_pd_reboot_init()” (bbc3110823eca), which changes the return
> to
>   `-ENODEV` on NULL and returns 0 otherwise:
>   drivers/pmdomain/thead/th1520-pm-domains.c:181. For stable
>   backporting, include this fix alongside the main commit to avoid
>   silently succeeding when the aux device creation fails.
> - Stable policy fit:
>   - Fixes a real functionality gap (reboot/poweroff) for TH1520
> users.
>   - Minimal code, clear intent, and contained to the TH1520 PM domain
>     driver.
>   - No feature creep or architectural refactoring.
> 
> Recommendation: Backport this commit together with the follow-up fix
> bbc3110823eca to ensure correct error handling.
> 
>  drivers/pmdomain/thead/th1520-pm-domains.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c
> b/drivers/pmdomain/thead/th1520-pm-domains.c
> index 9040b698e7f7f..5213994101a59 100644
> --- a/drivers/pmdomain/thead/th1520-pm-domains.c
> +++ b/drivers/pmdomain/thead/th1520-pm-domains.c
> @@ -173,6 +173,16 @@ static int th1520_pd_pwrseq_gpu_init(struct
> device *dev)
>                                         adev);
>  }
>  
> +static int th1520_pd_reboot_init(struct device *dev,
> +                                struct th1520_aon_chan *aon_chan)
> +{
> +       struct auxiliary_device *adev;
> +
> +       adev = devm_auxiliary_device_create(dev, "reboot", aon_chan);
> +
> +       return PTR_ERR_OR_ZERO(adev);
> +}
> +
>  static int th1520_pd_probe(struct platform_device *pdev)
>  {
>         struct generic_pm_domain **domains;
> @@ -235,6 +245,10 @@ static int th1520_pd_probe(struct
> platform_device *pdev)
>         if (ret)
>                 goto err_clean_provider;
>  
> +       ret = th1520_pd_reboot_init(dev, aon_chan);
> +       if (ret)
> +               goto err_clean_provider;
> +
>         return 0;
>  
>  err_clean_provider: