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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <44bc8f0a-cbdc-db4a-9a46-b8bae5cc37a2@cogentembedded.com>
Date:   Tue, 14 May 2019 23:27:50 +0300
From:   Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:     masonccyang@...c.com.tw, Geert Uytterhoeven <geert@...ux-m68k.org>,
        Lee Jones <lee.jones@...aro.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,
        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

On 05/14/2019 12:46 PM, masonccyang@...c.com.tw wrote:

>>>> 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! :-)

   s/fir/for/, not the firtree season anymore. :-)

>>> 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.
> 
> okay.
> 
>>>           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)...
> 
> Need it ?

   You seem to call clk_get_rate() on the module clock, I doubt that's
correct topologically...

> RPCD2 is derived from RPC and it's value is half of RPC,
> i.e., RPC = 160MHz, RPCD2 = 80 MHz

   I know.

>>    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:
> 
> okay, patch to
> 
> rpc_if: spi@<...>

   That, or just keep the node label.

>> 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@<...>.
> 
> what about rpc_if: hf@<...>

   Can't change the node name, as it's declared in the .dtsi files, not *.dts
ones. And "spi" works for the HF case as well -- no complaints from dtc. :-)

>>>                   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).
> 
> Patch new 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 RPC-IF registers,
> +                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"
> +- #address-cells: should be 1
> +- #size-cells: should be 0

   Still nothing about the "oower-domains" and "resets" props... :-(

> +
> +Example:
> +- SPI mode:
> +
> +                rpc_if: spi@...00000 {
> +                                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 = <0>;
[...]
> =======================================================================<
> 
> OK ?

   Yes, with the remaining issue fixed.
 
> thanks & best regards,
> Mason

[...]

MBR, Sergei

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