mbox series

[RFC,0/5] rockchip_sfc: add support for Rockchip SFC

Message ID 20210525214921.16606-1-macroalpha82@gmail.com
Headers show
Series rockchip_sfc: add support for Rockchip SFC | expand

Message

Chris Morgan May 25, 2021, 9:49 p.m. UTC
From: Chris Morgan <macromorgan@hotmail.com>

Requesting comments for a proposed patchset for adding the Rockchip
serial flash controller to u-boot. The goal of these patches is to
enable it for the Odroid Go Advance so that it may eventually boot
exclusively from the SFC on mainline U-boot (I have tested this and
it works).

The specific help I need with this patch is:

1) I don't know the best way to upstream the XTX25F128B flash chip.
This chip uses a continuation code for the manufacturer ID, however I
cannot seem to find any way to actually read the continuation code.
There is a risk of this driver, used as-is, to collide with another
chip which has the same manufacturer ID with a different continuation
code.

2) The Rockchip SFC driver itself (as it is mostly as-is from the BSP
U-Boot sources) supports SPI NAND and chips of varying sizes, but my
implementation only permits me to test with a single 128Mb flash chip.
The driver itself does some checking on the bitlen in the routine
rockchip_sfc_xfer() which is what is called for the dm_spi_ops.xfer.
I'm not sure if there is a better way to do this. Additionally, I have
to bit-shift the address written to the SFC as I suspect the value is
meant to be left justified, but I never tested it further.

Additionally, it might be worth mentioning but I noticed the Rockchip
BROM will only boot the TPL/SPL off of the SFC if I write it to address
0x10000. This is not documented and different than the address looked
at for SD card booting (512 * 64 = 0x8000 for SD Card booting). Also,
like the SD card driver I can confirm that if DMA is enabled at the SPL
stage A-TF seems to fail silently, then when Linux loads it hangs.
There is an ifdef to force FIFO mode only in the SPL stage.

Tested: Read (works)
	Write (works if you write to an erased sector)
	Erase (works)
	SPL Read (works if you edit the u-boot,spl-boot-order)

Chris Morgan (5):
  spi: rockchip_sfc: add support for Rockchip SFC
  rockchip: px30: Add support for using SFC
  rockchip: px30: add the serial flash controller
  mtd: spi-nor-ids: Add XTX XT25F128B
  rockchip: px30: add support for SFC for Odroid Go Advance

 arch/arm/dts/px30.dtsi                     |  38 ++
 arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  10 +-
 arch/arm/dts/rk3326-odroid-go2.dts         |  22 +
 arch/arm/mach-rockchip/px30/px30.c         |  64 ++
 drivers/mtd/spi/Kconfig                    |   6 +
 drivers/mtd/spi/spi-nor-ids.c              |   8 +
 drivers/spi/Kconfig                        |   8 +
 drivers/spi/Makefile                       |   1 +
 drivers/spi/rockchip_sfc.c                 | 652 +++++++++++++++++++++
 9 files changed, 926 insertions(+), 1 deletion(-)
 create mode 100644 drivers/spi/rockchip_sfc.c

Comments

Kever Yang June 1, 2021, 12:22 p.m. UTC | #1
Add Yifeng from rockchip.

Hi Chris,

     First of all, I think you should remain the origin author info in 
the signed-off.


Hi Yifeng,

     Please help to review this driver.


Thanks,

- Kever

On 2021/5/26 上午5:49, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Requesting comments for a proposed patchset for adding the Rockchip
> serial flash controller to u-boot. The goal of these patches is to
> enable it for the Odroid Go Advance so that it may eventually boot
> exclusively from the SFC on mainline U-boot (I have tested this and
> it works).
>
> The specific help I need with this patch is:
>
> 1) I don't know the best way to upstream the XTX25F128B flash chip.
> This chip uses a continuation code for the manufacturer ID, however I
> cannot seem to find any way to actually read the continuation code.
> There is a risk of this driver, used as-is, to collide with another
> chip which has the same manufacturer ID with a different continuation
> code.
>
> 2) The Rockchip SFC driver itself (as it is mostly as-is from the BSP
> U-Boot sources) supports SPI NAND and chips of varying sizes, but my
> implementation only permits me to test with a single 128Mb flash chip.
> The driver itself does some checking on the bitlen in the routine
> rockchip_sfc_xfer() which is what is called for the dm_spi_ops.xfer.
> I'm not sure if there is a better way to do this. Additionally, I have
> to bit-shift the address written to the SFC as I suspect the value is
> meant to be left justified, but I never tested it further.
>
> Additionally, it might be worth mentioning but I noticed the Rockchip
> BROM will only boot the TPL/SPL off of the SFC if I write it to address
> 0x10000. This is not documented and different than the address looked
> at for SD card booting (512 * 64 = 0x8000 for SD Card booting). Also,
> like the SD card driver I can confirm that if DMA is enabled at the SPL
> stage A-TF seems to fail silently, then when Linux loads it hangs.
> There is an ifdef to force FIFO mode only in the SPL stage.
>
> Tested: Read (works)
> 	Write (works if you write to an erased sector)
> 	Erase (works)
> 	SPL Read (works if you edit the u-boot,spl-boot-order)
>
> Chris Morgan (5):
>    spi: rockchip_sfc: add support for Rockchip SFC
>    rockchip: px30: Add support for using SFC
>    rockchip: px30: add the serial flash controller
>    mtd: spi-nor-ids: Add XTX XT25F128B
>    rockchip: px30: add support for SFC for Odroid Go Advance
>
>   arch/arm/dts/px30.dtsi                     |  38 ++
>   arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  10 +-
>   arch/arm/dts/rk3326-odroid-go2.dts         |  22 +
>   arch/arm/mach-rockchip/px30/px30.c         |  64 ++
>   drivers/mtd/spi/Kconfig                    |   6 +
>   drivers/mtd/spi/spi-nor-ids.c              |   8 +
>   drivers/spi/Kconfig                        |   8 +
>   drivers/spi/Makefile                       |   1 +
>   drivers/spi/rockchip_sfc.c                 | 652 +++++++++++++++++++++
>   9 files changed, 926 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/spi/rockchip_sfc.c
>
Chris Morgan June 1, 2021, 4:54 p.m. UTC | #2
On Tue, Jun 01, 2021 at 08:22:09PM +0800, Kever Yang wrote:
> Add Yifeng from rockchip.
> 
> Hi Chris,
> 
>     First of all, I think you should remain the origin author info in the
> signed-off.

