Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1695666 > unrolled thread

[PATCH 0/3] memory: atmel-ebi: Fix setting EBI timings through dts

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] memory: atmel-ebi: Fix setting EBI timings through dts Alexander Dahl <ada@thorsis.com> - 2017-07-25 14:10 +0200
    [PATCH 3/3] memory: atmel-ebi: Fix smc cycle xlate converter Alexander Dahl <ada@thorsis.com> - 2017-07-25 14:10 +0200
      Re: [PATCH 3/3] memory: atmel-ebi: Fix smc cycle xlate converter Lee Jones <lee.jones@linaro.org> - 2017-07-26 10:30 +0200
    [PATCH 1/3] memory: atmel-ebi: Fix smc timing return value evaluation Alexander Dahl <ada@thorsis.com> - 2017-07-25 14:10 +0200
    Re: [PATCH 0/3] memory: atmel-ebi: Fix setting EBI timings through  dts Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-07-26 20:30 +0200
    Re: [PATCH 0/3] memory: atmel-ebi: Fix setting EBI timings through  dts Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-07-26 22:40 +0200

#1695666 — [PATCH 0/3] memory: atmel-ebi: Fix setting EBI timings through dts

Hello,

this small patch series based on v4.13-rc2 fixes three things I found
when trying to run the latest rc on an at91samg20 based platform with
a SRAM like memory connected to the EBI interface, for which the
timings should be set through dts. 

For all register settings of interest I checked the hardware manuals
of the at91sam9g20 and the sama5d3x to not cause regressions, as well
as the actual register settings in the running system. (I used the
devmem tool from busybox for that.)

Greets
Alex

[toc] | [next] | [standalone]

#1695667 — [PATCH 3/3] memory: atmel-ebi: Fix smc cycle xlate converter

The converter function for translating ns timings in register values was
initialized with a wrong function pointer. This resulted in wrong
register values also for the setup and pulse registers when configuring
the EBI interface trough dts.

Includes a small fix in a comment of the smc driver, which was probably
just a copy'n'paste mistake.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/memory/atmel-ebi.c | 2 +-
 drivers/mfd/atmel-smc.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/atmel-ebi.c b/drivers/memory/atmel-ebi.c
