[v2] dt-bindings: mtd: Add sst25vf016b to the list of supported chip names

Message ID 1508853032-25229-1-git-send-email-fabrizio.castro@bp.renesas.com
State Rejected
Delegated to: Cyrille Pitchen
Headers show
Series
  • [v2] dt-bindings: mtd: Add sst25vf016b to the list of supported chip names
Related show

Commit Message

Fabrizio Castro Oct. 24, 2017, 1:50 p.m.
There are a few DT files that make use of sst25vf016b in their
compatible strings, and the driver supports this chip already.
This patch improves the documentation and therefore the result
of ./scripts/checkpatch.pl.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Thank you Rob, thank you Geert, and sorry for the delay on this one.
Here is v2.

Changes in v2:
* fixed subject prefix
* added changelog

 Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 +
 1 file changed, 1 insertion(+)

Comments

Fabrizio Castro Nov. 14, 2017, 9:38 a.m. | #1
Dear All,

how does this patch look like?

Thanks,
Fabrizio

> Subject: [PATCH v2] dt-bindings: mtd: Add sst25vf016b to the list of supported chip names
>
> There are a few DT files that make use of sst25vf016b in their
> compatible strings, and the driver supports this chip already.
> This patch improves the documentation and therefore the result
> of ./scripts/checkpatch.pl.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Thank you Rob, thank you Geert, and sorry for the delay on this one.
> Here is v2.
>
> Changes in v2:
> * fixed subject prefix
> * added changelog
>
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> index 4cab5d8..bf56d77 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> @@ -31,6 +31,7 @@ Required properties:
>                   s25sl12801
>                   s25fl008k
>                   s25fl064k
> +                 sst25vf016b
>                   sst25vf040b
>                   sst25wf040b
>                   m25p40
> --
> 2.7.4




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Cyrille Pitchen Nov. 17, 2017, 5:36 p.m. | #2
Hi Fabrizio,

sorry but I won't apply this patch.

New values for the 'compatible' DT properties should only be added for
memory parts not supporting the JEDEC READ ID (0x9F) command.

SST25 memories do support this command hence should use the "jedec,spi-nor"
value alone. For historical reasons, some DT nodes set their 'compatible'
string to something like "<vendor>,<model>", "jedec,spi-nor".
It should work as expected in most case however I discourage from doing so
in new device trees because it may have some side effects especially when
the m25p80.c driver is used between the spi-nor.c driver and the SPI
controller driver.

It's all about setting the 2nd parameter of spi_nor_scan(), the 'name'
parameter, as NULL when possible. This parameter should be set to a non NULL
value only for memories not supporting the JEDEC READ ID op code (0x9F).

Best regards,

Cyrille

Le 24/10/2017 à 15:50, Fabrizio Castro a écrit :
> There are a few DT files that make use of sst25vf016b in their
> compatible strings, and the driver supports this chip already.
> This patch improves the documentation and therefore the result
> of ./scripts/checkpatch.pl.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Thank you Rob, thank you Geert, and sorry for the delay on this one.
> Here is v2.
> 
> Changes in v2:
> * fixed subject prefix
> * added changelog
> 
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> index 4cab5d8..bf56d77 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> @@ -31,6 +31,7 @@ Required properties:
>                   s25sl12801
>                   s25fl008k
>                   s25fl064k
> +                 sst25vf016b
>                   sst25vf040b
>                   sst25wf040b
>                   m25p40
>
Fabrizio Castro Nov. 17, 2017, 5:40 p.m. | #3
Hello Cyrille,

it's ok, thank you for the feedback.

Best regards,
Fab

> Hi Fabrizio,

>

> sorry but I won't apply this patch.

>

> New values for the 'compatible' DT properties should only be added for

> memory parts not supporting the JEDEC READ ID (0x9F) command.

>

> SST25 memories do support this command hence should use the "jedec,spi-nor"

> value alone. For historical reasons, some DT nodes set their 'compatible'

> string to something like "<vendor>,<model>", "jedec,spi-nor".

> It should work as expected in most case however I discourage from doing so

> in new device trees because it may have some side effects especially when

> the m25p80.c driver is used between the spi-nor.c driver and the SPI

> controller driver.

>

> It's all about setting the 2nd parameter of spi_nor_scan(), the 'name'

> parameter, as NULL when possible. This parameter should be set to a non NULL

> value only for memories not supporting the JEDEC READ ID op code (0x9F).

>

> Best regards,

>

> Cyrille

>

> Le 24/10/2017 à 15:50, Fabrizio Castro a écrit :

> > There are a few DT files that make use of sst25vf016b in their

> > compatible strings, and the driver supports this chip already.

> > This patch improves the documentation and therefore the result

> > of ./scripts/checkpatch.pl.

> >

> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

> > Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>

> > Acked-by: Rob Herring <robh@kernel.org>

> > Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > ---

> > Thank you Rob, thank you Geert, and sorry for the delay on this one.

> > Here is v2.

> >

> > Changes in v2:

> > * fixed subject prefix

> > * added changelog

> >

> >  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 +

> >  1 file changed, 1 insertion(+)

> >

> > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-

> nor.txt

> > index 4cab5d8..bf56d77 100644

> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt

> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt

> > @@ -31,6 +31,7 @@ Required properties:

> >                   s25sl12801

> >                   s25fl008k

> >                   s25fl064k

> > +                 sst25vf016b

> >                   sst25vf040b

> >                   sst25wf040b

> >                   m25p40

> >

>

> --

> To unsubscribe from this list: send the line "unsubscribe devicetree" in

> the body of a message to majordomo@vger.kernel.org

> More majordomo info at  http://vger.kernel.org/majordomo-info.html




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Marek Vasut Nov. 17, 2017, 7:01 p.m. | #4
On 11/17/2017 06:36 PM, Cyrille Pitchen wrote:
> Hi Fabrizio,
> 
> sorry but I won't apply this patch.

ACK on that, if it can be detected with READID or SFDP, use that and
don't add redundant stuff into the DT bindings.

btw which renesas board has this SPI NOR on it ?

> New values for the 'compatible' DT properties should only be added for
> memory parts not supporting the JEDEC READ ID (0x9F) command.
> 
> SST25 memories do support this command hence should use the "jedec,spi-nor"
> value alone. For historical reasons, some DT nodes set their 'compatible'
> string to something like "<vendor>,<model>", "jedec,spi-nor".
> It should work as expected in most case however I discourage from doing so
> in new device trees because it may have some side effects especially when
> the m25p80.c driver is used between the spi-nor.c driver and the SPI
> controller driver.
> 
> It's all about setting the 2nd parameter of spi_nor_scan(), the 'name'
> parameter, as NULL when possible. This parameter should be set to a non NULL
> value only for memories not supporting the JEDEC READ ID op code (0x9F).
> 
> Best regards,
> 
> Cyrille
> 
> Le 24/10/2017 à 15:50, Fabrizio Castro a écrit :
>> There are a few DT files that make use of sst25vf016b in their
>> compatible strings, and the driver supports this chip already.
>> This patch improves the documentation and therefore the result
>> of ./scripts/checkpatch.pl.
>>
>> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
>> Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Thank you Rob, thank you Geert, and sorry for the delay on this one.
>> Here is v2.
>>
>> Changes in v2:
>> * fixed subject prefix
>> * added changelog
>>
>>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> index 4cab5d8..bf56d77 100644
>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> @@ -31,6 +31,7 @@ Required properties:
>>                   s25sl12801
>>                   s25fl008k
>>                   s25fl064k
>> +                 sst25vf016b
>>                   sst25vf040b
>>                   sst25wf040b
>>                   m25p40
>>
>

Patch

diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
index 4cab5d8..bf56d77 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
@@ -31,6 +31,7 @@  Required properties:
                  s25sl12801
                  s25fl008k
                  s25fl064k
+                 sst25vf016b
                  sst25vf040b
                  sst25wf040b
                  m25p40