Okay, I can do that. Please note that since I submitted this I was
asked to redo the upstream linux proposed driver to use the spi-mem
framework. I think for now honestly I'd like to abandon this patch
and resubmit a little later with one that is more or less the same
(using the spi-mem framework) as the Linux driver.

https://patchwork.ozlabs.org/project/linux-mtd/patch/20210528170020.26219-2-macroalpha82@gmail.com/

> 
> 
> Hi Yifeng,
> 
>     Please help to review this driver.
> 
> 
> Thanks,
> 
> - Kever
> 
> On 2021/5/26 上午5:49, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Requesting comments for a proposed patchset for adding the Rockchip
> > serial flash controller to u-boot. The goal of these patches is to
> > enable it for the Odroid Go Advance so that it may eventually boot
> > exclusively from the SFC on mainline U-boot (I have tested this and
> > it works).
> > 
> > The specific help I need with this patch is:
> > 
> > 1) I don't know the best way to upstream the XTX25F128B flash chip.
> > This chip uses a continuation code for the manufacturer ID, however I
> > cannot seem to find any way to actually read the continuation code.
> > There is a risk of this driver, used as-is, to collide with another
> > chip which has the same manufacturer ID with a different continuation
> > code.
> > 
> > 2) The Rockchip SFC driver itself (as it is mostly as-is from the BSP
> > U-Boot sources) supports SPI NAND and chips of varying sizes, but my
> > implementation only permits me to test with a single 128Mb flash chip.
> > The driver itself does some checking on the bitlen in the routine
> > rockchip_sfc_xfer() which is what is called for the dm_spi_ops.xfer.
> > I'm not sure if there is a better way to do this. Additionally, I have
> > to bit-shift the address written to the SFC as I suspect the value is
> > meant to be left justified, but I never tested it further.
> > 
> > Additionally, it might be worth mentioning but I noticed the Rockchip
> > BROM will only boot the TPL/SPL off of the SFC if I write it to address
> > 0x10000. This is not documented and different than the address looked
> > at for SD card booting (512 * 64 = 0x8000 for SD Card booting). Also,
> > like the SD card driver I can confirm that if DMA is enabled at the SPL
> > stage A-TF seems to fail silently, then when Linux loads it hangs.
> > There is an ifdef to force FIFO mode only in the SPL stage.
> > 
> > Tested: Read (works)
> > 	Write (works if you write to an erased sector)
> > 	Erase (works)
> > 	SPL Read (works if you edit the u-boot,spl-boot-order)
> > 
> > Chris Morgan (5):
> >    spi: rockchip_sfc: add support for Rockchip SFC
> >    rockchip: px30: Add support for using SFC
> >    rockchip: px30: add the serial flash controller
> >    mtd: spi-nor-ids: Add XTX XT25F128B
> >    rockchip: px30: add support for SFC for Odroid Go Advance
> > 
> >   arch/arm/dts/px30.dtsi                     |  38 ++
> >   arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  10 +-
> >   arch/arm/dts/rk3326-odroid-go2.dts         |  22 +
> >   arch/arm/mach-rockchip/px30/px30.c         |  64 ++
> >   drivers/mtd/spi/Kconfig                    |   6 +
> >   drivers/mtd/spi/spi-nor-ids.c              |   8 +
> >   drivers/spi/Kconfig                        |   8 +
> >   drivers/spi/Makefile                       |   1 +
> >   drivers/spi/rockchip_sfc.c                 | 652 +++++++++++++++++++++
> >   9 files changed, 926 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/spi/rockchip_sfc.c
> > 
> 
>
Kever Yang June 2, 2021, 1:27 a.m. UTC | #3
Hi Chris,

On 2021/6/2 上午12:54, Chris Morgan wrote:
> On Tue, Jun 01, 2021 at 08:22:09PM +0800, Kever Yang wrote:
>> Add Yifeng from rockchip.
>>
>> Hi Chris,
>>
>>      First of all, I think you should remain the origin author info in the
>> signed-off.
> Okay, I can do that. Please note that since I submitted this I was
> asked to redo the upstream linux proposed driver to use the spi-mem
> framework. I think for now honestly I'd like to abandon this patch
> and resubmit a little later with one that is more or less the same
> (using the spi-mem framework) as the Linux driver.


This sounds better, then we can wait for driver with new framework.

Thanks,

- Kever

