* [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR @ 2021-11-27 18:07 Olga Kitaina 2021-11-29 8:55 ` Miquel Raynal 0 siblings, 1 reply; 14+ messages in thread From: Olga Kitaina @ 2021-11-27 18:07 UTC (permalink / raw) To: miquel.raynal; +Cc: linux-mtd, Olga Kitaina According to the Arasan NAND controller spec, the flash clock rate for SDR must be <= 100 MHz, while for NV-DDR it must be the same as the rate of the CLK line for the mode. The driver previously always set 100 MHz for NV-DDR, which would result in incorrect behavior for NV-DDR modes 0-4. The appropriate clock rate can be calculated from the NV-DDR timing parameters as 1/tCK, or for rates measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. Signed-off-by: Olga Kitaina <okitain@gmail.com> --- drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c index 53bd10738418..ed4ee9942441 100644 --- a/drivers/mtd/nand/raw/arasan-nand-controller.c +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c @@ -18,6 +18,7 @@ #include <linux/gpio/consumer.h> #include <linux/interrupt.h> #include <linux/iopoll.h> +#include <linux/math64.h> #include <linux/module.h> #include <linux/mtd/mtd.h> #include <linux/mtd/partitions.h> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, DQS_BUFF_SEL_OUT(dqs_mode); } - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; + if (nand_interface_is_sdr) + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; + else + /* ONFI timings are defined in picoseconds */ + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, + conf->timings.nvddr.tCK_min); /* * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work base-commit: f53d4c109a666bf1a4883b45d546fba079258717 -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR 2021-11-27 18:07 [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR Olga Kitaina @ 2021-11-29 8:55 ` Miquel Raynal 2021-11-29 18:06 ` Olga Kitaina 0 siblings, 1 reply; 14+ messages in thread From: Miquel Raynal @ 2021-11-29 8:55 UTC (permalink / raw) To: Olga Kitaina; +Cc: linux-mtd Hi Olga, Please add all the MTD maintainers in copy, as requested by get_maintainers.pl. okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 +0300: > According to the Arasan NAND controller spec, > the flash clock rate for SDR must be <= 100 MHz, > while for NV-DDR it must be the same as the rate > of the CLK line for the mode. I completely missed that, where did you get the information? > The driver previously always set 100 MHz for NV-DDR, which > would result in incorrect behavior for NV-DDR modes 0-4. > > The appropriate clock rate can be calculated > from the NV-DDR timing parameters as 1/tCK, or for rates > measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. > You need a couple of Fixes + Cc: stable tags here, otherwise the patch looks good to me. > Signed-off-by: Olga Kitaina <okitain@gmail.com> > --- > drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c > index 53bd10738418..ed4ee9942441 100644 > --- a/drivers/mtd/nand/raw/arasan-nand-controller.c > +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c > @@ -18,6 +18,7 @@ > #include <linux/gpio/consumer.h> > #include <linux/interrupt.h> > #include <linux/iopoll.h> > +#include <linux/math64.h> > #include <linux/module.h> > #include <linux/mtd/mtd.h> > #include <linux/mtd/partitions.h> > @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, > DQS_BUFF_SEL_OUT(dqs_mode); > } > > - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > + if (nand_interface_is_sdr) > + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > + else > + /* ONFI timings are defined in picoseconds */ > + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, > + conf->timings.nvddr.tCK_min); > > /* > * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work > > base-commit: f53d4c109a666bf1a4883b45d546fba079258717 Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR 2021-11-29 8:55 ` Miquel Raynal @ 2021-11-29 18:06 ` Olga Kitaina 0 siblings, 0 replies; 14+ messages in thread From: Olga Kitaina @ 2021-11-29 18:06 UTC (permalink / raw) To: Miquel Raynal; +Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel Hi Miquel, On 29.11.2021 11:55, Miquel Raynal wrote: > Hi Olga, > > Please add all the MTD maintainers in copy, as requested by > get_maintainers.pl. > > okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 > +0300: > >> According to the Arasan NAND controller spec, >> the flash clock rate for SDR must be <= 100 MHz, >> while for NV-DDR it must be the same as the rate >> of the CLK line for the mode. > > I completely missed that, where did you get the information? The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. The same chapter should have information about necessary steps to switch from NV-DDR to SDR, which includes setting the flash clock to 100 MHz. Just to make sure i'm not shooting myself in the foot: am I changing the right clock? The documentation points out that we have to change flash_clk, which i thought was nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. In that case, does setting nfc->controller_clk to 100 MHz by default make sense? There isn't a hard limit on what the system clock might be (beyond a specific SoC), but there are timing requirements for the flash clock, and so setting a specific system clock frequency seems unnecessary for most devices. >> The driver previously always set 100 MHz for NV-DDR, which >> would result in incorrect behavior for NV-DDR modes 0-4. >> >> The appropriate clock rate can be calculated >> from the NV-DDR timing parameters as 1/tCK, or for rates >> measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. >> > > You need a couple of Fixes + Cc: stable tags here, otherwise the > patch looks good to me. > Will include in the next iteration of the patch, thank you. >> Signed-off-by: Olga Kitaina <okitain@gmail.com> >> --- >> drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c >> index 53bd10738418..ed4ee9942441 100644 >> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c >> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c >> @@ -18,6 +18,7 @@ >> #include <linux/gpio/consumer.h> >> #include <linux/interrupt.h> >> #include <linux/iopoll.h> >> +#include <linux/math64.h> >> #include <linux/module.h> >> #include <linux/mtd/mtd.h> >> #include <linux/mtd/partitions.h> >> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, >> DQS_BUFF_SEL_OUT(dqs_mode); >> } >> >> - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; >> + if (nand_interface_is_sdr) >> + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; >> + else >> + /* ONFI timings are defined in picoseconds */ >> + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, >> + conf->timings.nvddr.tCK_min); >> >> /* >> * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work >> >> base-commit: f53d4c109a666bf1a4883b45d546fba079258717 > > > Thanks, > Miquèl > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR @ 2021-11-29 18:06 ` Olga Kitaina 0 siblings, 0 replies; 14+ messages in thread From: Olga Kitaina @ 2021-11-29 18:06 UTC (permalink / raw) To: Miquel Raynal; +Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel Hi Miquel, On 29.11.2021 11:55, Miquel Raynal wrote: > Hi Olga, > > Please add all the MTD maintainers in copy, as requested by > get_maintainers.pl. > > okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 > +0300: > >> According to the Arasan NAND controller spec, >> the flash clock rate for SDR must be <= 100 MHz, >> while for NV-DDR it must be the same as the rate >> of the CLK line for the mode. > > I completely missed that, where did you get the information? The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. The same chapter should have information about necessary steps to switch from NV-DDR to SDR, which includes setting the flash clock to 100 MHz. Just to make sure i'm not shooting myself in the foot: am I changing the right clock? The documentation points out that we have to change flash_clk, which i thought was nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. In that case, does setting nfc->controller_clk to 100 MHz by default make sense? There isn't a hard limit on what the system clock might be (beyond a specific SoC), but there are timing requirements for the flash clock, and so setting a specific system clock frequency seems unnecessary for most devices. >> The driver previously always set 100 MHz for NV-DDR, which >> would result in incorrect behavior for NV-DDR modes 0-4. >> >> The appropriate clock rate can be calculated >> from the NV-DDR timing parameters as 1/tCK, or for rates >> measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. >> > > You need a couple of Fixes + Cc: stable tags here, otherwise the > patch looks good to me. > Will include in the next iteration of the patch, thank you. >> Signed-off-by: Olga Kitaina <okitain@gmail.com> >> --- >> drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c >> index 53bd10738418..ed4ee9942441 100644 >> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c >> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c >> @@ -18,6 +18,7 @@ >> #include <linux/gpio/consumer.h> >> #include <linux/interrupt.h> >> #include <linux/iopoll.h> >> +#include <linux/math64.h> >> #include <linux/module.h> >> #include <linux/mtd/mtd.h> >> #include <linux/mtd/partitions.h> >> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, >> DQS_BUFF_SEL_OUT(dqs_mode); >> } >> >> - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; >> + if (nand_interface_is_sdr) >> + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; >> + else >> + /* ONFI timings are defined in picoseconds */ >> + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, >> + conf->timings.nvddr.tCK_min); >> >> /* >> * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work >> >> base-commit: f53d4c109a666bf1a4883b45d546fba079258717 > > > Thanks, > Miquèl > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR 2021-11-29 18:06 ` Olga Kitaina @ 2021-11-30 7:20 ` Miquel Raynal -1 siblings, 0 replies; 14+ messages in thread From: Miquel Raynal @ 2021-11-30 7:20 UTC (permalink / raw) To: Olga Kitaina; +Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel Hi Olga, okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: > Hi Miquel, > > On 29.11.2021 11:55, Miquel Raynal wrote: > > Hi Olga, > > > > Please add all the MTD maintainers in copy, as requested by > > get_maintainers.pl. > > > > okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 > > +0300: > > > >> According to the Arasan NAND controller spec, > >> the flash clock rate for SDR must be <= 100 MHz, > >> while for NV-DDR it must be the same as the rate > >> of the CLK line for the mode. > > > > I completely missed that, where did you get the information? > > The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR > and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. > > The same chapter should have information about necessary steps to switch from NV-DDR to SDR, > which includes setting the flash clock to 100 MHz. > > > Just to make sure i'm not shooting myself in the foot: am I changing the right clock? > The documentation points out that we have to change flash_clk, which i thought was > nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. I believe I made a serious mistake, re-reading the code it feels like I'm changing the system's clock (which basically changes nothing in our case) instead of changing the NAND bus clock. > In that case, does setting nfc->controller_clk to 100 MHz by default make sense? > There isn't a hard limit on what the system clock might be (beyond a specific SoC), > but there are timing requirements for the flash clock, and so setting a specific > system clock frequency seems unnecessary for most devices. > Please create a two-patch series: 1- Setting the right clock in the current code base (inverting bus_clk and controller_clk where relevant, setting one to 100MHz and letting the other as it is) 2- Changing the default NV-DDR rate based on tCK (below patch). Do you have the necessary hardware for testing? > >> The driver previously always set 100 MHz for NV-DDR, which > >> would result in incorrect behavior for NV-DDR modes 0-4. > >> > >> The appropriate clock rate can be calculated > >> from the NV-DDR timing parameters as 1/tCK, or for rates > >> measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. > >> > > > > You need a couple of Fixes + Cc: stable tags here, otherwise the > > patch looks good to me. > > > > Will include in the next iteration of the patch, thank you. > > >> Signed-off-by: Olga Kitaina <okitain@gmail.com> > >> --- > >> drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c > >> index 53bd10738418..ed4ee9942441 100644 > >> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c > >> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c > >> @@ -18,6 +18,7 @@ > >> #include <linux/gpio/consumer.h> > >> #include <linux/interrupt.h> > >> #include <linux/iopoll.h> > >> +#include <linux/math64.h> > >> #include <linux/module.h> > >> #include <linux/mtd/mtd.h> > >> #include <linux/mtd/partitions.h> > >> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, > >> DQS_BUFF_SEL_OUT(dqs_mode); > >> } > >> > >> - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > >> + if (nand_interface_is_sdr) > >> + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > >> + else > >> + /* ONFI timings are defined in picoseconds */ > >> + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, > >> + conf->timings.nvddr.tCK_min); > >> > >> /* > >> * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work > >> > >> base-commit: f53d4c109a666bf1a4883b45d546fba079258717 > > > > > > Thanks, > > Miquèl > > Thanks, Miquèl ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR @ 2021-11-30 7:20 ` Miquel Raynal 0 siblings, 0 replies; 14+ messages in thread From: Miquel Raynal @ 2021-11-30 7:20 UTC (permalink / raw) To: Olga Kitaina; +Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel Hi Olga, okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: > Hi Miquel, > > On 29.11.2021 11:55, Miquel Raynal wrote: > > Hi Olga, > > > > Please add all the MTD maintainers in copy, as requested by > > get_maintainers.pl. > > > > okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 > > +0300: > > > >> According to the Arasan NAND controller spec, > >> the flash clock rate for SDR must be <= 100 MHz, > >> while for NV-DDR it must be the same as the rate > >> of the CLK line for the mode. > > > > I completely missed that, where did you get the information? > > The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR > and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. > > The same chapter should have information about necessary steps to switch from NV-DDR to SDR, > which includes setting the flash clock to 100 MHz. > > > Just to make sure i'm not shooting myself in the foot: am I changing the right clock? > The documentation points out that we have to change flash_clk, which i thought was > nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. I believe I made a serious mistake, re-reading the code it feels like I'm changing the system's clock (which basically changes nothing in our case) instead of changing the NAND bus clock. > In that case, does setting nfc->controller_clk to 100 MHz by default make sense? > There isn't a hard limit on what the system clock might be (beyond a specific SoC), > but there are timing requirements for the flash clock, and so setting a specific > system clock frequency seems unnecessary for most devices. > Please create a two-patch series: 1- Setting the right clock in the current code base (inverting bus_clk and controller_clk where relevant, setting one to 100MHz and letting the other as it is) 2- Changing the default NV-DDR rate based on tCK (below patch). Do you have the necessary hardware for testing? > >> The driver previously always set 100 MHz for NV-DDR, which > >> would result in incorrect behavior for NV-DDR modes 0-4. > >> > >> The appropriate clock rate can be calculated > >> from the NV-DDR timing parameters as 1/tCK, or for rates > >> measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. > >> > > > > You need a couple of Fixes + Cc: stable tags here, otherwise the > > patch looks good to me. > > > > Will include in the next iteration of the patch, thank you. > > >> Signed-off-by: Olga Kitaina <okitain@gmail.com> > >> --- > >> drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c > >> index 53bd10738418..ed4ee9942441 100644 > >> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c > >> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c > >> @@ -18,6 +18,7 @@ > >> #include <linux/gpio/consumer.h> > >> #include <linux/interrupt.h> > >> #include <linux/iopoll.h> > >> +#include <linux/math64.h> > >> #include <linux/module.h> > >> #include <linux/mtd/mtd.h> > >> #include <linux/mtd/partitions.h> > >> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, > >> DQS_BUFF_SEL_OUT(dqs_mode); > >> } > >> > >> - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > >> + if (nand_interface_is_sdr) > >> + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > >> + else > >> + /* ONFI timings are defined in picoseconds */ > >> + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, > >> + conf->timings.nvddr.tCK_min); > >> > >> /* > >> * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work > >> > >> base-commit: f53d4c109a666bf1a4883b45d546fba079258717 > > > > > > Thanks, > > Miquèl > > Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR 2021-11-30 7:20 ` Miquel Raynal @ 2021-11-30 20:08 ` Olga Kitaina -1 siblings, 0 replies; 14+ messages in thread From: Olga Kitaina @ 2021-11-30 20:08 UTC (permalink / raw) To: Miquel Raynal; +Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel Hi Miquel, On 30.11.2021 10:20, Miquel Raynal wrote: > Hi Olga, > > okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: > >> Hi Miquel, >> >> On 29.11.2021 11:55, Miquel Raynal wrote: >>> Hi Olga, >>> >>> Please add all the MTD maintainers in copy, as requested by >>> get_maintainers.pl. >>> >>> okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 >>> +0300: >>> >>>> According to the Arasan NAND controller spec, >>>> the flash clock rate for SDR must be <= 100 MHz, >>>> while for NV-DDR it must be the same as the rate >>>> of the CLK line for the mode. >>> >>> I completely missed that, where did you get the information? >> >> The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR >> and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. >> >> The same chapter should have information about necessary steps to switch from NV-DDR to SDR, >> which includes setting the flash clock to 100 MHz. >> >> >> Just to make sure i'm not shooting myself in the foot: am I changing the right clock? >> The documentation points out that we have to change flash_clk, which i thought was >> nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. > > I believe I made a serious mistake, re-reading the code it feels like > I'm changing the system's clock (which basically changes nothing in our > case) instead of changing the NAND bus clock. > >> In that case, does setting nfc->controller_clk to 100 MHz by default make sense? >> There isn't a hard limit on what the system clock might be (beyond a specific SoC), >> but there are timing requirements for the flash clock, and so setting a specific >> system clock frequency seems unnecessary for most devices. >> > > Please create a two-patch series: > 1- Setting the right clock in the current code base (inverting bus_clk > and controller_clk where relevant, setting one to 100MHz and letting > the other as it is) > 2- Changing the default NV-DDR rate based on tCK (below patch). > > Do you have the necessary hardware for testing? I'm sorry to say - I do not. The SoC this problem was initially noticed on can't run latest Linux, and even if it did I have no way of acquiring an NV-DDR-capable flash. Since Bootlin merged in NV-DDR support into the kernel, is it possible for you to test the next iteration of this patch series on NV-DDR hardware as well? Say, by purposefully preventing NV-DDR mode 5 from being chosen in anfc_setup_interface()? >>>> The driver previously always set 100 MHz for NV-DDR, which >>>> would result in incorrect behavior for NV-DDR modes 0-4. >>>> >>>> The appropriate clock rate can be calculated >>>> from the NV-DDR timing parameters as 1/tCK, or for rates >>>> measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. >>>> >>> >>> You need a couple of Fixes + Cc: stable tags here, otherwise the >>> patch looks good to me. >>> >> >> Will include in the next iteration of the patch, thank you. >> >>>> Signed-off-by: Olga Kitaina <okitain@gmail.com> >>>> --- >>>> drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- >>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c >>>> index 53bd10738418..ed4ee9942441 100644 >>>> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c >>>> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c >>>> @@ -18,6 +18,7 @@ >>>> #include <linux/gpio/consumer.h> >>>> #include <linux/interrupt.h> >>>> #include <linux/iopoll.h> >>>> +#include <linux/math64.h> >>>> #include <linux/module.h> >>>> #include <linux/mtd/mtd.h> >>>> #include <linux/mtd/partitions.h> >>>> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, >>>> DQS_BUFF_SEL_OUT(dqs_mode); >>>> } >>>> >>>> - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; >>>> + if (nand_interface_is_sdr) >>>> + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; >>>> + else >>>> + /* ONFI timings are defined in picoseconds */ >>>> + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, >>>> + conf->timings.nvddr.tCK_min); >>>> >>>> /* >>>> * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work >>>> >>>> base-commit: f53d4c109a666bf1a4883b45d546fba079258717 >>> >>> >>> Thanks, >>> Miquèl >>> > > > Thanks, > Miquèl > Thanks, Olga. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR @ 2021-11-30 20:08 ` Olga Kitaina 0 siblings, 0 replies; 14+ messages in thread From: Olga Kitaina @ 2021-11-30 20:08 UTC (permalink / raw) To: Miquel Raynal; +Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel Hi Miquel, On 30.11.2021 10:20, Miquel Raynal wrote: > Hi Olga, > > okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: > >> Hi Miquel, >> >> On 29.11.2021 11:55, Miquel Raynal wrote: >>> Hi Olga, >>> >>> Please add all the MTD maintainers in copy, as requested by >>> get_maintainers.pl. >>> >>> okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 >>> +0300: >>> >>>> According to the Arasan NAND controller spec, >>>> the flash clock rate for SDR must be <= 100 MHz, >>>> while for NV-DDR it must be the same as the rate >>>> of the CLK line for the mode. >>> >>> I completely missed that, where did you get the information? >> >> The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR >> and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. >> >> The same chapter should have information about necessary steps to switch from NV-DDR to SDR, >> which includes setting the flash clock to 100 MHz. >> >> >> Just to make sure i'm not shooting myself in the foot: am I changing the right clock? >> The documentation points out that we have to change flash_clk, which i thought was >> nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. > > I believe I made a serious mistake, re-reading the code it feels like > I'm changing the system's clock (which basically changes nothing in our > case) instead of changing the NAND bus clock. > >> In that case, does setting nfc->controller_clk to 100 MHz by default make sense? >> There isn't a hard limit on what the system clock might be (beyond a specific SoC), >> but there are timing requirements for the flash clock, and so setting a specific >> system clock frequency seems unnecessary for most devices. >> > > Please create a two-patch series: > 1- Setting the right clock in the current code base (inverting bus_clk > and controller_clk where relevant, setting one to 100MHz and letting > the other as it is) > 2- Changing the default NV-DDR rate based on tCK (below patch). > > Do you have the necessary hardware for testing? I'm sorry to say - I do not. The SoC this problem was initially noticed on can't run latest Linux, and even if it did I have no way of acquiring an NV-DDR-capable flash. Since Bootlin merged in NV-DDR support into the kernel, is it possible for you to test the next iteration of this patch series on NV-DDR hardware as well? Say, by purposefully preventing NV-DDR mode 5 from being chosen in anfc_setup_interface()? >>>> The driver previously always set 100 MHz for NV-DDR, which >>>> would result in incorrect behavior for NV-DDR modes 0-4. >>>> >>>> The appropriate clock rate can be calculated >>>> from the NV-DDR timing parameters as 1/tCK, or for rates >>>> measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. >>>> >>> >>> You need a couple of Fixes + Cc: stable tags here, otherwise the >>> patch looks good to me. >>> >> >> Will include in the next iteration of the patch, thank you. >> >>>> Signed-off-by: Olga Kitaina <okitain@gmail.com> >>>> --- >>>> drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- >>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c >>>> index 53bd10738418..ed4ee9942441 100644 >>>> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c >>>> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c >>>> @@ -18,6 +18,7 @@ >>>> #include <linux/gpio/consumer.h> >>>> #include <linux/interrupt.h> >>>> #include <linux/iopoll.h> >>>> +#include <linux/math64.h> >>>> #include <linux/module.h> >>>> #include <linux/mtd/mtd.h> >>>> #include <linux/mtd/partitions.h> >>>> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, >>>> DQS_BUFF_SEL_OUT(dqs_mode); >>>> } >>>> >>>> - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; >>>> + if (nand_interface_is_sdr) >>>> + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; >>>> + else >>>> + /* ONFI timings are defined in picoseconds */ >>>> + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, >>>> + conf->timings.nvddr.tCK_min); >>>> >>>> /* >>>> * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work >>>> >>>> base-commit: f53d4c109a666bf1a4883b45d546fba079258717 >>> >>> >>> Thanks, >>> Miquèl >>> > > > Thanks, > Miquèl > Thanks, Olga. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR 2021-11-30 20:08 ` Olga Kitaina @ 2021-12-01 8:09 ` Miquel Raynal -1 siblings, 0 replies; 14+ messages in thread From: Miquel Raynal @ 2021-12-01 8:09 UTC (permalink / raw) To: Olga Kitaina Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel, Michal Simek Hi Olga, + Michal (please add him in Cc of your next iteration) okitain@gmail.com wrote on Tue, 30 Nov 2021 23:08:20 +0300: > Hi Miquel, > > On 30.11.2021 10:20, Miquel Raynal wrote: > > Hi Olga, > > > > okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: > > > >> Hi Miquel, > >> > >> On 29.11.2021 11:55, Miquel Raynal wrote: > >>> Hi Olga, > >>> > >>> Please add all the MTD maintainers in copy, as requested by > >>> get_maintainers.pl. > >>> > >>> okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 > >>> +0300: > >>> > >>>> According to the Arasan NAND controller spec, > >>>> the flash clock rate for SDR must be <= 100 MHz, > >>>> while for NV-DDR it must be the same as the rate > >>>> of the CLK line for the mode. > >>> > >>> I completely missed that, where did you get the information? > >> > >> The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR > >> and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. > >> > >> The same chapter should have information about necessary steps to switch from NV-DDR to SDR, > >> which includes setting the flash clock to 100 MHz. > >> > >> > >> Just to make sure i'm not shooting myself in the foot: am I changing the right clock? > >> The documentation points out that we have to change flash_clk, which i thought was > >> nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. > > > > I believe I made a serious mistake, re-reading the code it feels like > > I'm changing the system's clock (which basically changes nothing in our > > case) instead of changing the NAND bus clock. > > > >> In that case, does setting nfc->controller_clk to 100 MHz by default make sense? > >> There isn't a hard limit on what the system clock might be (beyond a specific SoC), > >> but there are timing requirements for the flash clock, and so setting a specific > >> system clock frequency seems unnecessary for most devices. > >> > > > > Please create a two-patch series: > > 1- Setting the right clock in the current code base (inverting bus_clk > > and controller_clk where relevant, setting one to 100MHz and letting > > the other as it is) > > 2- Changing the default NV-DDR rate based on tCK (below patch). > > > > Do you have the necessary hardware for testing? > > I'm sorry to say - I do not. The SoC this problem was initially noticed on can't run latest Linux, > and even if it did I have no way of acquiring an NV-DDR-capable flash. > > Since Bootlin merged in NV-DDR support into the kernel, is it possible for you to test > the next iteration of this patch series on NV-DDR hardware as well? > Say, by purposefully preventing NV-DDR mode 5 from being chosen in anfc_setup_interface()? I don't have the hardware anymore. Please send a v2 with the necessary changes, then we will ask Naga (or somebody else from the same team) with access to the board to test it. > >>>> The driver previously always set 100 MHz for NV-DDR, which > >>>> would result in incorrect behavior for NV-DDR modes 0-4. > >>>> > >>>> The appropriate clock rate can be calculated > >>>> from the NV-DDR timing parameters as 1/tCK, or for rates > >>>> measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. > >>>> > >>> > >>> You need a couple of Fixes + Cc: stable tags here, otherwise the > >>> patch looks good to me. > >>> > >> > >> Will include in the next iteration of the patch, thank you. > >> > >>>> Signed-off-by: Olga Kitaina <okitain@gmail.com> > >>>> --- > >>>> drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- > >>>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c > >>>> index 53bd10738418..ed4ee9942441 100644 > >>>> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c > >>>> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c > >>>> @@ -18,6 +18,7 @@ > >>>> #include <linux/gpio/consumer.h> > >>>> #include <linux/interrupt.h> > >>>> #include <linux/iopoll.h> > >>>> +#include <linux/math64.h> > >>>> #include <linux/module.h> > >>>> #include <linux/mtd/mtd.h> > >>>> #include <linux/mtd/partitions.h> > >>>> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, > >>>> DQS_BUFF_SEL_OUT(dqs_mode); > >>>> } > >>>> > >>>> - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > >>>> + if (nand_interface_is_sdr) > >>>> + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > >>>> + else > >>>> + /* ONFI timings are defined in picoseconds */ > >>>> + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, > >>>> + conf->timings.nvddr.tCK_min); > >>>> > >>>> /* > >>>> * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work > >>>> > >>>> base-commit: f53d4c109a666bf1a4883b45d546fba079258717 > >>> > >>> > >>> Thanks, > >>> Miquèl > >>> > > > > > > Thanks, > > Miquèl > > > > Thanks, > Olga. Thanks, Miquèl ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR @ 2021-12-01 8:09 ` Miquel Raynal 0 siblings, 0 replies; 14+ messages in thread From: Miquel Raynal @ 2021-12-01 8:09 UTC (permalink / raw) To: Olga Kitaina Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel, Michal Simek Hi Olga, + Michal (please add him in Cc of your next iteration) okitain@gmail.com wrote on Tue, 30 Nov 2021 23:08:20 +0300: > Hi Miquel, > > On 30.11.2021 10:20, Miquel Raynal wrote: > > Hi Olga, > > > > okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: > > > >> Hi Miquel, > >> > >> On 29.11.2021 11:55, Miquel Raynal wrote: > >>> Hi Olga, > >>> > >>> Please add all the MTD maintainers in copy, as requested by > >>> get_maintainers.pl. > >>> > >>> okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 > >>> +0300: > >>> > >>>> According to the Arasan NAND controller spec, > >>>> the flash clock rate for SDR must be <= 100 MHz, > >>>> while for NV-DDR it must be the same as the rate > >>>> of the CLK line for the mode. > >>> > >>> I completely missed that, where did you get the information? > >> > >> The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR > >> and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. > >> > >> The same chapter should have information about necessary steps to switch from NV-DDR to SDR, > >> which includes setting the flash clock to 100 MHz. > >> > >> > >> Just to make sure i'm not shooting myself in the foot: am I changing the right clock? > >> The documentation points out that we have to change flash_clk, which i thought was > >> nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. > > > > I believe I made a serious mistake, re-reading the code it feels like > > I'm changing the system's clock (which basically changes nothing in our > > case) instead of changing the NAND bus clock. > > > >> In that case, does setting nfc->controller_clk to 100 MHz by default make sense? > >> There isn't a hard limit on what the system clock might be (beyond a specific SoC), > >> but there are timing requirements for the flash clock, and so setting a specific > >> system clock frequency seems unnecessary for most devices. > >> > > > > Please create a two-patch series: > > 1- Setting the right clock in the current code base (inverting bus_clk > > and controller_clk where relevant, setting one to 100MHz and letting > > the other as it is) > > 2- Changing the default NV-DDR rate based on tCK (below patch). > > > > Do you have the necessary hardware for testing? > > I'm sorry to say - I do not. The SoC this problem was initially noticed on can't run latest Linux, > and even if it did I have no way of acquiring an NV-DDR-capable flash. > > Since Bootlin merged in NV-DDR support into the kernel, is it possible for you to test > the next iteration of this patch series on NV-DDR hardware as well? > Say, by purposefully preventing NV-DDR mode 5 from being chosen in anfc_setup_interface()? I don't have the hardware anymore. Please send a v2 with the necessary changes, then we will ask Naga (or somebody else from the same team) with access to the board to test it. > >>>> The driver previously always set 100 MHz for NV-DDR, which > >>>> would result in incorrect behavior for NV-DDR modes 0-4. > >>>> > >>>> The appropriate clock rate can be calculated > >>>> from the NV-DDR timing parameters as 1/tCK, or for rates > >>>> measured in picoseconds, 10^12 / nand_nvddr_timings->tCK_min. > >>>> > >>> > >>> You need a couple of Fixes + Cc: stable tags here, otherwise the > >>> patch looks good to me. > >>> > >> > >> Will include in the next iteration of the patch, thank you. > >> > >>>> Signed-off-by: Olga Kitaina <okitain@gmail.com> > >>>> --- > >>>> drivers/mtd/nand/raw/arasan-nand-controller.c | 8 +++++++- > >>>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c > >>>> index 53bd10738418..ed4ee9942441 100644 > >>>> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c > >>>> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c > >>>> @@ -18,6 +18,7 @@ > >>>> #include <linux/gpio/consumer.h> > >>>> #include <linux/interrupt.h> > >>>> #include <linux/iopoll.h> > >>>> +#include <linux/math64.h> > >>>> #include <linux/module.h> > >>>> #include <linux/mtd/mtd.h> > >>>> #include <linux/mtd/partitions.h> > >>>> @@ -1043,7 +1044,12 @@ static int anfc_setup_interface(struct nand_chip *chip, int target, > >>>> DQS_BUFF_SEL_OUT(dqs_mode); > >>>> } > >>>> > >>>> - anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > >>>> + if (nand_interface_is_sdr) > >>>> + anand->clk = ANFC_XLNX_SDR_DFLT_CORE_CLK; > >>>> + else > >>>> + /* ONFI timings are defined in picoseconds */ > >>>> + anand->clk = div_u64((u64)NSEC_PER_SEC * 1000, > >>>> + conf->timings.nvddr.tCK_min); > >>>> > >>>> /* > >>>> * Due to a hardware bug in the ZynqMP SoC, SDR timing modes 0-1 work > >>>> > >>>> base-commit: f53d4c109a666bf1a4883b45d546fba079258717 > >>> > >>> > >>> Thanks, > >>> Miquèl > >>> > > > > > > Thanks, > > Miquèl > > > > Thanks, > Olga. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR 2021-12-01 8:09 ` Miquel Raynal @ 2021-12-01 8:16 ` Michal Simek -1 siblings, 0 replies; 14+ messages in thread From: Michal Simek @ 2021-12-01 8:16 UTC (permalink / raw) To: Miquel Raynal, Olga Kitaina, Amit Kumar Mahapatra Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel Hi, On 12/1/21 09:09, Miquel Raynal wrote: > Hi Olga, > > + Michal (please add him in Cc of your next iteration) > > okitain@gmail.com wrote on Tue, 30 Nov 2021 23:08:20 +0300: > >> Hi Miquel, >> >> On 30.11.2021 10:20, Miquel Raynal wrote: >>> Hi Olga, >>> >>> okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: >>> >>>> Hi Miquel, >>>> >>>> On 29.11.2021 11:55, Miquel Raynal wrote: >>>>> Hi Olga, >>>>> >>>>> Please add all the MTD maintainers in copy, as requested by >>>>> get_maintainers.pl. >>>>> >>>>> okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 >>>>> +0300: >>>>> >>>>>> According to the Arasan NAND controller spec, >>>>>> the flash clock rate for SDR must be <= 100 MHz, >>>>>> while for NV-DDR it must be the same as the rate >>>>>> of the CLK line for the mode. >>>>> >>>>> I completely missed that, where did you get the information? >>>> >>>> The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR >>>> and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. >>>> >>>> The same chapter should have information about necessary steps to switch from NV-DDR to SDR, >>>> which includes setting the flash clock to 100 MHz. >>>> >>>> >>>> Just to make sure i'm not shooting myself in the foot: am I changing the right clock? >>>> The documentation points out that we have to change flash_clk, which i thought was >>>> nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. >>> >>> I believe I made a serious mistake, re-reading the code it feels like >>> I'm changing the system's clock (which basically changes nothing in our >>> case) instead of changing the NAND bus clock. >>> >>>> In that case, does setting nfc->controller_clk to 100 MHz by default make sense? >>>> There isn't a hard limit on what the system clock might be (beyond a specific SoC), >>>> but there are timing requirements for the flash clock, and so setting a specific >>>> system clock frequency seems unnecessary for most devices. >>>> >>> >>> Please create a two-patch series: >>> 1- Setting the right clock in the current code base (inverting bus_clk >>> and controller_clk where relevant, setting one to 100MHz and letting >>> the other as it is) >>> 2- Changing the default NV-DDR rate based on tCK (below patch). >>> >>> Do you have the necessary hardware for testing? >> >> I'm sorry to say - I do not. The SoC this problem was initially noticed on can't run latest Linux, >> and even if it did I have no way of acquiring an NV-DDR-capable flash. >> >> Since Bootlin merged in NV-DDR support into the kernel, is it possible for you to test >> the next iteration of this patch series on NV-DDR hardware as well? >> Say, by purposefully preventing NV-DDR mode 5 from being chosen in anfc_setup_interface()? > > I don't have the hardware anymore. > > Please send a v2 with the necessary changes, then we will ask Naga (or > somebody else from the same team) with access to the board to test it. Keep Amit in loop. He has access to HW and able to test. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR @ 2021-12-01 8:16 ` Michal Simek 0 siblings, 0 replies; 14+ messages in thread From: Michal Simek @ 2021-12-01 8:16 UTC (permalink / raw) To: Miquel Raynal, Olga Kitaina, Amit Kumar Mahapatra Cc: linux-mtd, nagasure, richard, vigneshr, linux-kernel Hi, On 12/1/21 09:09, Miquel Raynal wrote: > Hi Olga, > > + Michal (please add him in Cc of your next iteration) > > okitain@gmail.com wrote on Tue, 30 Nov 2021 23:08:20 +0300: > >> Hi Miquel, >> >> On 30.11.2021 10:20, Miquel Raynal wrote: >>> Hi Olga, >>> >>> okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: >>> >>>> Hi Miquel, >>>> >>>> On 29.11.2021 11:55, Miquel Raynal wrote: >>>>> Hi Olga, >>>>> >>>>> Please add all the MTD maintainers in copy, as requested by >>>>> get_maintainers.pl. >>>>> >>>>> okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 >>>>> +0300: >>>>> >>>>>> According to the Arasan NAND controller spec, >>>>>> the flash clock rate for SDR must be <= 100 MHz, >>>>>> while for NV-DDR it must be the same as the rate >>>>>> of the CLK line for the mode. >>>>> >>>>> I completely missed that, where did you get the information? >>>> >>>> The "Data Interface Transitions" chapter of the spec contains timings for flash clock setup in NV-DDR >>>> and NV-DDR2 modes. The "time period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. >>>> >>>> The same chapter should have information about necessary steps to switch from NV-DDR to SDR, >>>> which includes setting the flash clock to 100 MHz. >>>> >>>> >>>> Just to make sure i'm not shooting myself in the foot: am I changing the right clock? >>>> The documentation points out that we have to change flash_clk, which i thought was >>>> nfc->controller_clk and set up by anand->clk, but it seems like it might actually be nfc->bus_clk. >>> >>> I believe I made a serious mistake, re-reading the code it feels like >>> I'm changing the system's clock (which basically changes nothing in our >>> case) instead of changing the NAND bus clock. >>> >>>> In that case, does setting nfc->controller_clk to 100 MHz by default make sense? >>>> There isn't a hard limit on what the system clock might be (beyond a specific SoC), >>>> but there are timing requirements for the flash clock, and so setting a specific >>>> system clock frequency seems unnecessary for most devices. >>>> >>> >>> Please create a two-patch series: >>> 1- Setting the right clock in the current code base (inverting bus_clk >>> and controller_clk where relevant, setting one to 100MHz and letting >>> the other as it is) >>> 2- Changing the default NV-DDR rate based on tCK (below patch). >>> >>> Do you have the necessary hardware for testing? >> >> I'm sorry to say - I do not. The SoC this problem was initially noticed on can't run latest Linux, >> and even if it did I have no way of acquiring an NV-DDR-capable flash. >> >> Since Bootlin merged in NV-DDR support into the kernel, is it possible for you to test >> the next iteration of this patch series on NV-DDR hardware as well? >> Say, by purposefully preventing NV-DDR mode 5 from being chosen in anfc_setup_interface()? > > I don't have the hardware anymore. > > Please send a v2 with the necessary changes, then we will ask Naga (or > somebody else from the same team) with access to the board to test it. Keep Amit in loop. He has access to HW and able to test. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR 2021-12-01 8:16 ` Michal Simek @ 2022-01-25 11:16 ` Amit Kumar Kumar Mahapatra -1 siblings, 0 replies; 14+ messages in thread From: Amit Kumar Kumar Mahapatra @ 2022-01-25 11:16 UTC (permalink / raw) To: Michal Simek, Miquel Raynal, Olga Kitaina Cc: linux-mtd, Naga Sureshkumar Relli, richard, vigneshr, linux-kernel Hello Olga, > -----Original Message----- > From: Michal Simek <monstr@monstr.eu> > Sent: Wednesday, December 1, 2021 1:46 PM > To: Miquel Raynal <miquel.raynal@bootlin.com>; Olga Kitaina > <okitain@gmail.com>; Amit Kumar Kumar Mahapatra > <akumarma@xilinx.com> > Cc: linux-mtd@lists.infradead.org; Naga Sureshkumar Relli > <nagasure@xlnx.xilinx.com>; richard@nod.at; vigneshr@ti.com; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR > > Hi, > > > On 12/1/21 09:09, Miquel Raynal wrote: > > Hi Olga, > > > > + Michal (please add him in Cc of your next iteration) > > > > okitain@gmail.com wrote on Tue, 30 Nov 2021 23:08:20 +0300: > > > >> Hi Miquel, > >> > >> On 30.11.2021 10:20, Miquel Raynal wrote: > >>> Hi Olga, > >>> > >>> okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: > >>> > >>>> Hi Miquel, > >>>> > >>>> On 29.11.2021 11:55, Miquel Raynal wrote: > >>>>> Hi Olga, > >>>>> > >>>>> Please add all the MTD maintainers in copy, as requested by > >>>>> get_maintainers.pl. > >>>>> > >>>>> okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 > >>>>> +0300: > >>>>> > >>>>>> According to the Arasan NAND controller spec, the flash clock > >>>>>> rate for SDR must be <= 100 MHz, while for NV-DDR it must be the > >>>>>> same as the rate of the CLK line for the mode. > >>>>> > >>>>> I completely missed that, where did you get the information? > >>>> > >>>> The "Data Interface Transitions" chapter of the spec contains > >>>> timings for flash clock setup in NV-DDR and NV-DDR2 modes. The "time > period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. > >>>> > >>>> The same chapter should have information about necessary steps to > >>>> switch from NV-DDR to SDR, which includes setting the flash clock to > 100 MHz. > >>>> > >>>> > >>>> Just to make sure i'm not shooting myself in the foot: am I changing the > right clock? > >>>> The documentation points out that we have to change flash_clk, > >>>> which i thought was > >>>> nfc->controller_clk and set up by anand->clk, but it seems like it might > actually be nfc->bus_clk. > >>> > >>> I believe I made a serious mistake, re-reading the code it feels > >>> like I'm changing the system's clock (which basically changes > >>> nothing in our > >>> case) instead of changing the NAND bus clock. > >>> > >>>> In that case, does setting nfc->controller_clk to 100 MHz by default > make sense? > >>>> There isn't a hard limit on what the system clock might be (beyond > >>>> a specific SoC), but there are timing requirements for the flash > >>>> clock, and so setting a specific system clock frequency seems > unnecessary for most devices. > >>>> > >>> > >>> Please create a two-patch series: > >>> 1- Setting the right clock in the current code base (inverting > >>> bus_clk and controller_clk where relevant, setting one to 100MHz and > >>> letting the other as it is) > >>> 2- Changing the default NV-DDR rate based on tCK (below patch). > >>> > >>> Do you have the necessary hardware for testing? > >> > >> I'm sorry to say - I do not. The SoC this problem was initially > >> noticed on can't run latest Linux, and even if it did I have no way of > acquiring an NV-DDR-capable flash. > >> > >> Since Bootlin merged in NV-DDR support into the kernel, is it > >> possible for you to test the next iteration of this patch series on NV-DDR > hardware as well? > >> Say, by purposefully preventing NV-DDR mode 5 from being chosen in > anfc_setup_interface()? > > > > I don't have the hardware anymore. > > > > Please send a v2 with the necessary changes, then we will ask Naga (or > > somebody else from the same team) with access to the board to test it. > > Keep Amit in loop. He has access to HW and able to test. Kindly send the next version, I have the required setup I will test it and let you know. Regards, Amit > > Thanks, > Michal > > -- > Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 > w: www.monstr.eu p: +42-0-721842854 > Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - > Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx > Microblaze/Zynq/ZynqMP/Versal SoCs ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR @ 2022-01-25 11:16 ` Amit Kumar Kumar Mahapatra 0 siblings, 0 replies; 14+ messages in thread From: Amit Kumar Kumar Mahapatra @ 2022-01-25 11:16 UTC (permalink / raw) To: Michal Simek, Miquel Raynal, Olga Kitaina Cc: linux-mtd, Naga Sureshkumar Relli, richard, vigneshr, linux-kernel Hello Olga, > -----Original Message----- > From: Michal Simek <monstr@monstr.eu> > Sent: Wednesday, December 1, 2021 1:46 PM > To: Miquel Raynal <miquel.raynal@bootlin.com>; Olga Kitaina > <okitain@gmail.com>; Amit Kumar Kumar Mahapatra > <akumarma@xilinx.com> > Cc: linux-mtd@lists.infradead.org; Naga Sureshkumar Relli > <nagasure@xlnx.xilinx.com>; richard@nod.at; vigneshr@ti.com; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR > > Hi, > > > On 12/1/21 09:09, Miquel Raynal wrote: > > Hi Olga, > > > > + Michal (please add him in Cc of your next iteration) > > > > okitain@gmail.com wrote on Tue, 30 Nov 2021 23:08:20 +0300: > > > >> Hi Miquel, > >> > >> On 30.11.2021 10:20, Miquel Raynal wrote: > >>> Hi Olga, > >>> > >>> okitain@gmail.com wrote on Mon, 29 Nov 2021 21:06:05 +0300: > >>> > >>>> Hi Miquel, > >>>> > >>>> On 29.11.2021 11:55, Miquel Raynal wrote: > >>>>> Hi Olga, > >>>>> > >>>>> Please add all the MTD maintainers in copy, as requested by > >>>>> get_maintainers.pl. > >>>>> > >>>>> okitain@gmail.com wrote on Sat, 27 Nov 2021 21:07:58 > >>>>> +0300: > >>>>> > >>>>>> According to the Arasan NAND controller spec, the flash clock > >>>>>> rate for SDR must be <= 100 MHz, while for NV-DDR it must be the > >>>>>> same as the rate of the CLK line for the mode. > >>>>> > >>>>> I completely missed that, where did you get the information? > >>>> > >>>> The "Data Interface Transitions" chapter of the spec contains > >>>> timings for flash clock setup in NV-DDR and NV-DDR2 modes. The "time > period" of those clocks is equal to tCK in NV-DDR and tRC in NV-DDR2. > >>>> > >>>> The same chapter should have information about necessary steps to > >>>> switch from NV-DDR to SDR, which includes setting the flash clock to > 100 MHz. > >>>> > >>>> > >>>> Just to make sure i'm not shooting myself in the foot: am I changing the > right clock? > >>>> The documentation points out that we have to change flash_clk, > >>>> which i thought was > >>>> nfc->controller_clk and set up by anand->clk, but it seems like it might > actually be nfc->bus_clk. > >>> > >>> I believe I made a serious mistake, re-reading the code it feels > >>> like I'm changing the system's clock (which basically changes > >>> nothing in our > >>> case) instead of changing the NAND bus clock. > >>> > >>>> In that case, does setting nfc->controller_clk to 100 MHz by default > make sense? > >>>> There isn't a hard limit on what the system clock might be (beyond > >>>> a specific SoC), but there are timing requirements for the flash > >>>> clock, and so setting a specific system clock frequency seems > unnecessary for most devices. > >>>> > >>> > >>> Please create a two-patch series: > >>> 1- Setting the right clock in the current code base (inverting > >>> bus_clk and controller_clk where relevant, setting one to 100MHz and > >>> letting the other as it is) > >>> 2- Changing the default NV-DDR rate based on tCK (below patch). > >>> > >>> Do you have the necessary hardware for testing? > >> > >> I'm sorry to say - I do not. The SoC this problem was initially > >> noticed on can't run latest Linux, and even if it did I have no way of > acquiring an NV-DDR-capable flash. > >> > >> Since Bootlin merged in NV-DDR support into the kernel, is it > >> possible for you to test the next iteration of this patch series on NV-DDR > hardware as well? > >> Say, by purposefully preventing NV-DDR mode 5 from being chosen in > anfc_setup_interface()? > > > > I don't have the hardware anymore. > > > > Please send a v2 with the necessary changes, then we will ask Naga (or > > somebody else from the same team) with access to the board to test it. > > Keep Amit in loop. He has access to HW and able to test. Kindly send the next version, I have the required setup I will test it and let you know. Regards, Amit > > Thanks, > Michal > > -- > Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 > w: www.monstr.eu p: +42-0-721842854 > Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - > Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx > Microblaze/Zynq/ZynqMP/Versal SoCs ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-01-25 11:19 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-27 18:07 [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR Olga Kitaina 2021-11-29 8:55 ` Miquel Raynal 2021-11-29 18:06 ` Olga Kitaina 2021-11-29 18:06 ` Olga Kitaina 2021-11-30 7:20 ` Miquel Raynal 2021-11-30 7:20 ` Miquel Raynal 2021-11-30 20:08 ` Olga Kitaina 2021-11-30 20:08 ` Olga Kitaina 2021-12-01 8:09 ` Miquel Raynal 2021-12-01 8:09 ` Miquel Raynal 2021-12-01 8:16 ` Michal Simek 2021-12-01 8:16 ` Michal Simek 2022-01-25 11:16 ` Amit Kumar Kumar Mahapatra 2022-01-25 11:16 ` Amit Kumar Kumar Mahapatra
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.