[U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select

2013-05-30 Thread Andrew Gabbasov
The number of gpio signal is packed inside CONFIG_SF_DEFAULT_CS macro
(shifted and or'ed with chip select), so it's incorrect to pass
that macro directly as an argument to gpio_direction_output() call.
The gpio number should be extracted (shifted back) before that.

Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com
---
 board/boundary/nitrogen6x/nitrogen6x.c|2 +-
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/boundary/nitrogen6x/nitrogen6x.c 
b/board/boundary/nitrogen6x/nitrogen6x.c
index cc071d6..b588ac2 100644
--- a/board/boundary/nitrogen6x/nitrogen6x.c
+++ b/board/boundary/nitrogen6x/nitrogen6x.c
@@ -342,7 +342,7 @@ iomux_v3_cfg_t const ecspi1_pads[] = {
 
 void setup_spi(void)
 {
-   gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1);
+   gpio_direction_output(CONFIG_SF_DEFAULT_CS  8, 1);
imx_iomux_v3_setup_multiple_pads(ecspi1_pads,
 ARRAY_SIZE(ecspi1_pads));
 }
diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c 
b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index 9f9cac8..8b71e03 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -312,7 +312,7 @@ iomux_v3_cfg_t const ecspi1_pads[] = {
 
 void setup_spi(void)
 {
-   gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1);
+   gpio_direction_output(CONFIG_SF_DEFAULT_CS  8, 1);
imx_iomux_v3_setup_multiple_pads(ecspi1_pads,
 ARRAY_SIZE(ecspi1_pads));
 }
-- 
1.7.10.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select

2013-05-30 Thread Dirk Behme

On 30.05.2013 12:02, Andrew Gabbasov wrote:

The number of gpio signal is packed inside CONFIG_SF_DEFAULT_CS macro
(shifted and or'ed with chip select), so it's incorrect to pass
that macro directly as an argument to gpio_direction_output() call.
The gpio number should be extracted (shifted back) before that.

Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com
---
  board/boundary/nitrogen6x/nitrogen6x.c|2 +-
  board/freescale/mx6qsabrelite/mx6qsabrelite.c |2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/boundary/nitrogen6x/nitrogen6x.c 
b/board/boundary/nitrogen6x/nitrogen6x.c
index cc071d6..b588ac2 100644
--- a/board/boundary/nitrogen6x/nitrogen6x.c
+++ b/board/boundary/nitrogen6x/nitrogen6x.c
@@ -342,7 +342,7 @@ iomux_v3_cfg_t const ecspi1_pads[] = {

  void setup_spi(void)
  {
-   gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1);
+   gpio_direction_output(CONFIG_SF_DEFAULT_CS  8, 1);
imx_iomux_v3_setup_multiple_pads(ecspi1_pads,
 ARRAY_SIZE(ecspi1_pads));
  }
diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c 
b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index 9f9cac8..8b71e03 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -312,7 +312,7 @@ iomux_v3_cfg_t const ecspi1_pads[] = {

  void setup_spi(void)
  {
-   gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1);
+   gpio_direction_output(CONFIG_SF_DEFAULT_CS  8, 1);
imx_iomux_v3_setup_multiple_pads(ecspi1_pads,
 ARRAY_SIZE(ecspi1_pads));
  }


To my understanding, above change is correct, but not complete ;)

The question is why has it worked with the wrong setting and nobody 
ever noticed that its wrong?


To my understanding the answer is because the SPI driver does it 
correctly:


http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/spi/mxc_spi.c;h=5bed858787f610a9c9a46bb2214665a51d60a9e9;hb=refs/heads/master#l376

So IMHO the gpio_direction_output() above can be removed completely.

Best regards

Dirk



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select

2013-05-30 Thread Gabbasov, Andrew
Hi Dirk,

 From: Behme, Dirk - Bosch
 Sent: Thursday, May 30, 2013 14:50
 To: Gabbasov, Andrew
 Cc: u-boot@lists.denx.de
 Subject: Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio 
 number in SF chip select

[skipped]

 To my understanding, above change is correct, but not complete ;)
 
 The question is why has it worked with the wrong setting and nobody
 ever noticed that its wrong?
 
 To my understanding the answer is because the SPI driver does it
 correctly:
 
 http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/spi/mxc_spi.c;h=5bed858787f610a9c9a46bb2214665a51d60a9e9;hb=refs/heads/master#l376
 
 So IMHO the gpio_direction_output() above can be removed completely.
 
 Best regards
 
 Dirk

Yes, the SPI driver correctly activates and deactivates the CS signal.
But before the first activation it relies on what signal state was set before 
that.
Setting it early at startup just adds some confidence that we have correct
(inactive) chip select state before the first activation by SPI driver.
Otherwise we have to rely on particular pad configuration (e.g. EIM_D19).
I understand, that we set its configuration to pull-up (and this is also
the reset default), and if we do nothing here, it will be recognized as high.
But in order to make sure, it's more safe to explicitly set the signal to 1.

Thanks.

Best regards,
Andrew




___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select

2013-05-30 Thread Dirk Behme

On 30.05.2013 13:32, Gabbasov, Andrew wrote:

Hi Dirk,


From: Behme, Dirk - Bosch
Sent: Thursday, May 30, 2013 14:50
To: Gabbasov, Andrew
Cc: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio 
number in SF chip select


[skipped]


To my understanding, above change is correct, but not complete ;)