>
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20210528170020.26219-2-macroalpha82@gmail.com/
>
>>
>> Hi Yifeng,
>>
>>      Please help to review this driver.
>>
>>
>> Thanks,
>>
>> - Kever
>>
>> On 2021/5/26 上午5:49, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> Requesting comments for a proposed patchset for adding the Rockchip
>>> serial flash controller to u-boot. The goal of these patches is to
>>> enable it for the Odroid Go Advance so that it may eventually boot
>>> exclusively from the SFC on mainline U-boot (I have tested this and
>>> it works).
>>>
>>> The specific help I need with this patch is:
>>>
>>> 1) I don't know the best way to upstream the XTX25F128B flash chip.
>>> This chip uses a continuation code for the manufacturer ID, however I
>>> cannot seem to find any way to actually read the continuation code.
>>> There is a risk of this driver, used as-is, to collide with another
>>> chip which has the same manufacturer ID with a different continuation
>>> code.
>>>
>>> 2) The Rockchip SFC driver itself (as it is mostly as-is from the BSP
>>> U-Boot sources) supports SPI NAND and chips of varying sizes, but my
>>> implementation only permits me to test with a single 128Mb flash chip.
>>> The driver itself does some checking on the bitlen in the routine
>>> rockchip_sfc_xfer() which is what is called for the dm_spi_ops.xfer.
>>> I'm not sure if there is a better way to do this. Additionally, I have
>>> to bit-shift the address written to the SFC as I suspect the value is
>>> meant to be left justified, but I never tested it further.
>>>
>>> Additionally, it might be worth mentioning but I noticed the Rockchip
>>> BROM will only boot the TPL/SPL off of the SFC if I write it to address
>>> 0x10000. This is not documented and different than the address looked
>>> at for SD card booting (512 * 64 = 0x8000 for SD Card booting). Also,
>>> like the SD card driver I can confirm that if DMA is enabled at the SPL
>>> stage A-TF seems to fail silently, then when Linux loads it hangs.
>>> There is an ifdef to force FIFO mode only in the SPL stage.
>>>
>>> Tested: Read (works)
>>> 	Write (works if you write to an erased sector)
>>> 	Erase (works)
>>> 	SPL Read (works if you edit the u-boot,spl-boot-order)
>>>
>>> Chris Morgan (5):
>>>     spi: rockchip_sfc: add support for Rockchip SFC
>>>     rockchip: px30: Add support for using SFC
>>>     rockchip: px30: add the serial flash controller
>>>     mtd: spi-nor-ids: Add XTX XT25F128B
>>>     rockchip: px30: add support for SFC for Odroid Go Advance
>>>
>>>    arch/arm/dts/px30.dtsi                     |  38 ++
>>>    arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  10 +-
>>>    arch/arm/dts/rk3326-odroid-go2.dts         |  22 +
>>>    arch/arm/mach-rockchip/px30/px30.c         |  64 ++
>>>    drivers/mtd/spi/Kconfig                    |   6 +
>>>    drivers/mtd/spi/spi-nor-ids.c              |   8 +
>>>    drivers/spi/Kconfig                        |   8 +
>>>    drivers/spi/Makefile                       |   1 +
>>>    drivers/spi/rockchip_sfc.c                 | 652 +++++++++++++++++++++
>>>    9 files changed, 926 insertions(+), 1 deletion(-)
>>>    create mode 100644 drivers/spi/rockchip_sfc.c
>>>
>>
>
Jon Lin June 4, 2021, 1:42 p.m. UTC | #4
Hi Chris:

It's my honor to read about spi-rockchip-sfc driver from you, and your code has made a big difference to me.

Recently, we happen to be willing to submit the spi-mem rk sfc drivers, and we have tried some of them
in the internal process. Compared with the RK internal code, I have the following thoughts on your code:
1) Only support spi-nor, not support spi-nand
2) Only support rx_dual, tx_quad, not support tx_dual, tx_quad
3) Some of the code may be further simplified, such as "rockchip_sfc_register_all" 
4) Some of the logic may be further simplified, such as "rockchip_sfc_init" 
5) Doesn't support SFC ver4 ver5 althought witch is compatibled with lower version

In order to support all these features, I adjust the drivers on the basis of your code witch is attach to these mail, these
drivers have been test in linux 5.10 rk3568 board, and I'm still doing more strict tests.

So I suggest that we complete it together, or we may consider transferring it to me for submission If I have the honor.

I'm still unfamiliar with the community culture. I hope I didn't offend you. If I have, I'll say sorry in advance. 
Best wishes.



jon.lin@rock-chips.com
 
From: Kever Yang
Date: 2021-06-02 09:27
To: Chris Morgan
CC: u-boot; heiko.stuebner; vigneshr; jagan; sjg; Chris Morgan; 赵仪峰; 林鼎强
Subject: Re: [RFC 0/5] rockchip_sfc: add support for Rockchip SFC
Hi Chris,
 
On 2021/6/2 上午12:54, Chris Morgan wrote:
> On Tue, Jun 01, 2021 at 08:22:09PM +0800, Kever Yang wrote:
>> Add Yifeng from rockchip.
>>
>> Hi Chris,
>>
>>      First of all, I think you should remain the origin author info in the
>> signed-off.
> Okay, I can do that. Please note that since I submitted this I was
> asked to redo the upstream linux proposed driver to use the spi-mem
> framework. I think for now honestly I'd like to abandon this patch
> and resubmit a little later with one that is more or less the same
> (using the spi-mem framework) as the Linux driver.
 
 
This sounds better, then we can wait for driver with new framework.
 
Thanks,
 
- Kever
 
