* linux-next: manual merge of the spi-nor tree with the imx-mxs tree
@ 2017-10-30 18:15 Mark Brown
2017-10-31 0:42 ` Cyrille Pitchen
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2017-10-30 18:15 UTC (permalink / raw)
To: Cyrille Pitchen, Yuan Yao, Hou Zhiqiang, Rob Herring, Shawn Guo,
Philipp Puschmann
Cc: linux-arm-kernel, Linux-Next Mailing List, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]
Hi Cyrille,
Today's linux-next merge of the spi-nor tree got a conflict in:
Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
between commit:
b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to sip-nor list")
from the imx-mxs tree and commit:
282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
from the spi-nor tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
index 4cab5d85cf6f,956bb046e599..000000000000
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
@@@ -13,7 -13,7 +13,8 @@@ Required properties
at25df321a
at25df641
at26df081a
+ en25s64
+ mr25h128
mr25h256
mr25h10
mr25h40
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: linux-next: manual merge of the spi-nor tree with the imx-mxs tree
2017-10-30 18:15 linux-next: manual merge of the spi-nor tree with the imx-mxs tree Mark Brown
@ 2017-10-31 0:42 ` Cyrille Pitchen
2017-10-31 2:16 ` Shawn Guo
2017-10-31 2:39 ` Z.q. Hou
0 siblings, 2 replies; 6+ messages in thread
From: Cyrille Pitchen @ 2017-10-31 0:42 UTC (permalink / raw)
To: Mark Brown, Yuan Yao, Hou Zhiqiang, Rob Herring, Shawn Guo,
Philipp Puschmann
Cc: linux-arm-kernel, Linux-Next Mailing List,
Linux Kernel Mailing List, Marek Vasut
Hi all,
+ Marek
Mark, thanks for this report.
Shawn, Yuan, if I don't make a mistake, patch "dt-bindings: mtd: add sst25wf040b
and en25s64 to sip-nor list" was not submitted to the linux-mtd mailing list
hence was neither reviewed nor acked by any spi-nor maintainer. If so, such a
patch should then be taken from the spi-nor/next branch of the l2-mtd tree.
So Shawn, could you please remove this patch from your tree ?
Yuan, could you please submit your patch to the linux-mtd mailing list for
proper review ?
Best regards,
Cyrille
Le 30/10/2017 à 19:15, Mark Brown a écrit :
> Hi Cyrille,
>
> Today's linux-next merge of the spi-nor tree got a conflict in:
>
> Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>
> between commit:
>
> b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to sip-nor list")
>
> from the imx-mxs tree and commit:
>
> 282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
>
> from the spi-nor tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> index 4cab5d85cf6f,956bb046e599..000000000000
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> @@@ -13,7 -13,7 +13,8 @@@ Required properties
> at25df321a
> at25df641
> at26df081a
> + en25s64
> + mr25h128
> mr25h256
> mr25h10
> mr25h40
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: linux-next: manual merge of the spi-nor tree with the imx-mxs tree
2017-10-31 0:42 ` Cyrille Pitchen
@ 2017-10-31 2:16 ` Shawn Guo
2017-11-01 12:47 ` Cyrille Pitchen
2017-10-31 2:39 ` Z.q. Hou
1 sibling, 1 reply; 6+ messages in thread
From: Shawn Guo @ 2017-10-31 2:16 UTC (permalink / raw)
To: Cyrille Pitchen, Arnd Bergmann
Cc: Mark Brown, Yuan Yao, Hou Zhiqiang, Rob Herring,
Philipp Puschmann, linux-arm-kernel, Linux-Next Mailing List,
Linux Kernel Mailing List, Marek Vasut
On Tue, Oct 31, 2017 at 01:42:30AM +0100, Cyrille Pitchen wrote:
> Hi all,
>
> + Marek
>
> Mark, thanks for this report.
>
> Shawn, Yuan, if I don't make a mistake, patch "dt-bindings: mtd: add sst25wf040b
> and en25s64 to sip-nor list" was not submitted to the linux-mtd mailing list
> hence was neither reviewed nor acked by any spi-nor maintainer. If so, such a
> patch should then be taken from the spi-nor/next branch of the l2-mtd tree.
I see the patch was Acked by DT maintainer and so applied it as part of
the series. I will leave such mtd bindings patch to l2-mtd maintainers
in the future.
> So Shawn, could you please remove this patch from your tree ?
I have already sent the patch to my upstream maintainers (arm-soc). I
guess it's okay to leave such trivial conflict to Linus. @Arnd, what do
you think?
Shawn
> Yuan, could you please submit your patch to the linux-mtd mailing list for
> proper review ?
>
> Best regards,
>
> Cyrille
>
>
> Le 30/10/2017 à 19:15, Mark Brown a écrit :
> > Hi Cyrille,
> >
> > Today's linux-next merge of the spi-nor tree got a conflict in:
> >
> > Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> >
> > between commit:
> >
> > b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to sip-nor list")
> >
> > from the imx-mxs tree and commit:
> >
> > 282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
> >
> > from the spi-nor tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging. You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> >
> > diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > index 4cab5d85cf6f,956bb046e599..000000000000
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > @@@ -13,7 -13,7 +13,8 @@@ Required properties
> > at25df321a
> > at25df641
> > at26df081a
> > + en25s64
> > + mr25h128
> > mr25h256
> > mr25h10
> > mr25h40
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: linux-next: manual merge of the spi-nor tree with the imx-mxs tree
2017-10-31 0:42 ` Cyrille Pitchen
2017-10-31 2:16 ` Shawn Guo
@ 2017-10-31 2:39 ` Z.q. Hou
1 sibling, 0 replies; 6+ messages in thread
From: Z.q. Hou @ 2017-10-31 2:39 UTC (permalink / raw)
To: Cyrille Pitchen, Mark Brown, Yuan Yao, Rob Herring, Shawn Guo,
Philipp Puschmann
Cc: linux-arm-kernel, Linux-Next Mailing List,
Linux Kernel Mailing List, Marek Vasut
Hi Cyrille,
> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@wedev4u.fr]
> Sent: 2017年10月31日 8:43
> To: Mark Brown <broonie@kernel.org>; Yuan Yao <yao.yuan@nxp.com>; Z.q.
> Hou <zhiqiang.hou@nxp.com>; Rob Herring <robh@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; Philipp Puschmann <pp@emlix.com>
> Cc: linux-arm-kernel@lists.infradead.org; Linux-Next Mailing List
> <linux-next@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; Marek Vasut <marek.vasut@gmail.com>
> Subject: Re: linux-next: manual merge of the spi-nor tree with the imx-mxs tree
>
> Hi all,
>
> + Marek
>
> Mark, thanks for this report.
>
> Shawn, Yuan, if I don't make a mistake, patch "dt-bindings: mtd: add
> sst25wf040b and en25s64 to sip-nor list" was not submitted to the linux-mtd
> mailing list hence was neither reviewed nor acked by any spi-nor maintainer. If
> so, such a patch should then be taken from the spi-nor/next branch of the
> l2-mtd tree.
>
> So Shawn, could you please remove this patch from your tree ?
>
> Yuan, could you please submit your patch to the linux-mtd mailing list for
> proper review ?
Only this single patch is needed to submit to linux-mtd mailing list, right?
Thanks,
Zhiqiang
> Best regards,
>
> Cyrille
>
>
> Le 30/10/2017 à 19:15, Mark Brown a écrit :
> > Hi Cyrille,
> >
> > Today's linux-next merge of the spi-nor tree got a conflict in:
> >
> > Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> >
> > between commit:
> >
> > b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to
> > sip-nor list")
> >
> > from the imx-mxs tree and commit:
> >
> > 282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
> >
> > from the spi-nor tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary. This is
> > now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your
> > tree is submitted for merging. You may also want to consider
> > cooperating with the maintainer of the conflicting tree to minimise
> > any particularly complex conflicts.
> >
> > diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > index 4cab5d85cf6f,956bb046e599..000000000000
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > @@@ -13,7 -13,7 +13,8 @@@ Required properties
> > at25df321a
> > at25df641
> > at26df081a
> > + en25s64
> > + mr25h128
> > mr25h256
> > mr25h10
> > mr25h40
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: linux-next: manual merge of the spi-nor tree with the imx-mxs tree
2017-10-31 2:16 ` Shawn Guo
@ 2017-11-01 12:47 ` Cyrille Pitchen
2017-11-02 8:38 ` Shawn Guo
0 siblings, 1 reply; 6+ messages in thread
From: Cyrille Pitchen @ 2017-11-01 12:47 UTC (permalink / raw)
To: Shawn Guo, Arnd Bergmann
Cc: Mark Brown, Yuan Yao, Hou Zhiqiang, Rob Herring,
Philipp Puschmann, linux-arm-kernel, Linux-Next Mailing List,
Linux Kernel Mailing List, Marek Vasut,
Linux MTD Development List
+ linux-mtd
Le 31/10/2017 à 03:16, Shawn Guo a écrit :
> On Tue, Oct 31, 2017 at 01:42:30AM +0100, Cyrille Pitchen wrote:
>> Hi all,
>>
>> + Marek
>>
>> Mark, thanks for this report.
>>
>> Shawn, Yuan, if I don't make a mistake, patch "dt-bindings: mtd: add sst25wf040b
>> and en25s64 to sip-nor list" was not submitted to the linux-mtd mailing list
>> hence was neither reviewed nor acked by any spi-nor maintainer. If so, such a
>> patch should then be taken from the spi-nor/next branch of the l2-mtd tree.
>
> I see the patch was Acked by DT maintainer and so applied it as part of
> the series. I will leave such mtd bindings patch to l2-mtd maintainers
> in the future.
>
>> So Shawn, could you please remove this patch from your tree ?
>
> I have already sent the patch to my upstream maintainers (arm-soc). I
> guess it's okay to leave such trivial conflict to Linus. @Arnd, what do
> you think?
>
My concern is not about the conflict, which indeed is trivial, but about
the fact that this patch should not be applied at all. I would have
like to discuss it on the linux-mtd mailing list since the
jedec,spi-nor.txt is maintained by MTD / spi-nor folks.
Some memory parts like mr25h128 don't support at all the JEDEC READ ID
(9Fh) command. In such a case, the "jedec,spi-nor" string is not suited
as a value inside the 'compatible' DT property because the memory is
not JEDEC compliant. In that precise case, we have no other choice but
to use another 'compatible' value, that's why we rely on chip name.
However, for memory parts that do support the JEDEC READ ID command,
like en25s64 and sst25wf040b, we require that the 'compatible' DT
property includes the "jedec,spi-nor" string. I guess for *legacy
reasons*, the chip name MAY be included too but this is not needed and
actually may have side effects, especially when the SPI NOR memory is
handled by the m25p80 driver. When possible, it's better to set the
2nd parameter of spi_nor_scan() to NULL. My understanding is that is
parameter is there for historical reason and is only used for:
1 - backward compatibility, when DT and the "jedec,spi-nor" 'compatible'
string were not introduced yet.
2 - provide the chip name of non-JEDEC memories.
So currently, I advise to avoid adding the chip name in the 'compatible'
string when not needed then to set it to "jedec,spi-nor" alone.
Also I think we should add new chip names in the "jedec,spi-nor.txt"
file only for non JEDEC memory parts, that is to say, when we have no
other choice.
This is my current feeling and I would have like to have a chance to
discuss it first with Marek and other MTD guys before taking a
decision because at the spi-nor side we've already had bad surprises
with chip names / Jedec ID mismatches. So IMHO, chip names are not so
reliable and we should avoid to use them when possible.
For instance, as long as they are not used in device trees, we can still
try to fix inconsistent naming in the entries of the spi_nor_ids[]
array of the drivers/mtd/spi-nor/spi-nor.c. In most cases the
name is only informational, not use for other purpose than display.
However, when the 2nd parameter of spi_nor_scan() is not NULL, which
is not true in the general case, the name is used as an index to find
out the relevant entry from the spi_nor_ids[] array. Such mechanism
should be used only for non-JEDEC memories, though the spi-nor driver
can handle the case where the 'compatible' DT property is set to
compatible = "<vendor>,<chip-name>", "jedec,spi-nor";
In such a case, and only when driven by the m25p80 driver, the
<chip-name> string is provided as the 2nd argument of spi_nor_scan().
This function first tries to look up the proper spi_nor_ids[] entry
based on the <chip-name> then reads the JEDEC ID and uses it as
another index in the spi_nor_ids[] array to find the real entry.
If both entries, the one found by name and the one found by JEDEC ID,
don't match, the spi-nor driver display a warning and keep the entry
found by ID. So adding the chip name in the 'compatible' string of
JEDEC memories is in the best case useless.
That's why, for now, I still think this patch should not be applied at
all, at least, not before having been reviewed and accepted by MTD guys.
Best regards,
Cyrille
> Shawn
>
>> Yuan, could you please submit your patch to the linux-mtd mailing list for
>> proper review ?
>>
>> Best regards,
>>
>> Cyrille
>>
>>
>> Le 30/10/2017 à 19:15, Mark Brown a écrit :
>>> Hi Cyrille,
>>>
>>> Today's linux-next merge of the spi-nor tree got a conflict in:
>>>
>>> Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>>
>>> between commit:
>>>
>>> b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to sip-nor list")
>>>
>>> from the imx-mxs tree and commit:
>>>
>>> 282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
>>>
>>> from the spi-nor tree.
>>>
>>> I fixed it up (see below) and can carry the fix as necessary. This
>>> is now fixed as far as linux-next is concerned, but any non trivial
>>> conflicts should be mentioned to your upstream maintainer when your tree
>>> is submitted for merging. You may also want to consider cooperating
>>> with the maintainer of the conflicting tree to minimise any particularly
>>> complex conflicts.
>>>
>>> diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>> index 4cab5d85cf6f,956bb046e599..000000000000
>>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
>>> @@@ -13,7 -13,7 +13,8 @@@ Required properties
>>> at25df321a
>>> at25df641
>>> at26df081a
>>> + en25s64
>>> + mr25h128
>>> mr25h256
>>> mr25h10
>>> mr25h40
>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: linux-next: manual merge of the spi-nor tree with the imx-mxs tree
2017-11-01 12:47 ` Cyrille Pitchen
@ 2017-11-02 8:38 ` Shawn Guo
0 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2017-11-02 8:38 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: Arnd Bergmann, Mark Brown, Yuan Yao, Hou Zhiqiang, Rob Herring,
Philipp Puschmann, linux-arm-kernel, Linux-Next Mailing List,
Linux Kernel Mailing List, Marek Vasut,
Linux MTD Development List
On Wed, Nov 01, 2017 at 01:47:04PM +0100, Cyrille Pitchen wrote:
> >> So Shawn, could you please remove this patch from your tree ?
> >
> > I have already sent the patch to my upstream maintainers (arm-soc). I
> > guess it's okay to leave such trivial conflict to Linus. @Arnd, what do
> > you think?
> >
>
> My concern is not about the conflict, which indeed is trivial, but about
> the fact that this patch should not be applied at all. I would have
> like to discuss it on the linux-mtd mailing list since the
> jedec,spi-nor.txt is maintained by MTD / spi-nor folks.
>
> Some memory parts like mr25h128 don't support at all the JEDEC READ ID
> (9Fh) command. In such a case, the "jedec,spi-nor" string is not suited
> as a value inside the 'compatible' DT property because the memory is
> not JEDEC compliant. In that precise case, we have no other choice but
> to use another 'compatible' value, that's why we rely on chip name.
>
> However, for memory parts that do support the JEDEC READ ID command,
> like en25s64 and sst25wf040b, we require that the 'compatible' DT
> property includes the "jedec,spi-nor" string. I guess for *legacy
> reasons*, the chip name MAY be included too but this is not needed and
> actually may have side effects, especially when the SPI NOR memory is
> handled by the m25p80 driver. When possible, it's better to set the
> 2nd parameter of spi_nor_scan() to NULL. My understanding is that is
> parameter is there for historical reason and is only used for:
> 1 - backward compatibility, when DT and the "jedec,spi-nor" 'compatible'
> string were not introduced yet.
> 2 - provide the chip name of non-JEDEC memories.
>
> So currently, I advise to avoid adding the chip name in the 'compatible'
> string when not needed then to set it to "jedec,spi-nor" alone.
> Also I think we should add new chip names in the "jedec,spi-nor.txt"
> file only for non JEDEC memory parts, that is to say, when we have no
> other choice.
>
> This is my current feeling and I would have like to have a chance to
> discuss it first with Marek and other MTD guys before taking a
> decision because at the spi-nor side we've already had bad surprises
> with chip names / Jedec ID mismatches. So IMHO, chip names are not so
> reliable and we should avoid to use them when possible.
>
> For instance, as long as they are not used in device trees, we can still
> try to fix inconsistent naming in the entries of the spi_nor_ids[]
> array of the drivers/mtd/spi-nor/spi-nor.c. In most cases the
> name is only informational, not use for other purpose than display.
> However, when the 2nd parameter of spi_nor_scan() is not NULL, which
> is not true in the general case, the name is used as an index to find
> out the relevant entry from the spi_nor_ids[] array. Such mechanism
> should be used only for non-JEDEC memories, though the spi-nor driver
> can handle the case where the 'compatible' DT property is set to
> compatible = "<vendor>,<chip-name>", "jedec,spi-nor";
> In such a case, and only when driven by the m25p80 driver, the
> <chip-name> string is provided as the 2nd argument of spi_nor_scan().
> This function first tries to look up the proper spi_nor_ids[] entry
> based on the <chip-name> then reads the JEDEC ID and uses it as
> another index in the spi_nor_ids[] array to find the real entry.
> If both entries, the one found by name and the one found by JEDEC ID,
> don't match, the spi-nor driver display a warning and keep the entry
> found by ID. So adding the chip name in the 'compatible' string of
> JEDEC memories is in the best case useless.
>
> That's why, for now, I still think this patch should not be applied at
> all, at least, not before having been reviewed and accepted by MTD guys.
Okay. If people all agreed that the patch was doing something wrong,
maybe a revert patch is more appropriate at this point, I guess.
Shawn
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-02 8:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-30 18:15 linux-next: manual merge of the spi-nor tree with the imx-mxs tree Mark Brown
2017-10-31 0:42 ` Cyrille Pitchen
2017-10-31 2:16 ` Shawn Guo
2017-11-01 12:47 ` Cyrille Pitchen
2017-11-02 8:38 ` Shawn Guo
2017-10-31 2:39 ` Z.q. Hou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).