Hello! On 05/13/2019 12:37 PM, masonccyang@xxxxxxxxxxx wrote: >> > > [...] >> > > >> > >> > On 4/24/19 11:23 PM, Rob Herring wrote: >> > > >> > >> > > On Wed, Apr 24, 2019 at 03:55:36PM +0800, Mason Yang wrote: >> > > >> > >> > >> Document the bindings used by the Renesas R-Car >> Gen3 RPC-IF MFD. >> > > >> > >> > >> >> > > >> > >> > >> Signed-off-by: Mason Yang <masonccyang@xxxxxxxxxxx> >> > > >> > >> > >> --- >> > > >> > >> > >> .../devicetree/bindings/mfd/mfd-renesas-rpc.txt >> | 40 ++++++ >> > > >> > >> ++++++++++++++++ >> > > >> > >> > >> 1 file changed, 40 insertions(+) >> > > >> > >> > >> create mode 100644 Documentation/devicetree/ >> bindings/mfd/mfd- >> > > >> > >> renesas-rpc.txt >> > > >> > >> > >> >> > > >> > >> > >> diff --git a/Documentation/devicetree/bindings/mfd/ >> mfd-renesas- >> > > >> > >> rpc.txt b/Documentation/devicetree/bindings/mfd/mfd- >> renesas-rpc.txt >> > > >> > >> > >> new file mode 100644 >> > > >> > >> > >> index 0000000..668b822 >> > > >> > >> > >> --- /dev/null >> > > >> > >> > >> +++ b/Documentation/devicetree/bindings/mfd/mfd- >> renesas-rpc.txt >> > > >> > >> > >> @@ -0,0 +1,40 @@ >> > > >> > >> > >> +Renesas R-Car Gen3 RPC-IF MFD Device Tree Bindings >> > > >> > >> > >> +-------------------------------------------------- >> > > >> > >> > > >> > > >> > >> > > Looks like a SPI flash controller from the example. What >> > > makes it an >> > > >> > >> > > MFD? >> > > >> > >> > >> > > >> > >> > It supports both SPI NOR and HyperFlash (CFI-compliantflash with >> > > >> > >> > different bus interface). >> > > >> > >> >> > > >> > >> Looks like you're registering one OR the other. >> > > >> > >> >> > > >> > >> Why don't you just do this from DT? >> > > >> > >> >> > > >> > >> No reason for this to be an MFD IMHO. >> > > >> > > >> > > >> > > >> > > >> > > okay, I will patch it back to SPI mode only. >> > > >> > >> > > >> > I don't think that's what Lee meant . The controller supports _both_ >> > > >> > modes , hence it would have the same compatible string. You >> just need to >> > > >> > extract the mode of operation from the DT. >> > > >> >> > > >> HiSilicon attempted to upstream something similar, only their >> > > >> controller provided NAND and NOR functionality. They used different >> > > >> compatible strings to differentiate between the varying >> > > >> technologies. >> > > >> >> > > >> They too tried to use MFD as a means to select between them (which was >> > > >> also NACKed). Not sure what they ended up doing, but the original >> > > >> submission and (half of) the conversation can be found at [0]. Some >> > > >> more of the thread continues at [1]. >> > > >> >> > > >> Hope that helps. >> > > >> >> > > >> [0] https://groups.google.com/forum/#!topic/fa.linux.kernel/F6i9o8sfOIw >> > > >> [1] https://marc.info/?l=devicetree&m=147669165104431&w=2 >> > > > >> > > > >> > > > Hi Marek, >> > > > >> > > > By Jones's comments: >> > > > >> -------------------------------------------------------------------------- >> > > >> From: Shunquan Lin <linshunquan1@xxxxxxxxxxxxx> >> > > >> >> > > >> This patch adds driver support for HiSilicon Flash Memory >> > > >> Controller(FMC). HiSilicon FMC is a multi-functions device which >> > > >> supports SPI Nor flash controller, SPI nand Flash controller and >> > > >> parallel nand flash controller. >> > > > >> > > > MFDs are for devices which span multiple subsystems. >> > > >> > > And we do! One of the subdrivers will live under drivers/ >> spi/, the other >> > > under drivers/mtd/... >> > > >> > >> > From my point of view, I think Jones mean to MFD's subsystems are >> working simultaneously >> > at the run-time, one period of time is working for sub-device-1 >> and later period of time >> > is working for sub-device-2 and so on. >> > >> > But for RPC-IF, SPI or HF mode is decided at boot time by pins >> configure and later in kernel >> > by dtb, RPC-IF can't switch SPI and HF mode at the run time. >> >> > So far, Jones seems don't agree RPC-IF to MFD and then RPC MFD >> will not applied >> > to mfd tree by him ! >> >> There's precedence for such constructs being an MFD: please see >> drivers/mfd/at91-usart.c, which registers a single MFD cell for either >> serial or SPI. Thanks fir your example, Geert! :-) > okay, many thanks for your information. > > How about to patch RPF-IF dts to: > --------------------------------------------------------------> > > Renesas R-Car Gen3 RPC-IF controller Device Tree Bindings > --------------------------------------------------------- > > RPC-IF supports both SPI NOR and HyperFlash (CFI-compliant flash) > > Required properties: > - compatible: should be an SoC-specific compatible value, followed by > "renesas,rcar-gen3-rpc" as a fallback. > supported SoC-specific values are: > "renesas,r8a77995-rpc" (R-Car D3) > - reg: should contain three register areas: > first for the base address of RPC-IF registers, I'd drop "the base address" here. > second for the direct mapping read mode and > third for the write buffer area. > - reg-names: should contain "regs", "dirmap" and "wbuf" > - clocks: should contain 1 entries for the module's clock > - clock-names: should contain "rpc" I suspect we'd need the RPC/RPCD2 clocks mentioned as well (not sure yet)... And how about "power-domains", "resets" (seen in the example below), also what about #address-cells & #size-cells? > > Example: Could you please indent with 1 or 2 tabs where you used 8 or 16 spaces? > - SPI mode: > > rpc: rpc-if@ee200000 { The node names should be generic, based on the device class. And in this case I'd like to use "spi@ee200000" as otherwise dtc keeps bitching like below: arch/arm64/boot/dts/renesas/r8a77980.dtsi:1344.21-1359.5: Warning (spi_bus_bridge): /soc/rpc@ee200000: node name for SPI buses should be 'spi' also defined at arch/arm64/boot/dts/renesas/r8a77980-condor.dts:283.6-343.3 arch/arm64/boot/dts/renesas/r8a77980-condor.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge' > - HF mode: > rpc: rpc-if@ee200000 { Again, spi@<...>. > compatible = "renesas,r8a77995-rpc", "renesas,rcar-gen3-rpc"; > reg = <0 0xee200000 0 0x200>, <0 0x08000000 0 0x4000000>, > <0 0xee208000 0 0x100>; > reg-names = "regs", "dirmap", "wbuf"; > clocks = <&cpg CPG_MOD 917>; > clock-names = "rpc"; > power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; > resets = <&cpg 917>; > #address-cells = <1>; > #size-cells = <1>; > > flash@0 { > compatible = "cfi-flash"; The working HF implementation has "cypress,hyperflash" before "cfi-flash". > reg = <0 0x4000000>; > }; > }; > > --------------------------------------------------------------< > > Is it OK ? Yeah, seems good (assuming you fix the issues above). [...] > thanks & best regards, > Mason MBR, Sergei