>
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20210528170020.26219-2-macroalpha82@gmail.com/
>
>>
>> Hi Yifeng,
>>
>>      Please help to review this driver.
>>
>>
>> Thanks,
>>
>> - Kever
>>
>> On 2021/5/26 上午5:49, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> Requesting comments for a proposed patchset for adding the Rockchip
>>> serial flash controller to u-boot. The goal of these patches is to
>>> enable it for the Odroid Go Advance so that it may eventually boot
>>> exclusively from the SFC on mainline U-boot (I have tested this and
>>> it works).
>>>
>>> The specific help I need with this patch is:
>>>
>>> 1) I don't know the best way to upstream the XTX25F128B flash chip.
>>> This chip uses a continuation code for the manufacturer ID, however I
>>> cannot seem to find any way to actually read the continuation code.
>>> There is a risk of this driver, used as-is, to collide with another
>>> chip which has the same manufacturer ID with a different continuation
>>> code.
>>>
>>> 2) The Rockchip SFC driver itself (as it is mostly as-is from the BSP
>>> U-Boot sources) supports SPI NAND and chips of varying sizes, but my
>>> implementation only permits me to test with a single 128Mb flash chip.
>>> The driver itself does some checking on the bitlen in the routine
>>> rockchip_sfc_xfer() which is what is called for the dm_spi_ops.xfer.
>>> I'm not sure if there is a better way to do this. Additionally, I have
>>> to bit-shift the address written to the SFC as I suspect the value is
>>> meant to be left justified, but I never tested it further.
>>>
>>> Additionally, it might be worth mentioning but I noticed the Rockchip
>>> BROM will only boot the TPL/SPL off of the SFC if I write it to address
>>> 0x10000. This is not documented and different than the address looked
>>> at for SD card booting (512 * 64 = 0x8000 for SD Card booting). Also,
>>> like the SD card driver I can confirm that if DMA is enabled at the SPL
>>> stage A-TF seems to fail silently, then when Linux loads it hangs.
>>> There is an ifdef to force FIFO mode only in the SPL stage.
>>>
>>> Tested: Read (works)
>>> Write (works if you write to an erased sector)
>>> Erase (works)
>>> SPL Read (works if you edit the u-boot,spl-boot-order)
>>>
>>> Chris Morgan (5):
>>>     spi: rockchip_sfc: add support for Rockchip SFC
>>>     rockchip: px30: Add support for using SFC
>>>     rockchip: px30: add the serial flash controller
>>>     mtd: spi-nor-ids: Add XTX XT25F128B
>>>     rockchip: px30: add support for SFC for Odroid Go Advance
>>>
>>>    arch/arm/dts/px30.dtsi                     |  38 ++
>>>    arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  10 +-
>>>    arch/arm/dts/rk3326-odroid-go2.dts         |  22 +
>>>    arch/arm/mach-rockchip/px30/px30.c         |  64 ++
>>>    drivers/mtd/spi/Kconfig                    |   6 +
>>>    drivers/mtd/spi/spi-nor-ids.c              |   8 +
>>>    drivers/spi/Kconfig                        |   8 +
>>>    drivers/spi/Makefile                       |   1 +
>>>    drivers/spi/rockchip_sfc.c                 | 652 +++++++++++++++++++++
>>>    9 files changed, 926 insertions(+), 1 deletion(-)
>>>    create mode 100644 drivers/spi/rockchip_sfc.c
>>>
>>
>
Chris Morgan June 4, 2021, 2:40 p.m. UTC | #5
On Fri, Jun 04, 2021 at 09:42:18PM +0800, 林鼎强 wrote:
> 
> Hi Chris:
> 
> It's my honor to read about spi-rockchip-sfc driver from you, and your code has made a big difference to me.
> 
> Recently, we happen to be willing to submit the spi-mem rk sfc drivers, and we have tried some of them
> in the internal process. Compared with the RK internal code, I have the following thoughts on your code:
> 1) Only support spi-nor, not support spi-nand
> 2) Only support rx_dual, tx_quad, not support tx_dual, tx_quad
> 3) Some of the code may be further simplified, such as "rockchip_sfc_register_all" 
> 4) Some of the logic may be further simplified, such as "rockchip_sfc_init" 
> 5) Doesn't support SFC ver4 ver5 althought witch is compatibled with lower version
> 
> In order to support all these features, I adjust the drivers on the basis of your code witch is attach to these mail, these
> drivers have been test in linux 5.10 rk3568 board, and I'm still doing more strict tests.
> 
> So I suggest that we complete it together, or we may consider transferring it to me for submission If I have the honor.
> 
> I'm still unfamiliar with the community culture. I hope I didn't offend you. If I have, I'll say sorry in advance. 
> Best wishes.

I am absolutely fine with you taking this on, and will be happy to help
you in any way I can. I only started working on support for this
because I wanted it supported in both Linux and U-Boot mainline, and
didn't see any progress being made (so I thought I would do it myself).
Basically, my goal all along has been to ensure the Odroid Go Advance
is fully supported in mainline Linux (and U-boot), and that requires
we get the audio (done), SFC (in progress), crypto, battery, charger,
and everything else (done) supported.

I trust you are better able to develop/test this than I am, as I only
have access to some technical documents and a single PX30 based device
to test this on.

Please CC me when you are ready to submit upstream to Linux and U-boot
and I will happily test it on my hardware. If you don't have a U-boot
driver in progress let me know and I can start working on one.

Thank you.

