[PATCH] riscv: dts: starfive: Update flash partition layout

[PATCH] riscv: dts: starfive: Update flash partition layout

[matthias.bgg]

From: Matthias Brugger <matthias.bgg@gmail.com>

Up to now, the describe flash partition layout has some gaps.
Use the whole flash chip by getting rid of the gaps.

Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>

---

 arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
index 8ff6ea64f0489..37b4c294ffcc5 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -321,16 +321,13 @@ partitions {
 			#size-cells = <1>;
 
 			spl@0 {
-				reg = <0x0 0x80000>;
+				reg = <0x0 0xf0000>;
 			};
 			uboot-env@f0000 {
 				reg = <0xf0000 0x10000>;
 			};
 			uboot@100000 {
-				reg = <0x100000 0x400000>;
-			};
-			reserved-data@600000 {
-				reg = <0x600000 0xa00000>;
+				reg = <0x100000 0xf00000>;
 			};
 		};
 	};
-- 
2.45.1

Re: [PATCH] riscv: dts: starfive: Update flash partition layout

[Heinrich Schuchardt]

On 03.06.24 17:07, matthias.bgg@kernel.org wrote:
> From: Matthias Brugger <matthias.bgg@gmail.com>
> 
> Up to now, the describe flash partition layout has some gaps.
> Use the whole flash chip by getting rid of the gaps.
> 
> Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>

For flashing larger firmware like EDK II it is helpful to maximize the 
partition sizes. Thanks for sending the patch.

Commit 8384087a ("riscv: dts: starfive: Add QSPI controller node for 
StarFive JH7110 SoC") 
https://lore.kernel.org/linux-riscv/20230804020254.291239-4-william.qiu@starfivetech.com/ 
introduced the current layout.

CCing Starfive's U-Boot reviewers.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> 
> ---
> 
>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index 8ff6ea64f0489..37b4c294ffcc5 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -321,16 +321,13 @@ partitions {
>   			#size-cells = <1>;
>   
>   			spl@0 {
> -				reg = <0x0 0x80000>;
> +				reg = <0x0 0xf0000>;
>   			};
>   			uboot-env@f0000 {
>   				reg = <0xf0000 0x10000>;
>   			};
>   			uboot@100000 {
> -				reg = <0x100000 0x400000>;
> -			};
> -			reserved-data@600000 {
> -				reg = <0x600000 0xa00000>;
> +				reg = <0x100000 0xf00000>;
>   			};
>   		};
>   	};

Re: [PATCH] riscv: dts: starfive: Update flash partition layout

[Emil Renner Berthing]

matthias.bgg@ wrote:
> From: Matthias Brugger <matthias.bgg@gmail.com>
>
> Up to now, the describe flash partition layout has some gaps.
> Use the whole flash chip by getting rid of the gaps.
>
> Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>

Hi Matthias,

Thanks for the patch.

