2021-11-29 22:49:26

by Olga Kitaina

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR

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.
>
> [email protected] 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 <[email protected]>
>> ---
>> 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
>


2021-11-30 07:20:23

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR

Hi Olga,

[email protected] 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.
> >
> > [email protected] 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 <[email protected]>
> >> ---
> >> 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

2021-11-30 20:08:37

by Olga Kitaina

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR

Hi Miquel,

On 30.11.2021 10:20, Miquel Raynal wrote:
> Hi Olga,
>
> [email protected] 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.
>>>
>>> [email protected] 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 <[email protected]>
>>>> ---
>>>> 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.

2021-12-01 08:09:49

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR

Hi Olga,

+ Michal (please add him in Cc of your next iteration)

[email protected] wrote on Tue, 30 Nov 2021 23:08:20 +0300:

> Hi Miquel,
>
> On 30.11.2021 10:20, Miquel Raynal wrote:
> > Hi Olga,
> >
> > [email protected] 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.
> >>>
> >>> [email protected] 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 <[email protected]>
> >>>> ---
> >>>> 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

2021-12-01 08:16:22

by Michal Simek

[permalink] [raw]
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)
>
> [email protected] wrote on Tue, 30 Nov 2021 23:08:20 +0300:
>
>> Hi Miquel,
>>
>> On 30.11.2021 10:20, Miquel Raynal wrote:
>>> Hi Olga,
>>>
>>> [email protected] 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.
>>>>>
>>>>> [email protected] 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: http://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


Subject: RE: [PATCH] mtd: rawnand: arasan: Fix clock rate in NV-DDR

Hello Olga,

> -----Original Message-----
> From: Michal Simek <[email protected]>
> Sent: Wednesday, December 1, 2021 1:46 PM
> To: Miquel Raynal <[email protected]>; Olga Kitaina
> <[email protected]>; Amit Kumar Kumar Mahapatra
> <[email protected]>
> Cc: [email protected]; Naga Sureshkumar Relli
> <[email protected]>; [email protected]; [email protected]; linux-
> [email protected]
> 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)
> >
> > [email protected] wrote on Tue, 30 Nov 2021 23:08:20 +0300:
> >
> >> Hi Miquel,
> >>
> >> On 30.11.2021 10:20, Miquel Raynal wrote:
> >>> Hi Olga,
> >>>
> >>> [email protected] 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.
> >>>>>
> >>>>> [email protected] 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: http://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