[v2,0/6] spi: Add Renesas SPIBSC controller
mbox series

Message ID 20191206134202.18784-1-chris.brandt@renesas.com
Headers show
Series
  • spi: Add Renesas SPIBSC controller
Related show

Message

Chris Brandt Dec. 6, 2019, 1:41 p.m. UTC
The Renesas SPI Bus Space Controller (SPIBSC) HW was specifically designed for
accessing Serial flash devices (QSPI, HyperFlash, Octa Flash). In the hardware
manuals, it is almost always labeled as the "Renesas SPI Multi I/O Bus Controller".
However, the HW IP is usually referred to within Renesas as the "SPI BSC".
Yes, the R-Car team nicknamed it RPC (for "Reduced Pin Count" flash) after HyperFash
support was added...but I personally think that RPC is not a good name for this
HW block.


This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.

The testing mostly consisted of formatting an area as JFFS2 and doing copying
of files and such.

While the HW changed a little between the RZ/A1 and RZ/A2 generations, the IP
block in the RZ/A2M was taken from the R-Car H3 design, so in theory this
driver should work for R-Car Gen3 as well.

=========================
Version 2 changes
=========================
* I got rid of all the critical clock stuff. The idea is is that if you are
  planning on using the SPI BSC, even in XIP mode, it should be described in DT.

* There is no actual 'runtime pm' implmented in the driver at the moment, and
  so just the standard enable/disable clock API is used.

* The compatible string "jedec,spi-nor" will be used to determine if a spi controller
  needs to be regitered or not. At the moment there is no setup needed for
  running in XIP mode, so we just need to signal that the peripheral clock should
  be left on and then we're done.




Chris Brandt (6):
  spi: Add SPIBSC driver
  dt-bindings: spi: Document Renesas SPIBSC bindings
  clk: renesas: r7s9210: Add SPIBSC clock
  ARM: dts: r7s72100: Add SPIBSC devices
  ARM: dts: r7s9210: Add SPIBSC device
  ARM: dts: gr-peach: Enable SPIBSC

 .../bindings/spi/renesas,spibsc.yaml          | 115 ++++
 arch/arm/boot/dts/r7s72100-gr-peach.dts       |   5 +
 arch/arm/boot/dts/r7s72100.dtsi               |  25 +-
 arch/arm/boot/dts/r7s9210.dtsi                |  11 +
 drivers/clk/renesas/r7s9210-cpg-mssr.c        |   1 +
 drivers/spi/Kconfig                           |   8 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-spibsc.c                      | 612 ++++++++++++++++++
 8 files changed, 776 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
 create mode 100644 drivers/spi/spi-spibsc.c

Comments

Sergei Shtylyov Dec. 7, 2019, 8:28 p.m. UTC | #1
Hello!

  Thank you for having mty on CC:, I might have missed oit otherwise... :-)

On 12/06/2019 04:41 PM, Chris Brandt wrote:

> The Renesas SPI Bus Space Controller (SPIBSC) HW was specifically designed for
> accessing Serial flash devices (QSPI,

   The initial design did only support SPI, hence the SPI in the name.

> HyperFlash, Octa Flash). In the hardware

   Only added in "2nd generation" controllers, like on R-Car gen3, RZ/A2. 

> manuals, it is almost always labeled as the "Renesas SPI Multi I/O Bus Controller".

   Not seeing "Renesas" but the rest looks consistent across the manuals.

> However, the HW IP is usually referred to within Renesas as the "SPI BSC".

   Poor name for the 2nd generation controllers which also support at least HyperFlash.

> Yes, the R-Car team nicknamed it RPC (for "Reduced Pin Count" flash) after HyperFash
> support was added...but I personally think that RPC is not a good name for this
> HW block.

   SPIBSC is also misleading... RPC-IF seems misleading too as it's only spelled out
in the R-Car gen3 and RZ/A2H manuals. 

> This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.

   In the SPI mode only, I assume?

   What I have now is the core driver (or rather a library) placed under drivers/memory/
and the SPI and HyperFlash front ends in drivers/spi/ and drivers/mtd/hyperbus/ respectfully.
I'm almost ready to post the core driver/bindings, the SPI driver still needs some Mark Brown's
comments addressed, and the HyperFlash driver is also ready but needs the existing HyperBus
infrastructure properly fixed up (having a draft patch now)...

> The testing mostly consisted of formatting an area as JFFS2 and doing copying
> of files and such.

   Did the same (or at least tried to :-) and I must admit that writing doesn't work with
any of the front ends... I still need to get this fixed.

> While the HW changed a little between the RZ/A1 and RZ/A2 generations, the IP
> block in the RZ/A2M was taken from the R-Car H3 design, so in theory this
> driver should work for R-Car Gen3 as well.

   I don't think it's a good idea to use the SPI dedicated driver on R-Car gen3, I would rather
see the RZ/A1 using the RPC-IF driver/library to reduce the code duplication...

> =========================
> Version 2 changes
> =========================
> * I got rid of all the critical clock stuff. The idea is is that if you are
>   planning on using the SPI BSC, even in XIP mode, it should be described in DT.
> 
> * There is no actual 'runtime pm' implmented in the driver at the moment, and
>   so just the standard enable/disable clock API is used.

   My code does have RPM enabled and used.

> * The compatible string "jedec,spi-nor" will be used to determine if a spi controller
>   needs to be regitered or not. At the moment there is no setup needed for
>   running in XIP mode, so we just need to signal that the peripheral clock should
>   be left on and then we're done.

[...]