index f8a01ae..ebf69ff 100644
--- a/drivers/memory/atmel-ebi.c
+++ b/drivers/memory/atmel-ebi.c
@@ -72,7 +72,7 @@ struct atmel_smc_timing_xlate {
 	{ .name = nm, .converter = atmel_smc_cs_conf_set_pulse, .shift = pos}
 
 #define ATMEL_SMC_CYCLE_XLATE(nm, pos)	\
-	{ .name = nm, .converter = atmel_smc_cs_conf_set_setup, .shift = pos}
+	{ .name = nm, .converter = atmel_smc_cs_conf_set_cycle, .shift = pos}
 
 static void at91sam9_ebi_get_config(struct atmel_ebi_dev *ebid,
 				    struct atmel_ebi_dev_config *conf)
diff --git a/drivers/mfd/atmel-smc.c b/drivers/mfd/atmel-smc.c
index 954cf0f..20cc0ea 100644
--- a/drivers/mfd/atmel-smc.c
+++ b/drivers/mfd/atmel-smc.c
@@ -206,7 +206,7 @@ EXPORT_SYMBOL_GPL(atmel_smc_cs_conf_set_pulse);
  *	     parameter
  *
  * This function encodes the @ncycles value as described in the datasheet
- * (section "SMC Pulse Register"), and then stores the result in the
+ * (section "SMC Cycle Register"), and then stores the result in the
  * @conf->setup field at @shift position.
  *
  * Returns -EINVAL if @shift is invalid, -ERANGE if @ncycles does not fit in
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1696875 — Re: [PATCH 3/3] memory: atmel-ebi: Fix smc cycle xlate converter

On Tue, 25 Jul 2017, Alexander Dahl wrote:

> The converter function for translating ns timings in register values was
> initialized with a wrong function pointer. This resulted in wrong
> register values also for the setup and pulse registers when configuring
> the EBI interface trough dts.
> 
> Includes a small fix in a comment of the smc driver, which was probably
> just a copy'n'paste mistake.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
>  drivers/memory/atmel-ebi.c | 2 +-
>  drivers/mfd/atmel-smc.c    | 2 +-

Acked-by: Lee Jones <lee.jones@linaro.org>

>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memory/atmel-ebi.c b/drivers/memory/atmel-ebi.c
> index f8a01ae..ebf69ff 100644
> --- a/drivers/memory/atmel-ebi.c
> +++ b/drivers/memory/atmel-ebi.c
> @@ -72,7 +72,7 @@ struct atmel_smc_timing_xlate {
>  	{ .name = nm, .converter = atmel_smc_cs_conf_set_pulse, .shift = pos}
>  
>  #define ATMEL_SMC_CYCLE_XLATE(nm, pos)	\
> -	{ .name = nm, .converter = atmel_smc_cs_conf_set_setup, .shift = pos}
> +	{ .name = nm, .converter = atmel_smc_cs_conf_set_cycle, .shift = pos}
>  
>  static void at91sam9_ebi_get_config(struct atmel_ebi_dev *ebid,
>  				    struct atmel_ebi_dev_config *conf)
> diff --git a/drivers/mfd/atmel-smc.c b/drivers/mfd/atmel-smc.c
> index 954cf0f..20cc0ea 100644
> --- a/drivers/mfd/atmel-smc.c
> +++ b/drivers/mfd/atmel-smc.c
> @@ -206,7 +206,7 @@ EXPORT_SYMBOL_GPL(atmel_smc_cs_conf_set_pulse);
>   *	     parameter
>   *
>   * This function encodes the @ncycles value as described in the datasheet
> - * (section "SMC Pulse Register"), and then stores the result in the
> + * (section "SMC Cycle Register"), and then stores the result in the
>   * @conf->setup field at @shift position.
>   *
>   * Returns -EINVAL if @shift is invalid, -ERANGE if @ncycles does not fit in

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1695669 — [PATCH 1/3] memory: atmel-ebi: Fix smc timing return value evaluation

Setting optional EBI/SMC properties through device tree always fails due
to wrong evaluation of the return value of
atmel_ebi_xslate_smc_timings().

If you put some of those properties in your dts file, but not
'atmel,smc-tdf-ns' the local variable 'required' in
atmel_ebi_xslate_smc_timings() stays on 'false' after the first 'if'
block. This leads to setting 'ret' to -EINVAL in the first run of the
following 'for' loop which is then the return value of this function.

However if you set 'atmel,smc-tdf-ns' in the dts file and everything in
atmel_ebi_xslate_smc_timings() works well, it returns the content of
'required' which is 'true' then.

So the function atmel_ebi_xslate_smc_timings() always returns non-zero
which lets its call in atmel_ebi_xslate_smc_config() always fail and
thus returning -EINVAL, so the EBI configuration for this node fails.

Judging from the following code evaluating the local 'required' variable
in atmel_ebi_xslate_smc_config() and the call of caps->xlate_config in
atmel_ebi_dev_setup() it's probably right to only let the call fail if a
negative error code is returned.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/memory/atmel-ebi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memory/atmel-ebi.c b/drivers/memory/atmel-ebi.c
index 99e644c..1cf34d2 100644
--- a/drivers/memory/atmel-ebi.c
+++ b/drivers/memory/atmel-ebi.c
@@ -263,7 +263,7 @@ static int atmel_ebi_xslate_smc_config(struct atmel_ebi_dev *ebid,
 	}
 
 	ret = atmel_ebi_xslate_smc_timings(ebid, np, &conf->smcconf);
-	if (ret)
+	if (ret < 0)
 		return -EINVAL;
 
 	if ((ret > 0 && !required) || (!ret && required)) {
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1697470 — Re: [PATCH 0/3] memory: atmel-ebi: Fix setting EBI timings through dts

Le Tue, 25 Jul 2017 14:00:21 +0200,
Alexander Dahl <ada@thorsis.com> a écrit :

> Hello,
> 
> this small patch series based on v4.13-rc2 fixes three things I found
> when trying to run the latest rc on an at91samg20 based platform with
> a SRAM like memory connected to the EBI interface, for which the
> timings should be set through dts. 
> 
> For all register settings of interest I checked the hardware manuals
> of the at91sam9g20 and the sama5d3x to not cause regressions, as well
> as the actual register settings in the running system. (I used the
> devmem tool from busybox for that.)

To the whole series:

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Alexandre, Nicolas, can you queue that for 4.13-rc3 or -rc4?

[toc] | [prev] | [next] | [standalone]

#1697551 — Re: [PATCH 0/3] memory: atmel-ebi: Fix setting EBI timings through dts

On 25/07/2017 at 14:00:21 +0200, Alexander Dahl wrote:
> Hello,
> 
> this small patch series based on v4.13-rc2 fixes three things I found
> when trying to run the latest rc on an at91samg20 based platform with
> a SRAM like memory connected to the EBI interface, for which the
> timings should be set through dts. 
> 
> For all register settings of interest I checked the hardware manuals
> of the at91sam9g20 and the sama5d3x to not cause regressions, as well
> as the actual register settings in the running system. (I used the
> devmem tool from busybox for that.)
> 

All applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web