| Message ID | 1383656135-8627-26-git-send-email-ezequiel.garcia@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Nov 05, 2013 at 09:55:32AM -0300, Ezequiel Garcia wrote: > The Armada 370 and Armada XP SoC have a NAND controller (aka NFCv2). > This commit adds support for it in Armada 370 and Armada XP SoC > common devicetree. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > arch/arm/boot/dts/armada-370-xp.dtsi | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi > index 01e69fc..b4e6898 100644 > --- a/arch/arm/boot/dts/armada-370-xp.dtsi > +++ b/arch/arm/boot/dts/armada-370-xp.dtsi > @@ -258,6 +258,15 @@ > status = "disabled"; > }; > > + nand@d0000 { > + compatible = "marvell,armada370-nand"; Could you please provide a separate patch updating the devicetree binding documentation? You can also Cc the entire series to the devicetree ml as long as the documentation patch is easy to find in the series. eg 'dt: binding: ...' thx, Jason.
On Tue, Nov 05, 2013 at 08:29:05AM -0500, Jason Cooper wrote: > On Tue, Nov 05, 2013 at 09:55:32AM -0300, Ezequiel Garcia wrote: > > The Armada 370 and Armada XP SoC have a NAND controller (aka NFCv2). > > This commit adds support for it in Armada 370 and Armada XP SoC > > common devicetree. > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > --- > > arch/arm/boot/dts/armada-370-xp.dtsi | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi > > index 01e69fc..b4e6898 100644 > > --- a/arch/arm/boot/dts/armada-370-xp.dtsi > > +++ b/arch/arm/boot/dts/armada-370-xp.dtsi > > @@ -258,6 +258,15 @@ > > status = "disabled"; > > }; > > > > + nand@d0000 { > > + compatible = "marvell,armada370-nand"; > > Could you please provide a separate patch updating the devicetree > binding documentation? You can also Cc the entire series to the > devicetree ml as long as the documentation patch is easy to find in the > series. eg 'dt: binding: ...' > Hm.. actually the controller already supports the new compatible string so the binding documentation should be added now. And I'd rather do that in a separate patch, to avoid cluttering the poor devicetree people with an unrelated 28-piece patch :-) BTW: who should take such a patch? I'm still a little lost regarding who takes the binding or dts patches for a given subsystem.
On Tue, Nov 05, 2013 at 10:51:46AM -0300, Ezequiel Garcia wrote: > On Tue, Nov 05, 2013 at 08:29:05AM -0500, Jason Cooper wrote: > > On Tue, Nov 05, 2013 at 09:55:32AM -0300, Ezequiel Garcia wrote: > > > The Armada 370 and Armada XP SoC have a NAND controller (aka NFCv2). > > > This commit adds support for it in Armada 370 and Armada XP SoC > > > common devicetree. > > > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > > --- > > > arch/arm/boot/dts/armada-370-xp.dtsi | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi > > > index 01e69fc..b4e6898 100644 > > > --- a/arch/arm/boot/dts/armada-370-xp.dtsi > > > +++ b/arch/arm/boot/dts/armada-370-xp.dtsi > > > @@ -258,6 +258,15 @@ > > > status = "disabled"; > > > }; > > > > > > + nand@d0000 { > > > + compatible = "marvell,armada370-nand"; > > > > Could you please provide a separate patch updating the devicetree > > binding documentation? You can also Cc the entire series to the > > devicetree ml as long as the documentation patch is easy to find in the > > series. eg 'dt: binding: ...' > > > > Hm.. actually the controller already supports the new compatible string > so the binding documentation should be added now. $ git grep -n 'marvell,armada370-nand' -- Documentation/devicetree/bindings/ $ Perhaps in v3.13-rc1? > And I'd rather do that in a separate patch, to avoid cluttering the poor > devicetree people with an unrelated 28-piece patch :-) No (really), according to Grant and Mark during the closing session, I asked this specific question. They _do_ want the entire series so they can refer to the corresponding code changes if necessary. As I stated above, we can make their job easier by making the binding a separate patch that is clearly marked as such. > BTW: who should take such a patch? I'm still a little lost regarding > who takes the binding or dts patches for a given subsystem. The appropriate sub-system maintainer still takes the patches, we simply wait a bit for the DT binding maintainers to chime in. If they don't after a few weeks, we can take it without their Ack. If the maintainer is unsure, or needs help reviewing the binding, they can always ping the DT folks for assistance. thx, Jason.
On Tue, Nov 05, 2013 at 10:15:31AM -0500, Jason Cooper wrote: > On Tue, Nov 05, 2013 at 10:51:46AM -0300, Ezequiel Garcia wrote: > > On Tue, Nov 05, 2013 at 08:29:05AM -0500, Jason Cooper wrote: > > > On Tue, Nov 05, 2013 at 09:55:32AM -0300, Ezequiel Garcia wrote: > > > > The Armada 370 and Armada XP SoC have a NAND controller (aka NFCv2). > > > > This commit adds support for it in Armada 370 and Armada XP SoC > > > > common devicetree. > > > > > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > > > --- > > > > arch/arm/boot/dts/armada-370-xp.dtsi | 9 +++++++++ > > > > 1 file changed, 9 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi > > > > index 01e69fc..b4e6898 100644 > > > > --- a/arch/arm/boot/dts/armada-370-xp.dtsi > > > > +++ b/arch/arm/boot/dts/armada-370-xp.dtsi > > > > @@ -258,6 +258,15 @@ > > > > status = "disabled"; > > > > }; > > > > > > > > + nand@d0000 { > > > > + compatible = "marvell,armada370-nand"; > > > > > > Could you please provide a separate patch updating the devicetree > > > binding documentation? You can also Cc the entire series to the > > > devicetree ml as long as the documentation patch is easy to find in the > > > series. eg 'dt: binding: ...' > > > > > > > Hm.. actually the controller already supports the new compatible string > > so the binding documentation should be added now. > > $ git grep -n 'marvell,armada370-nand' -- Documentation/devicetree/bindings/ > $ Well the controller supports it, but I never updated the binding: $ git grep -n 'marvell,armada370-nand' -- drivers/mtd/nand/pxa3xx-nand.c So that's why I think a separate patch to be taken by Brian now is more appropriate. > > And I'd rather do that in a separate patch, to avoid cluttering the poor > > devicetree people with an unrelated 28-piece patch :-) > > No (really), according to Grant and Mark during the closing session, I > asked this specific question. They _do_ want the entire series so they > can refer to the corresponding code changes if necessary. As I stated > above, we can make their job easier by making the binding a separate > patch that is clearly marked as such. > Ah, good to know. > > BTW: who should take such a patch? I'm still a little lost regarding > > who takes the binding or dts patches for a given subsystem. > > The appropriate sub-system maintainer still takes the patches, we simply > wait a bit for the DT binding maintainers to chime in. If they don't > after a few weeks, we can take it without their Ack. > > If the maintainer is unsure, or needs help reviewing the binding, they > can always ping the DT folks for assistance. > Ok, great.
Dear Jason Cooper, On Tue, 5 Nov 2013 10:15:31 -0500, Jason Cooper wrote: > > And I'd rather do that in a separate patch, to avoid cluttering the poor > > devicetree people with an unrelated 28-piece patch :-) > > No (really), according to Grant and Mark during the closing session, I > asked this specific question. They _do_ want the entire series so they > can refer to the corresponding code changes if necessary. As I stated > above, we can make their job easier by making the binding a separate > patch that is clearly marked as such. Interesting, because I had understood exactly the opposite from the discussions at the ARM kernel summit. But maybe the DT folks changed their mind between the ARM kernel summit and the closing session of the kernel summit :-) Thomas
Thomas, Adding Grant and Mark to the Cc to make sure they don't have buyer's remorse ;-) On Wed, Nov 06, 2013 at 09:24:35AM +0100, Thomas Petazzoni wrote: > Dear Jason Cooper, > > On Tue, 5 Nov 2013 10:15:31 -0500, Jason Cooper wrote: > > > > And I'd rather do that in a separate patch, to avoid cluttering the poor > > > devicetree people with an unrelated 28-piece patch :-) > > > > No (really), according to Grant and Mark during the closing session, I > > asked this specific question. They _do_ want the entire series so they > > can refer to the corresponding code changes if necessary. As I stated > > above, we can make their job easier by making the binding a separate > > patch that is clearly marked as such. > > Interesting, because I had understood exactly the opposite from the > discussions at the ARM kernel summit. But maybe the DT folks changed > their mind between the ARM kernel summit and the closing session of the > kernel summit :-) Yes, that's why I asked the question. Mark Rutland said he wanted to see the code changes when he was reviewing a binding. Which is indeed the opposite of what we had arrived at during the ARM mini-summit. So I asked Grant (last question of the Q&A) what he wanted us to tell the contributors. He said (bad paraphrasing here) "I have better filters now for finding the binding changes, so send the whole series" As him and Mark aren't the only reviewers, I took the extra step of saying send the DT bindings as a separate patch with it clearly identified in the subject line... thx, Jason.
Dear Jason Cooper, On Wed, 6 Nov 2013 06:42:44 -0500, Jason Cooper wrote: > > Interesting, because I had understood exactly the opposite from the > > discussions at the ARM kernel summit. But maybe the DT folks changed > > their mind between the ARM kernel summit and the closing session of > > the kernel summit :-) > > Yes, that's why I asked the question. Mark Rutland said he wanted to > see the code changes when he was reviewing a binding. Which is indeed > the opposite of what we had arrived at during the ARM mini-summit. > > So I asked Grant (last question of the Q&A) what he wanted us to tell > the contributors. He said (bad paraphrasing here) "I have better > filters now for finding the binding changes, so send the whole series" > > As him and Mark aren't the only reviewers, I took the extra step of > saying send the DT bindings as a separate patch with it clearly > identified in the subject line... Ok, thanks for the clarification. Those rules seem to be changing from one day to the next, not really easy to follow :) Is there some .txt file in Documentation/ that explains what the DT maintainers expect? Thanks, Thomas
On Wed, Nov 06, 2013 at 01:56:54PM +0100, Thomas Petazzoni wrote: > Dear Jason Cooper, > > On Wed, 6 Nov 2013 06:42:44 -0500, Jason Cooper wrote: > > > > Interesting, because I had understood exactly the opposite from the > > > discussions at the ARM kernel summit. But maybe the DT folks changed > > > their mind between the ARM kernel summit and the closing session of > > > the kernel summit :-) > > > > Yes, that's why I asked the question. Mark Rutland said he wanted to > > see the code changes when he was reviewing a binding. Which is indeed > > the opposite of what we had arrived at during the ARM mini-summit. > > > > So I asked Grant (last question of the Q&A) what he wanted us to tell > > the contributors. He said (bad paraphrasing here) "I have better > > filters now for finding the binding changes, so send the whole series" > > > > As him and Mark aren't the only reviewers, I took the extra step of > > saying send the DT bindings as a separate patch with it clearly > > identified in the subject line... > > Ok, thanks for the clarification. Those rules seem to be changing from > one day to the next, not really easy to follow :) > > Is there some .txt file in Documentation/ that explains what the DT > maintainers expect? hmmm. Give me a bit. :) thx, Jason.
diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi index 01e69fc..b4e6898 100644 --- a/arch/arm/boot/dts/armada-370-xp.dtsi +++ b/arch/arm/boot/dts/armada-370-xp.dtsi @@ -258,6 +258,15 @@ status = "disabled"; }; + nand@d0000 { + compatible = "marvell,armada370-nand"; + reg = <0xd0000 0x54>; + #address-cells = <1>; + #size-cells = <1>; + interrupts = <113>; + clocks = <&coredivclk 0>; + status = "disabled"; + }; }; };
The Armada 370 and Armada XP SoC have a NAND controller (aka NFCv2). This commit adds support for it in Armada 370 and Armada XP SoC common devicetree. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- arch/arm/boot/dts/armada-370-xp.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+)