MBR, Sergei
Chris Brandt Dec. 9, 2019, 3:10 p.m. UTC | #2
Hello Sergei,

On Sat, Dec 7, 2019, Sergei Shtylyov wrote:
> > The Renesas SPI Bus Space Controller (SPIBSC) HW was specifically
> > designed for accessing Serial flash devices (QSPI,
> 
>    The initial design did only support SPI, hence the SPI in the name.

The more important part is the "Bus Space Controller". Meaning the main 
purpose of this hardware was to allow the CPU to access serial flash 
directly (as in, XIP).

"SPI-BSC" was the internal name for the HW but does not appear in any of
the hardware manual. The hardware manuals (even the MCUs) only say "SPI
Multi I/O Bus Controller".
Even the R-car gen3 manual says 'SPI':  "SPI Multi I/O Bus Controller 
(RPC)".

I have no idea why the R-Car people felt they needed to put "RPC" in the
hardware manual as the title of the chapter. (Although, "Multi I/O" is 
just as bad as a name)
 
I did make the request to the RZ/G team to not put "RPC" in the title of
the chapter in any future RZ/G hardware manuals.

Since QSPI, HyperFlash and OctaFlash are all 'serial' Flash 
technologies, I would be find with a driver name of "SBSC" ("Serial Bus Space 
Controller") which at least looks closer to what is in all the hardware 
manuals.


>    SPIBSC is also misleading... RPC-IF seems misleading too as it's only
> spelled out in the R-Car gen3 and RZ/A2H manuals.

In the RZ/A2 manual, "RPC" is only used to label the 3 new external pins
that were added for HyperFlash.
  RPC_RESET# , RPC_WP# , RPC_INT#
But of course they were just copied from the R-Car manual.

But, maybe that's enough about the name for now.


> > This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.
> 
>    In the SPI mode only, I assume?

Yes. At the moment, there are only requests from users for QSPI flash access
(RZ/A and RZ/G users).

The RZ/A2M EVB was laid out to support all the different combinations of
serial flashes (by populating different chips). That is why there is 
already Segger J-link support for QSPI, Hyper and Octa for the RZ/A2.

I will admit, to developed this driver for the "SPI-BSC" HW, I have been
using an XIP kernel (XIP from another HyperFlash / HyperRAM combo chip 
on the board) because I didn't feel like moving all the switches to use 
SDRAM and a uImage kernel.
The RZ/A2M has a HyperFlash controller (for R/W), a OctaBus controller 
(for R/W) and the SPI BSC (Read-only).


>    What I have now is the core driver (or rather a library) placed under
> drivers/memory/ and the SPI and HyperFlash front ends in drivers/spi/ and
> drivers/mtd/hyperbus/ respectfully.
> I'm almost ready to post the core driver/bindings, the SPI driver still needs
> some Mark Brown's comments addressed, and the HyperFlash driver is also ready
> but needs the existing HyperBus infrastructure properly fixed up (having a
> draft patch now)...

But are these for the HyperBus controller? Or the SPI-BSC controller?
They are 2 different controllers, so you would think they would have 2 different drivers.


> > The testing mostly consisted of formatting an area as JFFS2 and doing
> > copying of files and such.
> 
>    Did the same (or at least tried to :-) and I must admit that writing
> doesn't work with any of the front ends... I still need to get this fixed.


That's the part I'm confused about. I saw the last patch series that 
made it up to v17 but still didn't get in. Although, it did look very 
complicated.
You can see from my SPI-BSC driver, it's basically 2 function: a SPI 
write and SPI read. The upper layer sends you down data to write, and you 
just write it. In theory, if a HyperFlash MTD layer was sending down 
data, the commands bytes would be different, but the procedure would be the 
same.


> > While the HW changed a little between the RZ/A1 and RZ/A2 generations,
> > the IP block in the RZ/A2M was taken from the R-Car H3 design, so in
> > theory this driver should work for R-Car Gen3 as well.
> 
>    I don't think it's a good idea to use the SPI dedicated driver on R-Car
> gen3, I would rather see the RZ/A1 using the RPC-IF driver/library to reduce
> the code duplication...

I agree on not having competing drivers. Especially since future RZ/A 
and RZ/G devices will most likely continue to include this HW.

However, the driver I posted is pretty simple and works. Does the 
HyperFlash MTD library that you are proposing have a very different API than 
just 'send bytes' and 'receive bytes'?


Chris
Sergei Shtylyov Dec. 11, 2019, 7:09 p.m. UTC | #3
On 12/09/2019 06:10 PM, Chris Brandt wrote:

>>> The Renesas SPI Bus Space Controller (SPIBSC) HW was specifically
>>> designed for accessing Serial flash devices (QSPI,
>>
>>    The initial design did only support SPI, hence the SPI in the name.
> 
> The more important part is the "Bus Space Controller". Meaning the main 
> purpose of this hardware was to allow the CPU to access serial flash 
> directly (as in, XIP).
> 
> "SPI-BSC" was the internal name for the HW but does not appear in any of
> the hardware manual. The hardware manuals (even the MCUs) only say "SPI
> Multi I/O Bus Controller".
> Even the R-car gen3 manual says 'SPI':  "SPI Multi I/O Bus Controller 
> (RPC)".
> 
> I have no idea why the R-Car people felt they needed to put "RPC" in the
> hardware manual as the title of the chapter. (Although, "Multi I/O" is 
> just as bad as a name)
>  
> I did make the request to the RZ/G team to not put "RPC" in the title of
> the chapter in any future RZ/G hardware manuals.
> 
> Since QSPI, HyperFlash and OctaFlash are all 'serial' Flash 
> technologies, I would be find with a driver name of "SBSC" ("Serial Bus Space 
> Controller") which at least looks closer to what is in all the hardware 
> manuals.

   How about "Serial Flash Controller" instead?

>>    SPIBSC is also misleading... RPC-IF seems misleading too as it's only
>> spelled out in the R-Car gen3 and RZ/A2H manuals.
> 
> In the RZ/A2 manual, "RPC" is only used to label the 3 new external pins
> that were added for HyperFlash.

   Sorry, I was to hasty to check the RZ/A2H manual before typing. :-/

>   RPC_RESET# , RPC_WP# , RPC_INT#
> But of course they were just copied from the R-Car manual.
> 
> But, maybe that's enough about the name for now.

   OK. :-)

>>> This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.
>>
>>    In the SPI mode only, I assume?
> 
> Yes. At the moment, there are only requests from users for QSPI flash access
> (RZ/A and RZ/G users).

   I keep being told by the management that we need HyperFlash too. :-)
In our BSP development, our engineers went "same hardware, 2 drivers"
way (with different "compatibles" per driver)...

> The RZ/A2M EVB was laid out to support all the different combinations of
> serial flashes (by populating different chips). That is why there is 
> already Segger J-link support for QSPI, Hyper and Octa for the RZ/A2.
> 
> I will admit, to developed this driver for the "SPI-BSC" HW, I have been
> using an XIP kernel (XIP from another HyperFlash / HyperRAM combo chip 
> on the board) because I didn't feel like moving all the switches to use 
> SDRAM and a uImage kernel.
> The RZ/A2M has a HyperFlash controller (for R/W), a OctaBus controller 
> (for R/W) and the SPI BSC (Read-only).

   Seen these...

>>    What I have now is the core driver (or rather a library) placed under
>> drivers/memory/ and the SPI and HyperFlash front ends in drivers/spi/ and
>> drivers/mtd/hyperbus/ respectfully.
>> I'm almost ready to post the core driver/bindings, the SPI driver still needs
>> some Mark Brown's comments addressed, and the HyperFlash driver is also ready
>> but needs the existing HyperBus infrastructure properly fixed up (having a
>> draft patch now)...

> But are these for the HyperBus controller? Or the SPI-BSC controller?
> They are 2 different controllers, so you would think they would have 2 different drivers.

   R-Car gen3 only has RPC-IF, no separate HyperBus controller, so the second case.

>>> The testing mostly consisted of formatting an area as JFFS2 and doing
>>> copying of files and such.
>>
>>    Did the same (or at least tried to :-) and I must admit that writing
>> doesn't work with any of the front ends... I still need to get this fixed.

   The last word from our BSP people was that JFFS2 doesn't work with the HyperFLash
dedicated BSP driver... :-/

> That's the part I'm confused about. I saw the last patch series that 
> made it up to v17 but still didn't get in. Although, it did look very 
> complicated.
> You can see from my SPI-BSC driver, it's basically 2 function: a SPI 

   I'll read/try it next thing...

> write and SPI read. The upper layer sends you down data to write, and you 
> just write it. In theory, if a HyperFlash MTD layer was sending down 
> data, the commands bytes would be different, but the procedure would be the 
> same.

   Yeah, the commands are different...

>>> While the HW changed a little between the RZ/A1 and RZ/A2 generations,
>>> the IP block in the RZ/A2M was taken from the R-Car H3 design, so in
>>> theory this driver should work for R-Car Gen3 as well.
>>
>>    I don't think it's a good idea to use the SPI dedicated driver on R-Car
>> gen3, I would rather see the RZ/A1 using the RPC-IF driver/library to reduce
>> the code duplication...
> 
> I agree on not having competing drivers. Especially since future RZ/A 
> and RZ/G devices will most likely continue to include this HW.

> However, the driver I posted is pretty simple and works. Does the 
> HyperFlash MTD

   There's no HF library, only front end driver.
   The real library covers both SPI and HF. The only difference between the two
is the h/w setup (minor difference).

> library that you are proposing have a very different API than 
> just 'send bytes' and 'receive bytes'?

   There's "prepare" and "transfer" APIs and also "direct map read" API.

> Chris

MBR, Sergei
Chris Brandt Dec. 12, 2019, 2:29 p.m. UTC | #4
On Wed, Dec 11, 2019, Sergei Shtylyov wrote:
> > Since QSPI, HyperFlash and OctaFlash are all 'serial' Flash
> > technologies, I would be find with a driver name of "SBSC" ("Serial
> > Bus Space
> > Controller") which at least looks closer to what is in all the
> > hardware manuals.
> 
>    How about "Serial Flash Controller" instead?

I would like that better than "RPC". At least it describes what it is.
RPC seems like a stupid name to me (but maybe that's just because I know
how that name was chosen...)
https://www.cypress.com/news/cypress-simplifies-embedded-system-design-new-low-pin-count-hyperram-memory
 "The HyperRAM and HyperFlash solution reduces pin count by at least 28 pins, ..."


As a side note, there is another HW block in Renesas that does the same 
thing as the SPI-BSC that they use in the MCU devices. That one they 
just named "QSPI".

> >>> This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.
> >>
> >>    In the SPI mode only, I assume?
> >
> > Yes. At the moment, there are only requests from users for QSPI flash
> > access (RZ/A and RZ/G users).
> 
>    I keep being told by the management that we need HyperFlash too. :-) In
> our BSP development, our engineers went "same hardware, 2 drivers"
> way (with different "compatibles" per driver)...

My plan was same HW, same "compatibles", same driver...but the driver 
would either register a SPI controller or a Hyperflash controller.

Note that the MMC/SDHI is the same HW but can act like 2 different peripherals.
We also have USB that can be either host or peripheral.


> >>> The testing mostly consisted of formatting an area as JFFS2 and
> >>> doing copying of files and such.
> >>
> >>    Did the same (or at least tried to :-) and I must admit that
> >> writing doesn't work with any of the front ends... I still need to get
> this fixed.
> 
>    The last word from our BSP people was that JFFS2 doesn't work with the
> HyperFLash dedicated BSP driver... :-/

Is that why this "RPC" patch series is taking so long?
It's a fairly simple piece of hardware.

When I first saw the series on the mailing list, my plan was to just wait
and then add RZ/A1 and RZ/A2 support. But....it looks like it all died.

So, I thought I would at least put in my own driver for SPI flash now, 
and then go back and add HyperFlash/OctaFlash once I get the chips 
swapped out on one of my RZ/A2 boards.


> > However, the driver I posted is pretty simple and works. Does the
> > HyperFlash MTD
> 
>    There's no HF library, only front end driver.
>    The real library covers both SPI and HF. The only difference between the
> two is the h/w setup (minor difference).

But is this "library" something specific to Renesas devices?
That's what I'm trying to understand.

My understanding is that HyperFlash uses standard CFI commands, so all 
we need to do is register a CFI device in the driver, just like we 
register a serial flash device.

(I guess I could go look at the sample code for our RTOS package and find out)


> > library that you are proposing have a very different API than just
> > 'send bytes' and 'receive bytes'?
> 
>    There's "prepare" and "transfer" APIs and also "direct map read" API.

I wonder what is the value of the "direct map read" (other than XIP in 
RZ/A systems). If you really want to directly access the flash (no 
buffering though the MTD layer), you need to register as a mtd-rom device, 
and then you don't really need an API at all.


Chris
Mark Brown Dec. 12, 2019, 3:28 p.m. UTC | #5
On Thu, Dec 12, 2019 at 02:29:07PM +0000, Chris Brandt wrote:
> On Wed, Dec 11, 2019, Sergei Shtylyov wrote:

> >    The last word from our BSP people was that JFFS2 doesn't work with the
> > HyperFLash dedicated BSP driver... :-/

> Is that why this "RPC" patch series is taking so long?
> It's a fairly simple piece of hardware.

The submitter appeared to be having difficulty with feedback from the
reviewers with knowledge of the hardware, then it looks like the last
version of the patch set didn't get any comments from any of those
reviewers.
Chris Brandt Dec. 12, 2019, 4:53 p.m. UTC | #6
On Thu, Dec 12, 2019 1, Mark Brown wrote:
> > Is that why this "RPC" patch series is taking so long?
> > It's a fairly simple piece of hardware.
> 
> The submitter appeared to be having difficulty with feedback from the
> reviewers with knowledge of the hardware, then it looks like the last
> version of the patch set didn't get any comments from any of those
> reviewers.

I went and looked at the driver and it was more complicated than what I 
did so I wasn't sure if there was some more advanced functionality or
something that was trying to be achieved.

I admit, V1 of this hardware (in the RZ/A1) was easier than V2 (RZ/A2 & R-Car),
so I had to re-write our BSP driver when it came time to support RZ/A2.
Basically, when they added HyperFlash support...it 'changed' how SPI
support was working. 

I agree with Sergei that it is always better to have a common driver for
common HW across different SoCs. However, it's not clear yet why there
needs to be increased complexity.

There were some good suggestions from the V2 series that I think complete
this driver. But I have not sent out a V3 until I can better understand
this 'competing' solution.

Chris
Mark Brown Dec. 12, 2019, 5:13 p.m. UTC | #7
On Thu, Dec 12, 2019 at 04:53:26PM +0000, Chris Brandt wrote:

> There were some good suggestions from the V2 series that I think complete
> this driver. But I have not sent out a V3 until I can better understand
> this 'competing' solution.

Oh, this is a new driver for the same hardware as the RPC driver :(

I don't really know enough about the device and there was *huge* amounts
of discussion which I'd have to try to page in so it'd be really good if
there were some agreement among those of you working with this device as
to what the best way forwards is.  I'm not sure any of the issues were
at the framework level so that's probably more sensible anyway.
Chris Brandt Dec. 12, 2019, 5:25 p.m. UTC | #8
On Thu, Dec 12, 2019, Mark Brown wrote:
> Oh, this is a new driver for the same hardware as the RPC driver :(

Correct. More or less.
Of the 3 devices (RZ/A1, RZ/A2, R-Car), RZ/A1 only supported SPI.
RZ/A2 and R-Car now support SPI and HyperFlash.

But in my laziness, I never upstreamed the RZ/A1 driver 2 year ago...so here we are :)

> I don't really know enough about the device and there was *huge* amounts
> of discussion which I'd have to try to page in so it'd be really good if
> there were some agreement among those of you working with this device as
> to what the best way forwards is.  I'm not sure any of the issues were
> at the framework level so that's probably more sensible anyway.

I'm trying to figure out if the differences are 'technical' or
'ideological' (ie, R-Car use cases vs RZ use cases).
Of course we can do anything we want in our vendor BSPs, but I'd like
to see a common upstream path.

Chris
Mark Brown Dec. 16, 2019, 3:21 p.m. UTC | #9
On Thu, Dec 12, 2019 at 05:25:43PM +0000, Chris Brandt wrote:
> On Thu, Dec 12, 2019, Mark Brown wrote:

> > I don't really know enough about the device and there was *huge* amounts
> > of discussion which I'd have to try to page in so it'd be really good if
> > there were some agreement among those of you working with this device as
> > to what the best way forwards is.  I'm not sure any of the issues were
> > at the framework level so that's probably more sensible anyway.

> I'm trying to figure out if the differences are 'technical' or
> 'ideological' (ie, R-Car use cases vs RZ use cases).
> Of course we can do anything we want in our vendor BSPs, but I'd like
> to see a common upstream path.

That'd certainly be good to achieve.  Hopefully we can get some
agreement between everyone on what the best way forwards is.  I don't
recall any substantial framework concerns with either driver so I think
it's more a renesas question.
Sergei Shtylyov Dec. 16, 2019, 8:31 p.m. UTC | #10
Hello!

On 12/12/2019 05:29 PM, Chris Brandt wrote:

>>> Since QSPI, HyperFlash and OctaFlash are all 'serial' Flash
>>> technologies, I would be find with a driver name of "SBSC" ("Serial
>>> Bus Space
>>> Controller") which at least looks closer to what is in all the
>>> hardware manuals.
>>
>>    How about "Serial Flash Controller" instead?
> 
> I would like that better than "RPC". At least it describes what it is.
> RPC seems like a stupid name to me (but maybe that's just because I know
> how that name was chosen...)
> https://www.cypress.com/news/cypress-simplifies-embedded-system-design-new-low-pin-count-hyperram-memory
>  "The HyperRAM and HyperFlash solution reduces pin count by at least 28 pins, ..."
> 
> As a side note, there is another HW block in Renesas that does the same 
> thing as the SPI-BSC that they use in the MCU devices. That one they 

   MCU?

> just named "QSPI".

   I thought QSPI stands for quad SPI?

>>>>> This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.
>>>>
>>>>    In the SPI mode only, I assume?
>>>
>>> Yes. At the moment, there are only requests from users for QSPI flash
>>> access (RZ/A and RZ/G users).
>>
>>    I keep being told by the management that we need HyperFlash too. :-) In
>> our BSP development, our engineers went "same hardware, 2 drivers"
>> way (with different "compatibles" per driver)...
> 
> My plan was same HW, same "compatibles", same driver...but the driver 
> would either register a SPI controller or a Hyperflash controller.

   I don't think this is a very good idea (and where to place such a driver,
in drives/memory/?... With the separate driver files, you can only build the
needed driver, SPI or HyperFash (or both -- which would be the only choice
with your approach?).

> Note that the MMC/SDHI is the same HW but can act like 2 different peripherals.

   Hm, not sure I understand... Isn't MMC and SD driven by the same subsystem?

> We also have USB that can be either host or peripheral.

   I know...

>>>>> The testing mostly consisted of formatting an area as JFFS2 and
>>>>> doing copying of files and such.
>>>>
>>>>    Did the same (or at least tried to :-) and I must admit that
>>>> writing doesn't work with any of the front ends... I still need to get
>> this fixed.
>>
>>    The last word from our BSP people was that JFFS2 doesn't work with the
>> HyperFLash dedicated BSP driver... :-/

   The BSP driver was initially written for H3/M3 SoCs and did work there...
But on V3H this driver fails...

> Is that why this "RPC" patch series is taking so long?
> It's a fairly simple piece of hardware.

   No. I didn't really use our BSP drivers as a base, so basically had to
write the HyperFLash part from scratch and fix the issues as they appeared...

> When I first saw the series on the mailing list, my plan was to just wait
> and then add RZ/A1 and RZ/A2 support. But....it looks like it all died.

   No. :-) It was worked on during all these months... I just finally decided
to stop, take a deep breath, and post what patches I had accumulated, without
the whole driver suite working first... I'm sorry it took so long....

> So, I thought I would at least put in my own driver for SPI flash now, 
> and then go back and add HyperFlash/OctaFlash once I get the chips 
> swapped out on one of my RZ/A2 boards.

   Ah! My HyperFlash driver is ready, just the HyperBus core needs some
fixing (I have a draft patch for that)...

>>> However, the driver I posted is pretty simple and works. Does the
>>> HyperFlash MTD
>>
>>    There's no HF library, only front end driver.
>>    The real library covers both SPI and HF. The only difference between the
>> two is the h/w setup (minor difference).
> 
> But is this "library" something specific to Renesas devices?

   Yes, it covers only RPC-IF (as described in the gen3 manual).

> That's what I'm trying to understand.
> 
> My understanding is that HyperFlash uses standard CFI commands, so all 

   The CFI command set driver needed some changes too (e.g. using the status
register to determine if a command is done).

> we need to do is register a CFI device in the driver, just like we 
> register a serial flash device.

> (I guess I could go look at the sample code for our RTOS package and find out)
> 
>>> library that you are proposing have a very different API than just
>>> 'send bytes' and 'receive bytes'?
>>
>>    There's "prepare" and "transfer" APIs and also "direct map read" API.

  The 1st one prepares the values to be written in either SPI mode or direct
read mode registers. Then you can call "transfer" or "direct mao read" which
would write out the register values into either set...

> I wonder what is the value of the "direct map read" (other than XIP in 
> RZ/A systems). If you really want to directly access the flash (no 
> buffering though the MTD layer), you need to register as a mtd-rom device, 
> and then you don't really need an API at all.

  I'd leave this question to Boris, else I never complete this msg. :-) 

> Chris

MBR, Sergei
Chris Brandt Dec. 16, 2019, 10:21 p.m. UTC | #11
Hello

On Mon, Dec 16, 2019, Sergei Shtylyov wrote:
> > As a side note, there is another HW block in Renesas that does the same
> > thing as the SPI-BSC that they use in the MCU devices. That one they
> 
>    MCU?
Yup.
  But...it has no significance to this discussion though :)


> > When I first saw the series on the mailing list, my plan was to just wait
> > and then add RZ/A1 and RZ/A2 support. But....it looks like it all died.
> 
>    No. :-) It was worked on during all these months... I just finally decided
> to stop, take a deep breath, and post what patches I had accumulated, without
> the whole driver suite working first... I'm sorry it took so long....