> 
> 
> 
> jon.lin@rock-chips.com
>  
> From: Kever Yang
> Date: 2021-06-02 09:27
> To: Chris Morgan
> CC: u-boot; heiko.stuebner; vigneshr; jagan; sjg; Chris Morgan; 赵仪峰; 林鼎强
> Subject: Re: [RFC 0/5] rockchip_sfc: add support for Rockchip SFC
> Hi Chris,
>  
> On 2021/6/2 上午12:54, Chris Morgan wrote:
> > On Tue, Jun 01, 2021 at 08:22:09PM +0800, Kever Yang wrote:
> >> Add Yifeng from rockchip.
> >>
> >> Hi Chris,
> >>
> >>      First of all, I think you should remain the origin author info in the
> >> signed-off.
> > Okay, I can do that. Please note that since I submitted this I was
> > asked to redo the upstream linux proposed driver to use the spi-mem
> > framework. I think for now honestly I'd like to abandon this patch
> > and resubmit a little later with one that is more or less the same
> > (using the spi-mem framework) as the Linux driver.
>  
>  
> This sounds better, then we can wait for driver with new framework.
>  
> Thanks,
>  
> - Kever
>  
> >
> > https://patchwork.ozlabs.org/project/linux-mtd/patch/20210528170020.26219-2-macroalpha82@gmail.com/
> >
> >>
> >> Hi Yifeng,
> >>
> >>      Please help to review this driver.
> >>
> >>
> >> Thanks,
> >>
> >> - Kever
> >>
> >> On 2021/5/26 上午5:49, Chris Morgan wrote:
> >>> From: Chris Morgan <macromorgan@hotmail.com>
> >>>
> >>> Requesting comments for a proposed patchset for adding the Rockchip
> >>> serial flash controller to u-boot. The goal of these patches is to
> >>> enable it for the Odroid Go Advance so that it may eventually boot
> >>> exclusively from the SFC on mainline U-boot (I have tested this and
> >>> it works).
> >>>
> >>> The specific help I need with this patch is:
> >>>
> >>> 1) I don't know the best way to upstream the XTX25F128B flash chip.
> >>> This chip uses a continuation code for the manufacturer ID, however I
> >>> cannot seem to find any way to actually read the continuation code.
> >>> There is a risk of this driver, used as-is, to collide with another
> >>> chip which has the same manufacturer ID with a different continuation
> >>> code.
> >>>
> >>> 2) The Rockchip SFC driver itself (as it is mostly as-is from the BSP
> >>> U-Boot sources) supports SPI NAND and chips of varying sizes, but my
> >>> implementation only permits me to test with a single 128Mb flash chip.
> >>> The driver itself does some checking on the bitlen in the routine
> >>> rockchip_sfc_xfer() which is what is called for the dm_spi_ops.xfer.
> >>> I'm not sure if there is a better way to do this. Additionally, I have
> >>> to bit-shift the address written to the SFC as I suspect the value is
> >>> meant to be left justified, but I never tested it further.
> >>>
> >>> Additionally, it might be worth mentioning but I noticed the Rockchip
> >>> BROM will only boot the TPL/SPL off of the SFC if I write it to address
> >>> 0x10000. This is not documented and different than the address looked
> >>> at for SD card booting (512 * 64 = 0x8000 for SD Card booting). Also,
> >>> like the SD card driver I can confirm that if DMA is enabled at the SPL
> >>> stage A-TF seems to fail silently, then when Linux loads it hangs.
> >>> There is an ifdef to force FIFO mode only in the SPL stage.
> >>>
> >>> Tested: Read (works)
> >>> Write (works if you write to an erased sector)
> >>> Erase (works)
> >>> SPL Read (works if you edit the u-boot,spl-boot-order)
> >>>
> >>> Chris Morgan (5):
> >>>     spi: rockchip_sfc: add support for Rockchip SFC
> >>>     rockchip: px30: Add support for using SFC
> >>>     rockchip: px30: add the serial flash controller
> >>>     mtd: spi-nor-ids: Add XTX XT25F128B
> >>>     rockchip: px30: add support for SFC for Odroid Go Advance
> >>>
> >>>    arch/arm/dts/px30.dtsi                     |  38 ++
> >>>    arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  10 +-
> >>>    arch/arm/dts/rk3326-odroid-go2.dts         |  22 +
> >>>    arch/arm/mach-rockchip/px30/px30.c         |  64 ++
> >>>    drivers/mtd/spi/Kconfig                    |   6 +
> >>>    drivers/mtd/spi/spi-nor-ids.c              |   8 +
> >>>    drivers/spi/Kconfig                        |   8 +
> >>>    drivers/spi/Makefile                       |   1 +
> >>>    drivers/spi/rockchip_sfc.c                 | 652 +++++++++++++++++++++
> >>>    9 files changed, 926 insertions(+), 1 deletion(-)
> >>>    create mode 100644 drivers/spi/rockchip_sfc.c
> >>>
> >>
> >
>  
>  
>
Jon Lin June 5, 2021, 7:37 a.m. UTC | #6
On 6/4/21 10:40 PM, Chris Morgan wrote:
> On Fri, Jun 04, 2021 at 09:42:18PM +0800, 林鼎强 wrote:
>> Hi Chris:
>>
>> It's my honor to read about spi-rockchip-sfc driver from you, and your code has made a big difference to me.
>>
>> Recently, we happen to be willing to submit the spi-mem rk sfc drivers, and we have tried some of them
>> in the internal process. Compared with the RK internal code, I have the following thoughts on your code:
>> 1) Only support spi-nor, not support spi-nand
>> 2) Only support rx_dual, tx_quad, not support tx_dual, tx_quad
>> 3) Some of the code may be further simplified, such as "rockchip_sfc_register_all"
>> 4) Some of the logic may be further simplified, such as "rockchip_sfc_init"
>> 5) Doesn't support SFC ver4 ver5 althought witch is compatibled with lower version
>>
>> In order to support all these features, I adjust the drivers on the basis of your code witch is attach to these mail, these
>> drivers have been test in linux 5.10 rk3568 board, and I'm still doing more strict tests.
>>
>> So I suggest that we complete it together, or we may consider transferring it to me for submission If I have the honor.
>>
>> I'm still unfamiliar with the community culture. I hope I didn't offend you. If I have, I'll say sorry in advance.
>> Best wishes.
> I am absolutely fine with you taking this on, and will be happy to help
> you in any way I can. I only started working on support for this
> because I wanted it supported in both Linux and U-Boot mainline, and
> didn't see any progress being made (so I thought I would do it myself).
> Basically, my goal all along has been to ensure the Odroid Go Advance
> is fully supported in mainline Linux (and U-boot), and that requires
> we get the audio (done), SFC (in progress), crypto, battery, charger,
> and everything else (done) supported.
>
> I trust you are better able to develop/test this than I am, as I only
> have access to some technical documents and a single PX30 based device
> to test this on.
>
> Please CC me when you are ready to submit upstream to Linux and U-boot
> and I will happily test it on my hardware. If you don't have a U-boot
> driver in progress let me know and I can start working on one.
>
> Thank you.


