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

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

[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/

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

[Miquel Raynal]

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/

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

[Olga Kitaina]

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/

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

[Olga Kitaina]

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
> 

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

[Miquel Raynal]

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

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

[Miquel Raynal]

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/

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

[Olga Kitaina]

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.

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

[Olga Kitaina]

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/

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

[Miquel Raynal]

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

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

[Miquel Raynal]

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/

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

[Michal Simek]

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

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

[Michal Simek]

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/

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

[Amit Kumar Kumar Mahapatra]

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/

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

[Amit Kumar Kumar Mahapatra]

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