So at the moment, there is nothing yet for me to 'try' on the RZ/A series, correct?


> > My understanding is that HyperFlash uses standard CFI commands, so all
> 
>    The CFI command set driver needed some changes too (e.g. using the status
> register to determine if a command is done).

So the existing MTD-SPI layer knows how to talk to SPI devices.
And, you've fixed up the existing CFI layer to talk to HyperFlash devices.
But, you do not want these MTD layer to talk directly to a Renesas HW driver?
You want to put another software layer in between?


I'm guessing that from this statement....

> >>> library that you are proposing have a very different API than just
> >>> 'send bytes' and 'receive bytes'?
> >>
> >>    There's "prepare" and "transfer" APIs and also "direct map read" API.
> 
>   The 1st one prepares the values to be written in either SPI mode or direct
> read mode registers. Then you can call "transfer" or "direct mao read" which
> would write out the register values into either set...

...that you want more control of the data stream being passed to the RPC-IF driver.
Correct??

It all keeps sounding complicated, unless I'm just not understanding the code
you are trying to implement.


Chris
Sergei Shtylyov Dec. 17, 2019, 7:30 p.m. UTC | #12
Hello!

On 12/17/2019 01:21 AM, Chris Brandt wrote:

>>> As a side note, there is another HW block in Renesas that does the same
>>> thing as the SPI-BSC that they use in the MCU devices. That one they
>>
>>    MCU?
> Yup.
>   But...it has no significance to this discussion though :)

   But what does the acronym mean?

