diff mbox series

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

Message ID 20191206134202.18784-3-chris.brandt@renesas.com (mailing list archive)
State Changes Requested
Delegated to: Geert Uytterhoeven
Headers show
Series spi: Add Renesas SPIBSC controller | expand

Commit Message

Chris Brandt Dec. 6, 2019, 1:41 p.m. UTC
Document the bindings used by the Renesas SPI bus space controller.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
 * change to YAML format
 * add interrupts property
 * Used more terms directly from the hardware manual
---
 .../bindings/spi/renesas,spibsc.yaml          | 115 ++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/renesas,spibsc.yaml

Comments

Geert Uytterhoeven Dec. 9, 2019, 2:09 p.m. UTC | #1
Hi Chris,

On Fri, Dec 6, 2019 at 2:43 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> Document the bindings used by the Renesas SPI bus space controller.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2:
>  * change to YAML format
>  * add interrupts property
>  * Used more terms directly from the hardware manual

Thanks for the update!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/renesas,spibsc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SPI Bus Space Controller (SPIBSC) Device Tree Bindings
> +
> +description: |
> +  Otherwise referred to as the "SPI Multi I/O Bus Controller" in SoC hardware
> +  manuals. This controller was designed specifically for accessing Serial flash
> +  devices such as SPI Flash, HyperFlash and OctaFlash. The HW can operate in two
> +  modes. One mode allows for normal byte by byte access (refereed to as
> +  "Manual Mode"). The other mode allows for direct memory mapped access (read
> +  only) to the flash area by the CPU (refereed to as "External Address Space
> +  Read Mode").
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +maintainers:
> +  - Chris Brandt <chris.brandt@renesas.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - renesas,r7s72100-spibsc     # RZ/A1
> +              - renesas,r7s9210-spibsc      # RZ/A2
> +
> +  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.

> +
> +  interrupts:
> +    description: Some HW versions do not contain interrupts
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  flash:
> +    description: |
> +      (Optional) In order to use the HW for R/W access ("Manual Mode"), a "flash"
> +      subnode must be present with a "compatible" property that contains
> +      "jedec,spi-nor". If a spi-nor property is not found, the HW is assumed to be
> +      already operating in "External Address Space Read Mode".
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - '#address-cells'
> +  - '#size-cells'

I would make the flash subnode mandatory.

> +
> +examples:
> +  - |
> +    # This example is for "Manual 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@0 {
> +            compatible = "jedec,spi-nor";
> +            reg = <0>;
> +            spi-max-frequency = <40000000>;
> +
> +            partitions {
> +                compatible = "fixed-partitions";
> +                #address-cells = <1>;
> +                #size-cells = <1>;
> +
> +                partition@0000000 {
> +                    label = "u-boot";
> +                    reg = <0x00000000 0x80000>;
> +                };
> +            };
> +        };
> +
> +    # 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.

I think when using:

    spibsc: spi@1f800000 {
                compatible = "renesas,r7s9210-spibsc", "simple-pm-bus";
                reg = <0x1f800000 0x100>;
                clocks = <&cpg CPG_MOD 83>;
                power-domains = <&cpg>;
                interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
                #address-cells = <1>;
                #size-cells = <1>;
                ranges = <0 0x20000000 0x10000000>;

                flash@0 {
                        ...
                };
    };

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

Of course, you still need your driver for using the FLASH in SPI mode.

> +        compatible = "mtd-rom";
> +        probe-type = "direct-mapped";
> +        reg = <0x20000000 0x4000000>;
> +        bank-width = <4>;
> +        device-width = <1>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
1> +
> +        partition@80000 {
> +            label ="uboot_env";
> +            reg = <0x00080000 0x010000>;
> +            read-only;
> +        };
> +    };

[1] https://lore.kernel.org/linux-mtd/20191209134823.13330-1-geert+renesas@glider.be/

Gr{oetje,eeting}s,

                        Geert
