Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] mm/arm: pgtable: remove young bit check for pte_valid_user
From: Brian Ruley @ 2026-04-10 11:01 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Will Deacon, Steve Capper, linux-arm-kernel, linux-kernel
In-Reply-To: <adfNN33QIOP3VfDm@shell.armlinux.org.uk>

On Apr 09, Russell King (Oracle) wrote:
> 
> On Thu, Apr 09, 2026 at 06:17:36PM +0300, Brian Ruley wrote:
> > However, in the case I describe, if VA_B is mapped immediately to pfn_q
> > after it been has unmapped and freed for VA_A, then it's quite possible
> > that the page is still indexed in the cache.
> 
> True.
> 
> > The hypothesis is that if
> > VA_A and VA_B land in the same I-cache set and VA_A old cache entry
> > still exists (tagged with pfn_q), then the CPU can fetch stale
> > instructions because the tag will match. That's one reason why we need
> > to invalidate the cache, but that will be skipped in the path:
> >
> >     migrate_pages
> >      migrate_pages_batch
> >       migrate_folio_move
> >        remove_migration_ptes
> >         remove_migration_pte
> >          set_pte_at
> >           set_ptes
> >            __sync_icache_dcache  (skipped if !young)
> >             set_pte_ext
> 
> In this case, if the old PTE was marked !young, then the new PTE will
> have:
>         pte = pte_mkold(pte);
> 
> on it, which marks it !young. As you say, __sync_icache_dcache() will
> be skipped. While a PTE entry will be set for the kernel, the code in
> set_pte_ext() will *not* establish a hardware PTE entry. For the
> 2-level pte code:
> 
>         tst     r1, #L_PTE_YOUNG        @ <- results in Z being set
>         tstne   r1, #L_PTE_VALID        @ <- not executed
>         eorne   r1, r1, #L_PTE_NONE     @ <- not executed
>         tstne   r1, #L_PTE_NONE         @ <- not executed
>         moveq   r3, #0                  @ <- hardware PTE value
>  ARM(   str     r3, [r0, #2048]! )      @ <- writes hardware PTE
> 
> So, for a !young PTE, the hardware PTE entry is written as zero,
> which means accesses should fault, which will then cause the PTE to
> be marked young.
> 
> For the 3-level case, the L_PTE_YOUNG bit corresponds with the AF bit
> in the PTE, and there aren't split Linux / hardware PTE entries. AF
> being clear should result in a page fault being generated for the
> kernel to handle making the PTE young.
> 
> In both of these cases, set_ptes() will need to be called with the
> updated PTE which will now be marked young, and that will result in
> the I-cache being flushed.

Hi Russell,

Thank you for the clarification, this is very educational for me.
I understand your scepticism, and I can't explain what's going on based
on what you replied. However, I do honestly believe there is a problem
here. I'll share the exact testing details and the instrumentation
we added that convinced us to reach out at the end. One idea we also
had was that could cache aliasing be happening here.

To clarify any potential misunderstanding, we've observed the
following:

- Sporadic SIGILL and SIGSEGV under memory pressure
- Scales with core count, i.e., quad core more likely to reproduce
  than dual core. We haven't observed an issue on single core.
- Coredumps show valid instructions at the faulting PC.
  The CPU executed something different from what's in memory.
  This pointed us to stale I-cache.
- Instrumentation indicates a correlation.
  A per-CPU ring buffer tracking exec page migrations was dumped on
  SIGILL. The faulting PC matched a recently migrated pages.
- We started seeing this after upgrade 6.1->6.12->6.18. We bisected
  two commits which had an impact, but we weren't convinced that
  either was the root cause: 5dfab109d5193e6c224d96cabf90e9cc2c039884
  and 6faea3422e3b4e8de44a55aa3e6e843320da66d2.
- Failed processes include systemd, tar, bash, ...
- Debug options, e.g., page poisoning, seems to hide the bug


> So you're saying that stress-ng doesn't reproduce this bug but
triggers the OOM-killer... confused.

Apologies for the confusion. I meant that with `stress-ng' we created
the memory pressure and OOM might have played a role in exposing the
"bug" as we (at the time) believed that anything that would trigger
memory free/reclaims and page migration was the key. One note I'll add
is that in our test we invoked stress-ng for 2 minutes (--timeout 2m)
and after each we would reboot the device. We had observed that reboots
seemed to have a discernible effect on the occurence in earlier testing
so we kept that in. I'm beginning to doubt if it had an effect now,
and unfortunately it's all anecdotal.

One more thing, even if you don't accept the patch, is this patch
harmful in any way or is it just sub-optimal?

I'll send the instrumentation patch as a follow-up, migh be there's a
flaw in it.

Best regards,
Brian

###TESTING###

1. stress-ng --vm 4 --vm-bytes 2G --vm-method zero-one --verify \
             --timeout 2m
2. reboot
3. repeat

Cleaned up logs of instrumentation:
```
kernel: [  104.610248] SIGILL at b6e6f1c0 pid 896, recent exec migrations:
kernel: [  104.610313]   cpu0: addr=b6e6f000 old_pfn=467d3 new_pfn=577fe pid=34 flushed=0
[...]
kernel: [  456.066661] SIGILL at b6d99f40 pid 455, recent exec migrations:
[...]
kernel: [  456.066963]   cpu0: addr=b6d99000 old_pfn=44270 new_pfn=7c9ea pid=34 flushed=0
[...] 
```


^ permalink raw reply

* Re: [PATCH v1 1/7] dt-bindings: arm: fsl: Add verdin imx8m[mp] and imx95 zinnia board
From: Krzysztof Kozlowski @ 2026-04-10 10:56 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frank Li,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Shawn Guo,
	Francesco Dolcini, devicetree, linux-kernel, imx,
	linux-arm-kernel
In-Reply-To: <20260409095855.61252-2-francesco@dolcini.it>

On Thu, Apr 09, 2026 at 11:58:47AM +0200, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Add Toradex Verdin Zinnia carrier board mated with Verdin
> iMX8M Plus, Verdin iMX8M Mini and Verdin iMX95.
> 
> Link: https://www.toradex.com/products/carrier-board/zinnia-carrier-board
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof



^ permalink raw reply

* Re: [PATCH 2/3] pwm: rp1: Add RP1 PWM controller driver
From: Uwe Kleine-König @ 2026-04-10 10:47 UTC (permalink / raw)
  To: Andrea della Porta
  Cc: linux-pwm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Broadcom internal kernel review list,
	devicetree, linux-rpi-kernel, linux-arm-kernel, linux-kernel,
	Naushir Patuck, Stanimir Varbanov
In-Reply-To: <adjQl37-6a--_y3Y@apocalypse>

[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]

Hello Andrea,

On Fri, Apr 10, 2026 at 12:27:35PM +0200, Andrea della Porta wrote:
> On 08:27 Fri 10 Apr     , Uwe Kleine-König wrote:
> > On Thu, Apr 09, 2026 at 06:16:41PM +0200, Andrea della Porta wrote:
> > > On 23:45 Sun 05 Apr     , Uwe Kleine-König wrote:
> > > > On Fri, Apr 03, 2026 at 04:31:55PM +0200, Andrea della Porta wrote:
> > > > > +static void rp1_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> > > > > +{
> > > > > +	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > > > > +	u32 value;
> > > > > +
> > > > > +	value = readl(rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> > > > > +	value &= ~PWM_MODE_MASK;
> > > > > +	writel(value, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> > > > > +
> > > > > +	rp1_pwm_apply_config(chip, pwm);
> > > > 
> > > > What is the purpose of this call?
> > > 
> > > To update the configuration on the next PWM strobe in order to avoid
> > > glitches. I'll add a short comment in the code.
> > 
> > .pwm_free() should not touch the hardware configuration. Changing the
> > pinmuxing (which I guess is the purpose of clearing PWM_MODE_MASK) is
> > somewhat a grey area. If that saves energy, that's okish. Otherwise
> > not interfering with the operation of the PWM (e.g. to keep a display on
> > during kexec or so) is preferred.
> 
> Sorry I should've been more clear on this. The pinmux/conf is not changed
> at all by this mask, only the PWM output mode is. The controller can output
> several type of waveforms and clearing PWM_MODE_MASK is just setting the
> controller to output a 0, which is the reset default i.e. the same value
> as just before exporting the channel.
> I guess this is the expected behaviour in case of a fan, it should stop
> spinning in case you unexport the pwm channel, but I see it could be
> different with displays.
> Honestly I don't have a strong opinion about that, please just let me
> know if I should drop that pwm_free entirely.

Yes, in this case drop the function completely. It's the responsibility
of the consumer to stop the PWM before releasing it.

> > > > > +static int rp1_pwm_resume(struct device *dev)
> > > > > +{
> > > > > +	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> > > > > +
> > > > > +	return clk_prepare_enable(rp1->clk);
> > > > 
> > > > Hmm, if this fails and then the driver is unbound, the clk operations
> > > > are not balanced.
> > > 
> > > I'll add some flags to check if the clock is really enabled or not.
> > 
> > To be honest, I guess that is a problem of several drivers, not only in
> > drivers/pwm. If this complicates the driver, I guess addressing this
> > isn't very critical.
> 
> I'll come up with something, we can always drop this check if deemed
> too 'noisy'. 

Great, thanks
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH v2 8/9] pmdomain: renesas: rcar-sysc: Drop GENPD_FLAG_NO_STAY_ON
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Due to the new fine grained sync_state support for onecell genpd provider
drivers, we should no longer need use the legacy behaviour. Therefore,
let's drop GENPD_FLAG_NO_STAY_ON.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/renesas/rcar-sysc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pmdomain/renesas/rcar-sysc.c b/drivers/pmdomain/renesas/rcar-sysc.c
index bd7bb9cbd9da..e4608c657629 100644
--- a/drivers/pmdomain/renesas/rcar-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-sysc.c
@@ -241,7 +241,6 @@ static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd)
 		}
 	}
 
-	genpd->flags |= GENPD_FLAG_NO_STAY_ON;
 	genpd->power_off = rcar_sysc_pd_power_off;
 	genpd->power_on = rcar_sysc_pd_power_on;
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 6/9] pmdomain: core: Export a common function for ->queue_sync_state()
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Along with of_genpd_sync_state() that genpd provider drivers may use to
manage sync_state, let's add and export of_genpd_queue_sync_state() for
those that may need it. It's expected that the genpd provider driver
assigns it's own ->queue_sync_state() callback and invoke the new helper
from there.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/core.c   | 14 +++++++++-----
 include/linux/pm_domain.h |  2 ++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index f11dc2110737..49e60cb67b3e 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2764,7 +2764,7 @@ static void genpd_parse_for_consumer(struct device_node *sup,
 	}
 }
 
-static void _genpd_queue_sync_state(struct device_node *np)
+static void genpd_queue_sync_state(struct device_node *np)
 {
 	struct generic_pm_domain *genpd;
 
@@ -2782,11 +2782,14 @@ static void _genpd_queue_sync_state(struct device_node *np)
 	mutex_unlock(&gpd_list_lock);
 }
 
-static void genpd_queue_sync_state(struct device *dev)
+void of_genpd_queue_sync_state(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
 	struct device_link *link;
 
+	if (!np)
+		return;
+
 	if (!genpd_should_wait_for_consumer(np))
 		return;
 
@@ -2810,8 +2813,9 @@ static void genpd_queue_sync_state(struct device *dev)
 		genpd_parse_for_consumer(np, consumer->of_node);
 	}
 
-	_genpd_queue_sync_state(np);
+	genpd_queue_sync_state(np);
 }
+EXPORT_SYMBOL_GPL(of_genpd_queue_sync_state);
 
 static void genpd_sync_state(struct device *dev)
 {
@@ -2922,7 +2926,7 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 		sync_state = true;
 	} else if (!dev_has_sync_state(dev)) {
 		dev_set_drv_sync_state(dev, genpd_sync_state);
-		dev_set_drv_queue_sync_state(dev, genpd_queue_sync_state);
+		dev_set_drv_queue_sync_state(dev, of_genpd_queue_sync_state);
 	}
 
 	put_device(dev);
@@ -3654,7 +3658,7 @@ static void genpd_provider_queue_sync_state(struct device *dev)
 	if (genpd->sync_state != GENPD_SYNC_STATE_ONECELL)
 		return;
 
-	genpd_queue_sync_state(dev);
+	of_genpd_queue_sync_state(dev);
 }
 
 static void genpd_provider_sync_state(struct device *dev)
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 7aa49721cde5..d428dd805c46 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -467,6 +467,7 @@ int of_genpd_remove_subdomain(const struct of_phandle_args *parent_spec,
 struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
 int of_genpd_parse_idle_states(struct device_node *dn,
 			       struct genpd_power_state **states, int *n);
+void of_genpd_queue_sync_state(struct device *dev);
 void of_genpd_sync_state(struct device_node *np);
 
 int genpd_dev_pm_attach(struct device *dev);
@@ -513,6 +514,7 @@ static inline int of_genpd_parse_idle_states(struct device_node *dn,
 	return -ENODEV;
 }
 
+static inline void of_genpd_queue_sync_state(struct device *dev) {}
 static inline void of_genpd_sync_state(struct device_node *np) {}
 
 static inline int genpd_dev_pm_attach(struct device *dev)
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 9/9] pmdomain: renesas: rmobile-sysc: Drop GENPD_FLAG_NO_STAY_ON
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Rmobile-sysc is not a onecell provider and didn't really needed
the GENPD_FLAG_NO_STAY_ON flag in the first place. Let's drop it.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/renesas/rmobile-sysc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pmdomain/renesas/rmobile-sysc.c b/drivers/pmdomain/renesas/rmobile-sysc.c
index 93103ff33d6e..e36f5d763c91 100644
--- a/drivers/pmdomain/renesas/rmobile-sysc.c
+++ b/drivers/pmdomain/renesas/rmobile-sysc.c
@@ -100,8 +100,7 @@ static void rmobile_init_pm_domain(struct rmobile_pm_domain *rmobile_pd)
 	struct generic_pm_domain *genpd = &rmobile_pd->genpd;
 	struct dev_power_governor *gov = rmobile_pd->gov;
 
-	genpd->flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP |
-		GENPD_FLAG_NO_STAY_ON;
+	genpd->flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
 	genpd->attach_dev = cpg_mstp_attach_dev;
 	genpd->detach_dev = cpg_mstp_detach_dev;
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 7/9] pmdomain: renesas: rcar-gen4-sysc: Drop GENPD_FLAG_NO_STAY_ON
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Due to the new fine grained sync_state support for onecell genpd provider
drivers, we should no longer need use the legacy behaviour. Therefore,
let's drop GENPD_FLAG_NO_STAY_ON.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/renesas/rcar-gen4-sysc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pmdomain/renesas/rcar-gen4-sysc.c b/drivers/pmdomain/renesas/rcar-gen4-sysc.c
index 0c6c639a91d0..81b154da725f 100644
--- a/drivers/pmdomain/renesas/rcar-gen4-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-gen4-sysc.c
@@ -251,7 +251,6 @@ static int __init rcar_gen4_sysc_pd_setup(struct rcar_gen4_sysc_pd *pd)
 		genpd->detach_dev = cpg_mssr_detach_dev;
 	}
 
-	genpd->flags |= GENPD_FLAG_NO_STAY_ON;
 	genpd->power_off = rcar_gen4_sysc_pd_power_off;
 	genpd->power_on = rcar_gen4_sysc_pd_power_on;
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 5/9] pmdomain: core: Extend fine grained sync_state to more onecell providers
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

A onecell power domain provider driver that we can assign a common
->sync_state() callback for, should be able to benefit from the improved
fine grained sync_state support in genpd. Therefore, let's also assign the
->queue_sync_state() callback for these types of provider drivers.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 783d6f981708..f11dc2110737 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2918,10 +2918,12 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 
 	fwnode = of_fwnode_handle(np);
 	dev = get_dev_from_fwnode(fwnode);
-	if (!dev)
+	if (!dev) {
 		sync_state = true;
-	else
+	} else if (!dev_has_sync_state(dev)) {
 		dev_set_drv_sync_state(dev, genpd_sync_state);
+		dev_set_drv_queue_sync_state(dev, genpd_queue_sync_state);
+	}
 
 	put_device(dev);
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 4/9] pmdomain: core: Add initial fine grained sync_state support
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

A onecell (#power-domain-cells = <1 or 2>; in DT) power domain provider
typically provides multiple independent power domains, each with their own
corresponding consumers. In these cases we have to wait for all consumers
for all the provided power domains before the ->sync_state() callback gets
called for the supplier.

In a first step to improve this, let's implement support for fine grained
sync_state support a per genpd basis by using the ->queue_sync_state()
callback. To take step by step, let's initially limit the improvement to
the internal genpd provider driver and to its corresponding genpd devices
for onecell providers.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/core.c   | 125 ++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h |   1 +
 2 files changed, 126 insertions(+)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index ad57846f02a3..783d6f981708 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2699,6 +2699,120 @@ static struct generic_pm_domain *genpd_get_from_provider(
 	return genpd;
 }
 
+static bool genpd_should_wait_for_consumer(struct device_node *np)
+{
+	struct generic_pm_domain *genpd;
+	bool should_wait = false;
+
+	mutex_lock(&gpd_list_lock);
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+		if (genpd->provider == of_fwnode_handle(np)) {
+			genpd_lock(genpd);
+
+			/* Clear the previous state before reevaluating. */
+			genpd->wait_for_consumer = false;
+
+			/*
+			 * Unless there is at least one genpd for the provider
+			 * that is being kept powered-on, we don't have to care
+			 * about waiting for consumers.
+			 */
+			if (genpd->stay_on)
+				should_wait = true;
+
+			genpd_unlock(genpd);
+		}
+	}
+	mutex_unlock(&gpd_list_lock);
+
+	return should_wait;
+}
+
+static void genpd_parse_for_consumer(struct device_node *sup,
+				     struct device_node *con)
+{
+	struct generic_pm_domain *genpd;
+	int i;
+
+	for (i = 0; ; i++) {
+		struct of_phandle_args pd_args;
+
+		if (of_parse_phandle_with_args(con, "power-domains",
+					       "#power-domain-cells",
+					        i, &pd_args))
+			break;
+
+		/*
+		 * The phandle must correspond to the supplier's genpd provider
+		 * to be relevant else let's move to the next index.
+		 */
+		if (sup != pd_args.np) {
+			of_node_put(pd_args.np);
+			continue;
+		}
+
+		mutex_lock(&gpd_list_lock);
+		genpd = genpd_get_from_provider(&pd_args);
+		if (!IS_ERR(genpd)) {
+			genpd_lock(genpd);
+			genpd->wait_for_consumer = true;
+			genpd_unlock(genpd);
+		}
+		mutex_unlock(&gpd_list_lock);
+
+		of_node_put(pd_args.np);
+	}
+}
+
+static void _genpd_queue_sync_state(struct device_node *np)
+{
+	struct generic_pm_domain *genpd;
+
+	mutex_lock(&gpd_list_lock);
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+		if (genpd->provider == of_fwnode_handle(np)) {
+			genpd_lock(genpd);
+			if (genpd->stay_on && !genpd->wait_for_consumer) {
+				genpd->stay_on = false;
+				genpd_queue_power_off_work(genpd);
+			}
+			genpd_unlock(genpd);
+		}
+	}
+	mutex_unlock(&gpd_list_lock);
+}
+
+static void genpd_queue_sync_state(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_link *link;
+
+	if (!genpd_should_wait_for_consumer(np))
+		return;
+
+	list_for_each_entry(link, &dev->links.consumers, s_node) {
+		struct device *consumer = link->consumer;
+
+		if (!device_link_test(link, DL_FLAG_MANAGED))
+			continue;
+
+		if (link->status == DL_STATE_ACTIVE)
+			continue;
+
+		if (!consumer->of_node)
+			continue;
+
+		/*
+		 * A consumer device has not been probed yet. Let's parse its
+		 * device node for the power-domains property, to find out the
+		 * genpds it may belong to and then prevent sync state for them.
+		 */
+		genpd_parse_for_consumer(np, consumer->of_node);
+	}
+
+	_genpd_queue_sync_state(np);
+}
+
 static void genpd_sync_state(struct device *dev)
 {
 	return of_genpd_sync_state(dev->of_node);
@@ -3531,6 +3645,16 @@ static int genpd_provider_probe(struct device *dev)
 	return 0;
 }
 
+static void genpd_provider_queue_sync_state(struct device *dev)
+{
+	struct generic_pm_domain *genpd = container_of(dev, struct generic_pm_domain, dev);
+
+	if (genpd->sync_state != GENPD_SYNC_STATE_ONECELL)
+		return;
+
+	genpd_queue_sync_state(dev);
+}
+
 static void genpd_provider_sync_state(struct device *dev)
 {
 	struct generic_pm_domain *genpd = container_of(dev, struct generic_pm_domain, dev);
@@ -3559,6 +3683,7 @@ static struct device_driver genpd_provider_drv = {
 	.name = "genpd_provider",
 	.bus = &genpd_provider_bus_type,
 	.probe = genpd_provider_probe,
+	.queue_sync_state = genpd_provider_queue_sync_state,
 	.sync_state = genpd_provider_sync_state,
 	.suppress_bind_attrs = true,
 };
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index b299dc0128d6..7aa49721cde5 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -215,6 +215,7 @@ struct generic_pm_domain {
 	cpumask_var_t cpus;		/* A cpumask of the attached CPUs */
 	bool synced_poweroff;		/* A consumer needs a synced poweroff */
 	bool stay_on;			/* Stay powered-on during boot. */
+	bool wait_for_consumer;		/* Consumers awaits to be probed. */
 	enum genpd_sync_state sync_state; /* How sync_state is managed. */
 	int (*power_off)(struct generic_pm_domain *domain);
 	int (*power_on)(struct generic_pm_domain *domain);
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 3/9] pmdomain: core: Move genpd_get_from_provider()
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

To prepare for subsequent changes and to avoid an unnecessary function
declaration, let's move genpd_get_from_provider() a bit earlier in the
code.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/core.c | 70 ++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 4d32fc676aaf..ad57846f02a3 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2664,6 +2664,41 @@ static bool genpd_present(const struct generic_pm_domain *genpd)
 	return ret;
 }
 
+/**
+ * genpd_get_from_provider() - Look-up PM domain
+ * @genpdspec: OF phandle args to use for look-up
+ *
+ * Looks for a PM domain provider under the node specified by @genpdspec and if
+ * found, uses xlate function of the provider to map phandle args to a PM
+ * domain.
+ *
+ * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
+ * on failure.
+ */
+static struct generic_pm_domain *genpd_get_from_provider(
+					const struct of_phandle_args *genpdspec)
+{
+	struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
+	struct of_genpd_provider *provider;
+
+	if (!genpdspec)
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&of_genpd_mutex);
+
+	/* Check if we have such a provider in our array */
+	list_for_each_entry(provider, &of_genpd_providers, link) {
+		if (provider->node == genpdspec->np)
+			genpd = provider->xlate(genpdspec, provider->data);
+		if (!IS_ERR(genpd))
+			break;
+	}
+
+	mutex_unlock(&of_genpd_mutex);
+
+	return genpd;
+}
+
 static void genpd_sync_state(struct device *dev)
 {
 	return of_genpd_sync_state(dev->of_node);
@@ -2889,41 +2924,6 @@ void of_genpd_del_provider(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
-/**
- * genpd_get_from_provider() - Look-up PM domain
- * @genpdspec: OF phandle args to use for look-up
- *
- * Looks for a PM domain provider under the node specified by @genpdspec and if
- * found, uses xlate function of the provider to map phandle args to a PM
- * domain.
- *
- * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
- * on failure.
- */
-static struct generic_pm_domain *genpd_get_from_provider(
-					const struct of_phandle_args *genpdspec)
-{
-	struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
-	struct of_genpd_provider *provider;
-
-	if (!genpdspec)
-		return ERR_PTR(-EINVAL);
-
-	mutex_lock(&of_genpd_mutex);
-
-	/* Check if we have such a provider in our array */
-	list_for_each_entry(provider, &of_genpd_providers, link) {
-		if (provider->node == genpdspec->np)
-			genpd = provider->xlate(genpdspec, provider->data);
-		if (!IS_ERR(genpd))
-			break;
-	}
-
-	mutex_unlock(&of_genpd_mutex);
-
-	return genpd;
-}
-
 /**
  * of_genpd_add_device() - Add a device to an I/O PM domain
  * @genpdspec: OF phandle args to use for look-up PM domain
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 1/9] driver core: Enable suppliers to implement fine grained sync_state support
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

The common sync_state support isn't fine grained enough for some types of
suppliers, like power domains for example. Especially when a supplier
provides multiple independent power domains, each with their own set of
consumers. In these cases we need to wait for all consumers for all the
provided power domains before invoking the supplier's ->sync_state().

To allow a more fine grained sync_state support to be implemented on per
supplier's driver basis, let's add a new optional callback. As soon as
there is an update worth to consider in regards to managing sync_state for
a supplier device, __device_links_queue_sync_state() invokes the callback.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/core.c           | 7 ++++++-
 include/linux/device/driver.h | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 09b98f02f559..4085a011d8ca 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1106,7 +1106,9 @@ int device_links_check_suppliers(struct device *dev)
  * Queues a device for a sync_state() callback when the device links write lock
  * isn't held. This allows the sync_state() execution flow to use device links
  * APIs.  The caller must ensure this function is called with
- * device_links_write_lock() held.
+ * device_links_write_lock() held.  Note, if the optional queue_sync_state()
+ * callback has been assigned too, it gets called for every update to allowing a
+ * more fine grained support to be implemented on per supplier basis.
  *
  * This function does a get_device() to make sure the device is not freed while
  * on this list.
@@ -1126,6 +1128,9 @@ static void __device_links_queue_sync_state(struct device *dev,
 	if (dev->state_synced)
 		return;
 
+	if (dev->driver && dev->driver->queue_sync_state)
+		dev->driver->queue_sync_state(dev);
+
 	list_for_each_entry(link, &dev->links.consumers, s_node) {
 		if (!device_link_test(link, DL_FLAG_MANAGED))
 			continue;
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index bbc67ec513ed..bc9ae1cbe03c 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -68,6 +68,12 @@ enum probe_type {
  *		be called at late_initcall_sync level. If the device has
  *		consumers that are never bound to a driver, this function
  *		will never get called until they do.
+ * @queue_sync_state: Similar to the ->sync_state() callback, but called to
+ *		allow syncing device state to software state in a more fine
+ *		grained way. It is called when there is an updated state that
+ *		may be worth to consider for any of the consumers linked to
+ *		this device. If implemented, the ->sync_state() callback is
+ *		required too.
  * @remove:	Called when the device is removed from the system to
  *		unbind a device from this driver.
  * @shutdown:	Called at shut-down time to quiesce the device.
@@ -110,6 +116,7 @@ struct device_driver {
 
 	int (*probe) (struct device *dev);
 	void (*sync_state)(struct device *dev);
+	void (*queue_sync_state)(struct device *dev);
 	int (*remove) (struct device *dev);
 	void (*shutdown) (struct device *dev);
 	int (*suspend) (struct device *dev, pm_message_t state);
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 2/9] driver core: Add dev_set_drv_queue_sync_state()
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Similar to the dev_set_drv_sync_state() helper, let's add another one to
allow subsystem level code to set the ->queue_sync_state() callback for a
driver that has not already set it.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 include/linux/device.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index e65d564f01cd..f812e70bdf22 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -994,6 +994,18 @@ static inline int dev_set_drv_sync_state(struct device *dev,
 	return 0;
 }
 
+static inline int dev_set_drv_queue_sync_state(struct device *dev,
+					       void (*fn)(struct device *dev))
+{
+	if (!dev || !dev->driver)
+		return 0;
+	if (dev->driver->queue_sync_state && dev->driver->queue_sync_state != fn)
+		return -EBUSY;
+	if (!dev->driver->queue_sync_state)
+		dev->driver->queue_sync_state = fn;
+	return 0;
+}
+
 static inline void dev_set_removable(struct device *dev,
 				     enum device_removable removable)
 {
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 0/9] driver core / pmdomain: Add support for fined grained sync_state
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel

Since the introduction [1] of the common sync_state support for pmdomains
(genpd), we have encountered a lot of various interesting problems. In most
cases the new behaviour of genpd triggered some weird platform specific bugs.

That said, in LPC in Tokyo me and Saravana hosted a session to walk through the
remaining limitations that we have found for genpd's sync state support. In
particular, we discussed the problems we have for the so-called onecell power
domain providers, where a single provider typically provides multiple
independent power domains, all with their own set of consumers.

Note that, onecell power domain providers are very common. It's being used by
many SoCs/platforms/technologies. To name a few:
SCMI, Qualcomm, NXP, Mediatek, Renesas, TI, etc.

Anyway, in these cases, the generic sync_state mechanism with fw_devlink isn't
fine grained enough, as we end up waiting for all consumers for all power
domains before the ->sync_callback gets called for the supplier/provider. In
other words, we may end up keeping unused power domains powered-on, for no good
reasons.

The series intends to fix this problem. Please have a look at the commit
messages for more details and help review/test!

Kind regards
Ulf Hansson

[1]
https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/

Ulf Hansson (9):
  driver core: Enable suppliers to implement fine grained sync_state
    support
  driver core: Add dev_set_drv_queue_sync_state()
  pmdomain: core: Move genpd_get_from_provider()
  pmdomain: core: Add initial fine grained sync_state support
  pmdomain: core: Extend fine grained sync_state to more onecell
    providers
  pmdomain: core: Export a common function for ->queue_sync_state()
  pmdomain: renesas: rcar-gen4-sysc: Drop GENPD_FLAG_NO_STAY_ON
  pmdomain: renesas: rcar-sysc: Drop GENPD_FLAG_NO_STAY_ON
  pmdomain: renesas: rmobile-sysc: Drop GENPD_FLAG_NO_STAY_ON

 drivers/base/core.c                       |   7 +-
 drivers/pmdomain/core.c                   | 205 ++++++++++++++++++----
 drivers/pmdomain/renesas/rcar-gen4-sysc.c |   1 -
 drivers/pmdomain/renesas/rcar-sysc.c      |   1 -
 drivers/pmdomain/renesas/rmobile-sysc.c   |   3 +-
 include/linux/device.h                    |  12 ++
 include/linux/device/driver.h             |   7 +
 include/linux/pm_domain.h                 |   3 +
 8 files changed, 197 insertions(+), 42 deletions(-)

-- 
2.43.0



^ permalink raw reply

* Re: [PATCH net-next v3 00/12] net: airoha: Support multiple net_devices connected to the same GDM port
From: Lorenzo Bianconi @ 2026-04-10 10:39 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Christian Marangi,
	Benjamin Larsson, linux-arm-kernel, linux-mediatek, netdev,
	devicetree, Xuegang Lu
In-Reply-To: <20260409195645.16c68979@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On Apr 09, Jakub Kicinski wrote:
> On Mon, 06 Apr 2026 12:34:05 +0200 Lorenzo Bianconi wrote:
> > EN7581 or AN7583 SoCs support connecting multiple external SerDes (e.g.
> > Ethernet or USB SerDes) to GDM3 or GDM4 ports via a hw arbiter that
> > manages the traffic in a TDM manner. As a result multiple net_devices can
> > connect to the same GDM{3,4} port and there is a theoretical "1:n"
> > relation between GDM ports and net_devices.
> 
> Still waiting for the device tree review. I'm going to blindly send out
> the Sashiko review, please comment if any of it makes sense?

ack, I will do.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v3 10/12] net: airoha: Do not stop GDM port if it is shared
From: Lorenzo Bianconi @ 2026-04-10 10:35 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
	ansuelsmth, benjamin.larsson, linux-arm-kernel, linux-mediatek,
	netdev, devicetree, Xuegang Lu
In-Reply-To: <20260410025726.1810041-1-kuba@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 4665 bytes --]

> This is an AI-generated review of your patch. The human sending this
> email says "For your consideration."
> ---
> net: airoha: Do not stop GDM port if it is shared
> 
> Theoretically, in the current codebase, two independent net_devices can
> be connected to the same GDM port so we need to check the GDM port is not
> used by any other running net_device before setting the forward
> configuration to FE_PSE_PORT_DROP.
> 
> Tested-by: Xuegang Lu <xuegang.lu@airoha.com>
> 
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index fd27ba13029bd..5b0cd37b155e4 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> 
> [ ... ]
> 
> > @@ -1688,18 +1693,18 @@ static int airoha_dev_stop(struct net_device *netdev)
> >  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> >  	struct airoha_gdm_port *port = dev->port;
> >  	struct airoha_qdma *qdma = dev->qdma;
> > -	int i, err;
> > +	int i;
> >  
> >  	netif_tx_disable(netdev);
> > -	err = airoha_set_vip_for_gdm_port(dev, false);
> > -	if (err)
> > -		return err;
> > -
> >  	for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++)
> >  		netdev_tx_reset_subqueue(netdev, i);
> >  
> > -	airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
> > -				    FE_PSE_PORT_DROP);
> > +	if (atomic_dec_and_test(&port->users)) {
> > +		airoha_set_vip_for_gdm_port(dev, false);
> > +		airoha_set_gdm_port_fwd_cfg(qdma->eth,
> > +					    REG_GDM_FWD_CFG(port->id),
> > +					    FE_PSE_PORT_DROP);
> > +	}
> >  
> >  	if (atomic_dec_and_test(&qdma->users)) {
> >  		airoha_qdma_clear(qdma, REG_QDMA_GLOBAL_CFG,
> > 				  GLOBAL_CFG_TX_DMA_EN_MASK |
> > 				  GLOBAL_CFG_RX_DMA_EN_MASK);
> > 
> > 		for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++) {
> > 			if (!qdma->q_tx[i].ndesc)
> > 				continue;
> > 
> > 			airoha_qdma_cleanup_tx_queue(&qdma->q_tx[i]);
> > 		}
> > 	}
> 
> This problem wasn't introduced by this patch, but does the QDMA teardown
> path have a DMA corruption vulnerability here?
> 
> When GLOBAL_CFG_TX_DMA_EN_MASK is cleared, it fails to poll
> GLOBAL_CFG_TX_DMA_BUSY_MASK, proceeding to unmap memory while the hardware
> DMA engine may still be processing in-flight descriptors.
> 
> Furthermore, airoha_qdma_cleanup_tx_queue() unmaps the DMA addresses and
> frees the SKBs, but it does not appear to clear the physical hardware
> descriptors or reset the hardware ring pointers (REG_TX_CPU_IDX /
> REG_TX_DMA_IDX).
> 
> When the device is later reopened in airoha_dev_open(),
> GLOBAL_CFG_TX_DMA_EN_MASK is set again without re-initializing the rings.
> Will the hardware DMA engine resume fetching from its previous index,
> processing stale descriptors whose backing memory was already unmapped
> and freed?

ack, I will work on it with a dedicated patch.

> 
> [ ... ]
> 
> > @@ -1851,10 +1856,22 @@ static void airoha_dev_get_stats64(struct net_device *netdev,
> >  static int airoha_dev_change_mtu(struct net_device *netdev, int mtu)
> >  {
> >  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> > +	u32 cur_len, len = ETH_HLEN + mtu + ETH_FCS_LEN;
> >  	struct airoha_gdm_port *port = dev->port;
> > -	u32 len = ETH_HLEN + mtu + ETH_FCS_LEN;
> >  	struct airoha_eth *eth = dev->eth;
> >  
> > +	cur_len = FIELD_GET(GDM_LONG_LEN_MASK,
> > +			    airoha_fe_rr(eth, REG_GDM_LEN_CFG(port->id)));
> > +	if (len < cur_len) {
> > +		u8 port_refcnt = atomic_read(&port->users);
> > +
> > +		/* We can decrease the device MTU just if the GDM port is
> > +		 * not shared or if the other device is not running.
> > +		 */
> > +		if (port_refcnt > 1 || (port_refcnt && !netif_running(netdev)))
> > +			return -EBUSY;
> > +	}
> > +
> 
> Does this MTU decrease validation create a state trap on shared ports?
> 
> If both eth0 and eth1 (sharing a port) are up, and a user increases eth0 MTU
> to 9000, the hardware MTU updates to 9000. If the user then tries to lower
> eth0 MTU back to 1500, len < cur_len evaluates to true. Because eth1 is also
> up (port_refcnt > 1), this returns -EBUSY.
> 
> This would prevent either interface from decreasing its MTU without fully
> bringing down the sibling interface.
> 
> Should the software netdev->mtu be decoupled from the hardware validation,
> allowing valid MTU changes in software while dynamically programming the
> hardware MTU to the maximum of all currently up interfaces on the shared
> port?

It seems more correct to me avoid receiving in the stack packet bigger than the
configured 'SW MTU".

Regards,
Lorenzo


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] pwm: rp1: Add RP1 PWM controller driver
From: Andrea della Porta @ 2026-04-10 10:27 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Andrea della Porta, linux-pwm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli,
	Broadcom internal kernel review list, devicetree,
	linux-rpi-kernel, linux-arm-kernel, linux-kernel, Naushir Patuck,
	Stanimir Varbanov
In-Reply-To: <adiW1tBC8Imd14LD@monoceros>

Hi Uwe,

On 08:27 Fri 10 Apr     , Uwe Kleine-König wrote:
> Hello Andrea,
> 
> On Thu, Apr 09, 2026 at 06:16:41PM +0200, Andrea della Porta wrote:
> > On 23:45 Sun 05 Apr     , Uwe Kleine-König wrote:
> > > On Fri, Apr 03, 2026 at 04:31:55PM +0200, Andrea della Porta wrote:
> > > > +static void rp1_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> > > > +{
> > > > +	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > > > +	u32 value;
> > > > +
> > > > +	value = readl(rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> > > > +	value &= ~PWM_MODE_MASK;
> > > > +	writel(value, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> > > > +
> > > > +	rp1_pwm_apply_config(chip, pwm);
> > > 
> > > What is the purpose of this call?
> > 
> > To update the configuration on the next PWM strobe in order to avoid
> > glitches. I'll add a short comment in the code.
> 
> .pwm_free() should not touch the hardware configuration. Changing the
> pinmuxing (which I guess is the purpose of clearing PWM_MODE_MASK) is
> somewhat a grey area. If that saves energy, that's okish. Otherwise
> not interfering with the operation of the PWM (e.g. to keep a display on
> during kexec or so) is preferred.

Sorry I should've been more clear on this. The pinmux/conf is not changed
at all by this mask, only the PWM output mode is. The controller can output
several type of waveforms and clearing PWM_MODE_MASK is just setting the
controller to output a 0, which is the reset default i.e. the same value
as just before exporting the channel.
I guess this is the expected behaviour in case of a fan, it should stop
spinning in case you unexport the pwm channel, but I see it could be
different with displays.
Honestly I don't have a strong opinion about that, please just let me
know if I should drop that pwm_free entirely.

> 
> > > > +static int rp1_pwm_resume(struct device *dev)
> > > > +{
> > > > +	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> > > > +
> > > > +	return clk_prepare_enable(rp1->clk);
> > > 
> > > Hmm, if this fails and then the driver is unbound, the clk operations
> > > are not balanced.
> > 
> > I'll add some flags to check if the clock is really enabled or not.
> 
> To be honest, I guess that is a problem of several drivers, not only in
> drivers/pwm. If this complicates the driver, I guess addressing this
> isn't very critical.

I'll come up with something, we can always drop this check if deemed
too 'noisy'. 

Many thanks,
Andrea

> 
> Best regards
> Uwe




^ permalink raw reply

* Re: [PATCH net-next v3 09/12] net: airoha: Support multiple net_devices for a single FE GDM port
From: Lorenzo Bianconi @ 2026-04-10 10:19 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
	ansuelsmth, benjamin.larsson, linux-arm-kernel, linux-mediatek,
	netdev, devicetree
In-Reply-To: <20260410025724.1810007-1-kuba@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 8640 bytes --]

On Apr 09, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "For your consideration."
> ---
> net: airoha: Support multiple net_devices for a single FE GDM port
> 
> This commit introduces support for mapping multiple net_device interfaces
> to a single Frame Engine (FE) GDM port via an external hardware arbiter.
> It updates the rx/tx paths, port allocation, and registration logic to
> handle the new net_device array.
> 
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> [ ... ]
> > @@ -615,9 +617,8 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
> >  		u32 hash, reason, msg1 = le32_to_cpu(desc->msg1);
> >  		struct page *page = virt_to_head_page(e->buf);
> >  		u32 desc_ctrl = le32_to_cpu(desc->ctrl);
> > -		struct airoha_gdm_port *port;
> > -		struct net_device *netdev;
> > -		int data_len, len, p;
> > +		struct airoha_gdm_dev *dev;
> > +		int data_len, len;
> >  
> >  		if (!(desc_ctrl & QDMA_DESC_DONE_MASK))
> >  			break;
> 
> While looking at this code, I noticed an issue that predates this patch.
> Can the lack of a memory barrier after checking the descriptor ownership bit
> cause stale reads?
> 
> The driver checks QDMA_DESC_DONE_MASK to verify that the hardware has finished
> writing the descriptor. However, it reads desc->msg1 before this check and
> without a dma_rmb():
> 
> 		u32 hash, reason, msg1 = le32_to_cpu(desc->msg1);
> 		struct page *page = virt_to_head_page(e->buf);
> 		u32 desc_ctrl = le32_to_cpu(desc->ctrl);
> 
> 		if (!(desc_ctrl & QDMA_DESC_DONE_MASK))
> 			break;
> 
> Since weakly ordered architectures can reorder memory reads, could the CPU
> prefetch msg1 before the DMA engine writes the new descriptor values, leading
> to routing the packet to the wrong gdm_dev?

This issue has been already fixed with the following commit:

commit 4ae0604a0673e11e2075b178387151fcad5111b5
Author: Lorenzo Bianconi <lorenzo@kernel.org>
Date:   Tue Apr 7 08:48:04 2026 +0200

    net: airoha: Add dma_rmb() and READ_ONCE() in airoha_qdma_rx_process()
    
    Add missing dma_rmb() in airoha_qdma_rx_process routine to make sure the
    DMA read operations are completed when the NIC reports the processing on
    the current descriptor is done. Moreover, add missing READ_ONCE() in
    airoha_qdma_rx_process() for DMA descriptor control fields in order to
    avoid any compiler reordering.

> 
> [ ... ]
> > @@ -1764,9 +1765,7 @@ static int airoha_set_gdm2_loopback(struct airoha_gdm_dev *dev)
> >  	airoha_fe_clear(eth, REG_FE_VIP_PORT_EN, BIT(AIROHA_GDM2_IDX));
> >  	airoha_fe_clear(eth, REG_FE_IFC_PORT_EN, BIT(AIROHA_GDM2_IDX));
> >  
> > -	/* XXX: handle XSI_USB_PORT and XSI_PCE1_PORT */
> > -	nbq = port->id == AIROHA_GDM3_IDX && airoha_is_7581(eth) ? 4 : 0;
> > -	src_port = eth->soc->ops.get_src_port_id(port, nbq);
> > +	src_port = eth->soc->ops.get_src_port_id(port, dev->nbq);
> >  	if (src_port < 0)
> >  		return src_port;
> 
> Can configuring the loopback for one device break connectivity for other
> devices sharing the same port?
> 
> When multiple net_devices (like PCIE0 and PCIE1 on GDM3) open and call
> airoha_set_gdm2_loopback(), they modify global hardware registers such as
> REG_FE_WAN_PORT.
> 
> Since WAN0_MASK is unconditionally updated to track the current device's
> src_port:
> 
> 	airoha_fe_rmw(eth, REG_FE_WAN_PORT,
> 		      WAN1_EN_MASK | WAN1_MASK | WAN0_MASK,
> 		      FIELD_PREP(WAN0_MASK, src_port));
> 
> won't the last configured device clobber the hardware state for the others?

This issue is fixed with the following patch in the series:

commit 008995bd8f39c5fdf37a933fa3c962e7ef52e8c6
Author: Lorenzo Bianconi <lorenzo@kernel.org>
Date:   Fri Apr 3 12:07:27 2026 +0200

    net: airoha: Introduce WAN device flag
    
    Introduce WAN flag to specify if a given device is used to transmit/receive
    WAN or LAN traffic. Current codebase supports specifying LAN/WAN device
    configuration in ndo_init() callback during device bootstrap.
    Please note it is possible to specify multiple LAN devices but just a
    single WAN one.

> 
> [ ... ]
> > @@ -1987,7 +1986,8 @@ static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb,
> >  	}
> >  
> >  	fport = airoha_get_fe_port(dev);
> > -	msg1 = FIELD_PREP(QDMA_ETH_TXMSG_FPORT_MASK, fport) |
> > +	msg1 = FIELD_PREP(QDMA_ETH_TXMSG_NBOQ_MASK, dev->nbq) |
> > +	       FIELD_PREP(QDMA_ETH_TXMSG_FPORT_MASK, fport) |
> >  	       FIELD_PREP(QDMA_ETH_TXMSG_METER_MASK, 0x7f);
> >  
> >  	q = &qdma->q_tx[qid];
> 
> Does sharing the hardware QDMA TX queues among multiple net_device instances
> introduce a permanent transmit stall?
> 
> In airoha_dev_xmit(), if a shared hardware queue is full, the driver stops the
> netdev_queue of the specific netdev making the call:
> 
> 	if (q->queued + nr_frags >= q->ndesc) {
> 		/* not enough space in the queue */
> 		netif_tx_stop_queue(txq);
> 		spin_unlock_bh(&q->lock);
> 		return NETDEV_TX_BUSY;
> 	}
> 
> However, the TX completion handler (airoha_tx_irq_process()) only wakes the
> queue of the netdev attached to the completed packet. If an interface's TX
> queue is stopped but it has no pending packets in the hardware queue (because
> another interface filled it), will it ever get woken up?

This issue is orthogonal to this series and it will be fixed with a dedicated
patch.

> 
> [ ... ]
> > @@ -2955,11 +2958,25 @@ static int airoha_alloc_gdm_device(struct airoha_eth *eth,
> >  			 netdev->dev_addr);
> >  	}
> >  
> > +	/* Allowed nbq for EN7581 on GDM3 port are 4 and 5 for PCIE0
> > +	 * and PCIE1 respectively.
> > +	 */
> > +	index = nbq;
> > +	if (index && airoha_is_7581(eth) && port->id == AIROHA_GDM3_IDX)
> > +		index -= 4;
> > +
> > +	if (index >= ARRAY_SIZE(port->devs) || port->devs[index]) {
> > +		dev_err(eth->dev, "invalid nbq id: %d\n", nbq);
> > +		return -EINVAL;
> > +	}
> > +
> > +	netdev->dev.of_node = of_node_get(np);
> >  	dev = netdev_priv(netdev);
> 
> Does this code leak the of_node reference?
> 
> The node reference is explicitly incremented via of_node_get(np). The networking
> core does not automatically drop explicit of_node references on free_netdev().
> Without a matching of_node_put() in the error paths or the driver's teardown
> function (airoha_remove()), will this reference permanently leak when the device
> is destroyed?

ack, I will fix it in v4.

> 
> [ ... ]
> > @@ -3003,7 +3021,43 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> >  	if (err)
> >  		return err;
> >  
> > -	return airoha_alloc_gdm_device(eth, port, np);
> > +	for_each_child_of_node(np, node) {
> > +		/* Multiple external serdes connected to the FE GDM port via an
> > +		 * external arbiter.
> > +		 */
> > +		const __be32 *nbq_ptr;
> > +		int nbq;
> > +
> > +		if (!of_device_is_compatible(node, "airoha,eth-port"))
> > +			continue;
> > +
> > +		d++;
> > +		if (!of_device_is_available(node))
> > +			continue;
> > +
> > +		nbq_ptr = of_get_property(node, "reg", NULL);
> > +		if (!nbq_ptr) {
> > +			dev_err(eth->dev, "missing nbq id\n");
> > +			of_node_put(node);
> > +			return -EINVAL;
> > +		}
> > +
> > +		/* Verify the provided nbq parameter is valid */
> > +		nbq = be32_to_cpup(nbq_ptr);
> > +		err = eth->soc->ops.get_src_port_id(port, nbq);
> > +		if (err < 0) {
> > +			of_node_put(node);
> > +			return err;
> > +		}
> > +
> > +		err = airoha_alloc_gdm_device(eth, port, nbq, node);
> > +		if (err) {
> > +			of_node_put(node);
> > +			return err;
> > +		}
> > +	}
> > +
> > +	return !d ? airoha_alloc_gdm_device(eth, port, 0, np) : 0;
> >  }
> 
> Does passing 0 for nbq in the fallback path break older device trees on the
> EN7581 SoC?
> 
> Later, when the interface is brought up, airoha_set_gdm2_loopback() calls
> airoha_en7581_get_src_port_id(), which returns -EINVAL for GDM3 if nbq is
> not 4 or 5:
> 
> 	switch (port->id) {
> 	case AIROHA_GDM3_IDX:
> 		/* 7581 SoC supports PCIe serdes on GDM3 port */
> 		if (nbq == 4)
> 			return HSGMII_LAN_7581_PCIE0_SRCPORT;
> 		if (nbq == 5)
> 			return HSGMII_LAN_7581_PCIE1_SRCPORT;
> 		break;
> 
> If nbq is hardcoded to 0 for legacy DTs, won't this cause interface
> initialization to fail unconditionally for GDM3?

ack, I will fix it in v4.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* [PATCH 4/4] arm64: dts: amlogic: t7: Add clk measure support
From: Jian Hu @ 2026-04-10 10:03 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
	Martin Blumenstingl, robh+dt, Rob Herring, Krzysztof Kozlowski
  Cc: Jian Hu, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20260410100329.3167482-1-jian.hu@amlogic.com>

Add the clock measure device to the T7 SoC family.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
index 7fe72c94ed62..cec2ea74850d 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
@@ -701,6 +701,11 @@ pwm_ao_cd: pwm@60000 {
 				status = "disabled";
 			};
 
+			clock-measurer@48000 {
+				compatible = "amlogic,t7-clk-measure";
+				reg = <0x0 0x48000 0x0 0x1c>;
+			};
+
 			sd_emmc_a: mmc@88000 {
 				compatible = "amlogic,t7-mmc", "amlogic,meson-axg-mmc";
 				reg = <0x0 0x88000 0x0 0x800>;
-- 
2.47.1



^ permalink raw reply related

* [PATCH 3/4] arm64: dts: meson: a1: Add clk measure support
From: Jian Hu @ 2026-04-10 10:03 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
	Martin Blumenstingl, robh+dt, Rob Herring, Krzysztof Kozlowski
  Cc: Jian Hu, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20260410100329.3167482-1-jian.hu@amlogic.com>

Add the clock measure device to the A1 SoC family.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 348411411f3d..6f6a6145cba1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -576,6 +576,11 @@ saradc: adc@2c00 {
 				status = "disabled";
 			};
 
+			clock-measurer@3400 {
+				compatible = "amlogic,a1-clk-measure";
+				reg = <0x0 0x3400 0x0 0x1c>;
+			};
+
 			i2c1: i2c@5c00 {
 				compatible = "amlogic,meson-axg-i2c";
 				status = "disabled";
-- 
2.47.1



^ permalink raw reply related

* [PATCH 2/4] soc: amlogic: clk-measure: Add A1 and T7 support
From: Jian Hu @ 2026-04-10 10:03 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
	Martin Blumenstingl, robh+dt, Rob Herring, Krzysztof Kozlowski
  Cc: Jian Hu, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20260410100329.3167482-1-jian.hu@amlogic.com>

Add support for the A1 and T7 SoC family in amlogic clk measure.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 drivers/soc/amlogic/meson-clk-measure.c | 272 ++++++++++++++++++++++++
 1 file changed, 272 insertions(+)

diff --git a/drivers/soc/amlogic/meson-clk-measure.c b/drivers/soc/amlogic/meson-clk-measure.c
index d862e30a244e..083524671b76 100644
--- a/drivers/soc/amlogic/meson-clk-measure.c
+++ b/drivers/soc/amlogic/meson-clk-measure.c
@@ -787,6 +787,258 @@ static const struct meson_msr_id clk_msr_s4[] = {
 
 };
 
+static struct meson_msr_id clk_msr_a1[] = {
+	CLK_MSR_ID(0, "tdmout_b_sclk"),
+	CLK_MSR_ID(1, "tdmout_a_sclk"),
+	CLK_MSR_ID(2, "tdmin_lb_sclk"),
+	CLK_MSR_ID(3, "tdmin_b_sclk"),
+	CLK_MSR_ID(4, "tdmin_a_sclk"),
+	CLK_MSR_ID(5, "vad"),
+	CLK_MSR_ID(6, "resamplea"),
+	CLK_MSR_ID(7, "pdm_sysclk"),
+	CLK_MSR_ID(8, "pdm_dclk"),
+	CLK_MSR_ID(9, "locker_out"),
+	CLK_MSR_ID(10, "locker_in"),
+	CLK_MSR_ID(11, "spdifin"),
+	CLK_MSR_ID(12, "tdmin_vad"),
+	CLK_MSR_ID(13, "au_adc"),
+	CLK_MSR_ID(14, "au_dac"),
+	CLK_MSR_ID(16, "spicc_a"),
+	CLK_MSR_ID(17, "spifc"),
+	CLK_MSR_ID(18, "sd_emmc_a"),
+	CLK_MSR_ID(19, "dmcx4"),
+	CLK_MSR_ID(20, "dmc"),
+	CLK_MSR_ID(21, "psram"),
+	CLK_MSR_ID(22, "cecb"),
+	CLK_MSR_ID(23, "ceca"),
+	CLK_MSR_ID(24, "ts"),
+	CLK_MSR_ID(25, "pwm_f"),
+	CLK_MSR_ID(26, "pwm_e"),
+	CLK_MSR_ID(27, "pwm_d"),
+	CLK_MSR_ID(28, "pwm_c"),
+	CLK_MSR_ID(29, "pwm_b"),
+	CLK_MSR_ID(30, "pwm_a"),
+	CLK_MSR_ID(31, "saradc"),
+	CLK_MSR_ID(32, "usb_bus"),
+	CLK_MSR_ID(33, "dsp_b"),
+	CLK_MSR_ID(34, "dsp_a"),
+	CLK_MSR_ID(35, "axi"),
+	CLK_MSR_ID(36, "sys"),
+	CLK_MSR_ID(40, "rng_ring_osc0"),
+	CLK_MSR_ID(41, "rng_ring_osc1"),
+	CLK_MSR_ID(42, "rng_ring_osc2"),
+	CLK_MSR_ID(43, "rng_ring_osc3"),
+	CLK_MSR_ID(44, "dds_out"),
+	CLK_MSR_ID(45, "cpu_clk_div16"),
+	CLK_MSR_ID(46, "gpio_msr"),
+	CLK_MSR_ID(50, "osc_ring_cpu0"),
+	CLK_MSR_ID(51, "osc_ring_cpu1"),
+	CLK_MSR_ID(54, "osc_ring_top0"),
+	CLK_MSR_ID(55, "osc_ring_top1"),
+	CLK_MSR_ID(56, "osc_ring_ddr"),
+	CLK_MSR_ID(57, "osc_ring_dmc"),
+	CLK_MSR_ID(58, "osc_ring_dspa"),
+	CLK_MSR_ID(59, "osc_ring_dspb"),
+	CLK_MSR_ID(60, "osc_ring_rama"),
+	CLK_MSR_ID(61, "osc_ring_ramb"),
+};
+
+static struct meson_msr_id clk_msr_t7[] = {
+	CLK_MSR_ID(0, "sys"),
+	CLK_MSR_ID(1, "axi"),
+	CLK_MSR_ID(2, "rtc"),
+	CLK_MSR_ID(3, "dspa"),
+	CLK_MSR_ID(4, "dspb"),
+	CLK_MSR_ID(5, "mali"),
+	CLK_MSR_ID(6, "sys_cpu_clk_div16"),
+	CLK_MSR_ID(7, "ceca"),
+	CLK_MSR_ID(8, "cecb"),
+	CLK_MSR_ID(10, "fclk_div5"),
+	CLK_MSR_ID(11, "mpll0"),
+	CLK_MSR_ID(12, "mpll1"),
+	CLK_MSR_ID(13, "mpll2"),
+	CLK_MSR_ID(14, "mpll3"),
+	CLK_MSR_ID(15, "mpll_50m"),
+	CLK_MSR_ID(16, "pcie_inp"),
+	CLK_MSR_ID(17, "pcie_inn"),
+	CLK_MSR_ID(18, "mpll_test_out"),
+	CLK_MSR_ID(19, "hifi_pll"),
+	CLK_MSR_ID(20, "gp0_pll"),
+	CLK_MSR_ID(21, "gp1_pll"),
+	CLK_MSR_ID(22, "eth_mppll_50m"),
+	CLK_MSR_ID(23, "sys_pll_div16"),
+	CLK_MSR_ID(24, "ddr_dpll_pt"),
+	CLK_MSR_ID(25, "earcrx_pll"),
+	CLK_MSR_ID(26, "paie1_clk_inp"),
+	CLK_MSR_ID(27, "paie1_clk_inn"),
+	CLK_MSR_ID(28, "amlgdc"),
+	CLK_MSR_ID(29, "gdc"),
+	CLK_MSR_ID(30, "mod_eth_phy_ref"),
+	CLK_MSR_ID(31, "mod_eth_tx"),
+	CLK_MSR_ID(32, "eth_clk125Mhz"),
+	CLK_MSR_ID(33, "eth_clk_rmii"),
+	CLK_MSR_ID(34, "co_clkin_to_mac"),
+	CLK_MSR_ID(35, "mod_eth_rx_clk_rmii"),
+	CLK_MSR_ID(36, "co_rx"),
+	CLK_MSR_ID(37, "co_tx"),
+	CLK_MSR_ID(38, "eth_phy_rxclk"),
+	CLK_MSR_ID(39, "eth_phy_plltxclk"),
+	CLK_MSR_ID(40, "ephy_test"),
+	CLK_MSR_ID(41, "dsi_b_meas"),
+	CLK_MSR_ID(42, "hdmirx_apl"),
+	CLK_MSR_ID(43, "hdmirx_tmds"),
+	CLK_MSR_ID(44, "hdmirx_cable"),
+	CLK_MSR_ID(45, "hdmirx_apll_clk_audio"),
+	CLK_MSR_ID(46, "hdmirx_5m"),
+	CLK_MSR_ID(47, "hdmirx_2m"),
+	CLK_MSR_ID(48, "hdmirx_cfg"),
+	CLK_MSR_ID(49, "hdmirx_hdcp2x_eclk"),
+	CLK_MSR_ID(50, "vid_pll0_div"),
+	CLK_MSR_ID(51, "hdmi_vid_pll"),
+	CLK_MSR_ID(54, "vdac_clk"),
+	CLK_MSR_ID(55, "vpu_clk_buf"),
+	CLK_MSR_ID(56, "mod_tcon_clko"),
+	CLK_MSR_ID(57, "lcd_an_clk_ph2"),
+	CLK_MSR_ID(58, "lcd_an_clk_ph3"),
+	CLK_MSR_ID(59, "hdmi_tx_pixel"),
+	CLK_MSR_ID(60, "vdin_meas"),
+	CLK_MSR_ID(61, "vpu_clk"),
+	CLK_MSR_ID(62, "vpu_clkb"),
+	CLK_MSR_ID(63, "vpu_clkb_tmp"),
+	CLK_MSR_ID(64, "vpu_clkc"),
+	CLK_MSR_ID(65, "vid_lock"),
+	CLK_MSR_ID(66, "vapbclk"),
+	CLK_MSR_ID(67, "ge2d"),
+	CLK_MSR_ID(68, "aud_pll"),
+	CLK_MSR_ID(69, "aud_sck"),
+	CLK_MSR_ID(70, "dsi_a_meas"),
+	CLK_MSR_ID(72, "mipi_csi_phy"),
+	CLK_MSR_ID(73, "mipi_isp"),
+	CLK_MSR_ID(76, "hdmitx_tmds"),
+	CLK_MSR_ID(77, "hdmitx_sys"),
+	CLK_MSR_ID(78, "hdmitx_fe"),
+	CLK_MSR_ID(80, "hdmitx_prif"),
+	CLK_MSR_ID(81, "hdmitx_200m"),
+	CLK_MSR_ID(82, "hdmitx_aud"),
+	CLK_MSR_ID(83, "hdmitx_pnx"),
+	CLK_MSR_ID(84, "spicc5"),
+	CLK_MSR_ID(85, "spicc4"),
+	CLK_MSR_ID(86, "spicc3"),
+	CLK_MSR_ID(87, "spicc2"),
+	CLK_MSR_ID(93, "vdec"),
+	CLK_MSR_ID(94, "wave521_aclk"),
+	CLK_MSR_ID(95, "wave521_cclk"),
+	CLK_MSR_ID(96, "wave521_bclk"),
+	CLK_MSR_ID(97, "hcodec"),
+	CLK_MSR_ID(98, "hevcb"),
+	CLK_MSR_ID(99, "hevcf"),
+	CLK_MSR_ID(100, "hdmi_aud_pll"),
+	CLK_MSR_ID(101, "hdmi_acr_ref"),
+	CLK_MSR_ID(102, "hdmi_meter"),
+	CLK_MSR_ID(103, "hdmi_vid"),
+	CLK_MSR_ID(104, "hdmi_aud"),
+	CLK_MSR_ID(105, "hdmi_dsd"),
+	CLK_MSR_ID(108, "dsi1_phy"),
+	CLK_MSR_ID(109, "dsi0_phy"),
+	CLK_MSR_ID(110, "smartcard"),
+	CLK_MSR_ID(111, "sar_adc"),
+	CLK_MSR_ID(113, "sd_emmc_c"),
+	CLK_MSR_ID(114, "sd_emmc_b"),
+	CLK_MSR_ID(115, "sd_emmc_a"),
+	CLK_MSR_ID(116, "gpio_msr"),
+	CLK_MSR_ID(117, "spicc1"),
+	CLK_MSR_ID(118, "spicc0"),
+	CLK_MSR_ID(119, "anakin"),
+	CLK_MSR_ID(121, "ts_clk(temp sensor)"),
+	CLK_MSR_ID(122, "ts_a73"),
+	CLK_MSR_ID(123, "ts_a53"),
+	CLK_MSR_ID(124, "ts_nna"),
+	CLK_MSR_ID(130, "audio_vad"),
+	CLK_MSR_ID(131, "acodec_dac_clk_x128"),
+	CLK_MSR_ID(132, "audio_locker_in"),
+	CLK_MSR_ID(133, "audio_locker_out"),
+	CLK_MSR_ID(134, "audio_tdmout_c_sclk"),
+	CLK_MSR_ID(135, "audio_tdmout_b_sclk"),
+	CLK_MSR_ID(136, "audio_tdmout_a_sclk"),
+	CLK_MSR_ID(137, "audio_tdmin_lb_sclk"),
+	CLK_MSR_ID(138, "audio_tdmin_c_sclk"),
+	CLK_MSR_ID(139, "audio_tdmin_b_sclk"),
+	CLK_MSR_ID(140, "audio_tdmin_a_sclk"),
+	CLK_MSR_ID(141, "audio_resamplea"),
+	CLK_MSR_ID(142, "audio_pdm_sysclk"),
+	CLK_MSR_ID(143, "audio_spdifoutb_mst"),
+	CLK_MSR_ID(144, "audio_spdifout_mst"),
+	CLK_MSR_ID(145, "audio_spdifin_mst"),
+	CLK_MSR_ID(146, "audio_pdm_dclk"),
+	CLK_MSR_ID(147, "audio_resampleb"),
+	CLK_MSR_ID(148, "earcrx_pll_dmac"),
+	CLK_MSR_ID(156, "pwm_ao_h"),
+	CLK_MSR_ID(157, "pwm_ao_g"),
+	CLK_MSR_ID(158, "pwm_ao_f"),
+	CLK_MSR_ID(159, "pwm_ao_e"),
+	CLK_MSR_ID(160, "pwm_ao_d"),
+	CLK_MSR_ID(161, "pwm_ao_c"),
+	CLK_MSR_ID(162, "pwm_ao_b"),
+	CLK_MSR_ID(163, "pwm_ao_a"),
+	CLK_MSR_ID(164, "pwm_f"),
+	CLK_MSR_ID(165, "pwm_e"),
+	CLK_MSR_ID(166, "pwm_d"),
+	CLK_MSR_ID(167, "pwm_c"),
+	CLK_MSR_ID(168, "pwm_b"),
+	CLK_MSR_ID(169, "pwm_a"),
+	CLK_MSR_ID(170, "aclkm"),
+	CLK_MSR_ID(171, "mclk_pll"),
+	CLK_MSR_ID(172, "a73_sys_pll_div16"),
+	CLK_MSR_ID(173, "a73_cpu_clk_div16"),
+	CLK_MSR_ID(176, "rng_ring_0"),
+	CLK_MSR_ID(177, "rng_ring_1"),
+	CLK_MSR_ID(178, "rng_ring_2"),
+	CLK_MSR_ID(179, "rng_ring_3"),
+	CLK_MSR_ID(180, "am_ring_out0"),
+	CLK_MSR_ID(181, "am_ring_out1"),
+	CLK_MSR_ID(182, "am_ring_out2"),
+	CLK_MSR_ID(183, "am_ring_out3"),
+	CLK_MSR_ID(184, "am_ring_out4"),
+	CLK_MSR_ID(185, "am_ring_out5"),
+	CLK_MSR_ID(186, "am_ring_out6"),
+	CLK_MSR_ID(187, "am_ring_out7"),
+	CLK_MSR_ID(188, "am_ring_out8"),
+	CLK_MSR_ID(189, "am_ring_out9"),
+	CLK_MSR_ID(190, "am_ring_out10"),
+	CLK_MSR_ID(191, "am_ring_out11"),
+	CLK_MSR_ID(192, "am_ring_out12"),
+	CLK_MSR_ID(193, "am_ring_out13"),
+	CLK_MSR_ID(194, "am_ring_out14"),
+	CLK_MSR_ID(195, "am_ring_out15"),
+	CLK_MSR_ID(196, "am_ring_out16"),
+	CLK_MSR_ID(197, "am_ring_out17"),
+	CLK_MSR_ID(198, "am_ring_out18"),
+	CLK_MSR_ID(199, "am_ring_out19"),
+	CLK_MSR_ID(200, "mipi_csi_phy0"),
+	CLK_MSR_ID(201, "mipi_csi_phy1"),
+	CLK_MSR_ID(202, "mipi_csi_phy2"),
+	CLK_MSR_ID(203, "mipi_csi_phy3"),
+	CLK_MSR_ID(204, "vid_pll1_div"),
+	CLK_MSR_ID(205, "vid_pll2_div"),
+	CLK_MSR_ID(206, "am_ring_out20"),
+	CLK_MSR_ID(207, "am_ring_out21"),
+	CLK_MSR_ID(208, "am_ring_out22"),
+	CLK_MSR_ID(209, "am_ring_out23"),
+	CLK_MSR_ID(210, "am_ring_out24"),
+	CLK_MSR_ID(211, "am_ring_out25"),
+	CLK_MSR_ID(212, "am_ring_out26"),
+	CLK_MSR_ID(213, "am_ring_out27"),
+	CLK_MSR_ID(214, "am_ring_out28"),
+	CLK_MSR_ID(215, "am_ring_out29"),
+	CLK_MSR_ID(216, "am_ring_out30"),
+	CLK_MSR_ID(217, "am_ring_out31"),
+	CLK_MSR_ID(218, "am_ring_out32"),
+	CLK_MSR_ID(219, "enc0_if"),
+	CLK_MSR_ID(220, "enc2"),
+	CLK_MSR_ID(221, "enc1"),
+	CLK_MSR_ID(222, "enc0")
+};
+
 static int meson_measure_id(struct meson_msr_id *clk_msr_id,
 			    unsigned int duration)
 {
@@ -1026,6 +1278,18 @@ static const struct meson_msr_data clk_msr_s4_data = {
 	.reg = &msr_reg_offset_v2,
 };
 
+static const struct meson_msr_data clk_msr_a1_data = {
+	.msr_table = (void *)clk_msr_a1,
+	.msr_count = ARRAY_SIZE(clk_msr_a1),
+	.reg = &msr_reg_offset_v2,
+};
+
+static const struct meson_msr_data clk_msr_t7_data = {
+	.msr_table = (void *)clk_msr_t7,
+	.msr_count = ARRAY_SIZE(clk_msr_t7),
+	.reg = &msr_reg_offset_v2,
+};
+
 static const struct of_device_id meson_msr_match_table[] = {
 	{
 		.compatible = "amlogic,meson-gx-clk-measure",
@@ -1059,6 +1323,14 @@ static const struct of_device_id meson_msr_match_table[] = {
 		.compatible = "amlogic,s4-clk-measure",
 		.data = &clk_msr_s4_data,
 	},
+	{
+		.compatible = "amlogic,a1-clk-measure",
+		.data = &clk_msr_a1_data,
+	},
+	{
+		.compatible = "amlogic,t7-clk-measure",
+		.data = &clk_msr_t7_data,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, meson_msr_match_table);
-- 
2.47.1



^ permalink raw reply related

* [PATCH 1/4] dt-bindings: soc: amlogic: clk-measure: Add A1 and T7 compatible
From: Jian Hu @ 2026-04-10 10:03 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
	Martin Blumenstingl, robh+dt, Rob Herring, Krzysztof Kozlowski
  Cc: Jian Hu, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20260410100329.3167482-1-jian.hu@amlogic.com>

Add the Amlogic A1 and T7 compatible for the clk-measurer IP.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 .../bindings/soc/amlogic/amlogic,meson-gx-clk-measure.yaml      | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-gx-clk-measure.yaml b/Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-gx-clk-measure.yaml
index 39d4637c2d08..b1200e6940ac 100644
--- a/Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-gx-clk-measure.yaml
+++ b/Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-gx-clk-measure.yaml
@@ -24,6 +24,8 @@ properties:
       - amlogic,meson-sm1-clk-measure
       - amlogic,c3-clk-measure
       - amlogic,s4-clk-measure
+      - amlogic,a1-clk-measure
+      - amlogic,t7-clk-measure
 
   reg:
     maxItems: 1
-- 
2.47.1



^ permalink raw reply related

* [PATCH 0/4] soc: amlogic: clk-measure: add A1 and T7 support
From: Jian Hu @ 2026-04-10 10:03 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
	Martin Blumenstingl, robh+dt, Rob Herring, Krzysztof Kozlowski
  Cc: Jian Hu, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel

This series adds Amlogic clock measurement support for A1 and T7 SoCs,
including binding updates, driver additions, and device tree enablement.

Jian Hu (4):
  dt-bindings: soc: amlogic: clk-measure: Add A1 and T7 compatible
  soc: amlogic: clk-measure: Add A1 and T7 support
  arm64: dts: meson: a1: Add clk measure support
  arm64: dts: amlogic: t7: Add clk measure support

 .../amlogic/amlogic,meson-gx-clk-measure.yaml |   2 +
 arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi   |   5 +
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi     |   5 +
 drivers/soc/amlogic/meson-clk-measure.c       | 272 ++++++++++++++++++
 4 files changed, 284 insertions(+)

-- 
2.47.1



^ permalink raw reply

* Re: [PATCH] pinctrl: mediatek: moore: implement gpio_chip::get_direction()
From: Bartosz Golaszewski @ 2026-04-10  9:54 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: bartosz.golaszewski, linux, sean.wang, linusw, matthias.bgg,
	angelogioacchino.delregno, linux-mediatek, linux-gpio,
	linux-kernel, linux-arm-kernel
In-Reply-To: <trinity-5e6f6a95-e576-4f97-9085-c6de21945eab-1775813076268@trinity-msg-rest-gmx-gmx-live-5cf7d7879b-qwfn5>

On Fri, Apr 10, 2026 at 11:24 AM Frank Wunderlich
<frank-w@public-files.de> wrote:
>
> > Gesendet: Freitag, 10. April 2026 um 09:09
> > Von: "Bartosz Golaszewski" <bartosz.golaszewski@oss.qualcomm.com>
> > An: "Frank Wunderlich" <linux@fw-web.de>, "Sean Wang" <sean.wang@kernel.org>, "Linus Walleij" <linusw@kernel.org>, "Matthias Brugger" <matthias.bgg@gmail.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>, "Bartosz Golaszewski" <brgl@kernel.org>
> > CC: linux-mediatek@lists.infradead.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Bartosz Golaszewski" <bartosz.golaszewski@oss.qualcomm.com>
> > Betreff: [PATCH] pinctrl: mediatek: moore: implement gpio_chip::get_direction()
> >
> > If the gpio_chip::get_direction() callback is not implemented by the GPIO
> > controller driver, GPIOLIB emits a warning.
> >
> > Implement get_direction() for the GPIO part of pinctrl-moore.
> >
> > Fixes: 471e998c0e31 ("gpiolib: remove redundant callback check")
> > Fixes: e623c4303ed1 ("gpiolib: sanitize the return value of gpio_chip::get_direction()")
> > Reported-by: Frank Wunderlich <linux@fw-web.de>
>
> please use the email i used for SoB in my linked patch (closes link below), the other email i use only for sending patches due to mail provider limitation.
>

Linus: Can you fix this when applying, please?

Frank: Can you also leave your Tested-by under the patch?

Thanks,
Bartosz


^ permalink raw reply

* Re: [GIT PULL] Rockchip dts32 changes for 7.1 #2
From: Heiko Stuebner @ 2026-04-10  9:54 UTC (permalink / raw)
  To: arm; +Cc: soc, linux-rockchip, linux-arm-kernel, Stephen Boyd, mturquette
In-Reply-To: <13980380.dW097sEU6C@phil>

Am Freitag, 3. April 2026, 15:39:31 Mitteleuropäische Sommerzeit schrieb Heiko Stuebner:
> Hi soc maintainers,
> 
> please find below a new ARM32 Rockchip SoC for 7.1 . This goes on top
> of the generic arm32 changes I just sent.
> 
> 
> I've split this off from the other ARM32 changes, because this contains
> a shared clock header, shared between the devicetree side and the clock-
> driver side.
> 
> The clock pull-request is sent [0], but not merged yet - probably after
> easter I guess.
> 
> And while in the past this has always come together in time for the
> merge-window, I wasn't sure if in the soc multi-maintainer context the
> handling changes. So depending on your preference this could also wait
> until after the clock-subsystem-side got merged.

clock maintainers seem to be on vacation since 2026-03-25, so I'm not
so sure merging the related clock driver will happen in time anymore,
hence this PR should probably be skipped for now.

Ill rebase the SoC changes onto 7.1-rc1 once the clock driver gets
merged.


Heiko




^ permalink raw reply

* Re: [PATCH 7/7] clk: sunxi-ng: Add Allwinner A733 RTC CCU support
From: Junhui Liu @ 2026-04-10  9:49 UTC (permalink / raw)
  To: wens, Junhui Liu
  Cc: Michael Turquette, Stephen Boyd, Jernej Skrabec, Samuel Holland,
	Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Maxime Ripard, linux-clk, linux-arm-kernel, linux-sunxi,
	linux-kernel, linux-rtc, devicetree, André Przywara
In-Reply-To: <CAGb2v64euL+QNXiJdTn0JygYLXg0WoguPSprKT4sKGZGVZbwug@mail.gmail.com>

On Sat Mar 28, 2026 at 10:41 PM CST, Chen-Yu Tsai wrote:
> On Wed, Jan 21, 2026 at 7:04 PM Junhui Liu <junhui.liu@pigmoral.tech> wrote:
>>
>> Add support for the internal CCU found in the RTC module of the Allwinner
>> A733 SoC. While the basic 16MHz (IOSC) and 32kHz logic remains compatible
>> with older SoCs like the sun6i, the A733 introduces several new features.
>>
>> The A733 RTC CCU supports choosing one of three external crystal
>> frequencies: 19.2MHz, 24MHz, and 26MHz. It features hardware detection
>> logic to automatically identify the frequency used on the board and
>> exports this DCXO signal as the "hosc" clock.
>>
>> Furthermore, the driver implements logic to derive a 32kHz reference
>> from the HOSC. This is achieved through a muxed clock path using fixed
>> pre-dividers to normalize the different crystal frequencies to ~32kHz.
>
> Have you tested whether the actually normalizes the frequency, i.e.
> selects a different divider based on the DCXO frequency? Otherwise
> we're just lying about the frequency.

I only have A733 boards with 26MHz crystals, so I couldn't test all
crystal configurations. However, I exported the "hosc_32k" clock
(referred to as dcxo24M_div32k_clk in the vendor driver) to a physical
pin via the fanout path and measured it with the oscilloscope.

Observations:

- Normal conditions: The frequency remains stable within the 32.744 kHz
  to 32.791 kHz range.
- Forced condition: I grounded the R24 resistor on radxa A7A board to
  trick the SoC into detecting a 24MHz crystal while the actual input
  remained 26MHz. In this case, the frequency became unstable but still
  stayed around the 32.2 kHz to 33.3 kHz range.

Based on these results, it appears the hardware does attempt to
normalize the frequency towards 32.768 kHz via some internal logic.

>
>> This path reuses the same hardware mux registers as the HOSC clock.
>>
>> Additionally, this CCU provides several gate clocks for specific
>> peripherals, including SerDes, HDMI, and UFS. The driver is implemented
>> as an auxiliary driver to be bound to the sun6i-rtc driver.
>>
>> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
>> ---
>>  drivers/clk/sunxi-ng/Kconfig               |   5 +
>>  drivers/clk/sunxi-ng/Makefile              |   2 +
>>  drivers/clk/sunxi-ng/ccu-sun60i-a733-rtc.c | 204 +++++++++++++++++++++++++++++
>>  drivers/clk/sunxi-ng/ccu-sun60i-a733-rtc.h |  18 +++
>>  drivers/clk/sunxi-ng/ccu_rtc.h             |   7 +
>>  5 files changed, 236 insertions(+)
>>

[...]

>> +
>> +static const struct clk_parent_data hosc_parents[] = {
>> +       { .fw_name = "osc24M" },
>> +       { .fw_name = "osc19M" },
>> +       { .fw_name = "osc26M" },
>> +       { .fw_name = "osc24M" },
>> +};
>
> As mentioned in my reply to the binding, this is wrong. There is only
> one input.
>
> The most you can do is check the rate of the parent clock against the
> detected one, and _scream_ that the DT is wrong. And maybe override
> the reported frequency.

I will add a warning message if the frequency detected by the driver
does not match the one in the DT.

>
> If you want to do the latter, you could add a new fixed rate gated
> clock type to our library. You would fill in the rate before the
> clocks get registered. I probably wouldn't go that far. We want people
> to have correct hardware descriptions.
>
> Funnily enough Allwinner's BSP actually implements a fixed rate gate
> for the next 24M-to-32k divider clock.

Yes, I noticed that as well. I agree, and I will model this path as a
simple fixed-rate clock (32768Hz) in v2.

>
>> +
>> +struct ccu_mux hosc_clk = {
>> +       .enable = DCXO_CTRL_DCXO_EN,
>> +       .mux    = _SUNXI_CCU_MUX(14, 2),
>> +       .common = {
>> +               .reg            = DCXO_CTRL_REG,
>> +               .hw.init        = CLK_HW_INIT_PARENTS_DATA("hosc",
>> +                                                          hosc_parents,
>> +                                                          &ccu_mux_ro_ops,
>> +                                                          0),
>> +       },
>> +};
>
> So this is wrong.
>
>> +
>> +static const struct ccu_mux_fixed_prediv hosc_32k_predivs[] = {
>> +       { .index = 0, .div = 732 },
>
> Why is it 732 instead of 750?

As mentioned above, the target frequency is 32.768kHz rather than
32.0kHz. However, since I will drop this prediv array and use a
fixed-rate clock instead, I think this will no longer be an issue.

-- 
Best regards,
Junhui Liu



^ permalink raw reply


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