>>> When I first saw the series on the mailing list, my plan was to just wait
>>> and then add RZ/A1 and RZ/A2 support. But....it looks like it all died.
>>
>>    No. :-) It was worked on during all these months... I just finally decided
>> to stop, take a deep breath, and post what patches I had accumulated, without
>> the whole driver suite working first... I'm sorry it took so long....
> 
> So at the moment, there is nothing yet for me to 'try' on the RZ/A series, correct?

   Why, I can send you a working version of the SPI driver, and even HF one if you're
interested.
 
>>> My understanding is that HyperFlash uses standard CFI commands, so all
>>
>>    The CFI command set driver needed some changes too (e.g. using the status
>> register to determine if a command is done).
> 
> So the existing MTD-SPI layer knows how to talk to SPI devices.

   SPI-NOR does it with a help of drivers/spi/spi-mem.c... As for the HyperFlash,
it needs a HyperBus driver (that I have a working patch for)...

> And, you've fixed up the existing CFI layer to talk to HyperFlash evices.

   Mostly Boris B. did it... :-)

> But, you do not want these MTD layer to talk directly to a Renesas HW driver?

   Yes, because the SPI-NOR and HyperBus have different driver APIs.

> You want to put another software layer in between?

   Yes.

> I'm guessing that from this statement....

