* [PATCH] arm: dts: socfpga: denali needs nand_x_clk too
@ 2018-06-19 12:07 Richard Weinberger
2018-06-20 4:52 ` Marek Vasut
2018-06-22 14:37 ` Masahiro Yamada
0 siblings, 2 replies; 15+ messages in thread
From: Richard Weinberger @ 2018-06-19 12:07 UTC (permalink / raw)
To: devicetree
Cc: linux-kernel, linux-mtd, dinguyen, robh+dt, mark.rutland,
marek.vasut, computersforpeace, dwmw2, miquel.raynal,
boris.brezillon, yamada.masahiro, Richard Weinberger
The denali NAND flash controller needs at least two clocks to operate,
nand_clk and nand_x_clk.
Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by
setup_data_interface()") nand_x_clk is used to derive timing settings.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock
is not present on this SoC.
But my SoCFPGA knowledge is very limited.
Thanks,
//richard
---
arch/arm/boot/dts/socfpga.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 486d4e7433ed..562f7b375bbd 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -754,7 +754,8 @@
reg-names = "nand_data", "denali_reg";
interrupts = <0x0 0x90 0x4>;
dma-mask = <0xffffffff>;
- clocks = <&nand_clk>;
+ clocks = <&nand_clk>, <&nand_x_clk>;
+ clock-names = "nand", "nand_x";
status = "disabled";
};
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-19 12:07 [PATCH] arm: dts: socfpga: denali needs nand_x_clk too Richard Weinberger @ 2018-06-20 4:52 ` Marek Vasut 2018-06-20 5:27 ` Richard Weinberger 2018-06-22 14:37 ` Masahiro Yamada 1 sibling, 1 reply; 15+ messages in thread From: Marek Vasut @ 2018-06-20 4:52 UTC (permalink / raw) To: Richard Weinberger, devicetree Cc: linux-kernel, linux-mtd, dinguyen, robh+dt, mark.rutland, computersforpeace, dwmw2, miquel.raynal, boris.brezillon, yamada.masahiro, See, Chin Liang On 06/19/2018 02:07 PM, Richard Weinberger wrote: > The denali NAND flash controller needs at least two clocks to operate, > nand_clk and nand_x_clk. > Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by > setup_data_interface()") nand_x_clk is used to derive timing settings. > > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock > is not present on this SoC. > But my SoCFPGA knowledge is very limited. > > Thanks, > //richard It looks sane, but I cannot test it right now, since I'm on vacation. I hope Dinh/Chin can jump in. > --- > arch/arm/boot/dts/socfpga.dtsi | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi > index 486d4e7433ed..562f7b375bbd 100644 > --- a/arch/arm/boot/dts/socfpga.dtsi > +++ b/arch/arm/boot/dts/socfpga.dtsi > @@ -754,7 +754,8 @@ > reg-names = "nand_data", "denali_reg"; > interrupts = <0x0 0x90 0x4>; > dma-mask = <0xffffffff>; > - clocks = <&nand_clk>; > + clocks = <&nand_clk>, <&nand_x_clk>; > + clock-names = "nand", "nand_x"; > status = "disabled"; > }; > > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-20 4:52 ` Marek Vasut @ 2018-06-20 5:27 ` Richard Weinberger 2018-06-21 4:41 ` Marek Vasut 0 siblings, 1 reply; 15+ messages in thread From: Richard Weinberger @ 2018-06-20 5:27 UTC (permalink / raw) To: Marek Vasut Cc: devicetree, linux-kernel, linux-mtd, dinguyen, robh+dt, mark.rutland, computersforpeace, dwmw2, miquel.raynal, boris.brezillon, yamada.masahiro, See, Chin Liang Marek, Am Mittwoch, 20. Juni 2018, 06:52:09 CEST schrieb Marek Vasut: > On 06/19/2018 02:07 PM, Richard Weinberger wrote: > > The denali NAND flash controller needs at least two clocks to operate, > > nand_clk and nand_x_clk. > > Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by > > setup_data_interface()") nand_x_clk is used to derive timing settings. > > > > Signed-off-by: Richard Weinberger <richard@nod.at> > > --- > > Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock > > is not present on this SoC. > > But my SoCFPGA knowledge is very limited. > > > > Thanks, > > //richard > > It looks sane, but I cannot test it right now, since I'm on vacation. > I hope Dinh/Chin can jump in. The patch was tested by me. So, at least it is not completely untested. BTW: I forgot to mention that it depends on Masahiro's Denali fixes. Thanks, //richard ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-20 5:27 ` Richard Weinberger @ 2018-06-21 4:41 ` Marek Vasut 0 siblings, 0 replies; 15+ messages in thread From: Marek Vasut @ 2018-06-21 4:41 UTC (permalink / raw) To: Richard Weinberger Cc: devicetree, linux-kernel, linux-mtd, dinguyen, robh+dt, mark.rutland, computersforpeace, dwmw2, miquel.raynal, boris.brezillon, yamada.masahiro, See, Chin Liang On 06/20/2018 07:27 AM, Richard Weinberger wrote: > Marek, > > Am Mittwoch, 20. Juni 2018, 06:52:09 CEST schrieb Marek Vasut: >> On 06/19/2018 02:07 PM, Richard Weinberger wrote: >>> The denali NAND flash controller needs at least two clocks to operate, >>> nand_clk and nand_x_clk. >>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by >>> setup_data_interface()") nand_x_clk is used to derive timing settings. >>> >>> Signed-off-by: Richard Weinberger <richard@nod.at> >>> --- >>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock >>> is not present on this SoC. >>> But my SoCFPGA knowledge is very limited. >>> >>> Thanks, >>> //richard >> >> It looks sane, but I cannot test it right now, since I'm on vacation. >> I hope Dinh/Chin can jump in. > > The patch was tested by me. So, at least it is not completely untested. > BTW: I forgot to mention that it depends on Masahiro's Denali fixes. Then that's perfect. Reviewed-by: Marek Vasut <marek.vasut@gmail.com> -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-19 12:07 [PATCH] arm: dts: socfpga: denali needs nand_x_clk too Richard Weinberger 2018-06-20 4:52 ` Marek Vasut @ 2018-06-22 14:37 ` Masahiro Yamada 2018-06-22 15:58 ` Richard Weinberger 1 sibling, 1 reply; 15+ messages in thread From: Masahiro Yamada @ 2018-06-22 14:37 UTC (permalink / raw) To: Richard Weinberger Cc: DTML, Linux Kernel Mailing List, linux-mtd, Dinh Nguyen, Rob Herring, Mark Rutland, Marek Vašut, Brian Norris, David Woodhouse, Miquel Raynal, Boris Brezillon Hi Richard, 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: > The denali NAND flash controller needs at least two clocks to operate, > nand_clk and nand_x_clk. > Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by > setup_data_interface()") nand_x_clk is used to derive timing settings. > > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock > is not present on this SoC. > But my SoCFPGA knowledge is very limited. > > Thanks, > //richard > --- > arch/arm/boot/dts/socfpga.dtsi | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi > index 486d4e7433ed..562f7b375bbd 100644 > --- a/arch/arm/boot/dts/socfpga.dtsi > +++ b/arch/arm/boot/dts/socfpga.dtsi > @@ -754,7 +754,8 @@ > reg-names = "nand_data", "denali_reg"; > interrupts = <0x0 0x90 0x4>; > dma-mask = <0xffffffff>; > - clocks = <&nand_clk>; > + clocks = <&nand_clk>, <&nand_x_clk>; > + clock-names = "nand", "nand_x"; IMHO, this should be clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; clock-names = "nand", "nand_x", "ecc"; A clock consumer (Denali in this case) should generally take the same number of clocks across SoCs. It is just some SoCs tie clocks together. It is the case for my UniPhier platform; "nand_x" and "ecc" are tied up because they are both 200MHz. SOCFPGA supports HW ECC correction, thus it surely needs ecc clock. > status = "disabled"; > }; > > -- > 2.17.1 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-22 14:37 ` Masahiro Yamada @ 2018-06-22 15:58 ` Richard Weinberger 2018-06-25 14:50 ` Dinh Nguyen 0 siblings, 1 reply; 15+ messages in thread From: Richard Weinberger @ 2018-06-22 15:58 UTC (permalink / raw) To: Masahiro Yamada Cc: DTML, Linux Kernel Mailing List, linux-mtd, Dinh Nguyen, Rob Herring, Mark Rutland, Marek Vašut, Brian Norris, David Woodhouse, Miquel Raynal, Boris Brezillon Masahiro, Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: > Hi Richard, > > > 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: > > The denali NAND flash controller needs at least two clocks to operate, > > nand_clk and nand_x_clk. > > Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by > > setup_data_interface()") nand_x_clk is used to derive timing settings. > > > > Signed-off-by: Richard Weinberger <richard@nod.at> > > --- > > Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock > > is not present on this SoC. > > But my SoCFPGA knowledge is very limited. > > > > Thanks, > > //richard > > --- > > arch/arm/boot/dts/socfpga.dtsi | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi > > index 486d4e7433ed..562f7b375bbd 100644 > > --- a/arch/arm/boot/dts/socfpga.dtsi > > +++ b/arch/arm/boot/dts/socfpga.dtsi > > @@ -754,7 +754,8 @@ > > reg-names = "nand_data", "denali_reg"; > > interrupts = <0x0 0x90 0x4>; > > dma-mask = <0xffffffff>; > > - clocks = <&nand_clk>; > > + clocks = <&nand_clk>, <&nand_x_clk>; > > + clock-names = "nand", "nand_x"; > > > IMHO, this should be > > clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; > clock-names = "nand", "nand_x", "ecc"; > > > > A clock consumer (Denali in this case) should generally > take the same number of clocks across SoCs. > > It is just some SoCs tie clocks together. > > It is the case for my UniPhier platform; > "nand_x" and "ecc" are tied up because they are both 200MHz. > > > SOCFPGA supports HW ECC correction, thus it surely needs ecc clock. Good point! Thanks, //richard ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-22 15:58 ` Richard Weinberger @ 2018-06-25 14:50 ` Dinh Nguyen 2018-06-25 14:55 ` Boris Brezillon 0 siblings, 1 reply; 15+ messages in thread From: Dinh Nguyen @ 2018-06-25 14:50 UTC (permalink / raw) To: Richard Weinberger, Masahiro Yamada Cc: DTML, Linux Kernel Mailing List, linux-mtd, Rob Herring, Mark Rutland, Marek Vašut, Brian Norris, David Woodhouse, Miquel Raynal, Boris Brezillon On 06/22/2018 10:58 AM, Richard Weinberger wrote: > Masahiro, > > Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: >> Hi Richard, >> >> >> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: >>> The denali NAND flash controller needs at least two clocks to operate, >>> nand_clk and nand_x_clk. >>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by >>> setup_data_interface()") nand_x_clk is used to derive timing settings. >>> >>> Signed-off-by: Richard Weinberger <richard@nod.at> >>> --- >>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock >>> is not present on this SoC. >>> But my SoCFPGA knowledge is very limited. >>> >>> Thanks, >>> //richard >>> --- >>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi >>> index 486d4e7433ed..562f7b375bbd 100644 >>> --- a/arch/arm/boot/dts/socfpga.dtsi >>> +++ b/arch/arm/boot/dts/socfpga.dtsi >>> @@ -754,7 +754,8 @@ >>> reg-names = "nand_data", "denali_reg"; >>> interrupts = <0x0 0x90 0x4>; >>> dma-mask = <0xffffffff>; >>> - clocks = <&nand_clk>; >>> + clocks = <&nand_clk>, <&nand_x_clk>; >>> + clock-names = "nand", "nand_x"; >> >> >> IMHO, this should be >> >> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; >> clock-names = "nand", "nand_x", "ecc"; No, it should be just the nand_x and ecc. There's already a patch to use the nand_x_clk and not the nand_clk. https://git.kernel.org/pub/scm/linux/kernel/git/dinguyen/linux.git/commit/?h=socfpga_for_next_v4.19_fixes_v1&id=1709ab58eb79b19bceb2287d111bf1bd2df1cf6d Dinh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-25 14:50 ` Dinh Nguyen @ 2018-06-25 14:55 ` Boris Brezillon 2018-06-26 2:38 ` Masahiro Yamada 0 siblings, 1 reply; 15+ messages in thread From: Boris Brezillon @ 2018-06-25 14:55 UTC (permalink / raw) To: Dinh Nguyen Cc: Richard Weinberger, Masahiro Yamada, DTML, Linux Kernel Mailing List, linux-mtd, Rob Herring, Mark Rutland, Marek Vašut, Brian Norris, David Woodhouse, Miquel Raynal On Mon, 25 Jun 2018 09:50:18 -0500 Dinh Nguyen <dinguyen@kernel.org> wrote: > On 06/22/2018 10:58 AM, Richard Weinberger wrote: > > Masahiro, > > > > Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: > >> Hi Richard, > >> > >> > >> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: > >>> The denali NAND flash controller needs at least two clocks to operate, > >>> nand_clk and nand_x_clk. > >>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by > >>> setup_data_interface()") nand_x_clk is used to derive timing settings. > >>> > >>> Signed-off-by: Richard Weinberger <richard@nod.at> > >>> --- > >>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock > >>> is not present on this SoC. > >>> But my SoCFPGA knowledge is very limited. > >>> > >>> Thanks, > >>> //richard > >>> --- > >>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- > >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi > >>> index 486d4e7433ed..562f7b375bbd 100644 > >>> --- a/arch/arm/boot/dts/socfpga.dtsi > >>> +++ b/arch/arm/boot/dts/socfpga.dtsi > >>> @@ -754,7 +754,8 @@ > >>> reg-names = "nand_data", "denali_reg"; > >>> interrupts = <0x0 0x90 0x4>; > >>> dma-mask = <0xffffffff>; > >>> - clocks = <&nand_clk>; > >>> + clocks = <&nand_clk>, <&nand_x_clk>; > >>> + clock-names = "nand", "nand_x"; > >> > >> > >> IMHO, this should be > >> > >> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; > >> clock-names = "nand", "nand_x", "ecc"; > > No, it should be just the nand_x and ecc. > > There's already a patch to use the nand_x_clk and not the nand_clk. > > https://git.kernel.org/pub/scm/linux/kernel/git/dinguyen/linux.git/commit/?h=socfpga_for_next_v4.19_fixes_v1&id=1709ab58eb79b19bceb2287d111bf1bd2df1cf6d Hm, are you sure this is accurate? I might be wrong but I find it weird that the denali NAND controller IP has been adapted by Xilinx to only take one clk. Isn't that the same clk is feeding all clk inputs of the denali block? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-25 14:55 ` Boris Brezillon @ 2018-06-26 2:38 ` Masahiro Yamada 2018-06-26 18:09 ` Miquel Raynal 0 siblings, 1 reply; 15+ messages in thread From: Masahiro Yamada @ 2018-06-26 2:38 UTC (permalink / raw) To: Boris Brezillon Cc: Dinh Nguyen, Richard Weinberger, DTML, Linux Kernel Mailing List, linux-mtd, Rob Herring, Mark Rutland, Marek Vašut, Brian Norris, David Woodhouse, Miquel Raynal 2018-06-25 23:55 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>: > On Mon, 25 Jun 2018 09:50:18 -0500 > Dinh Nguyen <dinguyen@kernel.org> wrote: > >> On 06/22/2018 10:58 AM, Richard Weinberger wrote: >> > Masahiro, >> > >> > Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: >> >> Hi Richard, >> >> >> >> >> >> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: >> >>> The denali NAND flash controller needs at least two clocks to operate, >> >>> nand_clk and nand_x_clk. >> >>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by >> >>> setup_data_interface()") nand_x_clk is used to derive timing settings. >> >>> >> >>> Signed-off-by: Richard Weinberger <richard@nod.at> >> >>> --- >> >>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock >> >>> is not present on this SoC. >> >>> But my SoCFPGA knowledge is very limited. >> >>> >> >>> Thanks, >> >>> //richard >> >>> --- >> >>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- >> >>> 1 file changed, 2 insertions(+), 1 deletion(-) >> >>> >> >>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi >> >>> index 486d4e7433ed..562f7b375bbd 100644 >> >>> --- a/arch/arm/boot/dts/socfpga.dtsi >> >>> +++ b/arch/arm/boot/dts/socfpga.dtsi >> >>> @@ -754,7 +754,8 @@ >> >>> reg-names = "nand_data", "denali_reg"; >> >>> interrupts = <0x0 0x90 0x4>; >> >>> dma-mask = <0xffffffff>; >> >>> - clocks = <&nand_clk>; >> >>> + clocks = <&nand_clk>, <&nand_x_clk>; >> >>> + clock-names = "nand", "nand_x"; >> >> >> >> >> >> IMHO, this should be >> >> >> >> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; >> >> clock-names = "nand", "nand_x", "ecc"; >> >> No, it should be just the nand_x and ecc. >> >> There's already a patch to use the nand_x_clk and not the nand_clk. Different people try to fix the problem in different ways. I think it is due to miscommunication across sub-systems. >> >> https://git.kernel.org/pub/scm/linux/kernel/git/dinguyen/linux.git/commit/?h=socfpga_for_next_v4.19_fixes_v1&id=1709ab58eb79b19bceb2287d111bf1bd2df1cf6d This does not break your board. However, it is not helpful any more. I already fix the Denali driver to use the hard-coded clock frequency if the old binding is used. BTW, Marek issued Reviewed-by to this patch. > Hm, are you sure this is accurate? I might be wrong but I find it weird > that the denali NAND controller IP has been adapted by Xilinx to only Xilinx? Do you mean Altera (Intel)? > take one clk. Isn't that the same clk is feeding all clk inputs of the > denali block? -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-26 2:38 ` Masahiro Yamada @ 2018-06-26 18:09 ` Miquel Raynal 2018-06-27 2:52 ` Masahiro Yamada 0 siblings, 1 reply; 15+ messages in thread From: Miquel Raynal @ 2018-06-26 18:09 UTC (permalink / raw) To: Masahiro Yamada Cc: Boris Brezillon, Dinh Nguyen, Richard Weinberger, DTML, Linux Kernel Mailing List, linux-mtd, Rob Herring, Mark Rutland, Marek Vašut, Brian Norris, David Woodhouse Hi Masahiro, On Tue, 26 Jun 2018 11:38:21 +0900, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2018-06-25 23:55 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>: > > On Mon, 25 Jun 2018 09:50:18 -0500 > > Dinh Nguyen <dinguyen@kernel.org> wrote: > > > >> On 06/22/2018 10:58 AM, Richard Weinberger wrote: > >> > Masahiro, > >> > > >> > Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: > >> >> Hi Richard, > >> >> > >> >> > >> >> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: > >> >>> The denali NAND flash controller needs at least two clocks to operate, > >> >>> nand_clk and nand_x_clk. > >> >>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by > >> >>> setup_data_interface()") nand_x_clk is used to derive timing settings. > >> >>> > >> >>> Signed-off-by: Richard Weinberger <richard@nod.at> > >> >>> --- > >> >>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock > >> >>> is not present on this SoC. > >> >>> But my SoCFPGA knowledge is very limited. > >> >>> > >> >>> Thanks, > >> >>> //richard > >> >>> --- > >> >>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- > >> >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >> >>> > >> >>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi > >> >>> index 486d4e7433ed..562f7b375bbd 100644 > >> >>> --- a/arch/arm/boot/dts/socfpga.dtsi > >> >>> +++ b/arch/arm/boot/dts/socfpga.dtsi > >> >>> @@ -754,7 +754,8 @@ > >> >>> reg-names = "nand_data", "denali_reg"; > >> >>> interrupts = <0x0 0x90 0x4>; > >> >>> dma-mask = <0xffffffff>; > >> >>> - clocks = <&nand_clk>; > >> >>> + clocks = <&nand_clk>, <&nand_x_clk>; > >> >>> + clock-names = "nand", "nand_x"; > >> >> > >> >> > >> >> IMHO, this should be > >> >> > >> >> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; > >> >> clock-names = "nand", "nand_x", "ecc"; > >> > >> No, it should be just the nand_x and ecc. > >> > >> There's already a patch to use the nand_x_clk and not the nand_clk. > > > Different people try to fix the problem in different ways. > > I think it is due to miscommunication across sub-systems. Is the series named mtd: rawnand: denali: add new clocks and improve setup_data_interface still valid? I am about to apply it (patches 2/5 to 5/5) but it looks like the discussion is still ongoing. Thanks, Miquèl ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-26 18:09 ` Miquel Raynal @ 2018-06-27 2:52 ` Masahiro Yamada 2018-06-27 14:55 ` Dinh Nguyen 0 siblings, 1 reply; 15+ messages in thread From: Masahiro Yamada @ 2018-06-27 2:52 UTC (permalink / raw) To: Miquel Raynal Cc: Mark Rutland, DTML, Dinh Nguyen, Richard Weinberger, Linux Kernel Mailing List, Boris Brezillon, Rob Herring, linux-mtd, Brian Norris, David Woodhouse, Marek Vašut 2018-06-27 3:09 GMT+09:00 Miquel Raynal <miquel.raynal@bootlin.com>: > Hi Masahiro, > > On Tue, 26 Jun 2018 11:38:21 +0900, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > >> 2018-06-25 23:55 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>: >> > On Mon, 25 Jun 2018 09:50:18 -0500 >> > Dinh Nguyen <dinguyen@kernel.org> wrote: >> > >> >> On 06/22/2018 10:58 AM, Richard Weinberger wrote: >> >> > Masahiro, >> >> > >> >> > Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: >> >> >> Hi Richard, >> >> >> >> >> >> >> >> >> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: >> >> >>> The denali NAND flash controller needs at least two clocks to operate, >> >> >>> nand_clk and nand_x_clk. >> >> >>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by >> >> >>> setup_data_interface()") nand_x_clk is used to derive timing settings. >> >> >>> >> >> >>> Signed-off-by: Richard Weinberger <richard@nod.at> >> >> >>> --- >> >> >>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock >> >> >>> is not present on this SoC. >> >> >>> But my SoCFPGA knowledge is very limited. >> >> >>> >> >> >>> Thanks, >> >> >>> //richard >> >> >>> --- >> >> >>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- >> >> >>> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> >>> >> >> >>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi >> >> >>> index 486d4e7433ed..562f7b375bbd 100644 >> >> >>> --- a/arch/arm/boot/dts/socfpga.dtsi >> >> >>> +++ b/arch/arm/boot/dts/socfpga.dtsi >> >> >>> @@ -754,7 +754,8 @@ >> >> >>> reg-names = "nand_data", "denali_reg"; >> >> >>> interrupts = <0x0 0x90 0x4>; >> >> >>> dma-mask = <0xffffffff>; >> >> >>> - clocks = <&nand_clk>; >> >> >>> + clocks = <&nand_clk>, <&nand_x_clk>; >> >> >>> + clock-names = "nand", "nand_x"; >> >> >> >> >> >> >> >> >> IMHO, this should be >> >> >> >> >> >> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; >> >> >> clock-names = "nand", "nand_x", "ecc"; >> >> >> >> No, it should be just the nand_x and ecc. >> >> >> >> There's already a patch to use the nand_x_clk and not the nand_clk. >> >> >> Different people try to fix the problem in different ways. >> >> I think it is due to miscommunication across sub-systems. > > Is the series named > > mtd: rawnand: denali: add new clocks and improve > setup_data_interface > > still valid? Yes. I believe V4 is valid. Information for Dinh Nguyen: http://patchwork.ozlabs.org/patch/933507/ http://patchwork.ozlabs.org/patch/933487/ http://patchwork.ozlabs.org/patch/933494/ http://patchwork.ozlabs.org/patch/933506/ If he is not convinced, I am open to discussion, though. > I am about to apply it (patches 2/5 to 5/5) but it looks like the > discussion is still ongoing. > > Thanks, > Miquèl > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-27 2:52 ` Masahiro Yamada @ 2018-06-27 14:55 ` Dinh Nguyen 2018-06-27 21:34 ` Boris Brezillon 2018-07-02 1:35 ` Masahiro Yamada 0 siblings, 2 replies; 15+ messages in thread From: Dinh Nguyen @ 2018-06-27 14:55 UTC (permalink / raw) To: Masahiro Yamada, Miquel Raynal Cc: Mark Rutland, DTML, Richard Weinberger, Linux Kernel Mailing List, Boris Brezillon, Rob Herring, linux-mtd, Brian Norris, David Woodhouse, Marek Vašut Hi Masahiro, On 06/26/2018 09:52 PM, Masahiro Yamada wrote: > 2018-06-27 3:09 GMT+09:00 Miquel Raynal <miquel.raynal@bootlin.com>: >> Hi Masahiro, >> >> On Tue, 26 Jun 2018 11:38:21 +0900, Masahiro Yamada >> <yamada.masahiro@socionext.com> wrote: >> >>> 2018-06-25 23:55 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>: >>>> On Mon, 25 Jun 2018 09:50:18 -0500 >>>> Dinh Nguyen <dinguyen@kernel.org> wrote: >>>> >>>>> On 06/22/2018 10:58 AM, Richard Weinberger wrote: >>>>>> Masahiro, >>>>>> >>>>>> Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: >>>>>>> Hi Richard, >>>>>>> >>>>>>> >>>>>>> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: >>>>>>>> The denali NAND flash controller needs at least two clocks to operate, >>>>>>>> nand_clk and nand_x_clk. >>>>>>>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by >>>>>>>> setup_data_interface()") nand_x_clk is used to derive timing settings. >>>>>>>> >>>>>>>> Signed-off-by: Richard Weinberger <richard@nod.at> >>>>>>>> --- >>>>>>>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock >>>>>>>> is not present on this SoC. >>>>>>>> But my SoCFPGA knowledge is very limited. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> //richard >>>>>>>> --- >>>>>>>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- >>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi >>>>>>>> index 486d4e7433ed..562f7b375bbd 100644 >>>>>>>> --- a/arch/arm/boot/dts/socfpga.dtsi >>>>>>>> +++ b/arch/arm/boot/dts/socfpga.dtsi >>>>>>>> @@ -754,7 +754,8 @@ >>>>>>>> reg-names = "nand_data", "denali_reg"; >>>>>>>> interrupts = <0x0 0x90 0x4>; >>>>>>>> dma-mask = <0xffffffff>; >>>>>>>> - clocks = <&nand_clk>; >>>>>>>> + clocks = <&nand_clk>, <&nand_x_clk>; >>>>>>>> + clock-names = "nand", "nand_x"; >>>>>>> >>>>>>> >>>>>>> IMHO, this should be >>>>>>> >>>>>>> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; >>>>>>> clock-names = "nand", "nand_x", "ecc"; >>>>> >>>>> No, it should be just the nand_x and ecc. >>>>> >>>>> There's already a patch to use the nand_x_clk and not the nand_clk. >>> >>> >>> Different people try to fix the problem in different ways. >>> >>> I think it is due to miscommunication across sub-systems. >> >> Is the series named >> >> mtd: rawnand: denali: add new clocks and improve >> setup_data_interface >> >> still valid? > > Yes. > I believe V4 is valid. > > > Information for Dinh Nguyen: > > http://patchwork.ozlabs.org/patch/933507/ > http://patchwork.ozlabs.org/patch/933487/ > http://patchwork.ozlabs.org/patch/933494/ > http://patchwork.ozlabs.org/patch/933506/ > > > If he is not convinced, I am open to discussion, though. I wasn't aware of these patches. This patch is staged to go into v4.17-rc3 through the arm-soc: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/arch/arm/boot/dts/socfpga.dtsi?h=fixes&id=4eda9b766b042ea38d84df91581b03f6145a2ab0 I think your patch will handle a case where only 1 clock is passed in, so it should be okay right? Dinh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-27 14:55 ` Dinh Nguyen @ 2018-06-27 21:34 ` Boris Brezillon 2018-07-02 1:35 ` Masahiro Yamada 1 sibling, 0 replies; 15+ messages in thread From: Boris Brezillon @ 2018-06-27 21:34 UTC (permalink / raw) To: Dinh Nguyen Cc: Masahiro Yamada, Miquel Raynal, Mark Rutland, DTML, Richard Weinberger, Linux Kernel Mailing List, Rob Herring, linux-mtd, Brian Norris, David Woodhouse, Marek Vašut On Wed, 27 Jun 2018 09:55:59 -0500 Dinh Nguyen <dinguyen@kernel.org> wrote: > Hi Masahiro, > > On 06/26/2018 09:52 PM, Masahiro Yamada wrote: > > 2018-06-27 3:09 GMT+09:00 Miquel Raynal <miquel.raynal@bootlin.com>: > >> Hi Masahiro, > >> > >> On Tue, 26 Jun 2018 11:38:21 +0900, Masahiro Yamada > >> <yamada.masahiro@socionext.com> wrote: > >> > >>> 2018-06-25 23:55 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>: > >>>> On Mon, 25 Jun 2018 09:50:18 -0500 > >>>> Dinh Nguyen <dinguyen@kernel.org> wrote: > >>>> > >>>>> On 06/22/2018 10:58 AM, Richard Weinberger wrote: > >>>>>> Masahiro, > >>>>>> > >>>>>> Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: > >>>>>>> Hi Richard, > >>>>>>> > >>>>>>> > >>>>>>> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: > >>>>>>>> The denali NAND flash controller needs at least two clocks to operate, > >>>>>>>> nand_clk and nand_x_clk. > >>>>>>>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by > >>>>>>>> setup_data_interface()") nand_x_clk is used to derive timing settings. > >>>>>>>> > >>>>>>>> Signed-off-by: Richard Weinberger <richard@nod.at> > >>>>>>>> --- > >>>>>>>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock > >>>>>>>> is not present on this SoC. > >>>>>>>> But my SoCFPGA knowledge is very limited. > >>>>>>>> > >>>>>>>> Thanks, > >>>>>>>> //richard > >>>>>>>> --- > >>>>>>>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- > >>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>>>>>> > >>>>>>>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi > >>>>>>>> index 486d4e7433ed..562f7b375bbd 100644 > >>>>>>>> --- a/arch/arm/boot/dts/socfpga.dtsi > >>>>>>>> +++ b/arch/arm/boot/dts/socfpga.dtsi > >>>>>>>> @@ -754,7 +754,8 @@ > >>>>>>>> reg-names = "nand_data", "denali_reg"; > >>>>>>>> interrupts = <0x0 0x90 0x4>; > >>>>>>>> dma-mask = <0xffffffff>; > >>>>>>>> - clocks = <&nand_clk>; > >>>>>>>> + clocks = <&nand_clk>, <&nand_x_clk>; > >>>>>>>> + clock-names = "nand", "nand_x"; > >>>>>>> > >>>>>>> > >>>>>>> IMHO, this should be > >>>>>>> > >>>>>>> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; > >>>>>>> clock-names = "nand", "nand_x", "ecc"; > >>>>> > >>>>> No, it should be just the nand_x and ecc. > >>>>> > >>>>> There's already a patch to use the nand_x_clk and not the nand_clk. > >>> > >>> > >>> Different people try to fix the problem in different ways. > >>> > >>> I think it is due to miscommunication across sub-systems. > >> > >> Is the series named > >> > >> mtd: rawnand: denali: add new clocks and improve > >> setup_data_interface > >> > >> still valid? > > > > Yes. > > I believe V4 is valid. > > > > > > Information for Dinh Nguyen: > > > > http://patchwork.ozlabs.org/patch/933507/ > > http://patchwork.ozlabs.org/patch/933487/ > > http://patchwork.ozlabs.org/patch/933494/ > > http://patchwork.ozlabs.org/patch/933506/ > > > > > > If he is not convinced, I am open to discussion, though. > > I wasn't aware of these patches. This patch is staged to go into > v4.17-rc3 through the arm-soc: > > https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/arch/arm/boot/dts/socfpga.dtsi?h=fixes&id=4eda9b766b042ea38d84df91581b03f6145a2ab0 I don't think that's a problem. Masahiro's changes are targeting 4.19, so these new DT changes should go in after that anyway. > > I think your patch will handle a case where only 1 clock is passed in, > so it should be okay right? Just because the DT changes you made fixed the problem doesn't mean it's the right solution. DT should represent HW blocks and HW connections. According to Masahiro, the denali NAND controller takes 3 clk signals in input, so those 3 clks should be represented in the DT with 3 entries in the "clocks" and "clock-names" props. The fact that, in the socfpga case, all clk input signals are actually connected to the same clk provider does not mean you should have a single entry in "clocks" and "clock-names". Instead, you should have all 3 entries of clocks pointing to the same clk provider. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-06-27 14:55 ` Dinh Nguyen 2018-06-27 21:34 ` Boris Brezillon @ 2018-07-02 1:35 ` Masahiro Yamada 2018-07-02 13:19 ` Dinh Nguyen 1 sibling, 1 reply; 15+ messages in thread From: Masahiro Yamada @ 2018-07-02 1:35 UTC (permalink / raw) To: Dinh Nguyen Cc: Miquel Raynal, Mark Rutland, DTML, Marek Vašut, Richard Weinberger, Linux Kernel Mailing List, Boris Brezillon, Rob Herring, linux-mtd, Brian Norris, David Woodhouse Hi Dinh, 2018-06-27 23:55 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>: > Hi Masahiro, > > On 06/26/2018 09:52 PM, Masahiro Yamada wrote: >> 2018-06-27 3:09 GMT+09:00 Miquel Raynal <miquel.raynal@bootlin.com>: >>> Hi Masahiro, >>> >>> On Tue, 26 Jun 2018 11:38:21 +0900, Masahiro Yamada >>> <yamada.masahiro@socionext.com> wrote: >>> >>>> 2018-06-25 23:55 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>: >>>>> On Mon, 25 Jun 2018 09:50:18 -0500 >>>>> Dinh Nguyen <dinguyen@kernel.org> wrote: >>>>> >>>>>> On 06/22/2018 10:58 AM, Richard Weinberger wrote: >>>>>>> Masahiro, >>>>>>> >>>>>>> Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: >>>>>>>> Hi Richard, >>>>>>>> >>>>>>>> >>>>>>>> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: >>>>>>>>> The denali NAND flash controller needs at least two clocks to operate, >>>>>>>>> nand_clk and nand_x_clk. >>>>>>>>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by >>>>>>>>> setup_data_interface()") nand_x_clk is used to derive timing settings. >>>>>>>>> >>>>>>>>> Signed-off-by: Richard Weinberger <richard@nod.at> >>>>>>>>> --- >>>>>>>>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock >>>>>>>>> is not present on this SoC. >>>>>>>>> But my SoCFPGA knowledge is very limited. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> //richard >>>>>>>>> --- >>>>>>>>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- >>>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>>>> >>>>>>>>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi >>>>>>>>> index 486d4e7433ed..562f7b375bbd 100644 >>>>>>>>> --- a/arch/arm/boot/dts/socfpga.dtsi >>>>>>>>> +++ b/arch/arm/boot/dts/socfpga.dtsi >>>>>>>>> @@ -754,7 +754,8 @@ >>>>>>>>> reg-names = "nand_data", "denali_reg"; >>>>>>>>> interrupts = <0x0 0x90 0x4>; >>>>>>>>> dma-mask = <0xffffffff>; >>>>>>>>> - clocks = <&nand_clk>; >>>>>>>>> + clocks = <&nand_clk>, <&nand_x_clk>; >>>>>>>>> + clock-names = "nand", "nand_x"; >>>>>>>> >>>>>>>> >>>>>>>> IMHO, this should be >>>>>>>> >>>>>>>> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; >>>>>>>> clock-names = "nand", "nand_x", "ecc"; >>>>>> >>>>>> No, it should be just the nand_x and ecc. >>>>>> >>>>>> There's already a patch to use the nand_x_clk and not the nand_clk. >>>> >>>> >>>> Different people try to fix the problem in different ways. >>>> >>>> I think it is due to miscommunication across sub-systems. >>> >>> Is the series named >>> >>> mtd: rawnand: denali: add new clocks and improve >>> setup_data_interface >>> >>> still valid? >> >> Yes. >> I believe V4 is valid. >> >> >> Information for Dinh Nguyen: >> >> http://patchwork.ozlabs.org/patch/933507/ >> http://patchwork.ozlabs.org/patch/933487/ >> http://patchwork.ozlabs.org/patch/933494/ >> http://patchwork.ozlabs.org/patch/933506/ >> >> >> If he is not convinced, I am open to discussion, though. > > I wasn't aware of these patches. This patch is staged to go into > v4.17-rc3 through the arm-soc: > > https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/arch/arm/boot/dts/socfpga.dtsi?h=fixes&id=4eda9b766b042ea38d84df91581b03f6145a2ab0 > > I think your patch will handle a case where only 1 clock is passed in, > so it should be okay right? > I should be OK, but please consider the proper fix for v4.19 as Boris suggested. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too 2018-07-02 1:35 ` Masahiro Yamada @ 2018-07-02 13:19 ` Dinh Nguyen 0 siblings, 0 replies; 15+ messages in thread From: Dinh Nguyen @ 2018-07-02 13:19 UTC (permalink / raw) To: Masahiro Yamada Cc: Miquel Raynal, Mark Rutland, DTML, Marek Vašut, Richard Weinberger, Linux Kernel Mailing List, Boris Brezillon, Rob Herring, linux-mtd, Brian Norris, David Woodhouse On 07/01/2018 08:35 PM, Masahiro Yamada wrote: > Hi Dinh, > > 2018-06-27 23:55 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>: >> Hi Masahiro, >> >> On 06/26/2018 09:52 PM, Masahiro Yamada wrote: >>> 2018-06-27 3:09 GMT+09:00 Miquel Raynal <miquel.raynal@bootlin.com>: >>>> Hi Masahiro, >>>> >>>> On Tue, 26 Jun 2018 11:38:21 +0900, Masahiro Yamada >>>> <yamada.masahiro@socionext.com> wrote: >>>> >>>>> 2018-06-25 23:55 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>: >>>>>> On Mon, 25 Jun 2018 09:50:18 -0500 >>>>>> Dinh Nguyen <dinguyen@kernel.org> wrote: >>>>>> >>>>>>> On 06/22/2018 10:58 AM, Richard Weinberger wrote: >>>>>>>> Masahiro, >>>>>>>> >>>>>>>> Am Freitag, 22. Juni 2018, 16:37:21 CEST schrieb Masahiro Yamada: >>>>>>>>> Hi Richard, >>>>>>>>> >>>>>>>>> >>>>>>>>> 2018-06-19 21:07 GMT+09:00 Richard Weinberger <richard@nod.at>: >>>>>>>>>> The denali NAND flash controller needs at least two clocks to operate, >>>>>>>>>> nand_clk and nand_x_clk. >>>>>>>>>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by >>>>>>>>>> setup_data_interface()") nand_x_clk is used to derive timing settings. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Richard Weinberger <richard@nod.at> >>>>>>>>>> --- >>>>>>>>>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock >>>>>>>>>> is not present on this SoC. >>>>>>>>>> But my SoCFPGA knowledge is very limited. >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> //richard >>>>>>>>>> --- >>>>>>>>>> arch/arm/boot/dts/socfpga.dtsi | 3 ++- >>>>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>>>>> >>>>>>>>>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi >>>>>>>>>> index 486d4e7433ed..562f7b375bbd 100644 >>>>>>>>>> --- a/arch/arm/boot/dts/socfpga.dtsi >>>>>>>>>> +++ b/arch/arm/boot/dts/socfpga.dtsi >>>>>>>>>> @@ -754,7 +754,8 @@ >>>>>>>>>> reg-names = "nand_data", "denali_reg"; >>>>>>>>>> interrupts = <0x0 0x90 0x4>; >>>>>>>>>> dma-mask = <0xffffffff>; >>>>>>>>>> - clocks = <&nand_clk>; >>>>>>>>>> + clocks = <&nand_clk>, <&nand_x_clk>; >>>>>>>>>> + clock-names = "nand", "nand_x"; >>>>>>>>> >>>>>>>>> >>>>>>>>> IMHO, this should be >>>>>>>>> >>>>>>>>> clocks = <&nand_clk>, <&nand_x_clk>, <&nand_x_clk>; >>>>>>>>> clock-names = "nand", "nand_x", "ecc"; >>>>>>> >>>>>>> No, it should be just the nand_x and ecc. >>>>>>> >>>>>>> There's already a patch to use the nand_x_clk and not the nand_clk. >>>>> >>>>> >>>>> Different people try to fix the problem in different ways. >>>>> >>>>> I think it is due to miscommunication across sub-systems. >>>> >>>> Is the series named >>>> >>>> mtd: rawnand: denali: add new clocks and improve >>>> setup_data_interface >>>> >>>> still valid? >>> >>> Yes. >>> I believe V4 is valid. >>> >>> >>> Information for Dinh Nguyen: >>> >>> http://patchwork.ozlabs.org/patch/933507/ >>> http://patchwork.ozlabs.org/patch/933487/ >>> http://patchwork.ozlabs.org/patch/933494/ >>> http://patchwork.ozlabs.org/patch/933506/ >>> >>> >>> If he is not convinced, I am open to discussion, though. >> >> I wasn't aware of these patches. This patch is staged to go into >> v4.17-rc3 through the arm-soc: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/arch/arm/boot/dts/socfpga.dtsi?h=fixes&id=4eda9b766b042ea38d84df91581b03f6145a2ab0 >> >> I think your patch will handle a case where only 1 clock is passed in, >> so it should be okay right? >> > > I should be OK, > but please consider the proper fix for v4.19 > as Boris suggested. > Yes, I will spin up a patch. Dinh ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-07-02 13:19 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-19 12:07 [PATCH] arm: dts: socfpga: denali needs nand_x_clk too Richard Weinberger 2018-06-20 4:52 ` Marek Vasut 2018-06-20 5:27 ` Richard Weinberger 2018-06-21 4:41 ` Marek Vasut 2018-06-22 14:37 ` Masahiro Yamada 2018-06-22 15:58 ` Richard Weinberger 2018-06-25 14:50 ` Dinh Nguyen 2018-06-25 14:55 ` Boris Brezillon 2018-06-26 2:38 ` Masahiro Yamada 2018-06-26 18:09 ` Miquel Raynal 2018-06-27 2:52 ` Masahiro Yamada 2018-06-27 14:55 ` Dinh Nguyen 2018-06-27 21:34 ` Boris Brezillon 2018-07-02 1:35 ` Masahiro Yamada 2018-07-02 13:19 ` Dinh Nguyen
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).