Chris Brandt Dec. 9, 2019, 3:45 p.m. UTC | #2
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
Geert Uytterhoeven Dec. 9, 2019, 7:34 p.m. UTC | #3
Hi Chris,

On Mon, Dec 9, 2019 at 4:45 PM Chris Brandt <Chris.Brandt@renesas.com> 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
Sergei Shtylyov Dec. 10, 2019, 8:07 p.m. UTC | #4
Hello!

On 12/09/2019 05:09 PM, Geert Uytterhoeven wrote:

>> Document the bindings used by the Renesas SPI bus space controller.
>>
>> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
>> ---
>> v2:
>>  * change to YAML format
>>  * add interrupts property
>>  * Used more terms directly from the hardware manual
> 
> Thanks for the update!
> 
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
[...]
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - renesas,r7s72100-spibsc     # RZ/A1
>> +              - renesas,r7s9210-spibsc      # RZ/A2
>> +
>> +  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.

   I'm not sure we can do that. The flash bus is accessed via a window
with the high bits in the DREAR reg, even in the direct read mode...

> 
>> +
>> +  interrupts:
>> +    description: Some HW versions do not contain interrupts
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  flash:
>> +    description: |
>> +      (Optional) In order to use the HW for R/W access ("Manual Mode"), a "flash"
>> +      subnode must be present with a "compatible" property that contains
>> +      "jedec,spi-nor". If a spi-nor property is not found, the HW is assumed to be
>> +      already operating in "External Address Space Read Mode".
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - '#address-cells'
>> +  - '#size-cells'
> 
> I would make the flash subnode mandatory.

   Agreed.

>> +
>> +examples:
>> +  - |
>> +    # This example is for "Manual 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@0 {
>> +            compatible = "jedec,spi-nor";
>> +            reg = <0>;
>> +            spi-max-frequency = <40000000>;
>> +
>> +            partitions {
>> +                compatible = "fixed-partitions";
>> +                #address-cells = <1>;
>> +                #size-cells = <1>;
>> +
>> +                partition@0000000 {
>> +                    label = "u-boot";
>> +                    reg = <0x00000000 0x80000>;
>> +                };
>> +            };
>> +        };
>> +
>> +    # 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.

   ACK.

[...]

> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergei
Geert Uytterhoeven Dec. 10, 2019, 8:17 p.m. UTC | #5
Hi Sergei,

On Tue, Dec 10, 2019 at 9:07 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 12/09/2019 05:09 PM, Geert Uytterhoeven wrote:
> >> Document the bindings used by the Renesas SPI bus space controller.
> >> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> >> ---
> >> v2:
> >>  * change to YAML format
> >>  * add interrupts property
> >>  * Used more terms directly from the hardware manual
> >
> > Thanks for the update!
> >
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
> [...]
> >> +properties:
> >> +  compatible:
> >> +    oneOf:
> >> +      - items:
> >> +          - enum:
> >> +              - renesas,r7s72100-spibsc     # RZ/A1
> >> +              - renesas,r7s9210-spibsc      # RZ/A2
> >> +
> >> +  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.
>
>    I'm not sure we can do that. The flash bus is accessed via a window
> with the high bits in the DREAR reg, even in the direct read mode...

So if the FLASH is too large, you cannot access all of it without
programming the high bits.
However, when using XIP,  you're limited to the window size anyway.

Gr{oetje,eeting}s,

                        Geert
Chris Brandt Dec. 10, 2019, 8:23 p.m. UTC | #6
On Tue, Dec 10, 2019, Sergei Shtylyov wrote:
> >> +    items:
> >> +      - description: Registers
> >> +      - description: Memory Mapped Address Space
> >
> > The second one is not needed, if you would add "ranges" for the
> > memory-mapped mode.
> 
>    I'm not sure we can do that. The flash bus is accessed via a window with
> the high bits in the DREAR reg, even in the direct read mode...

The purpose of this driver was to allow R/W access using the existing 
MTD/SPI layer.
When the HW is operating in this mode, the driver basically just needs 
to read the vale of "ranges" in the DT to know the base address, and 
that's all it uses it for.

