| Message ID | 20170117110338.10756-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Commit | 3a08e933415c58689797c5bdc825e78a808fffe1 |
| Headers | show |
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); >
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?
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 :)
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 ;)
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 ?
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
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>
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!
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);
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(-)