I'm glad to receive your reply and your understanding so soon. I'll talk 
about my plan directly. If there is no obvious objection,I'll go on with it.

Firstly, I will submit the support for the my first version of Linux 
drivers next week which is base on your series submission.

After browsing your submission, there will be the following changes:
1) [RFC,v4,1/8] change with the "Rob Herring" comment
2) [RFC,v4,2/8] update driver
3) All dt patches, make a little change to compatible with RK SDK

[RFC,v4,8/8] arm64: dts: rockchip: Enable SFC for Odroid Go Advance
[RFC,v4,7/8] arm64: dts: rockchip: Add SFC to RK3308
[RFC,v4,6/8] arm: dts: rockchip: Add SFC to RV1108
[RFC,v4,5/8] arm: dts: rockchip: Add SFC to RK3036
[RFC,v4,4/8] clk: rockchip: Add support for hclk_sfc on rk3036
[RFC,v4,3/8] arm64: dts: rockchip: Add SFC to PX30
[RFC,v4,2/8] spi: rockchip-sfc: add rockchip serial flash controller
[RFC,v4,1/8] dt-bindings: rockchip-sfc: Bindings for Rockchip serial 
flash controller

Secondly, Keep devolop uboot drivers base on your serieas submission 
https://patchwork.ozlabs.org/project/uboot/list//?series=245724
submit the code under uboot as soon as possible.

Finally I'll keep focus on the new comments about all these linux 
patches,then fix it until they are merged.

Best wishes.