>>>>> library that you are proposing have a very different API than just
>>>>> 'send bytes' and 'receive bytes'?
>>>>
>>>>    There's "prepare" and "transfer" APIs and also "direct map read" API.
>>
>>   The 1st one prepares the values to be written in either SPI mode or direct
>> read mode registers. Then you can call "transfer" or "direct mao read" which
>> would write out the register values into either set...
> 
> ...that you want more control of the data stream being passed to the RPC-IF driver.
> Correct??
> 
> It all keeps sounding complicated, unless I'm just not understanding the code
> you are trying to implement.

   Well, it reflects the fact that the "SPI mode" and "direct read" register sets
are largely identical. The "preparation" stage sets up the register values, the 
other stages use that data to set up the real register sets and then do the
needed transfers...

> Chris

MBR, Sergei
Sergei Shtylyov Dec. 17, 2019, 7:44 p.m. UTC | #13
On 12/16/2019 11:31 PM, Sergei Shtylyov wrote:

[...]
>> My understanding is that HyperFlash uses standard CFI commands, so all 
> 
>    The CFI command set driver needed some changes too (e.g. using the status
> register to determine if a command is done).
> 
>> we need to do is register a CFI device in the driver, just like we 
>> register a serial flash device.
> 
>> (I guess I could go look at the sample code for our RTOS package and find out)
>>
>>>> library that you are proposing have a very different API than just
>>>> 'send bytes' and 'receive bytes'?
>>>
>>>    There's "prepare" and "transfer" APIs and also "direct map read" API.
> 
>   The 1st one prepares the values to be written in either SPI mode or direct
> read mode registers. Then you can call "transfer" or "direct mao read" which
> would write out the register values into either set...
> 
>> I wonder what is the value of the "direct map read" (other than XIP in 
>> RZ/A systems). If you really want to directly access the flash (no 
>> buffering though the MTD layer), you need to register as a mtd-rom device, 
>> and then you don't really need an API at all.
> 
>   I'd leave this question to Boris, else I never complete this msg. :-) 

   Didn't really summon him, doing that now... :-)

