* [PATCH] ARM: dts: da850-evm: add clock properties to the nand node @ 2018-02-05 15:52 Bartosz Golaszewski [not found] ` <20180205155222.22189-1-brgl-ARrdPY/1zhM@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Bartosz Golaszewski @ 2018-02-05 15:52 UTC (permalink / raw) To: Sekhar Nori, Kevin Hilman, Rob Herring, Mark Rutland, Russell King, David Lechner Cc: devicetree, linux-kernel, linux-arm-kernel, Bartosz Golaszewski From: Bartosz Golaszewski <bgolaszewski@baylibre.com> Make nand work with the common clock framework by specifying which clock should be used and what name to look up. Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- arch/arm/boot/dts/da850-evm.dts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts index a86a8a1816f2..2602ad8e99ee 100644 --- a/arch/arm/boot/dts/da850-evm.dts +++ b/arch/arm/boot/dts/da850-evm.dts @@ -296,6 +296,9 @@ reg = <0 0x02000000 0x02000000 1 0x00000000 0x00008000>; + clocks = <&psc0 3>; + clock-names = "aemif"; + ti,davinci-chipselect = <1>; ti,davinci-mask-ale = <0>; ti,davinci-mask-cle = <0>; -- 2.16.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20180205155222.22189-1-brgl-ARrdPY/1zhM@public.gmane.org>]
* Re: [PATCH] ARM: dts: da850-evm: add clock properties to the nand node [not found] ` <20180205155222.22189-1-brgl-ARrdPY/1zhM@public.gmane.org> @ 2018-02-06 11:07 ` Sekhar Nori [not found] ` <3f171f6a-bcea-65ec-d56d-f6ae24660f34-l0cyMroinI0@public.gmane.org> 2018-02-11 5:06 ` kbuild test robot 1 sibling, 1 reply; 9+ messages in thread From: Sekhar Nori @ 2018-02-06 11:07 UTC (permalink / raw) To: Bartosz Golaszewski, Kevin Hilman, Rob Herring, Mark Rutland, Russell King, David Lechner Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Bartosz Golaszewski On Monday 05 February 2018 09:22 PM, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> > > Make nand work with the common clock framework by specifying which > clock should be used and what name to look up. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> > --- > arch/arm/boot/dts/da850-evm.dts | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts > index a86a8a1816f2..2602ad8e99ee 100644 > --- a/arch/arm/boot/dts/da850-evm.dts > +++ b/arch/arm/boot/dts/da850-evm.dts > @@ -296,6 +296,9 @@ > reg = <0 0x02000000 0x02000000 > 1 0x00000000 0x00008000>; > > + clocks = <&psc0 3>; > + clock-names = "aemif"; Looks like this is being added only to satisfy the devm_clk_get() call in nand_davinci_probe() which I think is superfluous since we also enable the same clock in aemif_probe(). Perhaps the better solution is to drip the clk code in drivers/mtd/nand/davinci_nand.c and shift legacy code to start using drivers/memory/aemif.c as well? This way we can also drop arch/arm/mach-davinci/aemif.c Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <3f171f6a-bcea-65ec-d56d-f6ae24660f34-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH] ARM: dts: da850-evm: add clock properties to the nand node [not found] ` <3f171f6a-bcea-65ec-d56d-f6ae24660f34-l0cyMroinI0@public.gmane.org> @ 2018-02-06 13:08 ` Bartosz Golaszewski 2018-02-06 13:51 ` Sekhar Nori 0 siblings, 1 reply; 9+ messages in thread From: Bartosz Golaszewski @ 2018-02-06 13:08 UTC (permalink / raw) To: Sekhar Nori Cc: Bartosz Golaszewski, Kevin Hilman, Rob Herring, Mark Rutland, Russell King, David Lechner, arm-soc, linux-devicetree, LKML 2018-02-06 12:07 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>: > On Monday 05 February 2018 09:22 PM, Bartosz Golaszewski wrote: >> From: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >> >> Make nand work with the common clock framework by specifying which >> clock should be used and what name to look up. >> >> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >> --- >> arch/arm/boot/dts/da850-evm.dts | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >> index a86a8a1816f2..2602ad8e99ee 100644 >> --- a/arch/arm/boot/dts/da850-evm.dts >> +++ b/arch/arm/boot/dts/da850-evm.dts >> @@ -296,6 +296,9 @@ >> reg = <0 0x02000000 0x02000000 >> 1 0x00000000 0x00008000>; >> >> + clocks = <&psc0 3>; >> + clock-names = "aemif"; > > Looks like this is being added only to satisfy the devm_clk_get() call > in nand_davinci_probe() which I think is superfluous since we also > enable the same clock in aemif_probe(). > > Perhaps the better solution is to drip the clk code in > drivers/mtd/nand/davinci_nand.c and shift legacy code to start using > drivers/memory/aemif.c as well? This way we can also drop > arch/arm/mach-davinci/aemif.c > > Thanks, > Sekhar Yes, this sounds good, but I think we should leave it for later as an additional improvement, once everything else is in place. I think these patches should be applied together with David's series in order to not break the support on davinci boards and the aemif work would go in later as a follow-up. How about that? Also: I don't have any keystone board to test whether such changes don't break the nand support there. Would you be able to test this? Thanks, Bartosz -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: dts: da850-evm: add clock properties to the nand node 2018-02-06 13:08 ` Bartosz Golaszewski @ 2018-02-06 13:51 ` Sekhar Nori [not found] ` <54dbdb98-e0e4-c8c9-fec4-2f050745d9be-l0cyMroinI0@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Sekhar Nori @ 2018-02-06 13:51 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Mark Rutland, linux-devicetree, David Lechner, Kevin Hilman, Bartosz Golaszewski, Russell King, LKML, Rob Herring, arm-soc On Tuesday 06 February 2018 06:38 PM, Bartosz Golaszewski wrote: > 2018-02-06 12:07 GMT+01:00 Sekhar Nori <nsekhar@ti.com>: >> On Monday 05 February 2018 09:22 PM, Bartosz Golaszewski wrote: >>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>> >>> Make nand work with the common clock framework by specifying which >>> clock should be used and what name to look up. >>> >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>> --- >>> arch/arm/boot/dts/da850-evm.dts | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>> index a86a8a1816f2..2602ad8e99ee 100644 >>> --- a/arch/arm/boot/dts/da850-evm.dts >>> +++ b/arch/arm/boot/dts/da850-evm.dts >>> @@ -296,6 +296,9 @@ >>> reg = <0 0x02000000 0x02000000 >>> 1 0x00000000 0x00008000>; >>> >>> + clocks = <&psc0 3>; >>> + clock-names = "aemif"; >> >> Looks like this is being added only to satisfy the devm_clk_get() call >> in nand_davinci_probe() which I think is superfluous since we also >> enable the same clock in aemif_probe(). >> >> Perhaps the better solution is to drip the clk code in >> drivers/mtd/nand/davinci_nand.c and shift legacy code to start using >> drivers/memory/aemif.c as well? This way we can also drop >> arch/arm/mach-davinci/aemif.c >> >> Thanks, >> Sekhar > > Yes, this sounds good, but I think we should leave it for later as an > additional improvement, once everything else is in place. I think > these patches should be applied together with David's series in order > to not break the support on davinci boards and the aemif work would go > in later as a follow-up. How about that? No, I dont think we should add temporary hacks to DT to work around driver issues (I do think its a hack since the clock belongs to aemif module not NAND flash). An easier driver hack might be to not treat devm_clk_get() failure in davinci_nand.c as catastrophic. It will safely fail in DT case and we should get the clock in legacy boot case. I think we are looking at a driver update dependency anyway. > > Also: I don't have any keystone board to test whether such changes > don't break the nand support there. Would you be able to test this? Yes, I have access to those boards. Thanks, Sekhar ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <54dbdb98-e0e4-c8c9-fec4-2f050745d9be-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH] ARM: dts: da850-evm: add clock properties to the nand node [not found] ` <54dbdb98-e0e4-c8c9-fec4-2f050745d9be-l0cyMroinI0@public.gmane.org> @ 2018-02-06 18:16 ` David Lechner [not found] ` <794024f3-f87a-58ed-2722-a4a2d09df3ce-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: David Lechner @ 2018-02-06 18:16 UTC (permalink / raw) To: Sekhar Nori, Bartosz Golaszewski Cc: Bartosz Golaszewski, Kevin Hilman, Rob Herring, Mark Rutland, Russell King, arm-soc, linux-devicetree, LKML On 02/06/2018 07:51 AM, Sekhar Nori wrote: > On Tuesday 06 February 2018 06:38 PM, Bartosz Golaszewski wrote: >> 2018-02-06 12:07 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>: >>> On Monday 05 February 2018 09:22 PM, Bartosz Golaszewski wrote: >>>> From: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>> >>>> Make nand work with the common clock framework by specifying which >>>> clock should be used and what name to look up. >>>> >>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>> --- >>>> arch/arm/boot/dts/da850-evm.dts | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>>> index a86a8a1816f2..2602ad8e99ee 100644 >>>> --- a/arch/arm/boot/dts/da850-evm.dts >>>> +++ b/arch/arm/boot/dts/da850-evm.dts >>>> @@ -296,6 +296,9 @@ >>>> reg = <0 0x02000000 0x02000000 >>>> 1 0x00000000 0x00008000>; >>>> >>>> + clocks = <&psc0 3>; >>>> + clock-names = "aemif"; >>> >>> Looks like this is being added only to satisfy the devm_clk_get() call >>> in nand_davinci_probe() which I think is superfluous since we also >>> enable the same clock in aemif_probe(). >>> >>> Perhaps the better solution is to drip the clk code in >>> drivers/mtd/nand/davinci_nand.c and shift legacy code to start using >>> drivers/memory/aemif.c as well? This way we can also drop >>> arch/arm/mach-davinci/aemif.c >>> >>> Thanks, >>> Sekhar >> >> Yes, this sounds good, but I think we should leave it for later as an >> additional improvement, once everything else is in place. I think >> these patches should be applied together with David's series in order >> to not break the support on davinci boards and the aemif work would go >> in later as a follow-up. How about that? > > No, I dont think we should add temporary hacks to DT to work around > driver issues (I do think its a hack since the clock belongs to aemif > module not NAND flash). > > An easier driver hack might be to not treat devm_clk_get() failure in > davinci_nand.c as catastrophic. It will safely fail in DT case and we > should get the clock in legacy boot case. > > I think we are looking at a driver update dependency anyway. It looks like keystone.dtsi is using the clock-ranges property in the aemif node to pass the clock to child nodes. Could we not do the same in da850.dtsi? > >> >> Also: I don't have any keystone board to test whether such changes >> don't break the nand support there. Would you be able to test this? > > Yes, I have access to those boards. > > Thanks, > Sekhar > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <794024f3-f87a-58ed-2722-a4a2d09df3ce-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>]
* Re: [PATCH] ARM: dts: da850-evm: add clock properties to the nand node [not found] ` <794024f3-f87a-58ed-2722-a4a2d09df3ce-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> @ 2018-02-06 18:25 ` David Lechner [not found] ` <4aa2ab13-7890-6904-86b3-e2dbcb6d6daa-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: David Lechner @ 2018-02-06 18:25 UTC (permalink / raw) To: Sekhar Nori, Bartosz Golaszewski Cc: Bartosz Golaszewski, Kevin Hilman, Rob Herring, Mark Rutland, Russell King, arm-soc, linux-devicetree, LKML On 02/06/2018 12:16 PM, David Lechner wrote: > On 02/06/2018 07:51 AM, Sekhar Nori wrote: >> On Tuesday 06 February 2018 06:38 PM, Bartosz Golaszewski wrote: >>> 2018-02-06 12:07 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>: >>>> On Monday 05 February 2018 09:22 PM, Bartosz Golaszewski wrote: >>>>> From: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>>> >>>>> Make nand work with the common clock framework by specifying which >>>>> clock should be used and what name to look up. >>>>> >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>>> --- >>>>> arch/arm/boot/dts/da850-evm.dts | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>>>> index a86a8a1816f2..2602ad8e99ee 100644 >>>>> --- a/arch/arm/boot/dts/da850-evm.dts >>>>> +++ b/arch/arm/boot/dts/da850-evm.dts >>>>> @@ -296,6 +296,9 @@ >>>>> reg = <0 0x02000000 0x02000000 >>>>> 1 0x00000000 0x00008000>; >>>>> >>>>> + clocks = <&psc0 3>; >>>>> + clock-names = "aemif"; >>>> >>>> Looks like this is being added only to satisfy the devm_clk_get() call >>>> in nand_davinci_probe() which I think is superfluous since we also >>>> enable the same clock in aemif_probe(). >>>> >>>> Perhaps the better solution is to drip the clk code in >>>> drivers/mtd/nand/davinci_nand.c and shift legacy code to start using >>>> drivers/memory/aemif.c as well? This way we can also drop >>>> arch/arm/mach-davinci/aemif.c >>>> >>>> Thanks, >>>> Sekhar >>> >>> Yes, this sounds good, but I think we should leave it for later as an >>> additional improvement, once everything else is in place. I think >>> these patches should be applied together with David's series in order >>> to not break the support on davinci boards and the aemif work would go >>> in later as a follow-up. How about that? >> >> No, I dont think we should add temporary hacks to DT to work around >> driver issues (I do think its a hack since the clock belongs to aemif >> module not NAND flash). >> >> An easier driver hack might be to not treat devm_clk_get() failure in >> davinci_nand.c as catastrophic. It will safely fail in DT case and we >> should get the clock in legacy boot case. >> >> I think we are looking at a driver update dependency anyway. > > It looks like keystone.dtsi is using the clock-ranges property in the > aemif node to pass the clock to child nodes. Could we not do the same > in da850.dtsi? Bartosz, please try this instead of your patch. FYI, this is just following the existing memory-controllers/ti-aemif.txt device tree bindings, so not a "hack". --- diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi index 3a1f2ce..ff9d807 100644 --- a/arch/arm/boot/dts/da850.dtsi +++ b/arch/arm/boot/dts/da850.dtsi @@ -796,6 +796,8 @@ ranges = <0 0 0x60000000 0x08000000 1 0 0x68000000 0x00008000>; clocks = <&psc0 3>; + clock-names = "aemif"; + clock-ranges; status = "disabled"; }; memctrl: memory-controller@b0000000 { --- -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <4aa2ab13-7890-6904-86b3-e2dbcb6d6daa-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>]
* Re: [PATCH] ARM: dts: da850-evm: add clock properties to the nand node [not found] ` <4aa2ab13-7890-6904-86b3-e2dbcb6d6daa-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> @ 2018-02-06 18:45 ` Bartosz Golaszewski 2018-02-07 13:06 ` Sekhar Nori 0 siblings, 1 reply; 9+ messages in thread From: Bartosz Golaszewski @ 2018-02-06 18:45 UTC (permalink / raw) To: David Lechner Cc: Sekhar Nori, Bartosz Golaszewski, Kevin Hilman, Rob Herring, Mark Rutland, Russell King, arm-soc, linux-devicetree, LKML 2018-02-06 19:25 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>: > On 02/06/2018 12:16 PM, David Lechner wrote: >> >> On 02/06/2018 07:51 AM, Sekhar Nori wrote: >>> >>> On Tuesday 06 February 2018 06:38 PM, Bartosz Golaszewski wrote: >>>> >>>> 2018-02-06 12:07 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>: >>>>> >>>>> On Monday 05 February 2018 09:22 PM, Bartosz Golaszewski wrote: >>>>>> >>>>>> From: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>>>> >>>>>> Make nand work with the common clock framework by specifying which >>>>>> clock should be used and what name to look up. >>>>>> >>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>>>> --- >>>>>> arch/arm/boot/dts/da850-evm.dts | 3 +++ >>>>>> 1 file changed, 3 insertions(+) >>>>>> >>>>>> diff --git a/arch/arm/boot/dts/da850-evm.dts >>>>>> b/arch/arm/boot/dts/da850-evm.dts >>>>>> index a86a8a1816f2..2602ad8e99ee 100644 >>>>>> --- a/arch/arm/boot/dts/da850-evm.dts >>>>>> +++ b/arch/arm/boot/dts/da850-evm.dts >>>>>> @@ -296,6 +296,9 @@ >>>>>> reg = <0 0x02000000 0x02000000 >>>>>> 1 0x00000000 0x00008000>; >>>>>> >>>>>> + clocks = <&psc0 3>; >>>>>> + clock-names = "aemif"; >>>>> >>>>> >>>>> Looks like this is being added only to satisfy the devm_clk_get() call >>>>> in nand_davinci_probe() which I think is superfluous since we also >>>>> enable the same clock in aemif_probe(). >>>>> >>>>> Perhaps the better solution is to drip the clk code in >>>>> drivers/mtd/nand/davinci_nand.c and shift legacy code to start using >>>>> drivers/memory/aemif.c as well? This way we can also drop >>>>> arch/arm/mach-davinci/aemif.c >>>>> >>>>> Thanks, >>>>> Sekhar >>>> >>>> >>>> Yes, this sounds good, but I think we should leave it for later as an >>>> additional improvement, once everything else is in place. I think >>>> these patches should be applied together with David's series in order >>>> to not break the support on davinci boards and the aemif work would go >>>> in later as a follow-up. How about that? >>> >>> >>> No, I dont think we should add temporary hacks to DT to work around >>> driver issues (I do think its a hack since the clock belongs to aemif >>> module not NAND flash). >>> >>> An easier driver hack might be to not treat devm_clk_get() failure in >>> davinci_nand.c as catastrophic. It will safely fail in DT case and we >>> should get the clock in legacy boot case. >>> >>> I think we are looking at a driver update dependency anyway. >> >> >> It looks like keystone.dtsi is using the clock-ranges property in the >> aemif node to pass the clock to child nodes. Could we not do the same >> in da850.dtsi? > > > Bartosz, please try this instead of your patch. > > FYI, this is just following the existing memory-controllers/ti-aemif.txt > device tree bindings, so not a "hack". > > --- > diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi > index 3a1f2ce..ff9d807 100644 > --- a/arch/arm/boot/dts/da850.dtsi > +++ b/arch/arm/boot/dts/da850.dtsi > @@ -796,6 +796,8 @@ > ranges = <0 0 0x60000000 0x08000000 > 1 0 0x68000000 0x00008000>; > clocks = <&psc0 3>; > + clock-names = "aemif"; > + clock-ranges; > status = "disabled"; > }; > memctrl: memory-controller@b0000000 { > --- Yes, this works. Sekhar: can we include it in David's series, while still keeping the plan to move legacy boards to using the aemif driver? Best regards, Bartosz -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: dts: da850-evm: add clock properties to the nand node 2018-02-06 18:45 ` Bartosz Golaszewski @ 2018-02-07 13:06 ` Sekhar Nori 0 siblings, 0 replies; 9+ messages in thread From: Sekhar Nori @ 2018-02-07 13:06 UTC (permalink / raw) To: Bartosz Golaszewski, David Lechner Cc: Mark Rutland, linux-devicetree, Kevin Hilman, Russell King, LKML, Bartosz Golaszewski, Rob Herring, arm-soc On Wednesday 07 February 2018 12:15 AM, Bartosz Golaszewski wrote: > 2018-02-06 19:25 GMT+01:00 David Lechner <david@lechnology.com>: >> On 02/06/2018 12:16 PM, David Lechner wrote: >>> >>> On 02/06/2018 07:51 AM, Sekhar Nori wrote: >>>> >>>> On Tuesday 06 February 2018 06:38 PM, Bartosz Golaszewski wrote: >>>>> >>>>> 2018-02-06 12:07 GMT+01:00 Sekhar Nori <nsekhar@ti.com>: >>>>>> >>>>>> On Monday 05 February 2018 09:22 PM, Bartosz Golaszewski wrote: >>>>>>> >>>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>>>>> >>>>>>> Make nand work with the common clock framework by specifying which >>>>>>> clock should be used and what name to look up. >>>>>>> >>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>>>>> --- >>>>>>> arch/arm/boot/dts/da850-evm.dts | 3 +++ >>>>>>> 1 file changed, 3 insertions(+) >>>>>>> >>>>>>> diff --git a/arch/arm/boot/dts/da850-evm.dts >>>>>>> b/arch/arm/boot/dts/da850-evm.dts >>>>>>> index a86a8a1816f2..2602ad8e99ee 100644 >>>>>>> --- a/arch/arm/boot/dts/da850-evm.dts >>>>>>> +++ b/arch/arm/boot/dts/da850-evm.dts >>>>>>> @@ -296,6 +296,9 @@ >>>>>>> reg = <0 0x02000000 0x02000000 >>>>>>> 1 0x00000000 0x00008000>; >>>>>>> >>>>>>> + clocks = <&psc0 3>; >>>>>>> + clock-names = "aemif"; >>>>>> >>>>>> >>>>>> Looks like this is being added only to satisfy the devm_clk_get() call >>>>>> in nand_davinci_probe() which I think is superfluous since we also >>>>>> enable the same clock in aemif_probe(). >>>>>> >>>>>> Perhaps the better solution is to drip the clk code in >>>>>> drivers/mtd/nand/davinci_nand.c and shift legacy code to start using >>>>>> drivers/memory/aemif.c as well? This way we can also drop >>>>>> arch/arm/mach-davinci/aemif.c >>>>>> >>>>>> Thanks, >>>>>> Sekhar >>>>> >>>>> >>>>> Yes, this sounds good, but I think we should leave it for later as an >>>>> additional improvement, once everything else is in place. I think >>>>> these patches should be applied together with David's series in order >>>>> to not break the support on davinci boards and the aemif work would go >>>>> in later as a follow-up. How about that? >>>> >>>> >>>> No, I dont think we should add temporary hacks to DT to work around >>>> driver issues (I do think its a hack since the clock belongs to aemif >>>> module not NAND flash). >>>> >>>> An easier driver hack might be to not treat devm_clk_get() failure in >>>> davinci_nand.c as catastrophic. It will safely fail in DT case and we >>>> should get the clock in legacy boot case. >>>> >>>> I think we are looking at a driver update dependency anyway. >>> >>> >>> It looks like keystone.dtsi is using the clock-ranges property in the >>> aemif node to pass the clock to child nodes. Could we not do the same >>> in da850.dtsi? >> >> >> Bartosz, please try this instead of your patch. >> >> FYI, this is just following the existing memory-controllers/ti-aemif.txt >> device tree bindings, so not a "hack". >> >> --- >> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi >> index 3a1f2ce..ff9d807 100644 >> --- a/arch/arm/boot/dts/da850.dtsi >> +++ b/arch/arm/boot/dts/da850.dtsi >> @@ -796,6 +796,8 @@ >> ranges = <0 0 0x60000000 0x08000000 >> 1 0 0x68000000 0x00008000>; >> clocks = <&psc0 3>; >> + clock-names = "aemif"; >> + clock-ranges; >> status = "disabled"; >> }; >> memctrl: memory-controller@b0000000 { >> --- > > Yes, this works. Sekhar: can we include it in David's series, while > still keeping the plan to move legacy boards to using the aemif > driver? Okay. Fine with me. Actually the chipselect node already has clock-ranges defined, but that doesn't have any meaning unless its also included in aemif node. Thanks, Sekhar ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: dts: da850-evm: add clock properties to the nand node [not found] ` <20180205155222.22189-1-brgl-ARrdPY/1zhM@public.gmane.org> 2018-02-06 11:07 ` Sekhar Nori @ 2018-02-11 5:06 ` kbuild test robot 1 sibling, 0 replies; 9+ messages in thread From: kbuild test robot @ 2018-02-11 5:06 UTC (permalink / raw) To: Bartosz Golaszewski Cc: kbuild-all-JC7UmRfGjtg, Sekhar Nori, Kevin Hilman, Rob Herring, Mark Rutland, Russell King, David Lechner, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Bartosz Golaszewski [-- Attachment #1: Type: text/plain, Size: 1078 bytes --] Hi Bartosz, I love your patch! Yet something to improve: [auto build test ERROR on robh/for-next] [also build test ERROR on v4.15 next-20180209] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Bartosz-Golaszewski/ARM-dts-da850-evm-add-clock-properties-to-the-nand-node/20180208-105626 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: arm-sunxi_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): >> ERROR: Input tree has errors, aborting (use -f to force output) --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 23273 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-11 5:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-05 15:52 [PATCH] ARM: dts: da850-evm: add clock properties to the nand node Bartosz Golaszewski [not found] ` <20180205155222.22189-1-brgl-ARrdPY/1zhM@public.gmane.org> 2018-02-06 11:07 ` Sekhar Nori [not found] ` <3f171f6a-bcea-65ec-d56d-f6ae24660f34-l0cyMroinI0@public.gmane.org> 2018-02-06 13:08 ` Bartosz Golaszewski 2018-02-06 13:51 ` Sekhar Nori [not found] ` <54dbdb98-e0e4-c8c9-fec4-2f050745d9be-l0cyMroinI0@public.gmane.org> 2018-02-06 18:16 ` David Lechner [not found] ` <794024f3-f87a-58ed-2722-a4a2d09df3ce-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> 2018-02-06 18:25 ` David Lechner [not found] ` <4aa2ab13-7890-6904-86b3-e2dbcb6d6daa-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> 2018-02-06 18:45 ` Bartosz Golaszewski 2018-02-07 13:06 ` Sekhar Nori 2018-02-11 5:06 ` kbuild test robot
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).