RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller

[Chris Brandt <Chris.Brandt@xxxxxxxxxxx>]

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