>> Chris

MBR, Sergei
Geert Uytterhoeven Dec. 17, 2019, 8:26 p.m. UTC | #14
Hi Sergei,

On Tue, Dec 17, 2019 at 8:30 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 12/17/2019 01:21 AM, Chris Brandt wrote:
> >>> As a side note, there is another HW block in Renesas that does the same
> >>> thing as the SPI-BSC that they use in the MCU devices. That one they
> >>
> >>    MCU?
> > Yup.
> >   But...it has no significance to this discussion though :)
>
>    But what does the acronym mean?

Микроконтро́ллер (англ. Micro Controller Unit, MCU)

Gr{oetje,eeting}s,

                        Geert
Boris Brezillon Dec. 18, 2019, 8:09 a.m. UTC | #15
On Tue, 17 Dec 2019 22:44:14 +0300
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> On 12/16/2019 11:31 PM, Sergei Shtylyov wrote:
> 
> [...]
> >> My understanding is that HyperFlash uses standard CFI commands, so all   
> > 
> >    The CFI command set driver needed some changes too (e.g. using the status
> > register to determine if a command is done).
> >   
> >> we need to do is register a CFI device in the driver, just like we 
> >> register a serial flash device.  
> >   
> >> (I guess I could go look at the sample code for our RTOS package and find out)
> >>  
> >>>> library that you are proposing have a very different API than just
> >>>> 'send bytes' and 'receive bytes'?  
> >>>
> >>>    There's "prepare" and "transfer" APIs and also "direct map read" API.  
> > 
> >   The 1st one prepares the values to be written in either SPI mode or direct
> > read mode registers. Then you can call "transfer" or "direct mao read" which
> > would write out the register values into either set...
> >   
> >> I wonder what is the value of the "direct map read" (other than XIP in 
> >> RZ/A systems). If you really want to directly access the flash (no 
> >> buffering though the MTD layer), you need to register as a mtd-rom device, 
> >> and then you don't really need an API at all.  
> > 
> >   I'd leave this question to Boris, else I never complete this msg. :-)   
> 
>    Didn't really summon him, doing that now... :-)