>>
>>
>> jon.lin@rock-chips.com
>>   
>> From: Kever Yang
>> Date: 2021-06-02 09:27
>> To: Chris Morgan
>> CC: u-boot; heiko.stuebner; vigneshr; jagan; sjg; Chris Morgan; 赵仪峰; 林鼎强
>> Subject: Re: [RFC 0/5] rockchip_sfc: add support for Rockchip SFC
>> Hi Chris,
>>   
>> On 2021/6/2 上午12:54, Chris Morgan wrote:
>>> On Tue, Jun 01, 2021 at 08:22:09PM +0800, Kever Yang wrote:
>>>> Add Yifeng from rockchip.
>>>>
>>>> Hi Chris,
>>>>
>>>>       First of all, I think you should remain the origin author info in the
>>>> signed-off.
>>> Okay, I can do that. Please note that since I submitted this I was
>>> asked to redo the upstream linux proposed driver to use the spi-mem
>>> framework. I think for now honestly I'd like to abandon this patch
>>> and resubmit a little later with one that is more or less the same
>>> (using the spi-mem framework) as the Linux driver.
>>   
>>   
>> This sounds better, then we can wait for driver with new framework.
>>   
>> Thanks,
>>   
>> - Kever
>>   
>>> https://patchwork.ozlabs.org/project/linux-mtd/patch/20210528170020.26219-2-macroalpha82@gmail.com/
>>>
>>>> Hi Yifeng,
>>>>
>>>>       Please help to review this driver.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> - Kever
>>>>
>>>> On 2021/5/26 上午5:49, Chris Morgan wrote:
>>>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>>>
>>>>> Requesting comments for a proposed patchset for adding the Rockchip
>>>>> serial flash controller to u-boot. The goal of these patches is to
>>>>> enable it for the Odroid Go Advance so that it may eventually boot
>>>>> exclusively from the SFC on mainline U-boot (I have tested this and
>>>>> it works).
>>>>>
>>>>> The specific help I need with this patch is:
>>>>>
>>>>> 1) I don't know the best way to upstream the XTX25F128B flash chip.
>>>>> This chip uses a continuation code for the manufacturer ID, however I
>>>>> cannot seem to find any way to actually read the continuation code.
>>>>> There is a risk of this driver, used as-is, to collide with another
>>>>> chip which has the same manufacturer ID with a different continuation
>>>>> code.
>>>>>
>>>>> 2) The Rockchip SFC driver itself (as it is mostly as-is from the BSP
>>>>> U-Boot sources) supports SPI NAND and chips of varying sizes, but my
>>>>> implementation only permits me to test with a single 128Mb flash chip.
>>>>> The driver itself does some checking on the bitlen in the routine
>>>>> rockchip_sfc_xfer() which is what is called for the dm_spi_ops.xfer.
>>>>> I'm not sure if there is a better way to do this. Additionally, I have
>>>>> to bit-shift the address written to the SFC as I suspect the value is
>>>>> meant to be left justified, but I never tested it further.
>>>>>
>>>>> Additionally, it might be worth mentioning but I noticed the Rockchip
>>>>> BROM will only boot the TPL/SPL off of the SFC if I write it to address
>>>>> 0x10000. This is not documented and different than the address looked
>>>>> at for SD card booting (512 * 64 = 0x8000 for SD Card booting). Also,
>>>>> like the SD card driver I can confirm that if DMA is enabled at the SPL
>>>>> stage A-TF seems to fail silently, then when Linux loads it hangs.
>>>>> There is an ifdef to force FIFO mode only in the SPL stage.
>>>>>
>>>>> Tested: Read (works)
>>>>> Write (works if you write to an erased sector)
>>>>> Erase (works)
>>>>> SPL Read (works if you edit the u-boot,spl-boot-order)
>>>>>
>>>>> Chris Morgan (5):
>>>>>      spi: rockchip_sfc: add support for Rockchip SFC
>>>>>      rockchip: px30: Add support for using SFC
>>>>>      rockchip: px30: add the serial flash controller
>>>>>      mtd: spi-nor-ids: Add XTX XT25F128B
>>>>>      rockchip: px30: add support for SFC for Odroid Go Advance
>>>>>
>>>>>     arch/arm/dts/px30.dtsi                     |  38 ++
>>>>>     arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  10 +-
>>>>>     arch/arm/dts/rk3326-odroid-go2.dts         |  22 +
>>>>>     arch/arm/mach-rockchip/px30/px30.c         |  64 ++
>>>>>     drivers/mtd/spi/Kconfig                    |   6 +
>>>>>     drivers/mtd/spi/spi-nor-ids.c              |   8 +
>>>>>     drivers/spi/Kconfig                        |   8 +
>>>>>     drivers/spi/Makefile                       |   1 +
>>>>>     drivers/spi/rockchip_sfc.c                 | 652 +++++++++++++++++++++
>>>>>     9 files changed, 926 insertions(+), 1 deletion(-)
>>>>>     create mode 100644 drivers/spi/rockchip_sfc.c
>>>>>
>>   
>>   
>>   
>
>
>
>
>
Chris Morgan June 7, 2021, 3:34 p.m. UTC | #7
On Sat, Jun 05, 2021 at 03:37:01PM +0800, Jon Lin wrote:
> 
> On 6/4/21 10:40 PM, Chris Morgan wrote:
> > On Fri, Jun 04, 2021 at 09:42:18PM +0800, 林鼎强 wrote:
> > > Hi Chris:
> > > 
> > > It's my honor to read about spi-rockchip-sfc driver from you, and your code has made a big difference to me.
> > > 
> > > Recently, we happen to be willing to submit the spi-mem rk sfc drivers, and we have tried some of them
> > > in the internal process. Compared with the RK internal code, I have the following thoughts on your code:
> > > 1) Only support spi-nor, not support spi-nand
> > > 2) Only support rx_dual, tx_quad, not support tx_dual, tx_quad
> > > 3) Some of the code may be further simplified, such as "rockchip_sfc_register_all"
> > > 4) Some of the logic may be further simplified, such as "rockchip_sfc_init"
> > > 5) Doesn't support SFC ver4 ver5 althought witch is compatibled with lower version
> > > 
> > > In order to support all these features, I adjust the drivers on the basis of your code witch is attach to these mail, these
> > > drivers have been test in linux 5.10 rk3568 board, and I'm still doing more strict tests.
> > > 
> > > So I suggest that we complete it together, or we may consider transferring it to me for submission If I have the honor.
> > > 
> > > I'm still unfamiliar with the community culture. I hope I didn't offend you. If I have, I'll say sorry in advance.
> > > Best wishes.
> > I am absolutely fine with you taking this on, and will be happy to help
> > you in any way I can. I only started working on support for this
> > because I wanted it supported in both Linux and U-Boot mainline, and
> > didn't see any progress being made (so I thought I would do it myself).
> > Basically, my goal all along has been to ensure the Odroid Go Advance
> > is fully supported in mainline Linux (and U-boot), and that requires
> > we get the audio (done), SFC (in progress), crypto, battery, charger,
> > and everything else (done) supported.
> > 
> > I trust you are better able to develop/test this than I am, as I only
> > have access to some technical documents and a single PX30 based device
> > to test this on.
> > 
> > Please CC me when you are ready to submit upstream to Linux and U-boot
> > and I will happily test it on my hardware. If you don't have a U-boot
> > driver in progress let me know and I can start working on one.
> > 
> > Thank you.
> 
> 
> I'm glad to receive your reply and your understanding so soon. I'll talk
> about my plan directly. If there is no obvious objection,I'll go on with it.
> 
> Firstly, I will submit the support for the my first version of Linux drivers
> next week which is base on your series submission.
> 
> After browsing your submission, there will be the following changes:
> 1) [RFC,v4,1/8] change with the "Rob Herring" comment
> 2) [RFC,v4,2/8] update driver
> 3) All dt patches, make a little change to compatible with RK SDK
> 
> [RFC,v4,8/8] arm64: dts: rockchip: Enable SFC for Odroid Go Advance
> [RFC,v4,7/8] arm64: dts: rockchip: Add SFC to RK3308
> [RFC,v4,6/8] arm: dts: rockchip: Add SFC to RV1108
> [RFC,v4,5/8] arm: dts: rockchip: Add SFC to RK3036
> [RFC,v4,4/8] clk: rockchip: Add support for hclk_sfc on rk3036
> [RFC,v4,3/8] arm64: dts: rockchip: Add SFC to PX30
> [RFC,v4,2/8] spi: rockchip-sfc: add rockchip serial flash controller
> [RFC,v4,1/8] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash
> controller
> 
> Secondly, Keep devolop uboot drivers base on your serieas submission
> https://patchwork.ozlabs.org/project/uboot/list//?series=245724
> submit the code under uboot as soon as possible.

I'm actually abandoning that series of commits, in favor of one more closely
related to the linux driver you're putting in place. I'd like to use the
spi-mem framework for U-boot as well.  I will pursue it instead and try to
have something ready soon. I'm not entirely clear though from this message
if you are planning on doing that as well or are asking me to pursue it.
I'm entirely okay with you pursuing it, but until you tell me you are I will
keep working on it so as to hopefully get something ready for the next pull
window.