>
> ---
>
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index 8ff6ea64f0489..37b4c294ffcc5 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -321,16 +321,13 @@ partitions {
>  			#size-cells = <1>;
>
>  			spl@0 {
> -				reg = <0x0 0x80000>;
> +				reg = <0x0 0xf0000>;

..this is definitely fine, but..

>  			};
>  			uboot-env@f0000 {
>  				reg = <0xf0000 0x10000>;
>  			};
>  			uboot@100000 {
> -				reg = <0x100000 0x400000>;
> -			};
> -			reserved-data@600000 {
> -				reg = <0x600000 0xa00000>;
> +				reg = <0x100000 0xf00000>;

Do we know that all of the VF2 1.2A, VF2 1.3B and Milk-V Mars boards have at
least 15kB SPI flash chips? In other words were there a reason this previously
ended at 10kB?

Also it looks like my Mars board and VF2 1.3B both report discovering a
"gd25lq128d" chip of 16kB, so why stop at 15kB?

/Emil

Re: [PATCH] riscv: dts: starfive: Update flash partition layout

[Heinrich Schuchardt]

On 6/3/24 18:10, Emil Renner Berthing wrote:
> matthias.bgg@ wrote:
>> From: Matthias Brugger <matthias.bgg@gmail.com>
>>
>> Up to now, the describe flash partition layout has some gaps.
>> Use the whole flash chip by getting rid of the gaps.
>>
>> Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> 
> Hi Matthias,
> 
> Thanks for the patch.
> 
>>
>> ---
>>
>>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> index 8ff6ea64f0489..37b4c294ffcc5 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -321,16 +321,13 @@ partitions {
>>   			#size-cells = <1>;
>>
>>   			spl@0 {
>> -				reg = <0x0 0x80000>;
>> +				reg = <0x0 0xf0000>;
> 
> ..this is definitely fine, but..
> 
>>   			};
>>   			uboot-env@f0000 {
>>   				reg = <0xf0000 0x10000>;
>>   			};
>>   			uboot@100000 {
>> -				reg = <0x100000 0x400000>;
>> -			};
>> -			reserved-data@600000 {
>> -				reg = <0x600000 0xa00000>;
>> +				reg = <0x100000 0xf00000>;
> 
> Do we know that all of the VF2 1.2A, VF2 1.3B and Milk-V Mars boards have at
> least 15kB SPI flash chips? In other words were there a reason this previously
> ended at 10kB?
> 
> Also it looks like my Mars board and VF2 1.3B both report discovering a
> "gd25lq128d" chip of 16kB, so why stop at 15kB?

Hello Emil,

0xf00000 (15 MiB) is the value of the size-cell. gd25lq128d has 128 Mbit 
(=16 MiB).

Cf. the examples in 
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml

The prior partition scheme had a partition 'reserved-data' also ending 
at 16 MiB. We don't change the expected size with the current patch.

Best regards

Heinrich

> 
> /Emil

Re: [PATCH] riscv: dts: starfive: Update flash partition layout

[Emil Renner Berthing]

Heinrich Schuchardt wrote:
> On 6/3/24 18:10, Emil Renner Berthing wrote:
> > matthias.bgg@ wrote:
> >> From: Matthias Brugger <matthias.bgg@gmail.com>
> >>
> >> Up to now, the describe flash partition layout has some gaps.
> >> Use the whole flash chip by getting rid of the gaps.
> >>
> >> Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> >
> > Hi Matthias,
> >
> > Thanks for the patch.
> >
> >>
> >> ---
> >>
> >>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 7 ++-----
> >>   1 file changed, 2 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >> index 8ff6ea64f0489..37b4c294ffcc5 100644
> >> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >> @@ -321,16 +321,13 @@ partitions {
> >>   			#size-cells = <1>;
> >>
> >>   			spl@0 {
> >> -				reg = <0x0 0x80000>;
> >> +				reg = <0x0 0xf0000>;
> >
> > ..this is definitely fine, but..
> >
> >>   			};
> >>   			uboot-env@f0000 {
> >>   				reg = <0xf0000 0x10000>;
> >>   			};
> >>   			uboot@100000 {
> >> -				reg = <0x100000 0x400000>;
> >> -			};
> >> -			reserved-data@600000 {
> >> -				reg = <0x600000 0xa00000>;
> >> +				reg = <0x100000 0xf00000>;
> >
> > Do we know that all of the VF2 1.2A, VF2 1.3B and Milk-V Mars boards have at
> > least 15kB SPI flash chips? In other words were there a reason this previously
> > ended at 10kB?
> >
> > Also it looks like my Mars board and VF2 1.3B both report discovering a
> > "gd25lq128d" chip of 16kB, so why stop at 15kB?
>
> Hello Emil,
>
> 0xf00000 (15 MiB) is the value of the size-cell. gd25lq128d has 128 Mbit
> (=16 MiB).
>
> Cf. the examples in
> Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>
> The prior partition scheme had a partition 'reserved-data' also ending
> at 16 MiB. We don't change the expected size with the current patch.

Oh, you're right. I totally misread that, sorry. You can add my

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

RE: [PATCH] riscv: dts: starfive: Update flash partition layout

[Hal Feng]

> On 03.06.24 23:27, Heinrich Schuchardt wrote:
> On 03.06.24 17:07, matthias.bgg@kernel.org wrote:
> > From: Matthias Brugger <matthias.bgg@gmail.com>
> >
> > Up to now, the describe flash partition layout has some gaps.
> > Use the whole flash chip by getting rid of the gaps.
> >
> > Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> 
> For flashing larger firmware like EDK II it is helpful to maximize the partition sizes.
> Thanks for sending the patch.
> 
> Commit 8384087a ("riscv: dts: starfive: Add QSPI controller node for StarFive
> JH7110 SoC") https://lore.kernel.org/linux-riscv/20230804020254.291239-4-
> william.qiu@starfivetech.com/
> introduced the current layout.
> 
> CCing Starfive's U-Boot reviewers.
> 
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

We added a "reserved-data" partition because the u-boot of VF2 didn't
require a large space and users can use this partition to do something
they want.

Best regards,
Hal

> 
> >
> > ---
> >
> >   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 7 ++-----
> >   1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > index 8ff6ea64f0489..37b4c294ffcc5 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > @@ -321,16 +321,13 @@ partitions {
> >   			#size-cells = <1>;
> >
> >   			spl@0 {
> > -				reg = <0x0 0x80000>;
> > +				reg = <0x0 0xf0000>;
> >   			};
> >   			uboot-env@f0000 {
> >   				reg = <0xf0000 0x10000>;
> >   			};
> >   			uboot@100000 {
> > -				reg = <0x100000 0x400000>;
> > -			};
> > -			reserved-data@600000 {
> > -				reg = <0x600000 0xa00000>;
> > +				reg = <0x100000 0xf00000>;
> >   			};
> >   		};
> >   	};

Re: [PATCH] riscv: dts: starfive: Update flash partition layout

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

On Mon, 03 Jun 2024 17:07:55 +0200, matthias.bgg@kernel.org wrote:
> Up to now, the describe flash partition layout has some gaps.
> Use the whole flash chip by getting rid of the gaps.
> 
> 

Applied to riscv-dt-for-next, thanks!

[1/1] riscv: dts: starfive: Update flash partition layout
      https://git.kernel.org/conor/c/1dc5f8f6ea73

Thanks,
Conor.