[PATCH v2] ARM64: dts: mcbin: fix SATA ports on Macchiatobin

[PATCH v2] ARM64: dts: mcbin: fix SATA ports on Macchiatobin

[Russell King (Oracle)]

Booting 6.16 on the Macchiatobin, I discover that I can no longer
access my disks, and thus the userspace boot fails. The cause appears
to be that one of the SATA controllers doesn't have any ports:

[    1.190312] ahci f4540000.sata: supply ahci not found, using dummy regulator
[    1.196255] ahci f4540000.sata: supply phy not found, using dummy regulator
[    1.202026] ahci f4540000.sata: No port enabled

This is as a result of the blamed commit below which added a default
disabled status to the .dtsi, but didn't properly update the mcbin
dtsi file. Fix this regression.

Fixes: 30023876aef4 ("arm64: dts: marvell: only enable complete sata nodes")
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
index 6170ca8f908f..8c7db2e87e30 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
@@ -388,11 +388,13 @@ &cp1_sata0 {
 	/* CPS Lane 1 - U32 */
 	sata-port@0 {
 		phys = <&cp1_comphy1 0>;
+		status = "okay";
 	};
 
 	/* CPS Lane 3 - U31 */
 	sata-port@1 {
 		phys = <&cp1_comphy3 1>;
+		status = "okay";
 	};
 };
 
-- 
2.30.2

Re: [PATCH v2] ARM64: dts: mcbin: fix SATA ports on Macchiatobin

[Russell King (Oracle)]

On Thu, Aug 21, 2025 at 06:43:28PM +0100, Russell King (Oracle) wrote:
> Booting 6.16 on the Macchiatobin, I discover that I can no longer
> access my disks, and thus the userspace boot fails. The cause appears
> to be that one of the SATA controllers doesn't have any ports:
> 
> [    1.190312] ahci f4540000.sata: supply ahci not found, using dummy regulator
> [    1.196255] ahci f4540000.sata: supply phy not found, using dummy regulator
> [    1.202026] ahci f4540000.sata: No port enabled
> 
> This is as a result of the blamed commit below which added a default
> disabled status to the .dtsi, but didn't properly update the mcbin
> dtsi file. Fix this regression.
> 
> Fixes: 30023876aef4 ("arm64: dts: marvell: only enable complete sata nodes")
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Frank,

I think this is also similarly broken by your patch:

arch/arm64/boot/dts/marvell/armada-8040-db.dts

as you've updated the ports on one SATA controller but not the other
in the same way as you omitted the second controller on mcbin.

I'd also question this:

arch/arm64/boot/dts/marvell/cn9132-clearfog.dts

as you updated the other cn9132, but not this one which was introduced
in 6.11, and your change was in 6.13. Please can you look at both of
these and send appropriate fixes?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2] ARM64: dts: mcbin: fix SATA ports on Macchiatobin

[Frank Wunderlich]

Am 21. August 2025 20:06:20 MESZ schrieb "Russell King (Oracle)" <linux@armlinux.org.uk>:
>On Thu, Aug 21, 2025 at 06:43:28PM +0100, Russell King (Oracle) wrote:
>> Booting 6.16 on the Macchiatobin, I discover that I can no longer
>> access my disks, and thus the userspace boot fails. The cause appears
>> to be that one of the SATA controllers doesn't have any ports:
>> 
>> [    1.190312] ahci f4540000.sata: supply ahci not found, using dummy regulator
>> [    1.196255] ahci f4540000.sata: supply phy not found, using dummy regulator
>> [    1.202026] ahci f4540000.sata: No port enabled
>> 
>> This is as a result of the blamed commit below which added a default
>> disabled status to the .dtsi, but didn't properly update the mcbin
>> dtsi file. Fix this regression.
>> 
>> Fixes: 30023876aef4 ("arm64: dts: marvell: only enable complete sata nodes")
>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>
>Frank,
>
>I think this is also similarly broken by your patch:
>
>arch/arm64/boot/dts/marvell/armada-8040-db.dts
>
>as you've updated the ports on one SATA controller but not the other
>in the same way as you omitted the second controller on mcbin.
>
>I'd also question this:
>
>arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
>
>as you updated the other cn9132, but not this one which was introduced
>in 6.11, and your change was in 6.13. Please can you look at both of
>these and send appropriate fixes?
>
>Thanks.
>

Hi,

I sent it at least twice..maybe this new was added in between.
But i have no marvell board for testing so i cannot verify my changes are correct.
I only tried to fix binding errors.
regards Frank

Re: [PATCH v2] ARM64: dts: mcbin: fix SATA ports on Macchiatobin

[Andrew Lunn]

> Hi,
> 
> I sent it at least twice..maybe this new was added in between.
> But i have no marvell board for testing so i cannot verify my changes are correct.
> I only tried to fix binding errors.
> regards Frank

You might be able to use ./scripts/dtc/dtx_diff

Generate the DTB before the change and after, and they should be
identical if the disable in the dtsi is correctly re-enabled in the
.dts file.

It would be good to check all Marvell DT blobs, both arm and arm64.

     Andrew

Re: [PATCH v2] ARM64: dts: mcbin: fix SATA ports on Macchiatobin

[Russell King (Oracle)]

On Thu, Aug 21, 2025 at 08:25:21PM +0200, Andrew Lunn wrote:
> > Hi,
> > 
> > I sent it at least twice..maybe this new was added in between.
> > But i have no marvell board for testing so i cannot verify my changes are correct.
> > I only tried to fix binding errors.
> > regards Frank
> 
> You might be able to use ./scripts/dtc/dtx_diff
> 
> Generate the DTB before the change and after, and they should be
> identical if the disable in the dtsi is correctly re-enabled in the
> .dts file.

I'm not sure that would help, because before Frank's patch, there were
no "status" properties in any of the sata-port nodes. After Frank's
patch, we end up with a mixture of "okay" and "disabled" depending
on whether the port is actually used - and the problem here is catching
all those that are actually being used on the hardware. In other words,
those notes that should have status = "okay" rather than status =
"disabled" added to them.

As I just stated, using grep:

$ grep -rA4 sata-port@ arch/arm64/boot/dts/marvell

is sufficient to find all the locations in the dts(i) files that have
a sata-port node defined, and in the context gives the rest of the
node properties, thus showing whether there is a status property present
or missing.

This is _exactly_ how I've identified that there are two more platforms
that are similarly broken.

This is not something that one can trivially check by doing a boot test
on platforms - I missed it despite booting the kernel on another
Macchiatobin as it's only a single kernel message amongst many that
says that something's wrong.

The only real way of doing this kind of change is to use grep and
similar to check that all sites have been caught.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2] ARM64: dts: mcbin: fix SATA ports on Macchiatobin

[Russell King (Oracle)]

On Thu, Aug 21, 2025 at 08:20:07PM +0200, Frank Wunderlich wrote:
> Am 21. August 2025 20:06:20 MESZ schrieb "Russell King (Oracle)" <linux@armlinux.org.uk>:
> >On Thu, Aug 21, 2025 at 06:43:28PM +0100, Russell King (Oracle) wrote:
> >> Booting 6.16 on the Macchiatobin, I discover that I can no longer
> >> access my disks, and thus the userspace boot fails. The cause appears
> >> to be that one of the SATA controllers doesn't have any ports:
> >> 
> >> [    1.190312] ahci f4540000.sata: supply ahci not found, using dummy regulator
> >> [    1.196255] ahci f4540000.sata: supply phy not found, using dummy regulator
> >> [    1.202026] ahci f4540000.sata: No port enabled
> >> 
> >> This is as a result of the blamed commit below which added a default
> >> disabled status to the .dtsi, but didn't properly update the mcbin
> >> dtsi file. Fix this regression.
> >> 
> >> Fixes: 30023876aef4 ("arm64: dts: marvell: only enable complete sata nodes")
> >> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> >
> >Frank,
> >
> >I think this is also similarly broken by your patch:
> >
> >arch/arm64/boot/dts/marvell/armada-8040-db.dts
> >
> >as you've updated the ports on one SATA controller but not the other
> >in the same way as you omitted the second controller on mcbin.
> >
> >I'd also question this:
> >
> >arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> >
> >as you updated the other cn9132, but not this one which was introduced
> >in 6.11, and your change was in 6.13. Please can you look at both of
> >these and send appropriate fixes?
> >
> >Thanks.
> >
> 
> Hi,
> 
> I sent it at least twice..maybe this new was added in between.

Nope. You patched Macchiatobin badly - you failed to use grep to find
all the sites that you needed to update.

$ grep -rA4 sata-port@ arch/arm64/boot/dts/marvell

would've shown you where you need to patch.

The timeline here is:

- I added support for Macchiatobin in November 2018. There was no need
  to describe the ports at that point.

- Miquel Raynal updated the description in July 2019 to add the
  sata-port nodes for each of the _three_ ports that are present on
  the hardware.

- You patched the file in January 2025, only touching the cp0_sata0
  device, ignoring the cp1_sata0 description below it.

So no, it has not been added since, it was always there.

> But i have no marvell board for testing so i cannot verify my changes
> are correct.

It will only show up if one has a platform that has disks connected
to all three SATA connectors, otherwise it's lost in the kernel boot
log noise.

The only way to do this is to take care, use grep to find all the
sites that need to be updated, make the changes, and then check again
using grep that you have indeed got all the necessary sites.

> I only tried to fix binding errors.

I have to say that this is the root of the problem - while it may
seem helpful to fix warnings etc one sees, it is only helpful if they
are done carefully and with utmost care to avoid the attempt causing
a regression.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2] ARM64: dts: mcbin: fix SATA ports on Macchiatobin

[Gregory CLEMENT]

"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> writes:

> Booting 6.16 on the Macchiatobin, I discover that I can no longer
> access my disks, and thus the userspace boot fails. The cause appears
> to be that one of the SATA controllers doesn't have any ports:
>
> [    1.190312] ahci f4540000.sata: supply ahci not found, using dummy regulator
> [    1.196255] ahci f4540000.sata: supply phy not found, using dummy regulator
> [    1.202026] ahci f4540000.sata: No port enabled
>
> This is as a result of the blamed commit below which added a default
> disabled status to the .dtsi, but didn't properly update the mcbin
> dtsi file. Fix this regression.
>
> Fixes: 30023876aef4 ("arm64: dts: marvell: only enable complete sata nodes")
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Applied on mvebu/fixes

Thanks,

Gregory

> ---
>  arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> index 6170ca8f908f..8c7db2e87e30 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> @@ -388,11 +388,13 @@ &cp1_sata0 {
>  	/* CPS Lane 1 - U32 */
>  	sata-port@0 {
>  		phys = <&cp1_comphy1 0>;
> +		status = "okay";
>  	};
>  
>  	/* CPS Lane 3 - U31 */
>  	sata-port@1 {
>  		phys = <&cp1_comphy3 1>;
> +		status = "okay";
>  	};
>  };
>  
> -- 
> 2.30.2
>

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com