| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b51d1cb7-b3b5-208f-ab4c-145ecb57805d@cogentembedded.com>
Date: Mon, 13 May 2019 22:08:30 +0300
From: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To: masonccyang@...c.com.tw, Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Boris Brezillon <bbrezillon@...nel.org>,
Mark Brown <broonie@...nel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Simon Horman <horms@...ge.net.au>, juliensu@...c.com.tw,
Lee Jones <lee.jones@...aro.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
linux-spi <linux-spi@...r.kernel.org>,
Marek Vasut <marek.vasut@...il.com>,
Mark Rutland <mark.rutland@....com>,
Rob Herring <robh@...nel.org>, zhengxunli@...c.com.tw
Subject: Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3
RPC-IF MFD bindings
Hello!
On 05/13/2019 12:37 PM, masonccyang@...c.com.tw 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@...c.com.tw>
>> > > >> > >> > >> ---
>> > > >> > >> > >> .../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@...ilicon.com>
>> > > >>
>> > > >> 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@...00000 {
The node names should be generic, based on the device class. And in this
case I'd like to use "spi@...00000" as otherwise dtc keeps bitching like below:
arch/arm64/boot/dts/renesas/r8a77980.dtsi:1344.21-1359.5: Warning (spi_bus_bridge):
/soc/rpc@...00000: 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@...00000 {
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
Powered by blists - more mailing lists