From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
To: Chris Brandt <Chris.Brandt@renesas.com>,
Mark Brown <broonie@kernel.org>, Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>
Cc: "linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>,
"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
Mason Yang <masonccyang@mxic.com.tw>
Subject: Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
Date: Wed, 11 Dec 2019 22:09:44 +0300 [thread overview]
Message-ID: <e6a73df5-31c4-3472-f7bc-a0984f1f5380@cogentembedded.com> (raw)
In-Reply-To: <TY1PR01MB156215E8668C0317FA0826B18A580@TY1PR01MB1562.jpnprd01.prod.outlook.com>
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
next prev parent reply other threads:[~2019-12-11 19:09 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
2019-12-06 13:41 ` [PATCH v2 1/6] spi: Add SPIBSC driver Chris Brandt
2019-12-12 19:36 ` Sergei Shtylyov
2019-12-12 20:19 ` Chris Brandt
2019-12-13 10:01 ` Geert Uytterhoeven
2019-12-13 14:45 ` Chris Brandt
2019-12-13 14:48 ` Geert Uytterhoeven
2019-12-13 19:37 ` Sergei Shtylyov
2019-12-13 18:36 ` Sergei Shtylyov
2019-12-13 19:40 ` Sergei Shtylyov
2019-12-13 20:43 ` Chris Brandt
2019-12-16 18:47 ` Sergei Shtylyov
2019-12-06 13:41 ` [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings Chris Brandt
2019-12-09 14:09 ` Geert Uytterhoeven
2019-12-09 15:45 ` Chris Brandt
2019-12-09 19:34 ` Geert Uytterhoeven
2019-12-10 20:07 ` Sergei Shtylyov
2019-12-10 20:17 ` Geert Uytterhoeven
2019-12-10 20:33 ` Chris Brandt
2019-12-10 20:23 ` Chris Brandt
2019-12-06 13:41 ` [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock Chris Brandt
2019-12-06 18:40 ` Sergei Shtylyov
2019-12-06 19:49 ` Chris Brandt
2019-12-20 14:38 ` Geert Uytterhoeven
2019-12-20 14:50 ` Chris Brandt
2019-12-06 13:42 ` [PATCH v2 4/6] ARM: dts: r7s72100: Add SPIBSC devices Chris Brandt
2019-12-06 13:42 ` [PATCH v2 5/6] ARM: dts: r7s9210: Add SPIBSC device Chris Brandt
2019-12-06 13:42 ` [PATCH v2 6/6] ARM: dts: gr-peach: Enable SPIBSC Chris Brandt
2019-12-07 20:28 ` [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Sergei Shtylyov
2019-12-09 15:10 ` Chris Brandt
2019-12-11 19:09 ` Sergei Shtylyov [this message]
2019-12-12 14:29 ` Chris Brandt
2019-12-12 15:28 ` Mark Brown
2019-12-12 16:53 ` Chris Brandt
2019-12-12 17:13 ` Mark Brown
2019-12-12 17:25 ` Chris Brandt
2019-12-16 15:21 ` Mark Brown
2019-12-16 20:31 ` Sergei Shtylyov
2019-12-16 22:21 ` Chris Brandt
2019-12-17 19:30 ` Sergei Shtylyov
2019-12-17 20:26 ` Geert Uytterhoeven
2019-12-19 16:57 ` Chris Brandt
2019-12-19 19:01 ` Sergei Shtylyov
2019-12-19 21:04 ` Chris Brandt
2019-12-20 1:45 ` masonccyang
2019-12-20 7:55 ` Geert Uytterhoeven
2019-12-24 16:58 ` Sergei Shtylyov
2019-12-27 0:58 ` Mark Brown
2019-12-17 19:44 ` Sergei Shtylyov
2019-12-18 8:09 ` Boris Brezillon
2019-12-19 16:32 ` Chris Brandt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e6a73df5-31c4-3472-f7bc-a0984f1f5380@cogentembedded.com \
--to=sergei.shtylyov@cogentembedded.com \
--cc=Chris.Brandt@renesas.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=geert+renesas@glider.be \
--cc=linux-clk@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=masonccyang@mxic.com.tw \
--cc=mturquette@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).