[PATCH] dts: riscv: spacemit: k3: Fix I/O power settings

[PATCH] dts: riscv: spacemit: k3: Fix I/O power settings

[Yixun Lan]

SpacemiT K3 SoC support dual-voltage I/O power domain, while initially
configure to 3.3v, and need to access register from APBC space to switch
to 1.8v domain.

Fix the GMAC0's I/O pins 1.8v switch failure that will result a broken
ethernet driver.

Fixes: d8944577496b ("riscv: dts: spacemit: k3: add pinctrl support")
Reported-by: Han Gao <gaohan@iscas.ac.cn>
Signed-off-by: Yixun Lan <dlan@kernel.org>
---
This issue is reported by Han while booting kernel from UFS storage, and
found the ethernet driver is broken, we can see the message as below:

 [    1.635881] mdio_bus stmmac-0: MDIO device at address 1 is missing.

This problem is caused by GMAC driver that unable to configure I/O power
domain to 1.8v, which leads to the MDIO communication failure.

We didn't find this problem due to using tftp(net) to load kernel images
while bootloader already configured the I/O power.
---
 arch/riscv/boot/dts/spacemit/k3.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
index 815debd16409..2b0bf9cd755b 100644
--- a/arch/riscv/boot/dts/spacemit/k3.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k3.dtsi
@@ -803,6 +803,7 @@ pinctrl: pinctrl@d401e000 {
 			clocks = <&syscon_apbc CLK_APBC_AIB>,
 				 <&syscon_apbc CLK_APBC_AIB_BUS>;
 			clock-names = "func", "bus";
+			spacemit,apbc = <&syscon_apbc>;
 		};
 
 		uart10: serial@d401f000 {

---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260518-07-dts-pinctrl-io-power-1bd33bfe5894

Best regards,
--  
Yixun Lan <dlan@kernel.org>

Re: [PATCH] dts: riscv: spacemit: k3: Fix I/O power settings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `spacemit,apbc` DT property triggers a spurious boot warning and relies on a fragile probe deferral dependency, because the target `syscon_apbc` node lacks the `"syscon"` compatible string.
--

commit 89bb8c89cecf7c32a2e825a2b8f604c5c45762bd
Author: Yixun Lan <dlan@kernel.org>

dts: riscv: spacemit: k3: Fix I/O power settings
    
SpacemiT K3 SoC support dual-voltage I/O power domain, while initially
configure to 3.3v, and need to access register from APBC space to switch
to 1.8v domain.
    
Fix the GMAC0's I/O pins 1.8v switch failure that will result a broken
ethernet driver.

> diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
> index 815debd16409b..2b0bf9cd755bd 100644
> --- a/arch/riscv/boot/dts/spacemit/k3.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k3.dtsi
> @@ -803,6 +803,7 @@ pinctrl: pinctrl@d401e000 {
>  			clocks = <&syscon_apbc CLK_APBC_AIB>,
>  				 <&syscon_apbc CLK_APBC_AIB_BUS>;
>  			clock-names = "func", "bus";
> +			spacemit,apbc = <&syscon_apbc>;

Does this newly added spacemit,apbc property trigger a spurious boot warning
and rely on a fragile probe deferral dependency?

Looking at the referenced syscon_apbc node, it lacks the "syscon" compatible
string.

This prevents syscon_node_to_regmap() from instantiating the regmap
on-the-fly, causing it to return -EPROBE_DEFER if the pinctrl driver probes
before the clock driver registers the regmap.

The pinctrl driver catches this with a blanket IS_ERR() check and prints a
now-spurious "no syscon found" warning during boot while discarding the
regmap pointer:

drivers/pinctrl/spacemit/pinctrl-k1.c:spacemit_pinctrl_probe() {
	...
	pctrl->regmap_apbc = syscon_regmap_lookup_by_phandle(np, "spacemit,apbc");
	if (IS_ERR(pctrl->regmap_apbc)) {
		dev_warn(dev, "no syscon found, disable power voltage switch functionality\n");
		pctrl->regmap_apbc = NULL;
	}
	...
}

The voltage switch functionality only ultimately succeeds because the
subsequent devm_clk_get_enabled() call against the same node also returns
-EPROBE_DEFER.

This gives the clock driver time to probe and register the regmap before the
pinctrl driver retries.

Would adding "syscon" to the syscon_apbc node's compatible string, or
explicitly handling -EPROBE_DEFER in the pinctrl driver, resolve this issue
and prevent the spurious warning?

>  		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-07-dts-pinctrl-io-power-v1-1-abe19c14a726@kernel.org?part=1

Re: [PATCH] dts: riscv: spacemit: k3: Fix I/O power settings

[Yixun Lan]

Hi 

On 21:15 Mon 18 May     , sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added `spacemit,apbc` DT property triggers a spurious boot warning and relies on a fragile probe deferral dependency, because the target `syscon_apbc` node lacks the `"syscon"` compatible string.
> --
> 
> commit 89bb8c89cecf7c32a2e825a2b8f604c5c45762bd
> Author: Yixun Lan <dlan@kernel.org>
> 
> dts: riscv: spacemit: k3: Fix I/O power settings
>     
> SpacemiT K3 SoC support dual-voltage I/O power domain, while initially
> configure to 3.3v, and need to access register from APBC space to switch
> to 1.8v domain.
>     
> Fix the GMAC0's I/O pins 1.8v switch failure that will result a broken
> ethernet driver.
> 
> > diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
> > index 815debd16409b..2b0bf9cd755bd 100644
> > --- a/arch/riscv/boot/dts/spacemit/k3.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k3.dtsi
> > @@ -803,6 +803,7 @@ pinctrl: pinctrl@d401e000 {
> >  			clocks = <&syscon_apbc CLK_APBC_AIB>,
> >  				 <&syscon_apbc CLK_APBC_AIB_BUS>;
> >  			clock-names = "func", "bus";
> > +			spacemit,apbc = <&syscon_apbc>;
> 
> Does this newly added spacemit,apbc property trigger a spurious boot warning
> and rely on a fragile probe deferral dependency?
> 
> Looking at the referenced syscon_apbc node, it lacks the "syscon" compatible
> string.
> 
> This prevents syscon_node_to_regmap() from instantiating the regmap
> on-the-fly, causing it to return -EPROBE_DEFER if the pinctrl driver probes
> before the clock driver registers the regmap.
> 
This is possible, I think it's worth the effort to add some logic to
handle the deferral probe situation

> The pinctrl driver catches this with a blanket IS_ERR() check and prints a
> now-spurious "no syscon found" warning during boot while discarding the
> regmap pointer:
> 
> drivers/pinctrl/spacemit/pinctrl-k1.c:spacemit_pinctrl_probe() {
> 	...
> 	pctrl->regmap_apbc = syscon_regmap_lookup_by_phandle(np, "spacemit,apbc");
> 	if (IS_ERR(pctrl->regmap_apbc)) {
> 		dev_warn(dev, "no syscon found, disable power voltage switch functionality\n");
> 		pctrl->regmap_apbc = NULL;
> 	}
> 	...
> }
> 
> The voltage switch functionality only ultimately succeeds because the
> subsequent devm_clk_get_enabled() call against the same node also returns
> -EPROBE_DEFER.
> 
> This gives the clock driver time to probe and register the regmap before the
> pinctrl driver retries.
> 
make sense

> Would adding "syscon" to the syscon_apbc node's compatible string, or
> explicitly handling -EPROBE_DEFER in the pinctrl driver, resolve this issue
> and prevent the spurious warning?
> 
I prefer to do the latter way

-- 
Yixun Lan (dlan)