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@xxxxxxxxxxx> >> --- >> 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