> 
> Finally I'll keep focus on the new comments about all these linux
> patches,then fix it until they are merged.
> 
> Best wishes.

Thank you. As you make changes I will test them and provide my tested-by tag.
For now I only have an rk3326 to test them on, but I will do my best to be
thorough.

> 
> 
> > > 
> > > 
> > > jon.lin@rock-chips.com
> > > From: Kever Yang
> > > Date: 2021-06-02 09:27
> > > To: Chris Morgan
> > > CC: u-boot; heiko.stuebner; vigneshr; jagan; sjg; Chris Morgan; 赵仪峰; 林鼎强
> > > Subject: Re: [RFC 0/5] rockchip_sfc: add support for Rockchip SFC
> > > Hi Chris,
> > > On 2021/6/2 上午12:54, Chris Morgan wrote:
> > > > On Tue, Jun 01, 2021 at 08:22:09PM +0800, Kever Yang wrote:
> > > > > Add Yifeng from rockchip.
> > > > > 
> > > > > Hi Chris,
> > > > > 
> > > > >       First of all, I think you should remain the origin author info in the
> > > > > signed-off.
> > > > Okay, I can do that. Please note that since I submitted this I was
> > > > asked to redo the upstream linux proposed driver to use the spi-mem
> > > > framework. I think for now honestly I'd like to abandon this patch
> > > > and resubmit a little later with one that is more or less the same
> > > > (using the spi-mem framework) as the Linux driver.
> > > This sounds better, then we can wait for driver with new framework.
> > > Thanks,
> > > - Kever
> > > > https://patchwork.ozlabs.org/project/linux-mtd/patch/20210528170020.26219-2-macroalpha82@gmail.com/
> > > > 
> > > > > Hi Yifeng,
> > > > > 
> > > > >       Please help to review this driver.
> > > > > 
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > - Kever
> > > > > 
> > > > > On 2021/5/26 上午5:49, Chris Morgan wrote:
> > > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > > > 
> > > > > > Requesting comments for a proposed patchset for adding the Rockchip
> > > > > > serial flash controller to u-boot. The goal of these patches is to
> > > > > > enable it for the Odroid Go Advance so that it may eventually boot
> > > > > > exclusively from the SFC on mainline U-boot (I have tested this and
> > > > > > it works).
> > > > > > 
> > > > > > The specific help I need with this patch is:
> > > > > > 
> > > > > > 1) I don't know the best way to upstream the XTX25F128B flash chip.
> > > > > > This chip uses a continuation code for the manufacturer ID, however I
> > > > > > cannot seem to find any way to actually read the continuation code.
> > > > > > There is a risk of this driver, used as-is, to collide with another
> > > > > > chip which has the same manufacturer ID with a different continuation
> > > > > > code.
> > > > > > 
> > > > > > 2) The Rockchip SFC driver itself (as it is mostly as-is from the BSP
> > > > > > U-Boot sources) supports SPI NAND and chips of varying sizes, but my
> > > > > > implementation only permits me to test with a single 128Mb flash chip.
> > > > > > The driver itself does some checking on the bitlen in the routine
> > > > > > rockchip_sfc_xfer() which is what is called for the dm_spi_ops.xfer.
> > > > > > I'm not sure if there is a better way to do this. Additionally, I have
> > > > > > to bit-shift the address written to the SFC as I suspect the value is
> > > > > > meant to be left justified, but I never tested it further.
> > > > > > 
> > > > > > Additionally, it might be worth mentioning but I noticed the Rockchip
> > > > > > BROM will only boot the TPL/SPL off of the SFC if I write it to address
> > > > > > 0x10000. This is not documented and different than the address looked
> > > > > > at for SD card booting (512 * 64 = 0x8000 for SD Card booting). Also,
> > > > > > like the SD card driver I can confirm that if DMA is enabled at the SPL
> > > > > > stage A-TF seems to fail silently, then when Linux loads it hangs.
> > > > > > There is an ifdef to force FIFO mode only in the SPL stage.
> > > > > > 
> > > > > > Tested: Read (works)
> > > > > > Write (works if you write to an erased sector)
> > > > > > Erase (works)
> > > > > > SPL Read (works if you edit the u-boot,spl-boot-order)
> > > > > > 
> > > > > > Chris Morgan (5):
> > > > > >      spi: rockchip_sfc: add support for Rockchip SFC
> > > > > >      rockchip: px30: Add support for using SFC
> > > > > >      rockchip: px30: add the serial flash controller
> > > > > >      mtd: spi-nor-ids: Add XTX XT25F128B
> > > > > >      rockchip: px30: add support for SFC for Odroid Go Advance
> > > > > > 
> > > > > >     arch/arm/dts/px30.dtsi                     |  38 ++
> > > > > >     arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  10 +-
> > > > > >     arch/arm/dts/rk3326-odroid-go2.dts         |  22 +
> > > > > >     arch/arm/mach-rockchip/px30/px30.c         |  64 ++
> > > > > >     drivers/mtd/spi/Kconfig                    |   6 +
> > > > > >     drivers/mtd/spi/spi-nor-ids.c              |   8 +
> > > > > >     drivers/spi/Kconfig                        |   8 +
> > > > > >     drivers/spi/Makefile                       |   1 +
> > > > > >     drivers/spi/rockchip_sfc.c                 | 652 +++++++++++++++++++++
> > > > > >     9 files changed, 926 insertions(+), 1 deletion(-)
> > > > > >     create mode 100644 drivers/spi/rockchip_sfc.c
> > > > > > 
> > 
> > 
> > 
> > 
> > 
> 
>