[v2,4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.

Message ID 1488960326-13858-1-git-send-email-jartur@cadence.com
State Superseded
Delegated to: Cyrille Pitchen
Headers show

Commit Message

Artur Jedrysek March 8, 2017, 8:05 a.m.
This patch updates Cadence QSPI Device Tree documentation to include
information about new compatible used to indicate, whether or not
Octal SPI transfers are supported by the device.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: Use new compatible, instead of boolean property, to indicate
Octal SPI support.
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Marek Vasut March 10, 2017, 3:39 a.m. | #1
On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
> This patch updates Cadence QSPI Device Tree documentation to include
> information about new compatible used to indicate, whether or not
> Octal SPI transfers are supported by the device.
> 
> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> ---
> Changelog:
> v2: Use new compatible, instead of boolean property, to indicate
> Octal SPI support.
> ---
>  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> index f248056..41d1e98 100644
> --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> @@ -1,7 +1,9 @@
>  * Cadence Quad SPI controller
>  
>  Required properties:
> -- compatible : Should be "cdns,qspi-nor".
> +- compatible : Should be "cdns,{qspi|ospi}-nor".

Please explicitly list all compatibles , ie.
Should be "cdns,qspi-nor" or "cdns,ospi-nor".

But I think the ospi is backward compatible with qspi, right ? So the
binding for ospi should list both, ie.
compatible = "cdns,ospi-nor", "cdns,qspi-nor";

> +  Use "cdns,qspi-nor" for Quad SPI controller.
> +  Use "cdns,ospi-nor" for Octal SPI controller.
>  - reg : Contains two entries, each of which is a tuple consisting of a
>  	physical address and length. The first entry is the address and
>  	length of the controller register set. The second entry is the
>
Artur Jedrysek March 10, 2017, 12:03 p.m. | #2
> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> Sent: 10 March 2017 04:39
> On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
> > This patch updates Cadence QSPI Device Tree documentation to include
> > information about new compatible used to indicate, whether or not
> > Octal SPI transfers are supported by the device.
> >
> > Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> > ---
> > Changelog:
> > v2: Use new compatible, instead of boolean property, to indicate
> > Octal SPI support.
> > ---
> >  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> > index f248056..41d1e98 100644
> > --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> > +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> > @@ -1,7 +1,9 @@
> >  * Cadence Quad SPI controller
> >
> >  Required properties:
> > -- compatible : Should be "cdns,qspi-nor".
> > +- compatible : Should be "cdns,{qspi|ospi}-nor".
> 
> Please explicitly list all compatibles , ie.
> Should be "cdns,qspi-nor" or "cdns,ospi-nor".
> 

Two possible options are also explicitly listed right below. I did that
in the way it was done in other driver for Cadence IP:
/Documentation/devicetree/bindings/net/macb.txt
Should I remove all of that and replace it with both options, separated
by an "or" word?

> But I think the ospi is backward compatible with qspi, right ? So the
> binding for ospi should list both, ie.
> compatible = "cdns,ospi-nor", "cdns,qspi-nor";
> 

Yes, the ospi is backwards compatible with qspi. However, the sole point
of introducing new compatible was to differentiate between them (which
was previously done using a boolean flag, but that was criticized).
Listing both bindings for ospi would only help for kernels not
containing driver update being subject of this patch series, as both
ospi and qspi are handled by the same driver - just with a slight
difference. I apologize if I got something wrong here.

> > +  Use "cdns,qspi-nor" for Quad SPI controller.
> > +  Use "cdns,ospi-nor" for Octal SPI controller.
> >  - reg : Contains two entries, each of which is a tuple consisting of a
> >  	physical address and length. The first entry is the address and
> >  	length of the controller register set. The second entry is the
> >
> 
> 
> --
> Best regards,
> Marek Vasut
Marek Vasut March 10, 2017, 12:52 p.m. | #3
On 03/10/2017 01:03 PM, Artur Jedrysek wrote:
> 
>> From: Marek Vasut [mailto:marek.vasut@gmail.com]
>> Sent: 10 March 2017 04:39
>> On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
>>> This patch updates Cadence QSPI Device Tree documentation to include
>>> information about new compatible used to indicate, whether or not
>>> Octal SPI transfers are supported by the device.
>>>
>>> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
>>> ---
>>> Changelog:
>>> v2: Use new compatible, instead of boolean property, to indicate
>>> Octal SPI support.
>>> ---
>>>  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>> b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> index f248056..41d1e98 100644
>>> --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> @@ -1,7 +1,9 @@
>>>  * Cadence Quad SPI controller
>>>
>>>  Required properties:
>>> -- compatible : Should be "cdns,qspi-nor".
>>> +- compatible : Should be "cdns,{qspi|ospi}-nor".
>>
>> Please explicitly list all compatibles , ie.
>> Should be "cdns,qspi-nor" or "cdns,ospi-nor".
>>
> 
> Two possible options are also explicitly listed right below. I did that
> in the way it was done in other driver for Cadence IP:
> /Documentation/devicetree/bindings/net/macb.txt
> Should I remove all of that and replace it with both options, separated
> by an "or" word?

Wait for Rob to confirm.

>> But I think the ospi is backward compatible with qspi, right ? So the
>> binding for ospi should list both, ie.
>> compatible = "cdns,ospi-nor", "cdns,qspi-nor";
>>
> 
> Yes, the ospi is backwards compatible with qspi. However, the sole point
> of introducing new compatible was to differentiate between them (which
> was previously done using a boolean flag, but that was criticized).
> Listing both bindings for ospi would only help for kernels not
> containing driver update being subject of this patch series, as both
> ospi and qspi are handled by the same driver - just with a slight
> difference. I apologize if I got something wrong here.

You got it right, I was just curious about the compatibility.

>>> +  Use "cdns,qspi-nor" for Quad SPI controller.
>>> +  Use "cdns,ospi-nor" for Octal SPI controller.
>>>  - reg : Contains two entries, each of which is a tuple consisting of a
>>>  	physical address and length. The first entry is the address and
>>>  	length of the controller register set. The second entry is the
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut
Rob Herring March 15, 2017, 8:23 p.m. | #4
On Fri, Mar 10, 2017 at 01:52:31PM +0100, Marek Vasut wrote:
> On 03/10/2017 01:03 PM, Artur Jedrysek wrote:
> > 
> >> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> >> Sent: 10 March 2017 04:39
> >> On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
> >>> This patch updates Cadence QSPI Device Tree documentation to include
> >>> information about new compatible used to indicate, whether or not
> >>> Octal SPI transfers are supported by the device.
> >>>
> >>> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> >>> ---
> >>> Changelog:
> >>> v2: Use new compatible, instead of boolean property, to indicate
> >>> Octal SPI support.
> >>> ---
> >>>  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> >> b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> >>> index f248056..41d1e98 100644
> >>> --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> >>> +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> >>> @@ -1,7 +1,9 @@
> >>>  * Cadence Quad SPI controller
> >>>
> >>>  Required properties:
> >>> -- compatible : Should be "cdns,qspi-nor".
> >>> +- compatible : Should be "cdns,{qspi|ospi}-nor".
> >>
> >> Please explicitly list all compatibles , ie.
> >> Should be "cdns,qspi-nor" or "cdns,ospi-nor".
> >>
> > 
> > Two possible options are also explicitly listed right below. I did that
> > in the way it was done in other driver for Cadence IP:
> > /Documentation/devicetree/bindings/net/macb.txt
> > Should I remove all of that and replace it with both options, separated
> > by an "or" word?
> 
> Wait for Rob to confirm.

As Marek suggested, but with one valid combination per line. I don't 
think checkpatch.pl will work with it as is.

> 
> >> But I think the ospi is backward compatible with qspi, right ? So the
> >> binding for ospi should list both, ie.
> >> compatible = "cdns,ospi-nor", "cdns,qspi-nor";
> >>
> > 
> > Yes, the ospi is backwards compatible with qspi. However, the sole point
> > of introducing new compatible was to differentiate between them (which
> > was previously done using a boolean flag, but that was criticized).
> > Listing both bindings for ospi would only help for kernels not
> > containing driver update being subject of this patch series, as both
> > ospi and qspi are handled by the same driver - just with a slight
> > difference. I apologize if I got something wrong here.
> 
> You got it right, I was just curious about the compatibility.

You are assuming every OS/user everywhere is updated. It doesn't hurt to 
have the backwards compatibility.

Rob

Patch

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index f248056..41d1e98 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -1,7 +1,9 @@ 
 * Cadence Quad SPI controller
 
 Required properties:
-- compatible : Should be "cdns,qspi-nor".
+- compatible : Should be "cdns,{qspi|ospi}-nor".
+  Use "cdns,qspi-nor" for Quad SPI controller.
+  Use "cdns,ospi-nor" for Octal SPI controller.
 - reg : Contains two entries, each of which is a tuple consisting of a
 	physical address and length. The first entry is the address and
 	length of the controller register set. The second entry is the