RE: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings

From: Chris Brandt <Chris.Brandt@renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: 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>,
	linux-spi <linux-spi@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	linux-clk <linux-clk@vger.kernel.org>,
	Mason Yang <masonccyang@mxic.com.tw>,
	Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Subject: RE: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
Date: Mon, 9 Dec 2019 15:45:08 +0000	[thread overview]
Message-ID: <TY1PR01MB1562BC84E2ECF81487A780D38A580@TY1PR01MB1562.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAMuHMdXW6_tCcx_DE66qBSTK8XmWyWm82ZD6h-N5YX_+xcvBtw@mail.gmail.com>

Hi Geert,

On Mon, Dec 9, 2019, Geert Uytterhoeven wrote:
> > +  reg:
> > +    minItems: 2
> > +    maxItems: 2
> > +    items:
> > +      - description: Registers
> > +      - description: Memory Mapped Address Space
> 
> The second one is not needed, if you would add "ranges" for the memory-mapped
> mode.

So for RZ/A2M:
  ranges = <0x0 0x20000000 0x10000000>;

> > +    # This example is for "External Address Space Read Mode"
> > +    spibsc: spi@1f800000 {
> > +        compatible = "renesas,r7s9210-spibsc";
> > +        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
> > +        clocks = <&cpg CPG_MOD 83>;
> > +        power-domains = <&cpg>;
> > +        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +    };
> > +    flash@20000000 {
> 
> This does not describe the hardware topology: the flash node should be a
> subnode of the spibsc node, as it relies on the spibsc being clocked.

So for the "XIP" case, I originally tried adding an "mtd-rom" flash node
under the spibsc node, but then the mtd-rom part never got probed. I 
guess that was because it didn't register a SPI controller.

But, I guess if we go your method...
>     spibsc: spi@1f800000 {
>                 compatible = "renesas,r7s9210-spibsc", "simple-pm-bus";

Then after the spibsc driver fails and the "simple-pm-bus" driver tries,
it will succeed and the simple-pm-bus driver will start probing the 
subnodes (in my case, the mtd-rom).

> and applying "[PATCH] mtd: maps: physmap: Add minimal Runtime PM support"[1],
> the memory-mapped case should work, without your spibsc driver.

Good.
So we can add the SPI-BSC clocks for RZ/A1 and RZ/A2 (even without the 
SPI-BSC driver) and still have a working solution for XIP_KERNEL.

So in the end, this all seems like a very simple solution to get 
everything I wanted with minimal complexity.

But, if Sergei is going a completely different route for R-Car, I guess 
I need to understand that first what he is trying to do before I really
push for this driver getting in.
Again, this driver was only when using the SPI-BSC HW, not the full 
(different) R/W HyperFlash controller HW. That would be a separate driver.

Chris