The dirmap API has not been designed with XIP in mind (though we could
theoretically extend it to support XIP). The main reason we added this
feature is because most controllers have either a slow PIO based path
where you can basically send every command you want and a fast
direct-mapping path which only support specific cmds or patterns. We
also hide things behind an API instead of returning a virtual mapping
because some controllers have extra constraints when accessing the
direct mapping (alignment, minimum size, limited direct mapping window
size, ...). Not to mention that some drivers might want to use DMA to
not stall the CPU on flash accesses.
Chris Brandt Dec. 19, 2019, 4:32 p.m. UTC | #16
On Wed, Dec 18, 2019, Boris Brezillon wrote:
> The dirmap API has not been designed with XIP in mind (though we could
> theoretically extend it to support XIP). The main reason we added this
[snip]

Boris,

Thank you for explaining.

Chris
Chris Brandt Dec. 19, 2019, 4:57 p.m. UTC | #17
On Tue, Dec 17, 2019, Sergei Shtylyov wrote:
>    But what does the acronym mean?
QSPI = "Quad SPI"


> > So at the moment, there is nothing yet for me to 'try' on the RZ/A series,
> correct?
> 
>    Why, I can send you a working version of the SPI driver, and even HF one
> if you're
> interested.

The point of this whole discussion is to determine if we should have 2 drivers
for the same Renesas HW IP.

There was a RPC-IF patch series that made it to v17....and is now dead.

You sent a new RFC series for a new method, but all it had was low level APIs,
no MTD framework, do it didn't really do anything.

If there was a complete patch set that I could try on the RZ/A SoCs and 
get a working SPI MTD device to show up, then I would drop my efforts of
getting my driver in and just add RZ/A support to the R-Car driver.

Geert suggested out an easy solution for the "XIP" use case for RZ/A 
devices, and that mostly happens outside this driver, so I'm not worried 
about that anymore.

Honestly, I'll be out of the office until January, so it's not like I'm 
going to do anything with it until then. But if there is a complete series
to try by then, I will see how it performs on RZ/A boards.

Chris
Sergei Shtylyov Dec. 19, 2019, 7:01 p.m. UTC | #18
Hello!

On 12/19/2019 07:57 PM, Chris Brandt wrote:

>>> So at the moment, there is nothing yet for me to 'try' on the RZ/A series,
>> correct?
>>
>>    Why, I can send you a working version of the SPI driver, and even HF one
>> if you're
>> interested.
> 
> The point of this whole discussion is to determine if we should have 2 drivers
> for the same Renesas HW IP.
> 
> There was a RPC-IF patch series that made it to v17....and is now dead.
> 
> You sent a new RFC series for a new method, but all it had was low level APIs,
> no MTD framework, do it didn't really do anything.

   Apparently you have missed the previous RFC iteration, the MFD/SPI drivers posted
at end of May:

https://patchwork.kernel.org/patch/10969211/
https://patchwork.kernel.org/patch/10969213/
https://patchwork.kernel.org/patch/10969217/

   There's not yet merged MTD patch you'd need too:

http://patchwork.ozlabs.org/patch/1199645/

   The MFD driver was shot down by Lee Jones who has advised placing the common