If the HW is going to be used in 'direct read mode', then the MTD/SPI 
layer will not be used. Instead the MTD/ROM driver will be used.
Configuring the HW for this mode is out of scope at the moment and 
should be done in the boot loader (just like DDR or SDRAM is configured by 
the boot loader).
The MTD/ROM driver will also handle all the ioremapping based on the 
values in the partition values under the flash node.

Chris
Chris Brandt Dec. 10, 2019, 8:33 p.m. UTC | #7
On Tue, Dec 10, 2019, Geert Uytterhoeven wrote:
> > > The second one is not needed, if you would add "ranges" for the
> > > memory-mapped mode.
> >
> >    I'm not sure we can do that. The flash bus is accessed via a window
> > with the high bits in the DREAR reg, even in the direct read mode...
> 
> So if the FLASH is too large, you cannot access all of it without programming
> the high bits.
> However, when using XIP,  you're limited to the window size anyway.

If your SPI flash supports 32-bit address commands, ("4READ" for 
example) then all 32-bit are sent out.

The 'high bits' that he's talking about is for accessing SPI that is 
beyond the XIP window size.

For example, the RZ/A1 has 64MB window size (RZ/A2=256MB). Meaning 
anything above 64MB is not accessible.
However, you could program this register to basically change the window 
starting point and make the upper 64MB of a 128MB flash visible and the
lower 64MB portion hidden.
For an XIP system, this is one way of doing a remote firmware update 
and keeping the old copy around just in case you need to revert back.

Anyway, the point is this doesn't matter in either the MTD/SPI or 
MTD/ROM operation of the driver as far as I can see.

Chris
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
new file mode 100644
index 000000000000..afbdc0824cc6
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
@@ -0,0 +1,115 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/renesas,spibsc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas SPI Bus Space Controller (SPIBSC) Device Tree Bindings
+
+description: |
+  Otherwise referred to as the "SPI Multi I/O Bus Controller" in SoC hardware
+  manuals. This controller was designed specifically for accessing Serial flash
+  devices such as SPI Flash, HyperFlash and OctaFlash. The HW can operate in two
+  modes. One mode allows for normal byte by byte access (refereed to as
+  "Manual Mode"). The other mode allows for direct memory mapped access (read
+  only) to the flash area by the CPU (refereed to as "External Address Space
+  Read Mode").
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+maintainers:
+  - Chris Brandt <chris.brandt@renesas.com>
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r7s72100-spibsc     # RZ/A1
+              - renesas,r7s9210-spibsc      # RZ/A2
+
+  reg:
+    minItems: 2
+    maxItems: 2
+    items:
+      - description: Registers
+      - description: Memory Mapped Address Space
+
+  interrupts:
+    description: Some HW versions do not contain interrupts
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  flash:
+    description: |
+      (Optional) In order to use the HW for R/W access ("Manual Mode"), a "flash"
+      subnode must be present with a "compatible" property that contains
+      "jedec,spi-nor". If a spi-nor property is not found, the HW is assumed to be
+      already operating in "External Address Space Read Mode".
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#address-cells'
+  - '#size-cells'
+
+examples:
+  - |
+    # This example is for "Manual 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@0 {
+            compatible = "jedec,spi-nor";
+            reg = <0>;
+            spi-max-frequency = <40000000>;
+
+            partitions {
+                compatible = "fixed-partitions";
+                #address-cells = <1>;
+                #size-cells = <1>;
+
+                partition@0000000 {
+                    label = "u-boot";
+                    reg = <0x00000000 0x80000>;
+                };
+            };
+        };
+
+    # 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 {
+        compatible = "mtd-rom";
+        probe-type = "direct-mapped";
+        reg = <0x20000000 0x4000000>;
+        bank-width = <4>;
+        device-width = <1>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        partition@80000 {
+            label ="uboot_env";
+            reg = <0x00080000 0x010000>;
+            read-only;
+        };
+    };
+
+...