[PATCH] spi/pl022: compare bitwidth to enums

[PATCH] spi/pl022: compare bitwidth to enums

[Linus Walleij]

The enums from <linux/amba/pl022.h> are actually off-by-one
since bitwidth 4 is defined as 0x03, causing weirdness when
we were trying out some 9-bit peripheral. Fix this by
comparing to the enum values rather than hardcoded values
and also fixing the enum to something reasonable.

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/spi/amba-pl022.c   |    4 ++--
 include/linux/amba/pl022.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
index 90a7e5d..981f78d 100644
--- a/drivers/spi/amba-pl022.c
+++ b/drivers/spi/amba-pl022.c
@@ -1605,12 +1605,12 @@ static int pl022_setup(struct spi_device *spi)
 	chip->xfer_type = chip_info->com_mode;
 	chip->cs_control = chip_info->cs_control;
 
-	if (chip_info->data_size <= 8) {
+	if (chip_info->data_size <= SSP_DATA_BITS_8) {
 		dev_dbg(&spi->dev, "1 <= n <=8 bits per word\n");
 		chip->n_bytes = 1;
 		chip->read = READING_U8;
 		chip->write = WRITING_U8;
-	} else if (chip_info->data_size <= 16) {
+	} else if (chip_info->data_size <= SSP_DATA_BITS_16) {
 		dev_dbg(&spi->dev, "9 <= n <= 16 bits per word\n");
 		chip->n_bytes = 2;
 		chip->read = READING_U16;
diff --git a/include/linux/amba/pl022.h b/include/linux/amba/pl022.h
index abf26cc..3c83c62 100644
--- a/include/linux/amba/pl022.h
+++ b/include/linux/amba/pl022.h
@@ -90,7 +90,7 @@ enum ssp_tx_endian {
  * enum ssp_data_size - number of bits in one data element
  */
 enum ssp_data_size {
-	SSP_DATA_BITS_4 = 0x03, SSP_DATA_BITS_5, SSP_DATA_BITS_6,
+	SSP_DATA_BITS_4 = 0x04, SSP_DATA_BITS_5, SSP_DATA_BITS_6,
 	SSP_DATA_BITS_7, SSP_DATA_BITS_8, SSP_DATA_BITS_9,
 	SSP_DATA_BITS_10, SSP_DATA_BITS_11, SSP_DATA_BITS_12,
 	SSP_DATA_BITS_13, SSP_DATA_BITS_14, SSP_DATA_BITS_15,
-- 
1.6.3.3

Re: [PATCH] spi/pl022: compare bitwidth to enums

[Linus Walleij]

2010/9/30 Linus Walleij <linus.walleij@stericsson.com>:

> The enums from <linux/amba/pl022.h> are actually off-by-one
> since bitwidth 4 is defined as 0x03, causing weirdness when
> we were trying out some 9-bit peripheral. Fix this by
> comparing to the enum values rather than hardcoded values
> and also fixing the enum to something reasonable.
>
> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> (...)
> -       if (chip_info->data_size <= 8) {
> +       if (chip_info->data_size <= SSP_DATA_BITS_8) {
>                dev_dbg(&spi->dev, "1 <= n <=8 bits per word\n");
>                chip->n_bytes = 1;
>                chip->read = READING_U8;
>                chip->write = WRITING_U8;
> -       } else if (chip_info->data_size <= 16) {
> +       } else if (chip_info->data_size <= SSP_DATA_BITS_16) {

I just realized that Kevin Wells patch actually patch away this problem
so merge his patch and disregard this for the optimal solution.

>  enum ssp_data_size {
> -       SSP_DATA_BITS_4 = 0x03, SSP_DATA_BITS_5, SSP_DATA_BITS_6,
> +       SSP_DATA_BITS_4 = 0x04, SSP_DATA_BITS_5, SSP_DATA_BITS_6,

This is not good because this is used for default register
settings too (I didn't notice, typical).

If anyone still wants a patch for older kernels I can provide a
patch against 2.6.36-rc6 with just the first two hunks.

Yours,
Linus Walleij

Re: [PATCH] spi/pl022: compare bitwidth to enums

[Grant Likely]

On Thu, Sep 30, 2010 at 8:34 PM, Linus Walleij
<linus.ml.walleij@gmail.com> wrote:
> 2010/9/30 Linus Walleij <linus.walleij@stericsson.com>:
>
>> The enums from <linux/amba/pl022.h> are actually off-by-one
>> since bitwidth 4 is defined as 0x03, causing weirdness when
>> we were trying out some 9-bit peripheral. Fix this by
>> comparing to the enum values rather than hardcoded values
>> and also fixing the enum to something reasonable.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
>> (...)
>> -       if (chip_info->data_size <= 8) {
>> +       if (chip_info->data_size <= SSP_DATA_BITS_8) {
>>                dev_dbg(&spi->dev, "1 <= n <=8 bits per word\n");
>>                chip->n_bytes = 1;
>>                chip->read = READING_U8;
>>                chip->write = WRITING_U8;
>> -       } else if (chip_info->data_size <= 16) {
>> +       } else if (chip_info->data_size <= SSP_DATA_BITS_16) {
>
> I just realized that Kevin Wells patch actually patch away this problem
> so merge his patch and disregard this for the optimal solution.
>
>>  enum ssp_data_size {
>> -       SSP_DATA_BITS_4 = 0x03, SSP_DATA_BITS_5, SSP_DATA_BITS_6,
>> +       SSP_DATA_BITS_4 = 0x04, SSP_DATA_BITS_5, SSP_DATA_BITS_6,
>
> This is not good because this is used for default register
> settings too (I didn't notice, typical).

And it creates a 1:1 mapping of name to the same number.  Insane!
Just use the integer number directly.  :-)

>
> If anyone still wants a patch for older kernels I can provide a
> patch against 2.6.36-rc6 with just the first two hunks.
>
> Yours,
> Linus Walleij
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

RE: [PATCH] spi/pl022: compare bitwidth to enums

[Kevin Wells]

> >> The enums from <linux/amba/pl022.h> are actually off-by-one
> >> since bitwidth 4 is defined as 0x03, causing weirdness when
> >> we were trying out some 9-bit peripheral. Fix this by
> >> comparing to the enum values rather than hardcoded values
> >> and also fixing the enum to something reasonable.
> >>
> >> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> >> (...)
> >> -       if (chip_info->data_size <= 8) {
> >> +       if (chip_info->data_size <= SSP_DATA_BITS_8) {
> >>                dev_dbg(&spi->dev, "1 <= n <=8 bits per word\n");
> >>                chip->n_bytes = 1;
> >>                chip->read = READING_U8;
> >>                chip->write = WRITING_U8;
> >> -       } else if (chip_info->data_size <= 16) {
> >> +       } else if (chip_info->data_size <= SSP_DATA_BITS_16) {
> >
> > I just realized that Kevin Wells patch actually patch away this problem
> > so merge his patch and disregard this for the optimal solution.
> >
> >>  enum ssp_data_size {
> >> -       SSP_DATA_BITS_4 = 0x03, SSP_DATA_BITS_5, SSP_DATA_BITS_6,
> >> +       SSP_DATA_BITS_4 = 0x04, SSP_DATA_BITS_5, SSP_DATA_BITS_6,
> >
> > This is not good because this is used for default register
> > settings too (I didn't notice, typical).
> 
> And it creates a 1:1 mapping of name to the same number.  Insane!
> Just use the integer number directly.  :-)

The spi->mode patch I submitted a few weeks ago actually removed the
data_size field from chip_info and uses spi->data_bus_bits instead.
The values passed into the dss field for the control0 register
subtracts 1 from the spi->data_bus_bits to get the correct value now.

Reference: 09/16/2010
[PATCH] spi: amba_pl022: Add spi->mode support to AMBA SPI driver

> 
> >
> > If anyone still wants a patch for older kernels I can provide a
> > patch against 2.6.36-rc6 with just the first two hunks.
> >
> > Yours,
> > Linus Walleij
> >
> 
> 
> 
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
> 
> --------------------------------------------------------------------------
> ----
> Start uncovering the many advantages of virtual appliances
> and start using them to simplify application deployment and
> accelerate your shift to cloud computing.
> http://p.sf.net/sfu/novell-sfdev2dev
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general