Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Sergei,
On Wed, May 22, 2019 at 7:23 PM Sergei Shtylyov
wrote:
> On 05/22/2019 08:05 PM, Geert Uytterhoeven wrote:
> >> On 05/20/2019 10:23 AM, masonccy...@mxic.com.tw wrote:
> >>> +- clocks: should contain 1 entries for the module's clock
> >>
> >>1 entry (clock node phandle and specifier).
> >
> > Doesn't "specifier" mean "phandle + optional arguments"?
>
>No.
>E.g. when specifying the IRQs, the "interrupts" prop contains one or more
> interrupt specifiers, the phandle is specified in the "interrupt-parent"
> prop (see the DT spec).
Mea culpa, I stand corrected.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
On 05/22/2019 08:05 PM, Geert Uytterhoeven wrote:
>> On 05/20/2019 10:23 AM, masonccy...@mxic.com.tw wrote:
>>> +- clocks: should contain 1 entries for the module's clock
>>
>>1 entry (clock node phandle and specifier).
>
> Doesn't "specifier" mean "phandle + optional arguments"?
No.
E.g. when specifying the IRQs, the "interrupts" prop contains one or more
interrupt specifiers, the phandle is specified in the "interrupt-parent"
prop (see the DT spec).
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergei
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Sergei,
On Wed, May 22, 2019 at 6:32 PM Sergei Shtylyov
wrote:
> On 05/20/2019 10:23 AM, masonccy...@mxic.com.tw wrote:
> > +- clocks: should contain 1 entries for the module's clock
>
>1 entry (clock node phandle and specifier).
Doesn't "specifier" mean "phandle + optional arguments"?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
On 05/20/2019 10:23 AM, masonccy...@mxic.com.tw wrote:
> -->
>
> 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...
clk_set_rate(), sorry.
>
> I think it's correct but just like Geert mentioned that there is no any
> patch
> in drivers/clk/renesas/r8a77995-cpg-mssr.c adding RPC-related clocks.
>
>
> I patched dt-bindings/clock/r8a77995-cpg-mssr.h for some simple testing
>
> -#define R8A77995_CLK_RPC 29
> -#define R8A77995_CLK_RPCD2 30
> +#define R8A77995_CLK_RPC 31
> +#define R8A77995_CLK_RPCD2 32
Hm, what does this do?
> by clk_prepare_enable() & clk_disable_unprepare() with CPG_MOD 917
> on D3 draak board, it is working fine.
> - SPI mode:git
>
> rpc: rpc-if@ee20 {
The node names should be generic, based on the device class. And in
this
case I'd like to use "spi@ee20" as otherwise dtc keeps bitching like
below:
>>>
>>> okay, patch to
>>>
>>> rpc_if: spi@<...>
>>
>>That, or just keep the node label.
>
> okay.
>
> - HF mode:
> rpc: rpc-if@ee20 {
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.
> :-)
Maybe it's possible using the "name" prop, don't know...
> okay,
>
> Patch 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
1 entry (clock node phandle and specifier).
> +- clock-names: should contain "rpc"
> +- power-domains: should contain system-controller(sysc) for
> power-domain-cell
What's "power-domain-cell"? I know "#power-domain-cells". I'd like this
to be "the power domain node's phandle and specifier".
> +- resets: should contain clock pulse generator(cpg) for reset-cell,
It's "#reset-cells" again. I'd like this to be "the reset node's phandle
and specifier".
> + power-domain-cell and clock-cell
Why mntion clock-cell at all here?
> +- #address-cells: should be 1
> +- #size-cells: should be 0
> +
> +Example:
> +- SPI mode:
> +
> +rpc: spi@ee20 {
> +compatible = "renesas,r8a77995-rpc",
> "renesas,rcar-gen3-rpc";
> +reg = <0 0xee20 0 0x200>, <0
> 0x0800 0 0x400>,
> + <0 0xee208000 0 0x100>;
> +reg-names = "regs", "dirmap", "wbuf";
> +clocks = < CPG_MOD 917>;
> +clock-names = "rpc";
> +power-domains = <
> R8A77995_PD_ALWAYS_ON>;
> +resets = < 917>;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Geert, > > On Mon, May 20, 2019 at 9:24 AM wrote: > > > >>> - 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... > > > > I think it's correct but just like Geert mentioned that there is no any > > patch > > in drivers/clk/renesas/r8a77995-cpg-mssr.c adding RPC-related clocks. > > > > > > I patched dt-bindings/clock/r8a77995-cpg-mssr.h for some simple testing > > > > -#define R8A77995_CLK_RPC 29 > > -#define R8A77995_CLK_RPCD2 30 > > +#define R8A77995_CLK_RPC 31 > > +#define R8A77995_CLK_RPCD2 32 > > That change doesn't make sense to me... > > > by clk_prepare_enable() & clk_disable_unprepare() with CPG_MOD 917 > > on D3 draak board, it is working fine. > > ... and is not sufficient to allow the above two calls. > > Besides, making explicit clk_prepare_enable() calls bypasses Runtime PM > and the automatic disabling of unused clocks, thus hiding bugs related to > Runtime PM. Which is probably why your driver doesn't work for Sergei... > Got it, thanks for your explanation. best regards, Mason CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. = CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. =
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Mason,
On Mon, May 20, 2019 at 9:24 AM wrote:
> > >>> - 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...
>
> I think it's correct but just like Geert mentioned that there is no any
> patch
> in drivers/clk/renesas/r8a77995-cpg-mssr.c adding RPC-related clocks.
>
>
> I patched dt-bindings/clock/r8a77995-cpg-mssr.h for some simple testing
>
> -#define R8A77995_CLK_RPC 29
> -#define R8A77995_CLK_RPCD2 30
> +#define R8A77995_CLK_RPC 31
> +#define R8A77995_CLK_RPCD2 32
That change doesn't make sense to me...
> by clk_prepare_enable() & clk_disable_unprepare() with CPG_MOD 917
> on D3 draak board, it is working fine.
... and is not sufficient to allow the above two calls.
Besides, making explicit clk_prepare_enable() calls bypasses Runtime PM
and the automatic disabling of unused clocks, thus hiding bugs related to
Runtime PM. Which is probably why your driver doesn't work for Sergei...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Sergei,
> >>> -->
> >>>
> >>> 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...
I think it's correct but just like Geert mentioned that there is no any
patch
in drivers/clk/renesas/r8a77995-cpg-mssr.c adding RPC-related clocks.
I patched dt-bindings/clock/r8a77995-cpg-mssr.h for some simple testing
-#define R8A77995_CLK_RPC 29
-#define R8A77995_CLK_RPCD2 30
+#define R8A77995_CLK_RPC 31
+#define R8A77995_CLK_RPCD2 32
by clk_prepare_enable() & clk_disable_unprepare() with CPG_MOD 917
on D3 draak board, it is working fine.
> >>
> >>> - SPI mode:git
> >>>
> >>> rpc: rpc-if@ee20 {
> >>
> >>The node names should be generic, based on the device class. And
in
> > this
> >> case I'd like to use "spi@ee20" as otherwise dtc keeps bitching
like
> > below:
> >
> > okay, patch to
> >
> > rpc_if: spi@<...>
>
>That, or just keep the node label.
okay.
> >>> - HF mode:
> >>> rpc: rpc-if@ee20 {
> >>
> >>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.
:-)
okay,
Patch 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"
+- power-domains: should contain system-controller(sysc) for
power-domain-cell
+- resets: should contain clock pulse generator(cpg) for reset-cell,
+ power-domain-cell and clock-cell
+- #address-cells: should be 1
+- #size-cells: should be 0
+
+Example:
+- SPI mode:
+
+rpc: spi@ee20 {
+compatible = "renesas,r8a77995-rpc",
"renesas,rcar-gen3-rpc";
+reg = <0 0xee20 0 0x200>, <0
0x0800 0 0x400>,
+ <0 0xee208000 0 0x100>;
+reg-names = "regs", "dirmap", "wbuf";
+clocks = < CPG_MOD 917>;
+clock-names = "rpc";
+power-domains = <
R8A77995_PD_ALWAYS_ON>;
+resets = < 917>;
+#address-cells = <1>;
+#size-cells = <0>;
+
+flash@0 {
+compatible =
"jedec,spi-nor";
+reg = <0>;
+spi-max-frequency =
<4000>;
+spi-tx-bus-width = <1>;
+spi-rx-bus-width = <1>;
+};
+};
+
+- HF mode:
+rpc: spi@ee20 {
+compatible = "renesas,r8a77995-rpc",
"renesas,rcar-gen3-rpc";
+
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
On 05/14/2019 12:46 PM, masonccy...@mxic.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@ee20 {
>>
>>The node names should be generic, based on the device class. And in
> this
>> case I'd like to use "spi@ee20" 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@ee20: 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@ee20 {
>>
>>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 0xee20 0 0x200>, <0 0x0800 0
> 0x400>,
>>> <0 0xee208000 0 0x100>;
>>> reg-names = "regs", "dirmap", "wbuf";
>>> clocks = < CPG_MOD 917>;
>>> clock-names = "rpc";
>>> power-domains = < R8A77995_PD_ALWAYS_ON>;
>>> resets = < 917>;
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>>
>>> flash@0 {
>>> compatible = "cfi-flash";
>>
>>The working HF implementation has "cypress,hyperflash" before
> "cfi-flash".
>>
>>> reg = <0 0x400>;
>>> };
>>> };
>>>
>>> --<
>>>
>>> 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... :-(
>
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Mason,
On Tue, May 14, 2019 at 11:46 AM wrote:
> > >> There's precedence for such constructs being an MFD: please see
> > >> drivers/mfd/at91-usart.c, which registers a single MFD cell for
> > > 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 ?
> RPCD2 is derived from RPC and it's value is half of RPC,
> i.e., RPC = 160MHz, RPCD2 = 80 MHz
That hierarchy depends on the actual SoC, right?
AFAIU, both are derived from RPCSRC, and can be controlled individually
on some SoCs.
BTW, I still haven't seen a patch for drivers/clk/renesas/r8a77995-cpg-mssr.c
adding RPC-related clocks, so I have no idea what's the Linux view of the
hierarchy on your system.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Sergei,
> >
> >Could you please indent with 1 or 2 tabs where you used 8 or 16
spaces?
Sorry, I just copy this new DTS from git to my Notes email system,
therefore, the "tabs space" is not alignment with original file.
>
> 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
> +
> +Example:
> +- SPI mode:
> +
> + rpc_if: spi@ee20 {
> + compatible = "renesas,r8a77995-rpc", "renesas,rcar-gen3-rpc";
> + reg = <0 0xee20 0 0x200>, <0 0x0800 0 0x400>,
> +<0 0xee208000 0 0x100>;
> + reg-names = "regs", "dirmap", "wbuf";
> + clocks = < CPG_MOD 917>;
> + clock-names = "rpc";
> + power-domains = < R8A77995_PD_ALWAYS_ON>;
> + resets = < 917>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + flash@0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + spi-max-frequency = <4000>;
> + spi-tx-bus-width = <1>;
> + spi-rx-bus-width = <1>;
> + };
> + };
> +
> +- HF mode:
> + rpc_if: hf@ee20 {
> + compatible = "renesas,r8a77995-rpc", "renesas,rcar-gen3-rpc";
> + reg = <0 0xee20 0 0x200>, <0 0x0800 0 0x400>,
> +<0 0xee208000 0 0x100>;
> + reg-names = "regs", "dirmap", "wbuf";
> + clocks = < CPG_MOD 917>;
> + clock-names = "rpc";
> + power-domains = < R8A77995_PD_ALWAYS_ON>;
> + resets = < 917>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + flash@0 {
> + compatible = "cypress,hyperflash", "cfi-flash";
> + reg = <0>;
> + };
> + };
>
> ===<
thanks for your review.
best regards,
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or
personal data, which is protected by applicable laws. Please be reminded that
duplication, disclosure, distribution, or use of this e-mail (and/or its
attachments) or any part thereof is prohibited. If you receive this e-mail in
error, please notify us immediately and delete this mail as well as its
attachment(s) from your system. In addition, please be informed that
collection, processing, and/or use of personal data is prohibited unless
expressly permitted by personal data protection laws. Thank you for your
attention and cooperation.
Macronix International Co., Ltd.
=
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hello,
> >> 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.
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 ?
RPCD2 is derived from RPC and it's value is half of RPC,
i.e., RPC = 160MHz, RPCD2 = 80 MHz
>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@ee20 {
>
>The node names should be generic, based on the device class. And in
this
> case I'd like to use "spi@ee20" as otherwise dtc keeps bitching like
below:
okay, patch to
rpc_if: spi@<...>
>
> arch/arm64/boot/dts/renesas/r8a77980.dtsi:1344.21-1359.5: Warning
(spi_bus_bridge):
> /soc/rpc@ee20: 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@ee20 {
>
>Again, spi@<...>.
what about rpc_if: hf@<...>
>
> > compatible = "renesas,r8a77995-rpc",
"renesas,rcar-gen3-rpc";
> > reg = <0 0xee20 0 0x200>, <0 0x0800 0
0x400>,
> > <0 0xee208000 0 0x100>;
> > reg-names = "regs", "dirmap", "wbuf";
> > clocks = < CPG_MOD 917>;
> > clock-names = "rpc";
> > power-domains = < R8A77995_PD_ALWAYS_ON>;
> > resets = < 917>;
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > flash@0 {
> > compatible = "cfi-flash";
>
>The working HF implementation has "cypress,hyperflash" before
"cfi-flash".
>
> > reg = <0 0x400>;
> > };
> > };
> >
> > --<
> >
> > 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
+
+Example:
+- SPI mode:
+
+rpc_if: spi@ee20 {
+compatible = "renesas,r8a77995-rpc",
"renesas,rcar-gen3-rpc";
+reg = <0 0xee20 0 0x200>, <0
0x0800 0 0x400>,
+ <0 0xee208000 0 0x100>;
+reg-names = "regs", "dirmap", "wbuf";
+clocks = < CPG_MOD 917>;
+clock-names = "rpc";
+power-domains = <
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Geert, > Subject > > Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings > > Hi Mason, > > Note that if you send multipart/text+html emails, they will be dropped silently > by most Linux mailing lists. > Hence I'm quoting your last email fully, to give other people a chance > reading it (and commenting). Thanks for your remind ! I have configured my Lotus Notes to plain text mode only and it should be OK for Linux mailing lists now. thanks & best regards, Mason CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. = CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. =
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, masonccy...@mxic.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 >> > > >> > >> > >> --- >> > > >> > >> > >> .../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 000..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=147669165104431=2 >> > > > >> > > > >> > > > Hi Marek, >> > > > >> > > > By Jones's comments: >> > > > >> -- >> > > >> From: Shunquan Lin >> > > >> >> > > >> 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
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Mason, Note that if you send multipart/text+html emails, they will be dropped silently by most Linux mailing lists. Hence I'm quoting your last email fully, to give other people a chance reading it (and commenting). On Mon, May 13, 2019 at 11:37 AM 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 > > > > >> > >> > >> --- > > > > >> > >> > >> .../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 000..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=147669165104431=2 > > > > > > > > > > > > > > > Hi Marek, > > > > > > > > > > By Jones's comments: > > > > > > > -- > > > > >> From: Shunquan Lin > > > > >> > > > > >> 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. > > 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 >
Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings
Hi Mason,
On Fri, May 10, 2019 at 3:09 AM wrote:
> > "Sergei Shtylyov"
> > 2019/05/10 上午 03:24
> > mark.rutl...@arm.com, "Rob Herring" ,
> > zhengxu...@mxic.com.tw
> > On 05/09/2019 05:06 AM, masonccy...@mxic.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
> > >> > >> > >> ---
> > >> > >> > >> .../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 000..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-compliant flash 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=147669165104431=2
> > >
> > >
> > > Hi Marek,
> > >
> > > By Jones's comments:
> > > --
> > >> From: Shunquan Lin
> > >>
> > >> 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.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
