[PATCH 1/2] rockchip: rk3588: Add SPINAND as boot device

[PATCH 1/2] rockchip: rk3588: Add SPINAND as boot device

[Slawomir Stepien]

Mark the flash@0 as BROM_BOOTSOURCE_SPINAND.
Fixes use of same-as-spl in u-boot,spl-boot-order prop on Orange Pi 5
Plus.

Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 arch/arm/mach-rockchip/rk3588/rk3588.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
index b1f535fad5..0c8f10a97c 100644
--- a/arch/arm/mach-rockchip/rk3588/rk3588.c
+++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
@@ -42,6 +42,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
 	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
 	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
+	[BROM_BOOTSOURCE_SPINAND] = "/spi@fe2b0000/flash@0",
 };
 
 static struct mm_region rk3588_mem_map[] = {
-- 
2.42.0

[PATCH 2/2] configs: orangepi-5-plus-rk3588: Build SPI image by default

[Slawomir Stepien]

Flashing this image to the on-board flash is useful when you want to
start Linux from a medium (e.g. nvme) without flashing u-boot elements
to it.

Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 configs/orangepi-5-plus-rk3588_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/orangepi-5-plus-rk3588_defconfig b/configs/orangepi-5-plus-rk3588_defconfig
index 0473699621..9af65026a0 100644
--- a/configs/orangepi-5-plus-rk3588_defconfig
+++ b/configs/orangepi-5-plus-rk3588_defconfig
@@ -103,3 +103,4 @@ CONFIG_USB_OHCI_GENERIC=y
 CONFIG_USB_DWC3=y
 CONFIG_USB_DWC3_GENERIC=y
 CONFIG_ERRNO_STR=y
+CONFIG_ROCKCHIP_SPI_IMAGE=y
-- 
2.42.0

Re: [PATCH 1/2] rockchip: rk3588: Add SPINAND as boot device

[Jonas Karlman]

On 2023-10-31 17:13, Slawomir Stepien wrote:
> Mark the flash@0 as BROM_BOOTSOURCE_SPINAND.
> Fixes use of same-as-spl in u-boot,spl-boot-order prop on Orange Pi 5
> Plus.
> 
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> ---
>  arch/arm/mach-rockchip/rk3588/rk3588.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
> index b1f535fad5..0c8f10a97c 100644
> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
> @@ -42,6 +42,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>  	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
>  	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
>  	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
> +	[BROM_BOOTSOURCE_SPINAND] = "/spi@fe2b0000/flash@0",

This is not correct, please see [1] for an open discussion on this issue.

Kever: Do you have any more insights into the question on [1].

Is my assessment about the value BootRom writes to bootsource reg
correct, see [1]?

- FSPI (M0): 3
- FSPI (M1): 4
- FSPI (M2): 6

[1] https://lore.kernel.org/u-boot/b82c02c0-0a37-8553-f498-b03f85394d59@kwiboo.se/

Regards,
Jonas

>  };
>  
>  static struct mm_region rk3588_mem_map[] = {

Re: [PATCH 2/2] configs: orangepi-5-plus-rk3588: Build SPI image by default

[Jonas Karlman]

On 2023-10-31 17:13, Slawomir Stepien wrote:
> Flashing this image to the on-board flash is useful when you want to
> start Linux from a medium (e.g. nvme) without flashing u-boot elements
> to it.
> 
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> ---
>  configs/orangepi-5-plus-rk3588_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/orangepi-5-plus-rk3588_defconfig b/configs/orangepi-5-plus-rk3588_defconfig
> index 0473699621..9af65026a0 100644
> --- a/configs/orangepi-5-plus-rk3588_defconfig
> +++ b/configs/orangepi-5-plus-rk3588_defconfig
> @@ -103,3 +103,4 @@ CONFIG_USB_OHCI_GENERIC=y
>  CONFIG_USB_DWC3=y
>  CONFIG_USB_DWC3_GENERIC=y
>  CONFIG_ERRNO_STR=y
> +CONFIG_ROCKCHIP_SPI_IMAGE=y

This was intentionally left out from original submission due to an
unfinished discussion at [1].

Also note that this option is not added at correct position in defconfig
file. Next time, please re-sync defconfig changes with a make
savedefconfig generated file.

[1] https://lore.kernel.org/u-boot/b82c02c0-0a37-8553-f498-b03f85394d59@kwiboo.se/

Regards,
Jonas

Re: [PATCH 2/2] configs: orangepi-5-plus-rk3588: Build SPI image by default

[Slawomir Stepien]

On paź 31, 2023 17:49, Jonas Karlman wrote:
> On 2023-10-31 17:13, Slawomir Stepien wrote:
> > Flashing this image to the on-board flash is useful when you want to
> > start Linux from a medium (e.g. nvme) without flashing u-boot elements
> > to it.
> > 
> > Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> > ---
> >  configs/orangepi-5-plus-rk3588_defconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/configs/orangepi-5-plus-rk3588_defconfig b/configs/orangepi-5-plus-rk3588_defconfig
> > index 0473699621..9af65026a0 100644
> > --- a/configs/orangepi-5-plus-rk3588_defconfig
> > +++ b/configs/orangepi-5-plus-rk3588_defconfig
> > @@ -103,3 +103,4 @@ CONFIG_USB_OHCI_GENERIC=y
> >  CONFIG_USB_DWC3=y
> >  CONFIG_USB_DWC3_GENERIC=y
> >  CONFIG_ERRNO_STR=y
> > +CONFIG_ROCKCHIP_SPI_IMAGE=y
> 
> This was intentionally left out from original submission due to an
> unfinished discussion at [1].
> 
> Also note that this option is not added at correct position in defconfig
> file. Next time, please re-sync defconfig changes with a make
> savedefconfig generated file.

Ah yes. I did forget about it. Sorry. I will regenerate it correctly...if there will be v2.

> [1] https://lore.kernel.org/u-boot/b82c02c0-0a37-8553-f498-b03f85394d59@kwiboo.se/
> 
> Regards,
> Jonas

-- 
Slawomir Stepien

Re: [PATCH 1/2] rockchip: rk3588: Add SPINAND as boot device

[Kever Yang]

Hi Jonas,

On 2023/11/1 00:45, Jonas Karlman wrote:
> On 2023-10-31 17:13, Slawomir Stepien wrote:
>> Mark the flash@0 as BROM_BOOTSOURCE_SPINAND.
>> Fixes use of same-as-spl in u-boot,spl-boot-order prop on Orange Pi 5
>> Plus.
>>
>> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
>> ---
>>   arch/arm/mach-rockchip/rk3588/rk3588.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
>> index b1f535fad5..0c8f10a97c 100644
>> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
>> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
>> @@ -42,6 +42,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>   	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
>>   	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
>>   	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
>> +	[BROM_BOOTSOURCE_SPINAND] = "/spi@fe2b0000/flash@0",
> This is not correct, please see [1] for an open discussion on this issue.
>
> Kever: Do you have any more insights into the question on [1].

As I reply in another mail, the FSPI M0~M2 is all the option for SPI 
NAND/NOR in rk3588.

Thanks,

- Kever

>
> Is my assessment about the value BootRom writes to bootsource reg
> correct, see [1]?
>
> - FSPI (M0): 3
> - FSPI (M1): 4
> - FSPI (M2): 6
>
> [1] https://lore.kernel.org/u-boot/b82c02c0-0a37-8553-f498-b03f85394d59@kwiboo.se/
>
> Regards,
> Jonas
>
>>   };
>>   
>>   static struct mm_region rk3588_mem_map[] = {

Re: [PATCH 1/2] rockchip: rk3588: Add SPINAND as boot device

[Slawomir Stepien]

Hi Jonas and Kever

Thanks for clarifying the situation Kever! Please see below my proposition. What do you think?

On lis 02, 2023 15:27, Kever Yang wrote:
> Hi Jonas,
> 
> On 2023/11/1 00:45, Jonas Karlman wrote:
> > On 2023-10-31 17:13, Slawomir Stepien wrote:
> > > Mark the flash@0 as BROM_BOOTSOURCE_SPINAND.
> > > Fixes use of same-as-spl in u-boot,spl-boot-order prop on Orange Pi 5
> > > Plus.
> > > 
> > > Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> > > ---
> > >   arch/arm/mach-rockchip/rk3588/rk3588.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
> > > index b1f535fad5..0c8f10a97c 100644
> > > --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
> > > +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
> > > @@ -42,6 +42,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
> > >   	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
> > >   	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
> > >   	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
> > > +	[BROM_BOOTSOURCE_SPINAND] = "/spi@fe2b0000/flash@0",
> > This is not correct, please see [1] for an open discussion on this issue.
> > 
> > Kever: Do you have any more insights into the question on [1].
> 
> As I reply in another mail, the FSPI M0~M2 is all the option for SPI
> NAND/NOR in rk3588.

So based on that info, something like this?:

1. BROM_BOOTSOURCE_SPINAND - removed (not used at all).
2. BROM_BOOTSOURCE_SPINOR_RK3588 - removed (not needed after this patch).
3. Updated also the rk3568.c since it also has FSPI M0 (only this one mux), so I would assume it
will be value of 3 there too.
4. The BROM_BOOTSOURCE_SPINOR is still there, since the rk3399 does not use the FSPI (that is my
understanding after checking the TRM doc).

diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h b/arch/arm/include/asm/arch-rockchip/bootrom.h
index 7dab18fbc3..9f16d28aec 100644
--- a/arch/arm/include/asm/arch-rockchip/bootrom.h
+++ b/arch/arm/include/asm/arch-rockchip/bootrom.h
@@ -46,9 +46,10 @@ enum {
 	BROM_BOOTSOURCE_NAND = 1,
 	BROM_BOOTSOURCE_EMMC = 2,
 	BROM_BOOTSOURCE_SPINOR = 3,
-	BROM_BOOTSOURCE_SPINAND = 4,
+	BROM_BOOTSOURCE_FSPI_M0 = 3,
+	BROM_BOOTSOURCE_FSPI_M1 = 4,
 	BROM_BOOTSOURCE_SD = 5,
-	BROM_BOOTSOURCE_SPINOR_RK3588 = 6,
+	BROM_BOOTSOURCE_FSPI_M2 = 6,
 	BROM_BOOTSOURCE_USB = 10,
 	BROM_LAST_BOOTSOURCE = BROM_BOOTSOURCE_USB
 };
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
index 69ef19cc85..ccb63c6a88 100644
--- a/arch/arm/mach-rockchip/rk3568/rk3568.c
+++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
@@ -83,7 +83,7 @@ static struct mm_region rk3568_mem_map[] = {
 
 const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 	[BROM_BOOTSOURCE_EMMC] = "/mmc@fe310000",
-	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe300000/flash@0",
+	[BROM_BOOTSOURCE_FSPI_M0] = "/spi@fe300000/flash@0",
 	[BROM_BOOTSOURCE_SD] = "/mmc@fe2b0000",
 };
 
diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
index b1f535fad5..c36f783d29 100644
--- a/arch/arm/mach-rockchip/rk3588/rk3588.c
+++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
@@ -39,9 +39,10 @@ DECLARE_GLOBAL_DATA_PTR;
 
 const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
 	[BROM_BOOTSOURCE_EMMC] = "/mmc@fe2e0000",
-	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
+	[BROM_BOOTSOURCE_FSPI_M0] = "/spi@fe2b0000/flash@0",
 	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
-	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
+	[BROM_BOOTSOURCE_FSPI_M1] = "/spi@fe2b0000/flash@0",
+	[BROM_BOOTSOURCE_FSPI_M2] = "/spi@fe2b0000/flash@0",
 };
 
 static struct mm_region rk3588_mem_map[] = {

> > Is my assessment about the value BootRom writes to bootsource reg
> > correct, see [1]?
> > 
> > - FSPI (M0): 3
> > - FSPI (M1): 4
> > - FSPI (M2): 6
> > 
> > [1] https://lore.kernel.org/u-boot/b82c02c0-0a37-8553-f498-b03f85394d59@kwiboo.se/
> > 
> > Regards,
> > Jonas
> > 
> > >   };
> > >   static struct mm_region rk3588_mem_map[] = {

-- 
Slawomir Stepien

Re: [PATCH 1/2] rockchip: rk3588: Add SPINAND as boot device

[Kever Yang]

Hi Slawomir,

On 2023/11/3 17:34, Slawomir Stepien wrote:
> Hi Jonas and Kever
>
> Thanks for clarifying the situation Kever! Please see below my proposition. What do you think?
>
> On lis 02, 2023 15:27, Kever Yang wrote:
>> Hi Jonas,
>>
>> On 2023/11/1 00:45, Jonas Karlman wrote:
>>> On 2023-10-31 17:13, Slawomir Stepien wrote:
>>>> Mark the flash@0 as BROM_BOOTSOURCE_SPINAND.
>>>> Fixes use of same-as-spl in u-boot,spl-boot-order prop on Orange Pi 5
>>>> Plus.
>>>>
>>>> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
>>>> ---
>>>>    arch/arm/mach-rockchip/rk3588/rk3588.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
>>>> index b1f535fad5..0c8f10a97c 100644
>>>> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
>>>> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
>>>> @@ -42,6 +42,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>>    	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
>>>>    	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
>>>>    	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
>>>> +	[BROM_BOOTSOURCE_SPINAND] = "/spi@fe2b0000/flash@0",
>>> This is not correct, please see [1] for an open discussion on this issue.
>>>
>>> Kever: Do you have any more insights into the question on [1].
>> As I reply in another mail, the FSPI M0~M2 is all the option for SPI
>> NAND/NOR in rk3588.
> So based on that info, something like this?:
>
> 1. BROM_BOOTSOURCE_SPINAND - removed (not used at all).
> 2. BROM_BOOTSOURCE_SPINOR_RK3588 - removed (not needed after this patch).
> 3. Updated also the rk3568.c since it also has FSPI M0 (only this one mux), so I would assume it
> will be value of 3 there too.
> 4. The BROM_BOOTSOURCE_SPINOR is still there, since the rk3399 does not use the FSPI (that is my
> understanding after checking the TRM doc).

This change looks good to me, I think this can cover all the SPI/FSPI 
boot for all the rockchip SoCs currently.


Thanks,

- Kever

>
> diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h b/arch/arm/include/asm/arch-rockchip/bootrom.h
> index 7dab18fbc3..9f16d28aec 100644
> --- a/arch/arm/include/asm/arch-rockchip/bootrom.h
> +++ b/arch/arm/include/asm/arch-rockchip/bootrom.h
> @@ -46,9 +46,10 @@ enum {
>   	BROM_BOOTSOURCE_NAND = 1,
>   	BROM_BOOTSOURCE_EMMC = 2,
>   	BROM_BOOTSOURCE_SPINOR = 3,
> -	BROM_BOOTSOURCE_SPINAND = 4,
> +	BROM_BOOTSOURCE_FSPI_M0 = 3,
> +	BROM_BOOTSOURCE_FSPI_M1 = 4,
>   	BROM_BOOTSOURCE_SD = 5,
> -	BROM_BOOTSOURCE_SPINOR_RK3588 = 6,
> +	BROM_BOOTSOURCE_FSPI_M2 = 6,
>   	BROM_BOOTSOURCE_USB = 10,
>   	BROM_LAST_BOOTSOURCE = BROM_BOOTSOURCE_USB
>   };
> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index 69ef19cc85..ccb63c6a88 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -83,7 +83,7 @@ static struct mm_region rk3568_mem_map[] = {
>   
>   const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>   	[BROM_BOOTSOURCE_EMMC] = "/mmc@fe310000",
> -	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe300000/flash@0",
> +	[BROM_BOOTSOURCE_FSPI_M0] = "/spi@fe300000/flash@0",
>   	[BROM_BOOTSOURCE_SD] = "/mmc@fe2b0000",
>   };
>   
> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
> index b1f535fad5..c36f783d29 100644
> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
> @@ -39,9 +39,10 @@ DECLARE_GLOBAL_DATA_PTR;
>   
>   const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>   	[BROM_BOOTSOURCE_EMMC] = "/mmc@fe2e0000",
> -	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
> +	[BROM_BOOTSOURCE_FSPI_M0] = "/spi@fe2b0000/flash@0",
>   	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
> -	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
> +	[BROM_BOOTSOURCE_FSPI_M1] = "/spi@fe2b0000/flash@0",
> +	[BROM_BOOTSOURCE_FSPI_M2] = "/spi@fe2b0000/flash@0",
>   };
>   
>   static struct mm_region rk3588_mem_map[] = {
>
>>> Is my assessment about the value BootRom writes to bootsource reg
>>> correct, see [1]?
>>>
>>> - FSPI (M0): 3
>>> - FSPI (M1): 4
>>> - FSPI (M2): 6
>>>
>>> [1] https://lore.kernel.org/u-boot/b82c02c0-0a37-8553-f498-b03f85394d59@kwiboo.se/
>>>
>>> Regards,
>>> Jonas
>>>
>>>>    };
>>>>    static struct mm_region rk3588_mem_map[] = {

Re: [PATCH 1/2] rockchip: rk3588: Add SPINAND as boot device

[Jonas Karlman]

On 2023-11-03 10:34, Slawomir Stepien wrote:
> Hi Jonas and Kever
> 
> Thanks for clarifying the situation Kever! Please see below my proposition. What do you think?

Yes, this matches my proposition at [1], and I have a series queued now
that we have gotten the answer.

Have included your commit to enable ROCKCHIP_SPI_IMAGE on Orange Pi 5
Plus, John Clark's commit to enable on NanoPC-T6 my own to enable on
Orange Pi 5.

Should hit the list later today after full runtime testing.

> 
> On lis 02, 2023 15:27, Kever Yang wrote:
>> Hi Jonas,
>>
>> On 2023/11/1 00:45, Jonas Karlman wrote:
>>> On 2023-10-31 17:13, Slawomir Stepien wrote:
>>>> Mark the flash@0 as BROM_BOOTSOURCE_SPINAND.
>>>> Fixes use of same-as-spl in u-boot,spl-boot-order prop on Orange Pi 5
>>>> Plus.
>>>>
>>>> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
>>>> ---
>>>>   arch/arm/mach-rockchip/rk3588/rk3588.c | 1 +
>>>>   1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
>>>> index b1f535fad5..0c8f10a97c 100644
>>>> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
>>>> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
>>>> @@ -42,6 +42,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>>>   	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
>>>>   	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
>>>>   	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
>>>> +	[BROM_BOOTSOURCE_SPINAND] = "/spi@fe2b0000/flash@0",
>>> This is not correct, please see [1] for an open discussion on this issue.
>>>
>>> Kever: Do you have any more insights into the question on [1].
>>
>> As I reply in another mail, the FSPI M0~M2 is all the option for SPI
>> NAND/NOR in rk3588.
> 
> So based on that info, something like this?:
> 
> 1. BROM_BOOTSOURCE_SPINAND - removed (not used at all).

Should not be removed, because this is still valid for old generation
SoCs.

> 2. BROM_BOOTSOURCE_SPINOR_RK3588 - removed (not needed after this patch).

Yes, this should be replaced with FSPI_M2.

> 3. Updated also the rk3568.c since it also has FSPI M0 (only this one mux), so I would assume it
> will be value of 3 there too.

No need to change this, the RK356x does not have iomux on fspi, so lets
treat this as an old generation SoCs.

> 4. The BROM_BOOTSOURCE_SPINOR is still there, since the rk3399 does not use the FSPI (that is my
> understanding after checking the TRM doc).

Agreed, should not be changed for same reasons as SPINAND.

> 
> diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h b/arch/arm/include/asm/arch-rockchip/bootrom.h
> index 7dab18fbc3..9f16d28aec 100644
> --- a/arch/arm/include/asm/arch-rockchip/bootrom.h
> +++ b/arch/arm/include/asm/arch-rockchip/bootrom.h
> @@ -46,9 +46,10 @@ enum {
>  	BROM_BOOTSOURCE_NAND = 1,
>  	BROM_BOOTSOURCE_EMMC = 2,
>  	BROM_BOOTSOURCE_SPINOR = 3,
> -	BROM_BOOTSOURCE_SPINAND = 4,
> +	BROM_BOOTSOURCE_FSPI_M0 = 3,
> +	BROM_BOOTSOURCE_FSPI_M1 = 4,
>  	BROM_BOOTSOURCE_SD = 5,
> -	BROM_BOOTSOURCE_SPINOR_RK3588 = 6,
> +	BROM_BOOTSOURCE_FSPI_M2 = 6,
>  	BROM_BOOTSOURCE_USB = 10,
>  	BROM_LAST_BOOTSOURCE = BROM_BOOTSOURCE_USB
>  };
> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index 69ef19cc85..ccb63c6a88 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -83,7 +83,7 @@ static struct mm_region rk3568_mem_map[] = {
>  
>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@fe310000",
> -	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe300000/flash@0",
> +	[BROM_BOOTSOURCE_FSPI_M0] = "/spi@fe300000/flash@0",
>  	[BROM_BOOTSOURCE_SD] = "/mmc@fe2b0000",
>  };
>  
> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
> index b1f535fad5..c36f783d29 100644
> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
> @@ -39,9 +39,10 @@ DECLARE_GLOBAL_DATA_PTR;
>  
>  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>  	[BROM_BOOTSOURCE_EMMC] = "/mmc@fe2e0000",
> -	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
> +	[BROM_BOOTSOURCE_FSPI_M0] = "/spi@fe2b0000/flash@0",
>  	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
> -	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
> +	[BROM_BOOTSOURCE_FSPI_M1] = "/spi@fe2b0000/flash@0",
> +	[BROM_BOOTSOURCE_FSPI_M2] = "/spi@fe2b0000/flash@0",

Fully agree with this change, and have made same/similar. However, the
logic for boot_devices will possible have to be changed/updated if there
is a board using SPI NAND with a different node name in the future.

Regards,
Jonas

>  };
>  
>  static struct mm_region rk3588_mem_map[] = {
> 
>>> Is my assessment about the value BootRom writes to bootsource reg
>>> correct, see [1]?
>>>
>>> - FSPI (M0): 3
>>> - FSPI (M1): 4
>>> - FSPI (M2): 6
>>>
>>> [1] https://lore.kernel.org/u-boot/b82c02c0-0a37-8553-f498-b03f85394d59@kwiboo.se/
>>>
>>> Regards,
>>> Jonas
>>>
>>>>   };
>>>>   static struct mm_region rk3588_mem_map[] = {
> 

Re: [PATCH 1/2] rockchip: rk3588: Add SPINAND as boot device

[Slawomir Stepien]

On lis 03, 2023 11:03, Jonas Karlman wrote:
> On 2023-11-03 10:34, Slawomir Stepien wrote:
> > Hi Jonas and Kever
> > 
> > Thanks for clarifying the situation Kever! Please see below my proposition. What do you think?
> 
> Yes, this matches my proposition at [1], and I have a series queued now
> that we have gotten the answer.
> 
> Have included your commit to enable ROCKCHIP_SPI_IMAGE on Orange Pi 5
> Plus, John Clark's commit to enable on NanoPC-T6 my own to enable on
> Orange Pi 5.
> 
> Should hit the list later today after full runtime testing.

Lovely, thank you! I have read all your comments from below. I'm trusting your judgment.

> > On lis 02, 2023 15:27, Kever Yang wrote:
> >> Hi Jonas,
> >>
> >> On 2023/11/1 00:45, Jonas Karlman wrote:
> >>> On 2023-10-31 17:13, Slawomir Stepien wrote:
> >>>> Mark the flash@0 as BROM_BOOTSOURCE_SPINAND.
> >>>> Fixes use of same-as-spl in u-boot,spl-boot-order prop on Orange Pi 5
> >>>> Plus.
> >>>>
> >>>> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> >>>> ---
> >>>>   arch/arm/mach-rockchip/rk3588/rk3588.c | 1 +
> >>>>   1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
> >>>> index b1f535fad5..0c8f10a97c 100644
> >>>> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
> >>>> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
> >>>> @@ -42,6 +42,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
> >>>>   	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
> >>>>   	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
> >>>>   	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
> >>>> +	[BROM_BOOTSOURCE_SPINAND] = "/spi@fe2b0000/flash@0",
> >>> This is not correct, please see [1] for an open discussion on this issue.
> >>>
> >>> Kever: Do you have any more insights into the question on [1].
> >>
> >> As I reply in another mail, the FSPI M0~M2 is all the option for SPI
> >> NAND/NOR in rk3588.
> > 
> > So based on that info, something like this?:
> > 
> > 1. BROM_BOOTSOURCE_SPINAND - removed (not used at all).
> 
> Should not be removed, because this is still valid for old generation
> SoCs.
> 
> > 2. BROM_BOOTSOURCE_SPINOR_RK3588 - removed (not needed after this patch).
> 
> Yes, this should be replaced with FSPI_M2.
> 
> > 3. Updated also the rk3568.c since it also has FSPI M0 (only this one mux), so I would assume it
> > will be value of 3 there too.
> 
> No need to change this, the RK356x does not have iomux on fspi, so lets
> treat this as an old generation SoCs.
> 
> > 4. The BROM_BOOTSOURCE_SPINOR is still there, since the rk3399 does not use the FSPI (that is my
> > understanding after checking the TRM doc).
> 
> Agreed, should not be changed for same reasons as SPINAND.
> 
> > 
> > diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h b/arch/arm/include/asm/arch-rockchip/bootrom.h
> > index 7dab18fbc3..9f16d28aec 100644
> > --- a/arch/arm/include/asm/arch-rockchip/bootrom.h
> > +++ b/arch/arm/include/asm/arch-rockchip/bootrom.h
> > @@ -46,9 +46,10 @@ enum {
> >  	BROM_BOOTSOURCE_NAND = 1,
> >  	BROM_BOOTSOURCE_EMMC = 2,
> >  	BROM_BOOTSOURCE_SPINOR = 3,
> > -	BROM_BOOTSOURCE_SPINAND = 4,
> > +	BROM_BOOTSOURCE_FSPI_M0 = 3,
> > +	BROM_BOOTSOURCE_FSPI_M1 = 4,
> >  	BROM_BOOTSOURCE_SD = 5,
> > -	BROM_BOOTSOURCE_SPINOR_RK3588 = 6,
> > +	BROM_BOOTSOURCE_FSPI_M2 = 6,
> >  	BROM_BOOTSOURCE_USB = 10,
> >  	BROM_LAST_BOOTSOURCE = BROM_BOOTSOURCE_USB
> >  };
> > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > index 69ef19cc85..ccb63c6a88 100644
> > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > @@ -83,7 +83,7 @@ static struct mm_region rk3568_mem_map[] = {
> >  
> >  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
> >  	[BROM_BOOTSOURCE_EMMC] = "/mmc@fe310000",
> > -	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe300000/flash@0",
> > +	[BROM_BOOTSOURCE_FSPI_M0] = "/spi@fe300000/flash@0",
> >  	[BROM_BOOTSOURCE_SD] = "/mmc@fe2b0000",
> >  };
> >  
> > diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c b/arch/arm/mach-rockchip/rk3588/rk3588.c
> > index b1f535fad5..c36f783d29 100644
> > --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
> > +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
> > @@ -39,9 +39,10 @@ DECLARE_GLOBAL_DATA_PTR;
> >  
> >  const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
> >  	[BROM_BOOTSOURCE_EMMC] = "/mmc@fe2e0000",
> > -	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe2b0000/flash@0",
> > +	[BROM_BOOTSOURCE_FSPI_M0] = "/spi@fe2b0000/flash@0",
> >  	[BROM_BOOTSOURCE_SD] = "/mmc@fe2c0000",
> > -	[BROM_BOOTSOURCE_SPINOR_RK3588] = "/spi@fe2b0000/flash@0",
> > +	[BROM_BOOTSOURCE_FSPI_M1] = "/spi@fe2b0000/flash@0",
> > +	[BROM_BOOTSOURCE_FSPI_M2] = "/spi@fe2b0000/flash@0",
> 
> Fully agree with this change, and have made same/similar. However, the
> logic for boot_devices will possible have to be changed/updated if there
> is a board using SPI NAND with a different node name in the future.
> 
> Regards,
> Jonas
> 
> >  };
> >  
> >  static struct mm_region rk3588_mem_map[] = {
> > 
> >>> Is my assessment about the value BootRom writes to bootsource reg
> >>> correct, see [1]?
> >>>
> >>> - FSPI (M0): 3
> >>> - FSPI (M1): 4
> >>> - FSPI (M2): 6
> >>>
> >>> [1] https://lore.kernel.org/u-boot/b82c02c0-0a37-8553-f498-b03f85394d59@kwiboo.se/
> >>>
> >>> Regards,
> >>> Jonas
> >>>
> >>>>   };
> >>>>   static struct mm_region rk3588_mem_map[] = {

-- 
Slawomir Stepien