mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
* [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on
@ 2026-09-14  8:56 Xianwei Zhao via B4 Relay
  2026-09-14  9:08 ` sashiko-bot
  2026-09-14 10:40 ` Martin Blumenstingl
  0 siblings, 2 replies; 6+ messages in thread
From: Xianwei Zhao via B4 Relay @ 2026-09-14  8:56 UTC (permalink / raw)
  To: Ulf Hansson, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: linux-pm, linux-arm-kernel, linux-amlogic, linux-kernel,
	Qingpeng Yang, Xianwei Zhao

From: Qingpeng Yang <qingpeng.yang@amlogic.com>

Mark the A9 ethernet power domain as always-on since it is required
for ethernet online wakeup and must not be powered down.

Signed-off-by: Qingpeng Yang <qingpeng.yang@amlogic.com>
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Keep Amlogic A9 ETH domain always on.
---
 drivers/pmdomain/amlogic/meson-secure-pwrc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pmdomain/amlogic/meson-secure-pwrc.c b/drivers/pmdomain/amlogic/meson-secure-pwrc.c
index 133549956efa..daf09cf86335 100644
--- a/drivers/pmdomain/amlogic/meson-secure-pwrc.c
+++ b/drivers/pmdomain/amlogic/meson-secure-pwrc.c
@@ -196,7 +196,8 @@ static const struct meson_secure_pwrc_domain_desc a9_pwrc_domains[] = {
 	SEC_PD(A9_VICP,		0),
 	SEC_PD(A9_SD_EMMC_A,	0),
 	SEC_PD(A9_SD_EMMC_B,	0),
-	SEC_PD(A9_ETH,		0),
+	/* ETH is for ethernet online wakeup, and should be always on */
+	SEC_PD(A9_ETH,		GENPD_FLAG_ALWAYS_ON),
 	SEC_PD(A9_PCIE_A,	0),
 	SEC_PD(A9_PCIE_B,	0),
 	SEC_PD(A9_NNA_4T,	0),

---
base-commit: 26b9ea4f00732908c4f87501483c20ecaaf4eca5
change-id: 20260914-a9-pwr-eth-6105f87f9f5e

Best regards,
-- 
Xianwei Zhao <xianwei.zhao@amlogic.com>



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on
  2026-09-14  8:56 [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on Xianwei Zhao via B4 Relay
@ 2026-09-14  9:08 ` sashiko-bot
  2026-09-14 10:40 ` Martin Blumenstingl
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-14  9:08 UTC (permalink / raw)
  To: Xianwei Zhao; +Cc: linux-amlogic, neil.armstrong

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The driver ignores the return value of `pm_genpd_init()`, which causes a critical Use-After-Free during system boot if initialization fails.
--

commit e3b7d61b7b7e8838767d2093bcc10638ec8bbd84
Author: Qingpeng Yang <qingpeng.yang@amlogic.com>

pmdomain: amlogic: Keep Amlogic A9 ETH domain always on
    
This commit marks the Amlogic A9 ethernet power domain as always-on.
The change is intended to ensure the domain remains powered to support
ethernet online wakeup functionality.

> diff --git a/drivers/pmdomain/amlogic/meson-secure-pwrc.c b/drivers/pmdomain/amlogic/meson-secure-pwrc.c
> index 133549956efa4..daf09cf86335b 100644
> --- a/drivers/pmdomain/amlogic/meson-secure-pwrc.c
> +++ b/drivers/pmdomain/amlogic/meson-secure-pwrc.c
> @@ -196,7 +196,8 @@ static const struct meson_secure_pwrc_domain_desc a9_pwrc_domains[] = {
>  	SEC_PD(A9_VICP,		0),
>  	SEC_PD(A9_SD_EMMC_A,	0),
>  	SEC_PD(A9_SD_EMMC_B,	0),
> -	SEC_PD(A9_ETH,		0),
> +	/* ETH is for ethernet online wakeup, and should be always on */
> +	SEC_PD(A9_ETH,		GENPD_FLAG_ALWAYS_ON),

[Severity: Critical]
This is a pre-existing issue, but does adding GENPD_FLAG_ALWAYS_ON here risk
a use-after-free during system boot if the ethernet domain fails to power on?

If the A9_ETH hardware state is off and the underlying firmware call fails to
power it on, pm_genpd_init() returns -EINVAL because it strictly enforces the
always-on constraint:

drivers/pmdomain/core.c:pm_genpd_init() {
    ...
    /* Always-on domains must be powered on at initialization. */
    if ((genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd)) &&
            !genpd_status_on(genpd)) {
        pr_err("always-on PM domain %s is not on\n", genpd->name);
        return -EINVAL;
    }
    ...
}

The probe function in the Amlogic secure power controller driver currently
ignores the return value of pm_genpd_init(), leaving the failed domain
uninitialized and out of the global gpd_list:

drivers/pmdomain/amlogic/meson-secure-pwrc.c:meson_secure_pwrc_probe() {
    ...
    for (i = 0; i < match->count; i++) {
        ...
        pm_genpd_init(&dom->base, NULL, match->domains[i].is_off(dom));
        pwrc->xlate.domains[i] = &dom->base;
    }
    ...
}

When the probe function later calls of_genpd_add_provider_onecell(), it
detects that the failed domain is missing from gpd_list and aborts provider
registration.

The probe failure then triggers devres cleanup, which frees the devm_kcalloc
allocated pwrc->domains array. However, the previously successfully initialized
domains (such as A9_DSPA) are never removed from the global gpd_list. 

Later, when genpd_power_off_unused() iterates over gpd_list during the late
initcalls, won't it dereference the freed memory from the pwrc->domains array?

>  	SEC_PD(A9_PCIE_A,	0),
>  	SEC_PD(A9_PCIE_B,	0),
>  	SEC_PD(A9_NNA_4T,	0),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-a9-pwr-eth-v1-1-752005131135@amlogic.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on
  2026-09-14  8:56 [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on Xianwei Zhao via B4 Relay
  2026-09-14  9:08 ` sashiko-bot
@ 2026-09-14 10:40 ` Martin Blumenstingl
  2026-09-15 15:47   ` Ulf Hansson
  2026-09-22  8:44   ` Qingpeng Yang
  1 sibling, 2 replies; 6+ messages in thread
From: Martin Blumenstingl @ 2026-09-14 10:40 UTC (permalink / raw)
  To: xianwei.zhao
  Cc: Ulf Hansson, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	linux-pm, linux-arm-kernel, linux-amlogic, linux-kernel,
	Qingpeng Yang

On Mon, Sep 14, 2026 at 10:56 AM Xianwei Zhao via B4 Relay
<devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
>
> From: Qingpeng Yang <qingpeng.yang@amlogic.com>
>
> Mark the A9 ethernet power domain as always-on since it is required
> for ethernet online wakeup and must not be powered down.
Is "online wakeup" a wake up from suspend?
If so: is the Ethernet controller listed with the "wakeup-source" flag
in .dts{,i}?
I'm wondering if the pmdomain and/or power (wakeup) frameworks in
Linux have other ways of managing this case (e.g. only keeping the
power domain active if needed).


Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on
  2026-09-14 10:40 ` Martin Blumenstingl
@ 2026-09-15 15:47   ` Ulf Hansson
  2026-09-22  9:04     ` Qingpeng Yang
  2026-09-22  8:44   ` Qingpeng Yang
  1 sibling, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2026-09-15 15:47 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: xianwei.zhao, Ulf Hansson, Neil Armstrong, Kevin Hilman,
	Jerome Brunet, linux-pm, linux-arm-kernel, linux-amlogic,
	linux-kernel, Qingpeng Yang

On Mon, Sep 14, 2026 at 12:40 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> On Mon, Sep 14, 2026 at 10:56 AM Xianwei Zhao via B4 Relay
> <devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
> >
> > From: Qingpeng Yang <qingpeng.yang@amlogic.com>
> >
> > Mark the A9 ethernet power domain as always-on since it is required
> > for ethernet online wakeup and must not be powered down.
> Is "online wakeup" a wake up from suspend?
> If so: is the Ethernet controller listed with the "wakeup-source" flag
> in .dts{,i}?
> I'm wondering if the pmdomain and/or power (wakeup) frameworks in
> Linux have other ways of managing this case (e.g. only keeping the
> power domain active if needed).

We certainly do.

For in-band wakeup we have GENPD_FLAG_ACTIVE_WAKEUP. If this is set
for the genpd in question and the device being suspended is configured
for system wakeup (device_awake_path() returns true), the genpd will
remain powered on during system suspend, see genpd_finish_suspend().

In genpd_finish_suspend() we also support out-band wakeups via
checking device_out_band_wakeup(). Although, I guess that's not the
case here.

Kind regards
Uffe

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on
  2026-09-14 10:40 ` Martin Blumenstingl
  2026-09-15 15:47   ` Ulf Hansson
@ 2026-09-22  8:44   ` Qingpeng Yang
  1 sibling, 0 replies; 6+ messages in thread
From: Qingpeng Yang @ 2026-09-22  8:44 UTC (permalink / raw)
  To: Martin Blumenstingl, xianwei.zhao
  Cc: Ulf Hansson, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	linux-pm, linux-arm-kernel, linux-amlogic, linux-kernel

Hi Martin,
     Thanks for your review.

On 9/14/2026 6:40 PM, Martin Blumenstingl wrote:
> 
> On Mon, Sep 14, 2026 at 10:56 AM Xianwei Zhao via B4 Relay
> <devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
>>
>> From: Qingpeng Yang <qingpeng.yang@amlogic.com>
>>
>> Mark the A9 ethernet power domain as always-on since it is required
>> for ethernet online wakeup and must not be powered down.
> Is "online wakeup" a wake up from suspend?
> If so: is the Ethernet controller listed with the "wakeup-source" flag
> in .dts{,i}?
> I'm wondering if the pmdomain and/or power (wakeup) frameworks in
> Linux have other ways of managing this case (e.g. only keeping the
> power domain active if needed).
> 

Yes, "online wakeup" here means wakeup from system suspend.
The Ethernet controller is not currently listed with the "wakeup-source"
flag in the .dtsi.

After internal discussion, we have decided not to support Ethernet
wakeup for now. So we will drop the always-on change for the Ethernet
power domain in this series.

> 
> Best regards,
> Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on
  2026-09-15 15:47   ` Ulf Hansson
@ 2026-09-22  9:04     ` Qingpeng Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Qingpeng Yang @ 2026-09-22  9:04 UTC (permalink / raw)
  To: Ulf Hansson, Martin Blumenstingl
  Cc: xianwei.zhao, Ulf Hansson, Neil Armstrong, Kevin Hilman,
	Jerome Brunet, linux-pm, linux-arm-kernel, linux-amlogic,
	linux-kernel

Hi Uffe,
     Thanks for your review.

On 9/15/2026 11:47 PM, Ulf Hansson wrote:
> 
> On Mon, Sep 14, 2026 at 12:40 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
>>
>> On Mon, Sep 14, 2026 at 10:56 AM Xianwei Zhao via B4 Relay
>> <devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
>>>
>>> From: Qingpeng Yang <qingpeng.yang@amlogic.com>
>>>
>>> Mark the A9 ethernet power domain as always-on since it is required
>>> for ethernet online wakeup and must not be powered down.
>> Is "online wakeup" a wake up from suspend?
>> If so: is the Ethernet controller listed with the "wakeup-source" flag
>> in .dts{,i}?
>> I'm wondering if the pmdomain and/or power (wakeup) frameworks in
>> Linux have other ways of managing this case (e.g. only keeping the
>> power domain active if needed).
> 
> We certainly do.
> 
> For in-band wakeup we have GENPD_FLAG_ACTIVE_WAKEUP. If this is set
> for the genpd in question and the device being suspended is configured
> for system wakeup (device_awake_path() returns true), the genpd will
> remain powered on during system suspend, see genpd_finish_suspend().
> 
> In genpd_finish_suspend() we also support out-band wakeups via
> checking device_out_band_wakeup(). Although, I guess that's not the
> case here.

Thanks for pointing out GENPD_FLAG_ACTIVE_WAKEUP and the
genpd_finish_suspend() logic. That is very helpful.

When we do need Ethernet wakeup support later, we will revisit this and
follow the approach you described, including the proper use of
GENPD_FLAG_ACTIVE_WAKEUP and the wakeup-source configuration.

> 
> Kind regards
> Uffe


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-22  9:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-14  8:56 [PATCH] pmdomain: amlogic: Keep Amlogic A9 ETH domain always on Xianwei Zhao via B4 Relay
2026-09-14  9:08 ` sashiko-bot
2026-09-14 10:40 ` Martin Blumenstingl
2026-09-15 15:47   ` Ulf Hansson
2026-09-22  9:04     ` Qingpeng Yang
2026-09-22  8:44   ` Qingpeng Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®