diff mbox series

[v13,3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF controller bindings

Message ID 1558423174-10748-4-git-send-email-masonccyang@mxic.com.tw (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series mfd: Add Renesas R-Car Gen3 RPC-IF MFD & SPI driver | expand

Commit Message

Mason Yang May 21, 2019, 7:19 a.m. UTC
Document the bindings used by the Renesas R-Car Gen3 RPC-IF controller.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 .../devicetree/bindings/mfd/renesas-rpc-if.txt     | 65 ++++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt

Comments

Sergei Shtylyov May 22, 2019, 4:50 p.m. UTC | #1
On 05/21/2019 10:19 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/mfd/renesas-rpc-if.txt     | 65 ++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> new file mode 100644
> index 0000000..20ec85b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> @@ -0,0 +1,65 @@
> +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

   That's just some nonsense, sorry...
   I suggest that you stop reposting your patches as I'm going to post
my version of this patchset RSN (based on your patches, of course) and I'm
going to take care of fixing this file as well.

> +- #address-cells: should be 1
> +- #size-cells: should be 0
[...]

MBR, Sergei
Mason Yang May 23, 2019, 9:16 a.m. UTC | #2
Hi Sergei,

> On 05/21/2019 10:19 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/mfd/renesas-rpc-if.txt     | 65 
++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 
Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt 
b/
> Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> > new file mode 100644
> > index 0000000..20ec85b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> > @@ -0,0 +1,65 @@
> > +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
> 
>    That's just some nonsense, sorry...
>    I suggest that you stop reposting your patches as I'm going to post
> my version of this patchset RSN (based on your patches, of course) and 
I'm
> going to take care of fixing this file as well.

okay, just let me know your patch then.

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.

=====================================================================
Lee Jones June 3, 2019, 1:04 p.m. UTC | #3
On Wed, 22 May 2019, Sergei Shtylyov wrote:

> On 05/21/2019 10:19 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/mfd/renesas-rpc-if.txt     | 65 ++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> > new file mode 100644
> > index 0000000..20ec85b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> > @@ -0,0 +1,65 @@
> > +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
> 
>    That's just some nonsense, sorry...
>    I suggest that you stop reposting your patches as I'm going to post
> my version of this patchset RSN (based on your patches, of course) and I'm
> going to take care of fixing this file as well.

Why is this necessary?

Why not just provide some constructive feedback instead?

> > +- #address-cells: should be 1
> > +- #size-cells: should be 0
> [...]
> 
> MBR, Sergei
Sergei Shtylyov June 3, 2019, 2:55 p.m. UTC | #4
Hello!

On 06/03/2019 04:04 PM, Lee Jones 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/mfd/renesas-rpc-if.txt     | 65 ++++++++++++++++++++++
>>>  1 file changed, 65 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
>>> new file mode 100644
>>> index 0000000..20ec85b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
>>> @@ -0,0 +1,65 @@
>>> +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
>>
>>    That's just some nonsense, sorry...
>>    I suggest that you stop reposting your patches as I'm going to post
>> my version of this patchset RSN (based on your patches, of course) and I'm
>> going to take care of fixing this file as well.
> 
> Why is this necessary?

   Because Mason doesn't want to develop the HyperFlash driver (or even move his code
in preparation to this driver being developed). I must develop this driver, and I'd
like to avoid the extra churn of mving the code between the MFD and SPI drivers.

> Why not just provide some constructive feedback instead?

   I was providing the feedback during these 13 revisions... unfortunately, it wasn't
fully considered.

>>> +- #address-cells: should be 1
>>> +- #size-cells: should be 0
>> [...]

MBR, Sergei
Mason Yang June 6, 2019, 7:40 a.m. UTC | #5
Hi Jones,


> Subject
> 
> Re: [PATCH v13 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF 

> controller bindings
> 
> Hello!
> 
> On 06/03/2019 04:04 PM, Lee Jones 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/mfd/renesas-rpc-if.txt     | 65 
++++++++++++++++++++++
> >>>  1 file changed, 65 insertions(+)
> >>>  create mode 100644 
Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> >>>
> >>> diff --git 
a/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt b/
> Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> >>> new file mode 100644
> >>> index 0000000..20ec85b
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
> >>> @@ -0,0 +1,65 @@
> >>> +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
> >>
> >>    That's just some nonsense, sorry...
> >>    I suggest that you stop reposting your patches as I'm going to 
post
> >> my version of this patchset RSN (based on your patches, of course) 
and I'm
> >> going to take care of fixing this file as well.
> > 
> > Why is this necessary?
> 
>    Because Mason doesn't want to develop the HyperFlash driver (or even 
move his code
> in preparation to this driver being developed). I must develop this 
driver, and I'd
> like to avoid the extra churn of mving the code between the MFD and SPI 
drivers.
> 

There might be some misunderstandings.

I had been requested to boot R-CAR from the OctaFlash and finally I have 
achieved it 
by patching SPI framework for OctaFlash operation and RPC-IF SPI driver. 

We were aware of the lacking support of RPC-IF in the Linux kernel at that 
time and 
I though I could contribute what I had developed.

At that time for my first submission of RPC-IF SPI on 15 NOv 2018, there 
was no any 
HyperFlash (or Hyper Bus) patches. And we did not consider it because the 
resource
of HyperFlash was shortage to us.

RPC-IF SPI was applied by Mark on 12,Feb 2019 but Marek comment to add 
supporting
MFD for RPC-IF and then I patched RPC-IF to MFD and SPI till this v13.

I always think about:

Is RPC-IF really good/suitable for MFD ?
RPC-IF works either in SPI or HyperFlash is decided by external hardware 
pins 
configuration and it can NOT switch it's operation mode in the run time. 
This is not like my understanding of MFD.

As your comments:
------------------------------------------------------------------------>
> +              flash = of_get_next_child(pdev->dev.of_node, NULL);
> +              if (!flash) {
> +                              dev_warn(&pdev->dev, "no flash node 
found\n");
> +                              return -ENODEV;
> +              }
> +
> +              if (of_device_is_compatible(flash, "jedec,spi-nor")) {
> +                              cell = &rpc_spi_ctlr;
> +              } else if (of_device_is_compatible(flash, "cfi-flash")) {
> +                              cell = &rpc_hf_ctlr;
> +              } else {
> +                              dev_warn(&pdev->dev, "unknown flash 
type\n");
> +                              return -ENODEV;
> +              }

Are there going to be more children coming?

If not, I'd argue that this is not an MFD.
<-------------------------------------------------------------------

I agreed with your opinion and I will resubmit RPC-IF in SPI only
if you also agree with it.


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.

=====================================================================
Marek Vasut June 18, 2019, 12:01 p.m. UTC | #6
On 6/6/19 9:40 AM, masonccyang@mxic.com.tw wrote:
[...]

> RPC-IF works either in SPI or HyperFlash is decided by external hardware 
> pins 
> configuration and it can NOT switch it's operation mode in the run time. 
> This is not like my understanding of MFD.

Which external hardware pins decide the RPC configuration ?

It seems to me like PHYCNT register, PHYMEM bitfield, selects what
device is connected, and then a couple of other bits control the
communication, but I see nothing which would be tied to any external
configuration pins.

[...]
Mason Yang June 19, 2019, 7:36 a.m. UTC | #7
Hi Marek, 

> Subject
> 
> Re: [PATCH v13 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF 

> controller bindings
> 
> On 6/6/19 9:40 AM, masonccyang@mxic.com.tw wrote:
> [...]
> 
> > RPC-IF works either in SPI or HyperFlash is decided by external 
hardware 
> > pins 
> > configuration and it can NOT switch it's operation mode in the run 
time. 
> > This is not like my understanding of MFD.
> 
> Which external hardware pins decide the RPC configuration ?
> 
> It seems to me like PHYCNT register, PHYMEM bitfield, selects what
> device is connected, and then a couple of other bits control the
> communication, but I see nothing which would be tied to any external
> configuration pins.
> 

You may refer to R-Car D3 Draak Eva. board hardware manual 
and start-up guide, i.e., 
Table 2.12 Pin Multiplexing of R-Car D3 Mode setting pins and
set by hardware switch 1, 2, 3, 13, 31 and 10 to configure
booting from SPI mode or HyperFlash mode.

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.

=====================================================================
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
new file mode 100644
index 0000000..20ec85b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/renesas-rpc-if.txt
@@ -0,0 +1,65 @@ 
+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@ee200000 {
+		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>;
+
+		flash@0 {
+			compatible = "jedec,spi-nor";
+			reg = <0>;
+			spi-max-frequency = <40000000>;
+			spi-tx-bus-width = <1>;
+			spi-rx-bus-width = <1>;
+		};
+	};
+
+- HF mode:
+	rpc: spi@ee200000 {
+		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>;
+
+		flash@0 {
+			compatible = "cypress,hyperflash", "cfi-flash";
+			reg = <0>;
+		};
+	};