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

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Chris,

On Mon, Dec 9, 2019 at 4:45 PM Chris Brandt <Chris.Brandt@xxxxxxxxxxx> wrote:
> On Mon, Dec 9, 2019, Geert Uytterhoeven wrote:
> > > +    # 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.

To probe subnodes, your node needs to either be compatible with e.g.
"simple-bus", or have its own driver that calls of_platform_populate().

> 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).

Yes, and unlike "simple-bus", "simple-pm-bus" does handle Runtime PM,
so the clock will be enabled when needed.
BTW, I still think "simple-bus" should handle Runtime PM, and
"simple-pm-bus" should not exist.

> > 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.

Exactly.

> 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.

Yeah, the "real" serial FLASH functionality needs its own driver.
How to fit all the SPI/QSPI/HF pieces together in a working driver is
still TBD.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds