[PATCH] mtd: spi-nor: add dt support for Everspin MRAMs

[PATCH] mtd: spi-nor: add dt support for Everspin MRAMs

[Uwe Kleine-König]

The MR25 family doesn't support JEDEC, so they need explicit mentioning
in the list of supported spi IDs. This makes it possible to add these
using for example:

	compatible = "everspin,mr25h40";

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

independent of the decision if -nonjedec is ok for m25p, this is needed
to make Everspin's MRAMs work.

Best regards
Uwe

 drivers/mtd/devices/m25p80.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
 	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
 	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
 
+	/* Everspin MRAMs without JEDEC support */
+	{ "mr25h256" }, /* 256 kib, 40 MHz */
+	{ "mr25h10" },  /*   1 Mib, 40 MHz */
+	{ "mr25h40" },  /*   4 Mib, 40 MHz */
+
 	{ },
 };
 MODULE_DEVICE_TABLE(spi, m25p_ids);
-- 
2.11.0

Re: [PATCH] mtd: spi-nor: add dt support for Everspin MRAMs

[Rafał Miłecki]

On 16 January 2017 at 14:35, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> in the list of supported spi IDs. This makes it possible to add these
> using for example:
>
>         compatible = "everspin,mr25h40";
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> independent of the decision if -nonjedec is ok for m25p, this is needed
> to make Everspin's MRAMs work.
>
> Best regards
> Uwe
>
>  drivers/mtd/devices/m25p80.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
>         {"m25p40-nonjedec"},    {"m25p80-nonjedec"},    {"m25p16-nonjedec"},
>         {"m25p32-nonjedec"},    {"m25p64-nonjedec"},    {"m25p128-nonjedec"},
>
> +       /* Everspin MRAMs without JEDEC support */
> +       { "mr25h256" }, /* 256 kib, 40 MHz */

A trivial thing: I think it should be Kib
https://en.wikipedia.org/wiki/Binary_prefix

-- 
Rafał

Re: [PATCH] mtd: spi-nor: add dt support for Everspin MRAMs

[Marek Vasut]

On 01/16/2017 02:55 PM, Rafał Miłecki wrote:
> On 16 January 2017 at 14:35, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
>> The MR25 family doesn't support JEDEC, so they need explicit mentioning
>> in the list of supported spi IDs. This makes it possible to add these
>> using for example:
>>
>>         compatible = "everspin,mr25h40";
>>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>> Hello,
>>
>> independent of the decision if -nonjedec is ok for m25p, this is needed
>> to make Everspin's MRAMs work.
>>
>> Best regards
>> Uwe
>>
>>  drivers/mtd/devices/m25p80.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
>>         {"m25p40-nonjedec"},    {"m25p80-nonjedec"},    {"m25p16-nonjedec"},
>>         {"m25p32-nonjedec"},    {"m25p64-nonjedec"},    {"m25p128-nonjedec"},
>>
>> +       /* Everspin MRAMs without JEDEC support */
>> +       { "mr25h256" }, /* 256 kib, 40 MHz */
> 
> A trivial thing: I think it should be Kib
> https://en.wikipedia.org/wiki/Binary_prefix
> 
Except for that, it looks good IMO.

-- 
Best regards,
Marek Vasut

Re: [PATCH] mtd: spi-nor: add dt support for Everspin MRAMs

[Uwe Kleine-König]

On Mon, Jan 16, 2017 at 02:55:01PM +0100, Rafał Miłecki wrote:
> On 16 January 2017 at 14:35, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > The MR25 family doesn't support JEDEC, so they need explicit mentioning
> > in the list of supported spi IDs. This makes it possible to add these
> > using for example:
> >
> >         compatible = "everspin,mr25h40";
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> >
> > independent of the decision if -nonjedec is ok for m25p, this is needed
> > to make Everspin's MRAMs work.
> >
> > Best regards
> > Uwe
> >
> >  drivers/mtd/devices/m25p80.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> > --- a/drivers/mtd/devices/m25p80.c
> > +++ b/drivers/mtd/devices/m25p80.c
> > @@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
> >         {"m25p40-nonjedec"},    {"m25p80-nonjedec"},    {"m25p16-nonjedec"},
> >         {"m25p32-nonjedec"},    {"m25p64-nonjedec"},    {"m25p128-nonjedec"},
> >
> > +       /* Everspin MRAMs without JEDEC support */
> > +       { "mr25h256" }, /* 256 kib, 40 MHz */
> 
> A trivial thing: I think it should be Kib
> https://en.wikipedia.org/wiki/Binary_prefix

Oh, really? 10^3 = k and 2^10 = Ki, stange. Should I resend for that?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] mtd: spi-nor: add dt support for Everspin MRAMs

[Rafał Miłecki]

On 16 January 2017 at 20:41, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Jan 16, 2017 at 02:55:01PM +0100, Rafał Miłecki wrote:
>> On 16 January 2017 at 14:35, Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>> > The MR25 family doesn't support JEDEC, so they need explicit mentioning
>> > in the list of supported spi IDs. This makes it possible to add these
>> > using for example:
>> >
>> >         compatible = "everspin,mr25h40";
>> >
>> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> > ---
>> > Hello,
>> >
>> > independent of the decision if -nonjedec is ok for m25p, this is needed
>> > to make Everspin's MRAMs work.
>> >
>> > Best regards
>> > Uwe
>> >
>> >  drivers/mtd/devices/m25p80.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> > --- a/drivers/mtd/devices/m25p80.c
>> > +++ b/drivers/mtd/devices/m25p80.c
>> > @@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
>> >         {"m25p40-nonjedec"},    {"m25p80-nonjedec"},    {"m25p16-nonjedec"},
>> >         {"m25p32-nonjedec"},    {"m25p64-nonjedec"},    {"m25p128-nonjedec"},
>> >
>> > +       /* Everspin MRAMs without JEDEC support */
>> > +       { "mr25h256" }, /* 256 kib, 40 MHz */
>>
>> A trivial thing: I think it should be Kib
>> https://en.wikipedia.org/wiki/Binary_prefix
>
> Oh, really? 10^3 = k and 2^10 = Ki, stange. Should I resend for that?

I don't think I understand you now. This is 262'144b device. You most
likely want to divide it by 1024 and use Kib then, right?

So it should be just 256 Kib instead of 256 kib.

-- 
Rafał

[PATCH v2] mtd: spi-nor: add dt support for Everspin MRAMs

[Uwe Kleine-König]

The MR25 family doesn't support JEDEC, so they need explicit mentioning
in the list of supported spi IDs. This makes it possible to add these
using for example:

	compatible = "everspin,mr25h40";

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1:
 - use Kib instead of kib

 drivers/mtd/devices/m25p80.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 9cf7fcd28034..aa50bd96de3a 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
 	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
 	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
 
+	/* Everspin MRAMs */
+	{ "mr25h256" }, /* 256 Kib, 40 MHz */
+	{ "mr25h10" },  /*   1 Mib, 40 MHz */
+	{ "mr25h40" },  /*   4 Mib, 40 MHz */
+
 	{ },
 };
 MODULE_DEVICE_TABLE(spi, m25p_ids);
-- 
2.11.0

Re: [PATCH v2] mtd: spi-nor: add dt support for Everspin MRAMs

[Rafał Miłecki]

On 16 January 2017 at 22:00, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> in the list of supported spi IDs. This makes it possible to add these
> using for example:
>
>         compatible = "everspin,mr25h40";
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Looks OK to me, thanks!

Re: [PATCH v2] mtd: spi-nor: add dt support for Everspin MRAMs

[Marek Vasut]

On 01/16/2017 10:00 PM, Uwe Kleine-König wrote:
> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> in the list of supported spi IDs. This makes it possible to add these
> using for example:
> 
> 	compatible = "everspin,mr25h40";
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Acked-by: Marek Vasut <marek.vasut@gmail.com>

> ---
> Changes since (implicit) v1:
>  - use Kib instead of kib
> 
>  drivers/mtd/devices/m25p80.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd28034..aa50bd96de3a 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
>  	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
>  	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
>  
> +	/* Everspin MRAMs */
> +	{ "mr25h256" }, /* 256 Kib, 40 MHz */
> +	{ "mr25h10" },  /*   1 Mib, 40 MHz */
> +	{ "mr25h40" },  /*   4 Mib, 40 MHz */
> +
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(spi, m25p_ids);
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v2] mtd: spi-nor: add dt support for Everspin MRAMs

[Cyrille Pitchen]

Hi,

Le 16/01/2017 à 22:00, Uwe Kleine-König a écrit :
> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> in the list of supported spi IDs. This makes it possible to add these
> using for example:
> 
> 	compatible = "everspin,mr25h40";
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Changes since (implicit) v1:
>  - use Kib instead of kib
> 
>  drivers/mtd/devices/m25p80.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd28034..aa50bd96de3a 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
>  	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
>  	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
>  
> +	/* Everspin MRAMs */
> +	{ "mr25h256" }, /* 256 Kib, 40 MHz */

There is already a "mr25h256" entry in the m25p_ids[] array.
It's a good idea to regroup everspin memories but it might be better to
reuse the existing entry.

The previous entry is between "at25df321a", ... and "mx25l4005a" so it
doesn't appear in your patch.

> +	{ "mr25h10" },  /*   1 Mib, 40 MHz */
> +	{ "mr25h40" },  /*   4 Mib, 40 MHz */
> +
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(spi, m25p_ids);
> 

Re: [PATCH v2] mtd: spi-nor: add dt support for Everspin MRAMs

[Cyrille Pitchen]

Le 17/01/2017 à 10:48, Cyrille Pitchen a écrit :
> Hi,
> 
> Le 16/01/2017 à 22:00, Uwe Kleine-König a écrit :
>> The MR25 family doesn't support JEDEC, so they need explicit mentioning
>> in the list of supported spi IDs. This makes it possible to add these
>> using for example:
>>
>> 	compatible = "everspin,mr25h40";
>>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>> Changes since (implicit) v1:
>>  - use Kib instead of kib
>>
>>  drivers/mtd/devices/m25p80.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index 9cf7fcd28034..aa50bd96de3a 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
>>  	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
>>  	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
>>  
>> +	/* Everspin MRAMs */
>> +	{ "mr25h256" }, /* 256 Kib, 40 MHz */
> 
> There is already a "mr25h256" entry in the m25p_ids[] array.
> It's a good idea to regroup everspin memories but it might be better to
> reuse the existing entry.
> 
> The previous entry is between "at25df321a", ... and "mx25l4005a" so it
> doesn't appear in your patch.
> 
>> +	{ "mr25h10" },  /*   1 Mib, 40 MHz */
>> +	{ "mr25h40" },  /*   4 Mib, 40 MHz */
>> +

Also if you add new entries, I think you should update the "Supported chip
names" section in Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt:

"mr25h256" is already there.

Best regards,

Cyrille


>>  	{ },
>>  };
>>  MODULE_DEVICE_TABLE(spi, m25p_ids);
>>
> 

[PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Uwe Kleine-König]

The MR25 family doesn't support JEDEC, so they need explicit mentioning
in the list of supported spi IDs. This makes it possible to add these
using for example:

	compatible = "everspin,mr25h40";

There was already an entry for mr25h256. Move that one out of the "keep
for compatibility" section and put in a new group for Everspin MRAMs.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1:

 - use Kib instead of kib

Changes since v2:

 - update dt docs
 - handle already existing mr25h256 in m25p_ids[]

Thanks to Cyrille for catching these.

 Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 2 ++
 drivers/mtd/devices/m25p80.c                            | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
index 2c91c03e7eb0..3e920ec5c4d3 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
@@ -14,6 +14,8 @@ Required properties:
                  at25df641
                  at26df081a
                  mr25h256
+                 mr25h10
+                 mr25h40
                  mx25l4005a
                  mx25l1606e
                  mx25l6405d
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 9cf7fcd28034..0e2d3a64651a 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -288,7 +288,6 @@ static const struct spi_device_id m25p_ids[] = {
 	 * should be kept for backward compatibility.
 	 */
 	{"at25df321a"},	{"at25df641"},	{"at26df081a"},
-	{"mr25h256"},
 	{"mx25l4005a"},	{"mx25l1606e"},	{"mx25l6405d"},	{"mx25l12805d"},
 	{"mx25l25635e"},{"mx66l51235l"},
 	{"n25q064"},	{"n25q128a11"},	{"n25q128a13"},	{"n25q512a"},
@@ -305,6 +304,11 @@ static const struct spi_device_id m25p_ids[] = {
 	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
 	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
 
+	/* Everspin MRAMs (non-JEDEC) */
+	{ "mr25h256" }, /* 256 Kib, 40 MHz */
+	{ "mr25h10" },  /*   1 Mib, 40 MHz */
+	{ "mr25h40" },  /*   4 Mib, 40 MHz */
+
 	{ },
 };
 MODULE_DEVICE_TABLE(spi, m25p_ids);
-- 
2.11.0

Re: [PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Cyrille Pitchen]

Le 17/01/2017 à 12:03, Uwe Kleine-König a écrit :
> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> in the list of supported spi IDs. This makes it possible to add these
> using for example:
> 
> 	compatible = "everspin,mr25h40";
> 
> There was already an entry for mr25h256. Move that one out of the "keep
> for compatibility" section and put in a new group for Everspin MRAMs.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

By the way, you've forgotten to collect Marek's ack from v1 but no need to
resend for that. I wait a little bit more for some DT guy ack if they want
but otherwise I think this patch is ready to be merged into the spi-nor tree.

Thanks !

> ---
> Changes since (implicit) v1:
> 
>  - use Kib instead of kib
> 
> Changes since v2:
> 
>  - update dt docs
>  - handle already existing mr25h256 in m25p_ids[]
> 
> Thanks to Cyrille for catching these.
> 
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 2 ++
>  drivers/mtd/devices/m25p80.c                            | 6 +++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> index 2c91c03e7eb0..3e920ec5c4d3 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> @@ -14,6 +14,8 @@ Required properties:
>                   at25df641
>                   at26df081a
>                   mr25h256
> +                 mr25h10
> +                 mr25h40
>                   mx25l4005a
>                   mx25l1606e
>                   mx25l6405d
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd28034..0e2d3a64651a 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -288,7 +288,6 @@ static const struct spi_device_id m25p_ids[] = {
>  	 * should be kept for backward compatibility.
>  	 */
>  	{"at25df321a"},	{"at25df641"},	{"at26df081a"},
> -	{"mr25h256"},
>  	{"mx25l4005a"},	{"mx25l1606e"},	{"mx25l6405d"},	{"mx25l12805d"},
>  	{"mx25l25635e"},{"mx66l51235l"},
>  	{"n25q064"},	{"n25q128a11"},	{"n25q128a13"},	{"n25q512a"},
> @@ -305,6 +304,11 @@ static const struct spi_device_id m25p_ids[] = {
>  	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
>  	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
>  
> +	/* Everspin MRAMs (non-JEDEC) */
> +	{ "mr25h256" }, /* 256 Kib, 40 MHz */
> +	{ "mr25h10" },  /*   1 Mib, 40 MHz */
> +	{ "mr25h40" },  /*   4 Mib, 40 MHz */
> +
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(spi, m25p_ids);
> 

Re: [PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Rafał Miłecki]

On 17 January 2017 at 12:03, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> in the list of supported spi IDs. This makes it possible to add these
> using for example:
>
>         compatible = "everspin,mr25h40";

(...)

> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> index 2c91c03e7eb0..3e920ec5c4d3 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> @@ -14,6 +14,8 @@ Required properties:
>                   at25df641
>                   at26df081a
>                   mr25h256
> +                 mr25h10
> +                 mr25h40
>                   mx25l4005a
>                   mx25l1606e
>                   mx25l6405d

Uh, this is getting a never-ending-story...
If these chipsets don't support JEDEC, should we keep them in jedec,spi-nor.txt?

-- 
Rafał

Re: [PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Cyrille Pitchen]

Le 17/01/2017 à 14:16, Rafał Miłecki a écrit :
> On 17 January 2017 at 12:03, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
>> The MR25 family doesn't support JEDEC, so they need explicit mentioning
>> in the list of supported spi IDs. This makes it possible to add these
>> using for example:
>>
>>         compatible = "everspin,mr25h40";
> 
> (...)
> 
>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> index 2c91c03e7eb0..3e920ec5c4d3 100644
>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>> @@ -14,6 +14,8 @@ Required properties:
>>                   at25df641
>>                   at26df081a
>>                   mr25h256
>> +                 mr25h10
>> +                 mr25h40
>>                   mx25l4005a
>>                   mx25l1606e
>>                   mx25l6405d
> 
> Uh, this is getting a never-ending-story...
> If these chipsets don't support JEDEC, should we keep them in jedec,spi-nor.txt?
> 

Maybe not but I think the new compatible strings should be documented
somewhere. Currently jedec,spi-nor.txt already documents all the
"m25p*-nonjedec" memories. So maybe just renaming the jedec,spi-nor.txt
file into spi-nor.txt or mtd,spi-nor.txt could be a solution. Otherwise, we
can let it as is. I have no idea of what would be the best solution.

To be honest, I don't always fully understand the DT policy/philosophy and
its requirements. I just thought when a new property or a new value is
introduced it has to be documented.
Generally speaking, when DT is involved in some series of patches, it often
generates many discussions about the proper way to do thinks and about
choosing the best between many technically functional solutions.

If you think jedec,spi-nor.txt is not suited to document the new value for
the compatible string, why not, I perfectly understand your point.

I don't mind choosing another way. I just want to be sure that, if not all,
most of people agree on that solution and if possible, it is compliant with
DT policy so everybody is happy and works together.
That's why I involve DT people, even if it's a small detail, so they can
advise us.

Anyway, at some point we have to take a decision to carry on thinks.
So actually, I would like to avoid a never-ending story :)

Re: [PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Rafał Miłecki]

On 17 January 2017 at 14:57, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
> Le 17/01/2017 à 14:16, Rafał Miłecki a écrit :
>> On 17 January 2017 at 12:03, Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>>> The MR25 family doesn't support JEDEC, so they need explicit mentioning
>>> in the list of supported spi IDs. This makes it possible to add these
>>> using for example:
>>>
>>>         compatible = "everspin,mr25h40";
>>
>> (...)
>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>> index 2c91c03e7eb0..3e920ec5c4d3 100644
>>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>> @@ -14,6 +14,8 @@ Required properties:
>>>                   at25df641
>>>                   at26df081a
>>>                   mr25h256
>>> +                 mr25h10
>>> +                 mr25h40
>>>                   mx25l4005a
>>>                   mx25l1606e
>>>                   mx25l6405d
>>
>> Uh, this is getting a never-ending-story...
>> If these chipsets don't support JEDEC, should we keep them in jedec,spi-nor.txt?
>>
>
> Maybe not but I think the new compatible strings should be documented
> somewhere. Currently jedec,spi-nor.txt already documents all the
> "m25p*-nonjedec" memories. So maybe just renaming the jedec,spi-nor.txt
> file into spi-nor.txt or mtd,spi-nor.txt could be a solution. Otherwise, we
> can let it as is. I have no idea of what would be the best solution.
>
> To be honest, I don't always fully understand the DT policy/philosophy and
> its requirements. I just thought when a new property or a new value is
> introduced it has to be documented.
> Generally speaking, when DT is involved in some series of patches, it often
> generates many discussions about the proper way to do thinks and about
> choosing the best between many technically functional solutions.
>
> If you think jedec,spi-nor.txt is not suited to document the new value for
> the compatible string, why not, I perfectly understand your point.
>
> I don't mind choosing another way. I just want to be sure that, if not all,
> most of people agree on that solution and if possible, it is compliant with
> DT policy so everybody is happy and works together.
> That's why I involve DT people, even if it's a small detail, so they can
> advise us.
>
> Anyway, at some point we have to take a decision to carry on thinks.
> So actually, I would like to avoid a never-ending story :)

Sounds OK to me, I'm not DT expert though ;)

-- 
Rafał

Re: [PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Marek Vasut]

On 01/17/2017 04:49 PM, Rafał Miłecki wrote:
> On 17 January 2017 at 14:57, Cyrille Pitchen <cyrille.pitchen@atmel.com> wrote:
>> Le 17/01/2017 à 14:16, Rafał Miłecki a écrit :
>>> On 17 January 2017 at 12:03, Uwe Kleine-König
>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>> The MR25 family doesn't support JEDEC, so they need explicit mentioning
>>>> in the list of supported spi IDs. This makes it possible to add these
>>>> using for example:
>>>>
>>>>         compatible = "everspin,mr25h40";
>>>
>>> (...)
>>>
>>>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>>> index 2c91c03e7eb0..3e920ec5c4d3 100644
>>>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>>> @@ -14,6 +14,8 @@ Required properties:
>>>>                   at25df641
>>>>                   at26df081a
>>>>                   mr25h256
>>>> +                 mr25h10
>>>> +                 mr25h40
>>>>                   mx25l4005a
>>>>                   mx25l1606e
>>>>                   mx25l6405d
>>>
>>> Uh, this is getting a never-ending-story...
>>> If these chipsets don't support JEDEC, should we keep them in jedec,spi-nor.txt?
>>>
>>
>> Maybe not but I think the new compatible strings should be documented
>> somewhere. Currently jedec,spi-nor.txt already documents all the
>> "m25p*-nonjedec" memories. So maybe just renaming the jedec,spi-nor.txt
>> file into spi-nor.txt or mtd,spi-nor.txt could be a solution. Otherwise, we
>> can let it as is. I have no idea of what would be the best solution.
>>
>> To be honest, I don't always fully understand the DT policy/philosophy and
>> its requirements. I just thought when a new property or a new value is
>> introduced it has to be documented.
>> Generally speaking, when DT is involved in some series of patches, it often
>> generates many discussions about the proper way to do thinks and about
>> choosing the best between many technically functional solutions.
>>
>> If you think jedec,spi-nor.txt is not suited to document the new value for
>> the compatible string, why not, I perfectly understand your point.
>>
>> I don't mind choosing another way. I just want to be sure that, if not all,
>> most of people agree on that solution and if possible, it is compliant with
>> DT policy so everybody is happy and works together.
>> That's why I involve DT people, even if it's a small detail, so they can
>> advise us.
>>
>> Anyway, at some point we have to take a decision to carry on thinks.
>> So actually, I would like to avoid a never-ending story :)
> 
> Sounds OK to me, I'm not DT expert though ;)

So ok, we already have a few non-jedec bindings documented in
jedec,spi-nor,text . Let's just apply this patch and if someone wants to
split the
binding document, patch is welcome. Good ?

-- 
Best regards,
Marek Vasut

Re: [PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Rob Herring]

On Tue, Jan 17, 2017 at 02:57:22PM +0100, Cyrille Pitchen wrote:
> Le 17/01/2017 à 14:16, Rafał Miłecki a écrit :
> > On 17 January 2017 at 12:03, Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> >> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> >> in the list of supported spi IDs. This makes it possible to add these
> >> using for example:
> >>
> >>         compatible = "everspin,mr25h40";
> > 
> > (...)
> > 
> >> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> >> index 2c91c03e7eb0..3e920ec5c4d3 100644
> >> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> >> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> >> @@ -14,6 +14,8 @@ Required properties:
> >>                   at25df641
> >>                   at26df081a
> >>                   mr25h256
> >> +                 mr25h10
> >> +                 mr25h40
> >>                   mx25l4005a
> >>                   mx25l1606e
> >>                   mx25l6405d
> > 
> > Uh, this is getting a never-ending-story...
> > If these chipsets don't support JEDEC, should we keep them in jedec,spi-nor.txt?
> > 
> 
> Maybe not but I think the new compatible strings should be documented
> somewhere. Currently jedec,spi-nor.txt already documents all the
> "m25p*-nonjedec" memories. So maybe just renaming the jedec,spi-nor.txt
> file into spi-nor.txt or mtd,spi-nor.txt could be a solution. Otherwise, we
> can let it as is. I have no idea of what would be the best solution.

As I read the description, the non-jedec chips don't support READ ID, 
but I would assume they otherwise follow the JEDEC spec(s)?

> To be honest, I don't always fully understand the DT policy/philosophy and
> its requirements. I just thought when a new property or a new value is
> introduced it has to be documented.
> Generally speaking, when DT is involved in some series of patches, it often
> generates many discussions about the proper way to do thinks and about
> choosing the best between many technically functional solutions.

Doesn't that apply to any code review? Sounds like the kernel process to 
me. If the DT review is more stringent, then I'll take that as a 
complement.

> If you think jedec,spi-nor.txt is not suited to document the new value for
> the compatible string, why not, I perfectly understand your point.
> 
> I don't mind choosing another way. I just want to be sure that, if not all,
> most of people agree on that solution and if possible, it is compliant with
> DT policy so everybody is happy and works together.
> That's why I involve DT people, even if it's a small detail, so they can
> advise us.
> 
> Anyway, at some point we have to take a decision to carry on thinks.
> So actually, I would like to avoid a never-ending story :)

I don't know what's the right answer here with regards to renaming or 
spliting things. In either case, that's a separate issue from this 
patch.

Rob

Re: [PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Rob Herring]

On Tue, Jan 17, 2017 at 12:03:38PM +0100, Uwe Kleine-König wrote:
> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> in the list of supported spi IDs. This makes it possible to add these
> using for example:
> 
> 	compatible = "everspin,mr25h40";
> 
> There was already an entry for mr25h256. Move that one out of the "keep
> for compatibility" section and put in a new group for Everspin MRAMs.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Changes since (implicit) v1:
> 
>  - use Kib instead of kib
> 
> Changes since v2:
> 
>  - update dt docs
>  - handle already existing mr25h256 in m25p_ids[]
> 
> Thanks to Cyrille for catching these.
> 
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 2 ++
>  drivers/mtd/devices/m25p80.c                            | 6 +++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)

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

Re: [PATCH v3] mtd: spi-nor: add dt support for Everspin MRAMs

[Cyrille Pitchen]

Le 19/01/2017 à 18:56, Rob Herring a écrit :
> On Tue, Jan 17, 2017 at 12:03:38PM +0100, Uwe Kleine-König wrote:
>> The MR25 family doesn't support JEDEC, so they need explicit mentioning
>> in the list of supported spi IDs. This makes it possible to add these
>> using for example:
>>
>> 	compatible = "everspin,mr25h40";
>>
>> There was already an entry for mr25h256. Move that one out of the "keep
>> for compatibility" section and put in a new group for Everspin MRAMs.
>>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>> Changes since (implicit) v1:
>>
>>  - use Kib instead of kib
>>
>> Changes since v2:
>>
>>  - update dt docs
>>  - handle already existing mr25h256 in m25p_ids[]
>>
>> Thanks to Cyrille for catching these.
>>
>>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 2 ++
>>  drivers/mtd/devices/m25p80.c                            | 6 +++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 
Applied to git://github.com/spi-nor/linux.git

Thanks!