code into drivers/memory/ instead... I don't want to re-post the SPI driver as
I haven't yet addressed all of Mark Brown's comments...

> If there was a complete patch set that I could try on the RZ/A SoCs and 
> get a working SPI MTD device to show up, then I would drop my efforts of
> getting my driver in and just add RZ/A support to the R-Car driver.

   Please try these patches, there's a big chance they'll work. 

[...]
> Honestly, I'll be out of the office until January, so it's not like I'm 
> going to do anything with it until then. But if there is a complete series
> to try by then, I will see how it performs on RZ/A boards.

   Hopefully I will have addressed Mark's feedback by then and post the new SPI
driver... Have happy holidays! (Ours happen on 1/1 and last till 1/8 this year.)

> Chris

MBR, Sergei
Chris Brandt Dec. 19, 2019, 9:04 p.m. UTC | #19
On Thu, Dec 19, 2019, Sergei Shtylyov wrote:
>    Apparently you have missed the previous RFC iteration, the MFD/SPI drivers
> posted
> at end of May:

OK, thanks. I'll go get them.
End of May....that was a while back.

>    The MFD driver was shot down by Lee Jones who has advised placing the
> common
> code into drivers/memory/ instead... I don't want to re-post the SPI driver
> as
> I haven't yet addressed all of Mark Brown's comments...

For now, I just want to check if functionally it works the same for RZ/A.

>    Please try these patches, there's a big chance they'll work.

If it does, that would be nice.
This HW is going to be continued to be used in new SoCs.


> Have happy holidays! (Ours happen on 1/1 and last till 1/8 this
> year.)

You too!

Chris

             *
            /.\
           /..'\
           /'.'\
          /.''.'\
          /.'.'.\
         /'.''.'.\
         ^^^[_]^^^
Mason Yang Dec. 20, 2019, 1:45 a.m. UTC | #20
Hello,
 
> On 12/19/2019 07:57 PM, Chris Brandt wrote:
> 
> >>> So at the moment, there is nothing yet for me to 'try' on the RZ/A 
series,
> >> correct?
> >>
> >>    Why, I can send you a working version of the SPI driver, and even 
HF one
> >> if you're
> >> interested.
> > 
> > The point of this whole discussion is to determine if we should have 2 
drivers
> > for the same Renesas HW IP.
> > 
> > There was a RPC-IF patch series that made it to v17....and is now 
dead.


It's under review by Geert Uytterhoeven

https://patchwork.kernel.org/project/linux-renesas-soc/list/?submitter=181859 


https://patchwork.kernel.org/patch/11078131/ 
https://patchwork.kernel.org/patch/11078133/ 


thanks & best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================
Geert Uytterhoeven Dec. 20, 2019, 7:55 a.m. UTC | #21
Hi Mason,

On Fri, Dec 20, 2019 at 2:45 AM <masonccyang@mxic.com.tw> wrote:
> > On 12/19/2019 07:57 PM, Chris Brandt wrote:
> > >>> So at the moment, there is nothing yet for me to 'try' on the RZ/A
> series,
> > >> correct?
> > >>
> > >>    Why, I can send you a working version of the SPI driver, and even
> HF one
> > >> if you're
> > >> interested.
> > >
> > > The point of this whole discussion is to determine if we should have 2
> drivers
> > > for the same Renesas HW IP.
> > >
> > > There was a RPC-IF patch series that made it to v17....and is now
> dead.
>
> It's under review by Geert Uytterhoeven
>
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?submitter=181859
>
>
> https://patchwork.kernel.org/patch/11078131/
> https://patchwork.kernel.org/patch/11078133/

It's marked "Under Review" in patchwork, as there haven't been any comments
on v17 yet.

Gr{oetje,eeting}s,

                        Geert
Sergei Shtylyov Dec. 24, 2019, 4:58 p.m. UTC | #22
Hello!

On 12/20/2019 04:45 AM, masonccyang@mxic.com.tw wrote:

>>>>> So at the moment, there is nothing yet for me to 'try' on the RZ/A series,
>>>> correct?
>>>>
>>>>    Why, I can send you a working version of the SPI driver, and even HF one
>>>> if you're
>>>> interested.
>>>
>>> The point of this whole discussion is to determine if we should have 2 drivers
>>> for the same Renesas HW IP.
>>>
>>> There was a RPC-IF patch series that made it to v17....and is now dead.
> 
> It's under review by Geert Uytterhoeven
> 
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?submitter=181859 
> https://patchwork.kernel.org/patch/11078131/ 
> https://patchwork.kernel.org/patch/11078133/

https://patchwork.kernel.org/patch/11078137/
https://patchwork.kernel.org/patch/11078139/ 

   It doesn't matter much what's in the renesas-soc patchwork, the patch would
be merged thru the linux-spi repo, and in their patchwork the status of your v17
patches is "New, archived"...

> thanks & best regards,
> Mason

MBR, Sergei
Mark Brown Dec. 27, 2019, 12:58 a.m. UTC | #23
On Tue, Dec 24, 2019 at 07:58:21PM +0300, Sergei Shtylyov wrote:

>    It doesn't matter much what's in the renesas-soc patchwork, the patch would
> be merged thru the linux-spi repo, and in their patchwork the status of your v17
> patches is "New, archived"...

To be fair patchwork isn't exactly used in the review process for
SPI like it is with some other subsystems so other than automated
updates when things get applied or superceeded the states don't
mean a huge amount.  Like I said elsewhere in the thread I'd been
expecting some review from other people working with this
hardware since up until that point it had been pretty active.