The question is why has it worked with the wrong setting and nobody
ever noticed that its wrong?

To my understanding the answer is because the SPI driver does it
correctly:

http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/spi/mxc_spi.c;h=5bed858787f610a9c9a46bb2214665a51d60a9e9;hb=refs/heads/master#l376

So IMHO the gpio_direction_output() above can be removed completely.

Best regards

Dirk


Yes, the SPI driver correctly activates and deactivates the CS signal.
But before the first activation it relies on what signal state was set before 
that.
Setting it early at startup just adds some confidence that we have correct
(inactive) chip select state before the first activation by SPI driver.
Otherwise we have to rely on particular pad configuration (e.g. EIM_D19).
I understand, that we set its configuration to pull-up (and this is also
the reset default), and if we do nothing here, it will be recognized as high.
But in order to make sure, it's more safe to explicitly set the signal to 1.


Hmm, what's 'unsure' in the time between calling setup_spi() the first 
time and calling spi_setup_slave() the first time?


Are they even called in this order? How long is the time between these 
two calls? What's 'unsafe' in this time frame? Why isn't it unsafe 
_until_ setup_spi() is called, then?


Best regards

Dirk






___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select

2013-05-30 Thread Gabbasov, Andrew


 From: Behme, Dirk - Bosch
 Sent: Thursday, May 30, 2013 15:50
 To: Gabbasov, Andrew
 Cc: u-boot@lists.denx.de
 Subject: Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio 
 number in SF chip select
 
 On 30.05.2013 13:32, Gabbasov, Andrew wrote:
  Hi Dirk,
  
  From: Behme, Dirk - Bosch
  Sent: Thursday, May 30, 2013 14:50
  To: Gabbasov, Andrew
  Cc: u-boot@lists.denx.de
  Subject: Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of 
  gpio number in SF chip select
 
  [skipped]
 
  To my understanding, above change is correct, but not complete ;)
 
  The question is why has it worked with the wrong setting and nobody
  ever noticed that its wrong?
 
  To my understanding the answer is because the SPI driver does it
  correctly:
 
  http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/spi/mxc_spi.c;h=5bed858787f610a9c9a46bb2214665a51d60a9e9;hb=refs/heads/master#l376
 
  So IMHO the gpio_direction_output() above can be removed completely.
 
  Best regards
 
  Dirk
 
  Yes, the SPI driver correctly activates and deactivates the CS signal.
  But before the first activation it relies on what signal state was set 
  before that.
  Setting it early at startup just adds some confidence that we have correct
  (inactive) chip select state before the first activation by SPI driver.
  Otherwise we have to rely on particular pad configuration (e.g. EIM_D19).
  I understand, that we set its configuration to pull-up (and this is also
  the reset default), and if we do nothing here, it will be recognized as 
  high.
  But in order to make sure, it's more safe to explicitly set the signal to 1.
 
 Hmm, what's 'unsure' in the time between calling setup_spi() the first
 time and calling spi_setup_slave() the first time?
 
 Are they even called in this order? How long is the time between these
 two calls? What's 'unsafe' in this time frame? Why isn't it unsafe
 _until_ setup_spi() is called, then?
 
 Best regards
 
 Dirk

Hi Dirk,

It turns out I overlooked that the pad configuration for chip select gpio is
actually pull-down, so, since erroneous gpio_direction_output call does not
make effect, the signal seems to be kept low, i.e. active, until first 
spi_setup_slave
call (where it is set correctly before any actual activity is being done on SPI 
bus).

And since indeed nobody yet complained about that, it must be ok to just remove
the gpio related call in setup_spi.

So, OK, you convinced me ;-) I'll prepare another patch shortly, that just 
removes
gpio_direction_output calls from setup_spi function for both mx6qsabrelite
and nitrogen6x.

Thanks.

Best regards,
Andrew
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select

2013-05-30 Thread Gabbasov, Andrew

 From: Gabbasov, Andrew
 Sent: Thursday, May 30, 2013 18:36
 To: Behme, Dirk - Bosch
 Cc: u-boot@lists.denx.de
 Subject: RE: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio 
 number in SF chip select

[skipped]

 So, OK, you convinced me ;-) I'll prepare another patch shortly, that just 
 removes
 gpio_direction_output calls from setup_spi function for both mx6qsabrelite
 and nitrogen6x.

I have submitted the mentioned patch. See Subject line
mx6: mx6qsabrelite/nitrogen6x: Remove incorrect setting of gpio CS signal

Thanks.

Best regards,
Andrew
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot