[PATCH 1/2] arm64: dts: uDPU: update partition table

[PATCH 1/2] arm64: dts: uDPU: update partition table

[Robert Marko]

Partition currently called "uboot" does not only contain U-boot, but
rather it contains TF-A, U-boot and U-boot environment.

So, to avoid accidentally deleting the U-boot environment which is
located at 0x180000 split the partition.

"uboot" is not the correct name as you can't boot these boards with U-boot
only, TF-A must be present as well, so rename the "uboot" partition to
"firmware".

While we are here, describe the NOR node as "spi-flash@0" instead of
"m25p80@0" which is the old SPI-NOR driver name.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
index 95d46e8d081c..ac64949bb53e 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
@@ -99,7 +99,7 @@ &spi0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&spi_quad_pins>;
 
-	m25p80@0 {
+	spi-flash@0 {
 		compatible = "jedec,spi-nor";
 		reg = <0>;
 		spi-max-frequency = <54000000>;
@@ -108,10 +108,15 @@ partitions {
 			compatible = "fixed-partitions";
 			#address-cells = <1>;
 			#size-cells = <1>;
-			/* only bootloader is located on the SPI */
+
 			partition@0 {
-				label = "uboot";
-				reg = <0 0x400000>;
+				label = "firmware";
+				reg = <0x0 0x180000>;
+			};
+
+			partition@180000 {
+				label = "u-boot-env";
+				reg = <0x180000 0x10000>;
 			};
 		};
 	};
-- 
2.35.1

[PATCH 2/2] arm64: dts: uDPU: correct temperature sensors

[Robert Marko]

uDPU has a pair of NCT375 temperature sensors, which are TMP75C compatible
as far as the driver is concerned.

The current LM75 compatible worked as all of the LM75 compatible sensors
are backwards compatible with the original part, but it meant that lower
resolution and incorrect sample rate was being used.

The "lm75" compatible has been deprecated anyway and is meant as fallback
in order to keep older DTS-es working.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
index ac64949bb53e..1f534c0c65f7 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
@@ -153,15 +153,15 @@ &i2c1 {
 	scl-gpios = <&gpionb 2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
 	sda-gpios = <&gpionb 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
 
-	lm75@48 {
+	nct375@48 {
 		status = "okay";
-		compatible = "lm75";
+		compatible = "ti,tmp75c";
 		reg = <0x48>;
 	};
 
-	lm75@49 {
+	nct375@49 {
 		status = "okay";
-		compatible = "lm75";
+		compatible = "ti,tmp75c";
 		reg = <0x49>;
 	};
 };
-- 
2.35.1

Re: [PATCH 1/2] arm64: dts: uDPU: update partition table

[Andrew Lunn]

On Mon, Mar 21, 2022 at 01:17:27PM +0100, Robert Marko wrote:
> Partition currently called "uboot" does not only contain U-boot, but
> rather it contains TF-A, U-boot and U-boot environment.
> 
> So, to avoid accidentally deleting the U-boot environment which is
> located at 0x180000 split the partition.
> 
> "uboot" is not the correct name as you can't boot these boards with U-boot
> only, TF-A must be present as well, so rename the "uboot" partition to
> "firmware".

Are there any ABI issues here? If these names are being used
somewhere, you are potentially breaking the boot. At minimum, i would
like to see something in the commit message which indicates you have
considered this and why you don't expect it to be a problem.

	   Andrew

Re: [PATCH 1/2] arm64: dts: uDPU: update partition table

[Robert Marko]

On Mon, Mar 21, 2022 at 1:41 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Mar 21, 2022 at 01:17:27PM +0100, Robert Marko wrote:
> > Partition currently called "uboot" does not only contain U-boot, but
> > rather it contains TF-A, U-boot and U-boot environment.
> >
> > So, to avoid accidentally deleting the U-boot environment which is
> > located at 0x180000 split the partition.
> >
> > "uboot" is not the correct name as you can't boot these boards with U-boot
> > only, TF-A must be present as well, so rename the "uboot" partition to
> > "firmware".
>
> Are there any ABI issues here? If these names are being used
> somewhere, you are potentially breaking the boot. At minimum, i would
> like to see something in the commit message which indicates you have
> considered this and why you don't expect it to be a problem.

Hi Andrew, this won't break booting as BootROM does not care about
partitions nor naming, it will just go to 0x0 of the NOR and boot it.

The same renaming had already been done:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/marvell?h=v5.17&id=a9d9bfcadfb43b856dbcf9419de75f7420d5a225

I can update the commit message if required.
Regards,
Robert

>
>            Andrew



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH 1/2] arm64: dts: uDPU: update partition table

[Pali Rohár]

On Monday 21 March 2022 13:41:39 Andrew Lunn wrote:
> On Mon, Mar 21, 2022 at 01:17:27PM +0100, Robert Marko wrote:
> > Partition currently called "uboot" does not only contain U-boot, but
> > rather it contains TF-A, U-boot and U-boot environment.
> > 
> > So, to avoid accidentally deleting the U-boot environment which is
> > located at 0x180000 split the partition.
> > 
> > "uboot" is not the correct name as you can't boot these boards with U-boot
> > only, TF-A must be present as well, so rename the "uboot" partition to
> > "firmware".
> 
> Are there any ABI issues here? If these names are being used
> somewhere, you are potentially breaking the boot. At minimum, i would
> like to see something in the commit message which indicates you have
> considered this and why you don't expect it to be a problem.

Hello! Similar change/fix was done also for Turris Mox DTS file:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9d9bfcadfb43b856dbcf9419de75f7420d5a225

Difference between uDPU and Mox is that uDPU has concatenated
CM3 firmware (called secure firmware) and A53 firmware (TF-A + u-boot)
into one binary, Mox has these two parts separated. So uDPU partition
should be called just firmware.

Same partition name "firmware" is used also on another A3720 device
Espressobin. But here partition names are filled/injected into DTB at
runtime by bootloader.

So I think this change for uDPU looks reasonable. Partition name
"u-boot" is used for flashing u-boot binary and renaming it could really
fix automated scripts to stop flashing into SPI NOR something which
makes device unbootable.

Re: [PATCH 1/2] arm64: dts: uDPU: update partition table

[Andrew Lunn]

On Mon, Mar 21, 2022 at 01:47:21PM +0100, Robert Marko wrote:
> On Mon, Mar 21, 2022 at 1:41 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Mon, Mar 21, 2022 at 01:17:27PM +0100, Robert Marko wrote:
> > > Partition currently called "uboot" does not only contain U-boot, but
> > > rather it contains TF-A, U-boot and U-boot environment.
> > >
> > > So, to avoid accidentally deleting the U-boot environment which is
> > > located at 0x180000 split the partition.
> > >
> > > "uboot" is not the correct name as you can't boot these boards with U-boot
> > > only, TF-A must be present as well, so rename the "uboot" partition to
> > > "firmware".
> >
> > Are there any ABI issues here? If these names are being used
> > somewhere, you are potentially breaking the boot. At minimum, i would
> > like to see something in the commit message which indicates you have
> > considered this and why you don't expect it to be a problem.
> 
> Hi Andrew, this won't break booting as BootROM does not care about
> partitions nor naming, it will just go to 0x0 of the NOR and boot it.
> 
> The same renaming had already been done:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/marvell?h=v5.17&id=a9d9bfcadfb43b856dbcf9419de75f7420d5a225
> 
> I can update the commit message if required.

Yes, please. It is always good to indicate you have thought about
issues like this. Not breaking the ABI is very important, and could
result in this getting reverted at some point. So indicating why you
think this is safe is useful information.

      Andrew

Re: [PATCH 1/2] arm64: dts: uDPU: update partition table

[Pali Rohár]

On Monday 21 March 2022 13:17:27 Robert Marko wrote:
> Partition currently called "uboot" does not only contain U-boot, but
> rather it contains TF-A, U-boot and U-boot environment.
> 
> So, to avoid accidentally deleting the U-boot environment which is
> located at 0x180000 split the partition.
> 
> "uboot" is not the correct name as you can't boot these boards with U-boot
> only, TF-A must be present as well, so rename the "uboot" partition to
> "firmware".
> 
> While we are here, describe the NOR node as "spi-flash@0" instead of
> "m25p80@0" which is the old SPI-NOR driver name.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
>  arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)

FYI, U-Boot now contains copy of kernel's A3720 DTS files, so you should
send same patch also to U-Boot project.

> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> index 95d46e8d081c..ac64949bb53e 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> @@ -99,7 +99,7 @@ &spi0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&spi_quad_pins>;
>  
> -	m25p80@0 {
> +	spi-flash@0 {
>  		compatible = "jedec,spi-nor";
>  		reg = <0>;
>  		spi-max-frequency = <54000000>;
> @@ -108,10 +108,15 @@ partitions {
>  			compatible = "fixed-partitions";
>  			#address-cells = <1>;
>  			#size-cells = <1>;
> -			/* only bootloader is located on the SPI */
> +
>  			partition@0 {
> -				label = "uboot";
> -				reg = <0 0x400000>;
> +				label = "firmware";
> +				reg = <0x0 0x180000>;
> +			};
> +
> +			partition@180000 {
> +				label = "u-boot-env";
> +				reg = <0x180000 0x10000>;
>  			};
>  		};
>  	};
> -- 
> 2.35.1
> 

Re: [PATCH 1/2] arm64: dts: uDPU: update partition table

[Robert Marko]

On Mon, Mar 21, 2022 at 2:16 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Mar 21, 2022 at 01:47:21PM +0100, Robert Marko wrote:
> > On Mon, Mar 21, 2022 at 1:41 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > On Mon, Mar 21, 2022 at 01:17:27PM +0100, Robert Marko wrote:
> > > > Partition currently called "uboot" does not only contain U-boot, but
> > > > rather it contains TF-A, U-boot and U-boot environment.
> > > >
> > > > So, to avoid accidentally deleting the U-boot environment which is
> > > > located at 0x180000 split the partition.
> > > >
> > > > "uboot" is not the correct name as you can't boot these boards with U-boot
> > > > only, TF-A must be present as well, so rename the "uboot" partition to
> > > > "firmware".
> > >
> > > Are there any ABI issues here? If these names are being used
> > > somewhere, you are potentially breaking the boot. At minimum, i would
> > > like to see something in the commit message which indicates you have
> > > considered this and why you don't expect it to be a problem.
> >
> > Hi Andrew, this won't break booting as BootROM does not care about
> > partitions nor naming, it will just go to 0x0 of the NOR and boot it.
> >
> > The same renaming had already been done:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/marvell?h=v5.17&id=a9d9bfcadfb43b856dbcf9419de75f7420d5a225
> >
> > I can update the commit message if required.
>
> Yes, please. It is always good to indicate you have thought about
> issues like this. Not breaking the ABI is very important, and could
> result in this getting reverted at some point. So indicating why you
> think this is safe is useful information.

Thanks, will do it in v2.

Regards,
Robert
>
>       Andrew



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH 1/2] arm64: dts: uDPU: update partition table

[Robert Marko]

On Mon, Mar 21, 2022 at 2:41 PM Pali Rohár <pali@kernel.org> wrote:
>
> On Monday 21 March 2022 13:17:27 Robert Marko wrote:
> > Partition currently called "uboot" does not only contain U-boot, but
> > rather it contains TF-A, U-boot and U-boot environment.
> >
> > So, to avoid accidentally deleting the U-boot environment which is
> > located at 0x180000 split the partition.
> >
> > "uboot" is not the correct name as you can't boot these boards with U-boot
> > only, TF-A must be present as well, so rename the "uboot" partition to
> > "firmware".
> >
> > While we are here, describe the NOR node as "spi-flash@0" instead of
> > "m25p80@0" which is the old SPI-NOR driver name.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> >  arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
>
> FYI, U-Boot now contains copy of kernel's A3720 DTS files, so you should
> send same patch also to U-Boot project.

I am working on uDPU fixes for U-boot as well so it will get synced as
part of the series.
Regards,
Robert
>
> > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> > index 95d46e8d081c..ac64949bb53e 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> > @@ -99,7 +99,7 @@ &spi0 {
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&spi_quad_pins>;
> >
> > -     m25p80@0 {
> > +     spi-flash@0 {
> >               compatible = "jedec,spi-nor";
> >               reg = <0>;
> >               spi-max-frequency = <54000000>;
> > @@ -108,10 +108,15 @@ partitions {
> >                       compatible = "fixed-partitions";
> >                       #address-cells = <1>;
> >                       #size-cells = <1>;
> > -                     /* only bootloader is located on the SPI */
> > +
> >                       partition@0 {
> > -                             label = "uboot";
> > -                             reg = <0 0x400000>;
> > +                             label = "firmware";
> > +                             reg = <0x0 0x180000>;
> > +                     };
> > +
> > +                     partition@180000 {
> > +                             label = "u-boot-env";
> > +                             reg = <0x180000 0x10000>;
> >                       };
> >               };
> >       };
> > --
> > 2.35.1
> >



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr