| Message ID | 1546921020-20436-3-git-send-email-masonccyang@mxic.com.tw (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: Add Renesas R-Car Gen3 RPC-IF SPI driver | expand |
On 1/8/19 5:17 AM, Mason Yang wrote: > Document the bindings used by the Renesas R-Car Gen3 RPC-IF controller. > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> > --- > .../devicetree/bindings/spi/spi-renesas-rpc.txt | 37 ++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt > > diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt > new file mode 100644 > index 0000000..5f96532 > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt > @@ -0,0 +1,37 @@ > +Renesas R-Car Gen3 RPC-IF controller Device Tree Bindings > +---------------------------------------------------------- > + > +Required properties: > +- compatible: should be "renesas,r8a77995-rpc" > +- #address-cells: should be 1 > +- #size-cells: should be 0 > +- reg: should contain 2 entries, one for the registers and one for the direct > + mapping area > +- reg-names: should contain "regs" and "dirmap" > +- clock-names: should contain "rpc" > +- clocks: should contain 1 entries for the module's clock > +- renesas,rpc-mode: should contain "spi" for rpc spi mode or > + "hyperflash" for rpc hyperflash mode. Why do we need this property ? I believe it is possible to detect the configuration based on the type of child of the RPC node. If the driver was properly designed, it could well behave as either CFI NOR driver or SPI flash driver and all would be good, but it seems this is written with it being SPI flash driver only and once the HF mode would need to be added, it'd mean a tremendous undertaking to rework the entire driver.
Hi Mason, On Tue, Jan 8, 2019 at 5:17 AM Mason Yang <masonccyang@mxic.com.tw> wrote: > Document the bindings used by the Renesas R-Car Gen3 RPC-IF controller. > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> Thanks for your patch! > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt > @@ -0,0 +1,37 @@ > +Renesas R-Car Gen3 RPC-IF controller Device Tree Bindings > +---------------------------------------------------------- > + > +Required properties: > +- compatible: should be "renesas,r8a77995-rpc" Would it make sense to have a family-specific fallback "renesas,rcar-gen3-rpc", bseides the SoC-specific compatible value? </masonccyang@mxic.com.tw></masonccyang@mxic.com.tw> > +- #address-cells: should be 1 > +- #size-cells: should be 0 > +- reg: should contain 2 entries, one for the registers and one for the direct > + mapping area > +- reg-names: should contain "regs" and "dirmap" > +- clock-names: should contain "rpc" > +- clocks: should contain 1 entries for the module's clock Doesn't the driver have a need to access the RPCD2 clock? At least on R-Car V3M, it needs to program the Divider Clock Register (DIVREG). > +- renesas,rpc-mode: should contain "spi" for rpc spi mode or > + "hyperflash" for rpc hyperflash mode. Can't this be derived from the flash subnode? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
On 1/10/19 10:31 AM, masonccyang@mxic.com.tw wrote: > Hi Marek, Hi, >> "Marek Vasut" <marek.vasut@gmail.com> >> 2019/01/08 下午 08:06 >> > diff --git a/Documentation/devicetree/bindings/spi/spi-renesas- >> rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt >> > new file mode 100644 >> > index 0000000..5f96532 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt >> > @@ -0,0 +1,37 @@ >> > +Renesas R-Car Gen3 RPC-IF controller Device Tree Bindings >> > +---------------------------------------------------------- >> > + >> > +Required properties: >> > +- compatible: should be "renesas,r8a77995-rpc" >> > +- #address-cells: should be 1 >> > +- #size-cells: should be 0 >> > +- reg: should contain 2 entries, one for the registers and one >> for the direct >> > + mapping area >> > +- reg-names: should contain "regs" and "dirmap" >> > +- clock-names: should contain "rpc" >> > +- clocks: should contain 1 entries for the module's clock >> > +- renesas,rpc-mode: should contain "spi" for rpc spi mode or >> > + "hyperflash" for rpc hyperflash mode. >> >> Why do we need this property ? I believe it is possible to detect the >> configuration based on the type of child of the RPC node. If the driver >> was properly designed, it could well behave as either CFI NOR driver or >> SPI flash driver and all would be good, but it seems this is written >> with it being SPI flash driver only and once the HF mode would need to >> be added, it'd mean a tremendous undertaking to rework the entire driver. >> > > Except to check if there are any SPI NOR child nodes by "jedec,spi-nor" > compatible, is any other way better ? > > any suggestion is welcome. That's the one. A MFD RPC driver can sanitize it's child nodes, verify that the config is valid and configure the RPC accordingly. All of the devices that can be connected to the RPC are either valid jedec-nor or CFI NOR (HF), so this should work fine.
diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt new file mode 100644 index 0000000..5f96532 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt @@ -0,0 +1,37 @@ +Renesas R-Car Gen3 RPC-IF controller Device Tree Bindings +---------------------------------------------------------- + +Required properties: +- compatible: should be "renesas,r8a77995-rpc" +- #address-cells: should be 1 +- #size-cells: should be 0 +- reg: should contain 2 entries, one for the registers and one for the direct + mapping area +- reg-names: should contain "regs" and "dirmap" +- clock-names: should contain "rpc" +- clocks: should contain 1 entries for the module's clock +- renesas,rpc-mode: should contain "spi" for rpc spi mode or + "hyperflash" for rpc hyperflash mode. + +Example: + + rpc: rpc@ee200000 { + compatible = "renesas,r8a77995-rpc"; + reg = <0 0xee200000 0 0x8100>, <0 0x08000000 0 0x4000000>; + reg-names = "regs", "dirmap"; + clocks = <&cpg CPG_MOD 917>; + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; + resets = <&cpg 917>; + clock-names = "rpc"; + renesas,rpc-mode = "spi"; + #address-cells = <1>; + #size-cells = <0>; + + flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <40000000>; + spi-tx-bus-width = <1>; + spi-rx-bus-width = <1>; + }; + };
Document the bindings used by the Renesas R-Car Gen3 RPC-IF controller. Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> --- .../devicetree/bindings/spi/spi-renesas-rpc.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt