Re: [PATCH V2] spi: remove check for bits_per_word on transfer from low level driver

2012-12-22 Thread Grant Likely 
On Thu, 20 Dec 2012 11:33:47 +0530, Laxman Dewangan  
wrote:
> On Wednesday 19 December 2012 09:54 PM, Grant Likely wrote:
> > On Tue, 18 Dec 2012 14:25:43 +0530, Laxman Dewangan  
> > wrote:
> >> The spi core make sure that each transfer structure have the proper
> >> setting for bits_per_word before calling low level transfer APIs.
> >>
> >> Hence it is no more require to check again in low level driver for
> >> this field whether this is set correct or not. Removing such code
> >> from low level driver.
> >>
> >> Signed-off-by: Laxman Dewangan
> > [...]
> >> */
> >>
> >>if (prev_speed_hz != speed_hz
> >> @@ -316,9 +315,8 @@ static int txx9spi_transfer(struct spi_device *spi, 
> >> struct spi_message *m)
> >>/* check each transfer's parameters */
> >>list_for_each_entry (t,>transfers, transfer_list) {
> >>u32 speed_hz = t->speed_hz ? : spi->max_speed_hz;
> >> -  u8 bits_per_word = t->bits_per_word ? : spi->bits_per_word;
> >> +  u8 bits_per_word = t->bits_per_word;
> >>
> >> -  bits_per_word = bits_per_word ? : 8;
> > Have you verified here that bits_per_word can never be '0' here? What is
> > the path that ensures spi->bits_per_word (and hence t->bits_per_word) is
> > set to 8 here?
> >
> > Otherwise the patch looks good. Thanks for doing this work.
> 
> 
> When we do the spi_add_device(), the spi_setup() get called.
> In spi_setup, it make  sure that spi->bits_per_word is not zero.
> 
> in spi_setup(spi.c)
> 
> if (!spi->bits_per_word)
>  spi->bits_per_word = 8;

Applied, thanks.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] spi: remove check for bits_per_word on transfer from low level driver

2012-12-22 Thread Grant Likely 
On Thu, 20 Dec 2012 11:33:47 +0530, Laxman Dewangan ldewan...@nvidia.com 
wrote:
 On Wednesday 19 December 2012 09:54 PM, Grant Likely wrote:
  On Tue, 18 Dec 2012 14:25:43 +0530, Laxman Dewanganldewan...@nvidia.com  
  wrote:
  The spi core make sure that each transfer structure have the proper
  setting for bits_per_word before calling low level transfer APIs.
 
  Hence it is no more require to check again in low level driver for
  this field whether this is set correct or not. Removing such code
  from low level driver.
 
  Signed-off-by: Laxman Dewanganldewan...@nvidia.com
  [...]
  */
 
 if (prev_speed_hz != speed_hz
  @@ -316,9 +315,8 @@ static int txx9spi_transfer(struct spi_device *spi, 
  struct spi_message *m)
 /* check each transfer's parameters */
 list_for_each_entry (t,m-transfers, transfer_list) {
 u32 speed_hz = t-speed_hz ? : spi-max_speed_hz;
  -  u8 bits_per_word = t-bits_per_word ? : spi-bits_per_word;
  +  u8 bits_per_word = t-bits_per_word;
 
  -  bits_per_word = bits_per_word ? : 8;
  Have you verified here that bits_per_word can never be '0' here? What is
  the path that ensures spi-bits_per_word (and hence t-bits_per_word) is
  set to 8 here?
 
  Otherwise the patch looks good. Thanks for doing this work.
 
 
 When we do the spi_add_device(), the spi_setup() get called.
 In spi_setup, it make  sure that spi-bits_per_word is not zero.
 
 in spi_setup(spi.c)
 
 if (!spi-bits_per_word)
  spi-bits_per_word = 8;

Applied, thanks.

g.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] spi: remove check for bits_per_word on transfer from low level driver

2012-12-19 Thread Laxman Dewangan 

On Wednesday 19 December 2012 09:54 PM, Grant Likely wrote:

On Tue, 18 Dec 2012 14:25:43 +0530, Laxman Dewangan  
wrote:

The spi core make sure that each transfer structure have the proper
setting for bits_per_word before calling low level transfer APIs.

Hence it is no more require to check again in low level driver for
this field whether this is set correct or not. Removing such code
from low level driver.

Signed-off-by: Laxman Dewangan

[...]

*/

if (prev_speed_hz != speed_hz
@@ -316,9 +315,8 @@ static int txx9spi_transfer(struct spi_device *spi, struct 
spi_message *m)
/* check each transfer's parameters */
list_for_each_entry (t,>transfers, transfer_list) {
u32 speed_hz = t->speed_hz ? : spi->max_speed_hz;
-   u8 bits_per_word = t->bits_per_word ? : spi->bits_per_word;
+   u8 bits_per_word = t->bits_per_word;

-   bits_per_word = bits_per_word ? : 8;

Have you verified here that bits_per_word can never be '0' here? What is
the path that ensures spi->bits_per_word (and hence t->bits_per_word) is
set to 8 here?

Otherwise the patch looks good. Thanks for doing this work.



When we do the spi_add_device(), the spi_setup() get called.
In spi_setup, it make  sure that spi->bits_per_word is not zero.

in spi_setup(spi.c)

if (!spi->bits_per_word)
spi->bits_per_word = 8;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] spi: remove check for bits_per_word on transfer from low level driver

2012-12-19 Thread Grant Likely 
On Tue, 18 Dec 2012 14:25:43 +0530, Laxman Dewangan  
wrote:
> The spi core make sure that each transfer structure have the proper
> setting for bits_per_word before calling low level transfer APIs.
> 
> Hence it is no more require to check again in low level driver for
> this field whether this is set correct or not. Removing such code
> from low level driver.
> 
> Signed-off-by: Laxman Dewangan 
[...]
> diff --git a/drivers/spi/spi-txx9.c b/drivers/spi/spi-txx9.c
> index d5a3cbb..550b5f4 100644
> --- a/drivers/spi/spi-txx9.c
> +++ b/drivers/spi/spi-txx9.c
> @@ -189,9 +189,8 @@ static void txx9spi_work_one(struct txx9spi *c, struct 
> spi_message *m)
>   unsigned int len = t->len;
>   unsigned int wsize;
>   u32 speed_hz = t->speed_hz ? : spi->max_speed_hz;
> - u8 bits_per_word = t->bits_per_word ? : spi->bits_per_word;
> + u8 bits_per_word = t->bits_per_word;
>  
> - bits_per_word = bits_per_word ? : 8;
>   wsize = bits_per_word >> 3; /* in bytes */
>  
>   if (prev_speed_hz != speed_hz
> @@ -316,9 +315,8 @@ static int txx9spi_transfer(struct spi_device *spi, 
> struct spi_message *m)
>   /* check each transfer's parameters */
>   list_for_each_entry (t, >transfers, transfer_list) {
>   u32 speed_hz = t->speed_hz ? : spi->max_speed_hz;
> - u8 bits_per_word = t->bits_per_word ? : spi->bits_per_word;
> + u8 bits_per_word = t->bits_per_word;
>  
> - bits_per_word = bits_per_word ? : 8;

Have you verified here that bits_per_word can never be '0' here? What is
the path that ensures spi->bits_per_word (and hence t->bits_per_word) is
set to 8 here?

Otherwise the patch looks good. Thanks for doing this work.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] spi: remove check for bits_per_word on transfer from low level driver

2012-12-19 Thread Grant Likely 
On Tue, 18 Dec 2012 14:25:43 +0530, Laxman Dewangan ldewan...@nvidia.com 
wrote:
 The spi core make sure that each transfer structure have the proper
 setting for bits_per_word before calling low level transfer APIs.
 
 Hence it is no more require to check again in low level driver for
 this field whether this is set correct or not. Removing such code
 from low level driver.
 
 Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
[...]
 diff --git a/drivers/spi/spi-txx9.c b/drivers/spi/spi-txx9.c
 index d5a3cbb..550b5f4 100644
 --- a/drivers/spi/spi-txx9.c
 +++ b/drivers/spi/spi-txx9.c
 @@ -189,9 +189,8 @@ static void txx9spi_work_one(struct txx9spi *c, struct 
 spi_message *m)
   unsigned int len = t-len;
   unsigned int wsize;
   u32 speed_hz = t-speed_hz ? : spi-max_speed_hz;
 - u8 bits_per_word = t-bits_per_word ? : spi-bits_per_word;
 + u8 bits_per_word = t-bits_per_word;
  
 - bits_per_word = bits_per_word ? : 8;
   wsize = bits_per_word  3; /* in bytes */
  
   if (prev_speed_hz != speed_hz
 @@ -316,9 +315,8 @@ static int txx9spi_transfer(struct spi_device *spi, 
 struct spi_message *m)
   /* check each transfer's parameters */
   list_for_each_entry (t, m-transfers, transfer_list) {
   u32 speed_hz = t-speed_hz ? : spi-max_speed_hz;
 - u8 bits_per_word = t-bits_per_word ? : spi-bits_per_word;
 + u8 bits_per_word = t-bits_per_word;
  
 - bits_per_word = bits_per_word ? : 8;

Have you verified here that bits_per_word can never be '0' here? What is
the path that ensures spi-bits_per_word (and hence t-bits_per_word) is
set to 8 here?

Otherwise the patch looks good. Thanks for doing this work.

g.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] spi: remove check for bits_per_word on transfer from low level driver

2012-12-19 Thread Laxman Dewangan 

On Wednesday 19 December 2012 09:54 PM, Grant Likely wrote:

On Tue, 18 Dec 2012 14:25:43 +0530, Laxman Dewanganldewan...@nvidia.com  
wrote:

The spi core make sure that each transfer structure have the proper
setting for bits_per_word before calling low level transfer APIs.

Hence it is no more require to check again in low level driver for
this field whether this is set correct or not. Removing such code
from low level driver.

Signed-off-by: Laxman Dewanganldewan...@nvidia.com

[...]

*/

if (prev_speed_hz != speed_hz
@@ -316,9 +315,8 @@ static int txx9spi_transfer(struct spi_device *spi, struct 
spi_message *m)
/* check each transfer's parameters */
list_for_each_entry (t,m-transfers, transfer_list) {
u32 speed_hz = t-speed_hz ? : spi-max_speed_hz;
-   u8 bits_per_word = t-bits_per_word ? : spi-bits_per_word;
+   u8 bits_per_word = t-bits_per_word;

-   bits_per_word = bits_per_word ? : 8;

Have you verified here that bits_per_word can never be '0' here? What is
the path that ensures spi-bits_per_word (and hence t-bits_per_word) is
set to 8 here?

Otherwise the patch looks good. Thanks for doing this work.



When we do the spi_add_device(), the spi_setup() get called.
In spi_setup, it make  sure that spi-bits_per_word is not zero.

in spi_setup(spi.c)

if (!spi-bits_per_word)
spi-bits_per_word = 8;


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2] spi: remove check for bits_per_word on transfer from low level driver

2012-12-18 Thread Laxman Dewangan 
The spi core make sure that each transfer structure have the proper
setting for bits_per_word before calling low level transfer APIs.

Hence it is no more require to check again in low level driver for
this field whether this is set correct or not. Removing such code
from low level driver.

Signed-off-by: Laxman Dewangan 
---
This is the continuation of feedback got from Jonas on
change
spi: make sure all transfer has bits_per_word set
where I made the change in only tegra slink driver.
This patch remove the similar code form all drivers.

Changes from V1:
- No change in code.
- Description is rewritten to match with the change.

 drivers/spi/spi-altera.c|2 +-
 drivers/spi/spi-bfin-sport.c|3 +--
 drivers/spi/spi-bfin5xx.c   |3 +--
 drivers/spi/spi-bitbang.c   |6 +++---
 drivers/spi/spi-clps711x.c  |2 +-
 drivers/spi/spi-coldfire-qspi.c |3 +--
 drivers/spi/spi-ep93xx.c|2 +-
 drivers/spi/spi-s3c64xx.c   |2 +-
 drivers/spi/spi-sirf.c  |3 +--
 drivers/spi/spi-tegra20-slink.c |9 +++--
 drivers/spi/spi-txx9.c  |6 ++
 11 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
index 5e7314a..a537f8d 100644
--- a/drivers/spi/spi-altera.c
+++ b/drivers/spi/spi-altera.c
@@ -134,7 +134,7 @@ static int altera_spi_txrx(struct spi_device *spi, struct 
spi_transfer *t)
hw->tx = t->tx_buf;
hw->rx = t->rx_buf;
hw->count = 0;
-   hw->bytes_per_word = (t->bits_per_word ? : spi->bits_per_word) / 8;
+   hw->bytes_per_word = t->bits_per_word / 8;
hw->len = t->len / hw->bytes_per_word;
 
if (hw->irq >= 0) {
diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
index ac7ffca..39b0d17 100644
--- a/drivers/spi/spi-bfin-sport.c
+++ b/drivers/spi/spi-bfin-sport.c
@@ -416,8 +416,7 @@ bfin_sport_spi_pump_transfers(unsigned long data)
drv_data->cs_change = transfer->cs_change;
 
/* Bits per word setup */
-   bits_per_word = transfer->bits_per_word ? :
-   message->spi->bits_per_word ? : 8;
+   bits_per_word = transfer->bits_per_word;
if (bits_per_word % 16 == 0)
drv_data->ops = _sport_transfer_ops_u16;
else
diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
index 0429d83..7d7c991 100644
--- a/drivers/spi/spi-bfin5xx.c
+++ b/drivers/spi/spi-bfin5xx.c
@@ -642,8 +642,7 @@ static void bfin_spi_pump_transfers(unsigned long data)
drv_data->cs_change = transfer->cs_change;
 
/* Bits per word setup */
-   bits_per_word = transfer->bits_per_word ? :
-   message->spi->bits_per_word ? : 8;
+   bits_per_word = transfer->bits_per_word;
if (bits_per_word % 16 == 0) {
drv_data->n_bytes = bits_per_word/8;
drv_data->len = (transfer->len) >> 1;
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
index 8b3d8ef..61beaec 100644
--- a/drivers/spi/spi-bitbang.c
+++ b/drivers/spi/spi-bitbang.c
@@ -69,7 +69,7 @@ static unsigned bitbang_txrx_8(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t->bits_per_word ? : spi->bits_per_word;
+   unsignedbits = t->bits_per_word;
unsignedcount = t->len;
const u8*tx = t->tx_buf;
u8  *rx = t->rx_buf;
@@ -95,7 +95,7 @@ static unsigned bitbang_txrx_16(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t->bits_per_word ? : spi->bits_per_word;
+   unsignedbits = t->bits_per_word;
unsignedcount = t->len;
const u16   *tx = t->tx_buf;
u16 *rx = t->rx_buf;
@@ -121,7 +121,7 @@ static unsigned bitbang_txrx_32(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t->bits_per_word ? : spi->bits_per_word;
+   unsignedbits = t->bits_per_word;
unsignedcount = t->len;
const u32   *tx = t->tx_buf;
u32 *rx = t->rx_buf;
diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
index 1366c46..a11cbf0 100644
--- a/drivers/spi/spi-clps711x.c
+++ b/drivers/spi/spi-clps711x.c
@@ -68,7 +68,7 @@ static int spi_clps711x_setup_xfer(struct spi_device *spi,
   struct spi_transfer *xfer)
 {
u32 speed = xfer->speed_hz ? : spi->max_speed_hz;
-   u8 bpw = xfer->bits_per_word ? : spi->bits_per_word;
+   u8 bpw = xfer->bits_per_word;
struct spi_clps711x_data *hw = spi_master_get_devdata(spi->master);
 
if (bpw != 8) {
diff --git a/drivers/spi/spi-coldfire-qspi.c b/drivers/spi/spi-coldfire-qspi.c
index 58466b8..7b5cc9e 100644

[PATCH V2] spi: remove check for bits_per_word on transfer from low level driver

2012-12-18 Thread Laxman Dewangan 
The spi core make sure that each transfer structure have the proper
setting for bits_per_word before calling low level transfer APIs.

Hence it is no more require to check again in low level driver for
this field whether this is set correct or not. Removing such code
from low level driver.

Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
This is the continuation of feedback got from Jonas on
change
spi: make sure all transfer has bits_per_word set
where I made the change in only tegra slink driver.
This patch remove the similar code form all drivers.

Changes from V1:
- No change in code.
- Description is rewritten to match with the change.

 drivers/spi/spi-altera.c|2 +-
 drivers/spi/spi-bfin-sport.c|3 +--
 drivers/spi/spi-bfin5xx.c   |3 +--
 drivers/spi/spi-bitbang.c   |6 +++---
 drivers/spi/spi-clps711x.c  |2 +-
 drivers/spi/spi-coldfire-qspi.c |3 +--
 drivers/spi/spi-ep93xx.c|2 +-
 drivers/spi/spi-s3c64xx.c   |2 +-
 drivers/spi/spi-sirf.c  |3 +--
 drivers/spi/spi-tegra20-slink.c |9 +++--
 drivers/spi/spi-txx9.c  |6 ++
 11 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
index 5e7314a..a537f8d 100644
--- a/drivers/spi/spi-altera.c
+++ b/drivers/spi/spi-altera.c
@@ -134,7 +134,7 @@ static int altera_spi_txrx(struct spi_device *spi, struct 
spi_transfer *t)
hw-tx = t-tx_buf;
hw-rx = t-rx_buf;
hw-count = 0;
-   hw-bytes_per_word = (t-bits_per_word ? : spi-bits_per_word) / 8;
+   hw-bytes_per_word = t-bits_per_word / 8;
hw-len = t-len / hw-bytes_per_word;
 
if (hw-irq = 0) {
diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
index ac7ffca..39b0d17 100644
--- a/drivers/spi/spi-bfin-sport.c
+++ b/drivers/spi/spi-bfin-sport.c
@@ -416,8 +416,7 @@ bfin_sport_spi_pump_transfers(unsigned long data)
drv_data-cs_change = transfer-cs_change;
 
/* Bits per word setup */
-   bits_per_word = transfer-bits_per_word ? :
-   message-spi-bits_per_word ? : 8;
+   bits_per_word = transfer-bits_per_word;
if (bits_per_word % 16 == 0)
drv_data-ops = bfin_sport_transfer_ops_u16;
else
diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
index 0429d83..7d7c991 100644
--- a/drivers/spi/spi-bfin5xx.c
+++ b/drivers/spi/spi-bfin5xx.c
@@ -642,8 +642,7 @@ static void bfin_spi_pump_transfers(unsigned long data)
drv_data-cs_change = transfer-cs_change;
 
/* Bits per word setup */
-   bits_per_word = transfer-bits_per_word ? :
-   message-spi-bits_per_word ? : 8;
+   bits_per_word = transfer-bits_per_word;
if (bits_per_word % 16 == 0) {
drv_data-n_bytes = bits_per_word/8;
drv_data-len = (transfer-len)  1;
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
index 8b3d8ef..61beaec 100644
--- a/drivers/spi/spi-bitbang.c
+++ b/drivers/spi/spi-bitbang.c
@@ -69,7 +69,7 @@ static unsigned bitbang_txrx_8(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u8*tx = t-tx_buf;
u8  *rx = t-rx_buf;
@@ -95,7 +95,7 @@ static unsigned bitbang_txrx_16(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u16   *tx = t-tx_buf;
u16 *rx = t-rx_buf;
@@ -121,7 +121,7 @@ static unsigned bitbang_txrx_32(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u32   *tx = t-tx_buf;
u32 *rx = t-rx_buf;
diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
index 1366c46..a11cbf0 100644
--- a/drivers/spi/spi-clps711x.c
+++ b/drivers/spi/spi-clps711x.c
@@ -68,7 +68,7 @@ static int spi_clps711x_setup_xfer(struct spi_device *spi,
   struct spi_transfer *xfer)
 {
u32 speed = xfer-speed_hz ? : spi-max_speed_hz;
-   u8 bpw = xfer-bits_per_word ? : spi-bits_per_word;
+   u8 bpw = xfer-bits_per_word;
struct spi_clps711x_data *hw = spi_master_get_devdata(spi-master);
 
if (bpw != 8) {
diff --git a/drivers/spi/spi-coldfire-qspi.c b/drivers/spi/spi-coldfire-qspi.c
index 58466b8..7b5cc9e 100644
---