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

[Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>]

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