dts: nxp: mxs: Update device tree description for LWE imx28 based devices

[PATCH] dts: nxp: mxs: Update device tree description for LWE imx28 based devices

Posted by Lukasz Majewski 1 year, 3 months ago

The description for imx28 based devices needs to be updated after some
further development.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi | 38 +++++++++---------------
 1 file changed, 14 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi b/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi
index 69fcb0dde940..6fa1f3e25b08 100644
--- a/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi
+++ b/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi
@@ -55,23 +55,6 @@ &i2c0 {
 	status = "okay";
 };
 
-&saif0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&saif0_pins_a>;
-	#sound-dai-cells = <0>;
-	assigned-clocks = <&clks 53>;
-	assigned-clock-rates = <12000000>;
-	status = "okay";
-};
-
-&saif1 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&saif1_pins_a>;
-	fsl,saif-master = <&saif0>;
-	#sound-dai-cells = <0>;
-	status = "okay";
-};
-
 &spi3_pins_a {
 	fsl,pinmux-ids = <
 		MX28_PAD_AUART2_RX__SSP3_D4
@@ -109,7 +92,7 @@ &ssp3 {
 
 	flash@0 {
 		compatible = "jedec,spi-nor";
-		spi-max-frequency = <40000000>;
+		spi-max-frequency = <20000000>;
 		reg = <0>;
 
 		partitions {
@@ -133,14 +116,21 @@ partition@90000 {
 				reg = <0x90000 0x10000>;
 			};
 
-			partition@100000 {
-				label = "kernel";
-				reg = <0x100000 0x400000>;
+			partition@3 {
+				label = "rescue";
+				reg = <0xA0000 0xF40000>;
+			};
+
+			partition@4 {
+				/* 1st sector for SPL boot img source data */
+				label = "spl-boot-data1";
+				reg = <0xFE0000 0x10000>;
 			};
 
-			partition@500000 {
-				label = "swupdate";
-				reg = <0x500000 0x800000>;
+			partition@5 {
+				/* 2nd sector for SPL boot img source data */
+				label = "spl-boot-data2";
+				reg = <0xFF0000 0x10000>;
 			};
 		};
 	};
-- 
2.39.2

Re: [PATCH] dts: nxp: mxs: Update device tree description for LWE imx28 based devices

Posted by Krzysztof Kozlowski 1 year, 3 months ago

On 27/08/2024 10:42, Lukasz Majewski wrote:
> The description for imx28 based devices needs to be updated after some
> further development.

Everything is an update, please be specific in commit subject and message.

> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
>  arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi | 38 +++++++++---------------
>  1 file changed, 14 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi b/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi
> index 69fcb0dde940..6fa1f3e25b08 100644
> --- a/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi
> +++ b/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi
> @@ -55,23 +55,6 @@ &i2c0 {
>  	status = "okay";
>  };
>  
> -&saif0 {
> -	pinctrl-names = "default";
> -	pinctrl-0 = <&saif0_pins_a>;
> -	#sound-dai-cells = <0>;
> -	assigned-clocks = <&clks 53>;
> -	assigned-clock-rates = <12000000>;
> -	status = "okay";
> -};
> -
> -&saif1 {
> -	pinctrl-names = "default";
> -	pinctrl-0 = <&saif1_pins_a>;
> -	fsl,saif-master = <&saif0>;
> -	#sound-dai-cells = <0>;
> -	status = "okay";
> -};
> -
>  &spi3_pins_a {
>  	fsl,pinmux-ids = <
>  		MX28_PAD_AUART2_RX__SSP3_D4
> @@ -109,7 +92,7 @@ &ssp3 {
>  
>  	flash@0 {
>  		compatible = "jedec,spi-nor";
> -		spi-max-frequency = <40000000>;
> +		spi-max-frequency = <20000000>;
>  		reg = <0>;
>  
>  		partitions {
> @@ -133,14 +116,21 @@ partition@90000 {
>  				reg = <0x90000 0x10000>;
>  			};
>  
> -			partition@100000 {
> -				label = "kernel";
> -				reg = <0x100000 0x400000>;
> +			partition@3 {

This does not look right. It is neither explained in commit msg, nor
matching reg.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

> +				label = "rescue";
> +				reg = <0xA0000 0xF40000>;

Lowercase hex

> +			};
> +

Best regards,
Krzysztof

Re: [PATCH] dts: nxp: mxs: Update device tree description for LWE imx28 based devices

Posted by Lukasz Majewski 1 year, 3 months ago

Hi Krzysztof,

> On 27/08/2024 10:42, Lukasz Majewski wrote:
> > The description for imx28 based devices needs to be updated after
> > some further development.  
> 
> Everything is an update, please be specific in commit subject and
> message.

Ok.

> 
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> >  arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi | 38
> > +++++++++--------------- 1 file changed, 14 insertions(+), 24
> > deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi
> > b/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi index
> > 69fcb0dde940..6fa1f3e25b08 100644 ---
> > a/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi +++
> > b/arch/arm/boot/dts/nxp/mxs/imx28-lwe.dtsi @@ -55,23 +55,6 @@ &i2c0
> > { status = "okay";
> >  };
> >  
> > -&saif0 {
> > -	pinctrl-names = "default";
> > -	pinctrl-0 = <&saif0_pins_a>;
> > -	#sound-dai-cells = <0>;
> > -	assigned-clocks = <&clks 53>;
> > -	assigned-clock-rates = <12000000>;
> > -	status = "okay";
> > -};
> > -
> > -&saif1 {
> > -	pinctrl-names = "default";
> > -	pinctrl-0 = <&saif1_pins_a>;
> > -	fsl,saif-master = <&saif0>;
> > -	#sound-dai-cells = <0>;
> > -	status = "okay";
> > -};
> > -
> >  &spi3_pins_a {
> >  	fsl,pinmux-ids = <
> >  		MX28_PAD_AUART2_RX__SSP3_D4
> > @@ -109,7 +92,7 @@ &ssp3 {
> >  
> >  	flash@0 {
> >  		compatible = "jedec,spi-nor";
> > -		spi-max-frequency = <40000000>;
> > +		spi-max-frequency = <20000000>;
> >  		reg = <0>;
> >  
> >  		partitions {
> > @@ -133,14 +116,21 @@ partition@90000 {
> >  				reg = <0x90000 0x10000>;
> >  			};
> >  
> > -			partition@100000 {
> > -				label = "kernel";
> > -				reg = <0x100000 0x400000>;
> > +			partition@3 {  
> 
> This does not look right. It is neither explained in commit msg, nor
> matching reg.

The SPI-NOR memory layout has evolved during time lifetime of the
device - for example special partitions to keep track of booted devices
for A/B booting scheme was added.

> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).

Could you be more specific here?

For example:
CROSS_COMPILE="*-gcc" ARCH=arm make -j4 dtbs_check W=1

Show:
arch/arm/boot/dts/nxp/mxs/imx28-xea.dtb:
/apb@80000000/apbx@80040000/saif@80042000: failed to match any schema
with compatible: ['fsl,imx28-saif']

Where the 'saif' sound driver is already supported:
git grep -n "fsl,imx28-saif"
Documentation/devicetree/bindings/sound/mxs-saif.txt:27: compatible = "fsl,imx28-saif";
Documentation/devicetree/bindings/sound/mxs-saif.txt:35: compatible = "fsl,imx28-saif";
arch/arm/boot/dts/nxp/mxs/imx28.dtsi:1110: compatible = "fsl,imx28-saif";
arch/arm/boot/dts/nxp/mxs/imx28.dtsi:1127: compatible = "fsl,imx28-saif";

Changes from this patch do not touch the generic imx28.{dtsi} code at
all.

I would assume, that the portion of my patch shall not bring any new,
extra warnings/errors?

> 
> > +				label = "rescue";
> > +				reg = <0xA0000 0xF40000>;  
> 
> Lowercase hex

Ok.

> 
> > +			};
> > +  
> 
> Best regards,
> Krzysztof
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Re: [PATCH] dts: nxp: mxs: Update device tree description for LWE imx28 based devices

Posted by Krzysztof Kozlowski 1 year, 3 months ago

On 27/08/2024 14:26, Lukasz Majewski wrote:
>>>  
>>> -			partition@100000 {
>>> -				label = "kernel";
>>> -				reg = <0x100000 0x400000>;
>>> +			partition@3 {  
>>
>> This does not look right. It is neither explained in commit msg, nor
>> matching reg.
> 
> The SPI-NOR memory layout has evolved during time lifetime of the
> device - for example special partitions to keep track of booted devices
> for A/B booting scheme was added.
> 
>>
>> It does not look like you tested the DTS against bindings. Please run
>> `make dtbs_check W=1` (see
>> Documentation/devicetree/bindings/writing-schema.rst or
>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
>> for instructions).
> 
> Could you be more specific here?

The @3 does not match "reg" as I wrote above. reg property and unit
address should match.

> 
> For example:
> CROSS_COMPILE="*-gcc" ARCH=arm make -j4 dtbs_check W=1
> 
> Show:
> arch/arm/boot/dts/nxp/mxs/imx28-xea.dtb:
> /apb@80000000/apbx@80040000/saif@80042000: failed to match any schema
> with compatible: ['fsl,imx28-saif']
> 
> Where the 'saif' sound driver is already supported:
> git grep -n "fsl,imx28-saif"
> Documentation/devicetree/bindings/sound/mxs-saif.txt:27: compatible = "fsl,imx28-saif";
> Documentation/devicetree/bindings/sound/mxs-saif.txt:35: compatible = "fsl,imx28-saif";
> arch/arm/boot/dts/nxp/mxs/imx28.dtsi:1110: compatible = "fsl,imx28-saif";
> arch/arm/boot/dts/nxp/mxs/imx28.dtsi:1127: compatible = "fsl,imx28-saif";
> 
> Changes from this patch do not touch the generic imx28.{dtsi} code at
> all.
> 
> I would assume, that the portion of my patch shall not bring any new,
> extra warnings/errors?

Heh, I would swear we have a warning for this... well, still this should
be fixed.


Best regards,
Krzysztof