[PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary

[PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary

[James Getzendanner]

From: Andy Getzendanner <james.getzendanner-9geRo0GdX4k6efY3Smnssodd74u8MsAO@public.gmane.org>

This patch corrects the behavior of bfin_spi_setup() to clear bits LSBF,
CPHA, and CPOL in the SPI control register when required by the desired
SPI configuration.  The patch also modifies bfin_spi_setup() to
set/clear the SIZE bit as appropriate.
Signed-off-by: Andy Getzendanner <james.getzendanner-9geRo0GdX4k6efY3Smnssodd74u8MsAO@public.gmane.org>
---
The control bits LSBF, CPHA, and CPOL are set by bfin_spi_setup() when
appropriate, but the function appears to assume they are clear at entry
and leaves them untouched when the desired configuration calls for them
to be clear.  The SIZE bit is never set or cleared.
bfin_spi_setup() is triggered in response to an ioctl which calls for
the SPI configuration to be changed, but the settings controlled by
LSBF, CPHA, and CPOL cannot be changed away from the setting implied by
the value 1.  Additionally, the word length setting (controlled by SIZE)
is not set correctly.
The patch has been tested against 2.6.30.4 (it applies against 2.6.32.1)
and observed to correct the behavior of the affected ioctl.
Please CC me at james.getzendanner-9geRo0GdX4k6efY3Smnssodd74u8MsAO@public.gmane.org in any replies as I
am not on the list.
--- linux-2.6.32.1/drivers/spi/spi_bfin5xx.c.orig	2009-12-14 12:47:25.000000000 -0500
+++ linux-2.6.32.1/drivers/spi/spi_bfin5xx.c	2009-12-18 00:27:10.000000000 -0500
@@ -1051,10 +1051,16 @@ static int bfin_spi_setup(struct spi_dev
 	/* translate common spi framework into our register */
 	if (spi->mode & SPI_CPOL)
 		chip->ctl_reg |= CPOL;
+	else
+		chip->ctl_reg &= ~CPOL;
 	if (spi->mode & SPI_CPHA)
 		chip->ctl_reg |= CPHA;
+	else
+		chip->ctl_reg &= ~CPHA;
 	if (spi->mode & SPI_LSB_FIRST)
 		chip->ctl_reg |= LSBF;
+	else
+		chip->ctl_reg &= ~LSBF;
 	/* we dont support running in slave mode (yet?) */
 	chip->ctl_reg |= MSTR;
 
@@ -1106,6 +1112,7 @@ static int bfin_spi_setup(struct spi_dev
 			bfin_spi_u8_cs_chg_writer : bfin_spi_u8_writer;
 		chip->duplex = chip->cs_change_per_word ?
 			bfin_spi_u8_cs_chg_duplex : bfin_spi_u8_duplex;
+		chip->ctl_reg &= ~SIZE;
 		break;
 
 	case 16:
@@ -1117,6 +1124,7 @@ static int bfin_spi_setup(struct spi_dev
 			bfin_spi_u16_cs_chg_writer : bfin_spi_u16_writer;
 		chip->duplex = chip->cs_change_per_word ?
 			bfin_spi_u16_cs_chg_duplex : bfin_spi_u16_duplex;
+		chip->ctl_reg |= SIZE;
 		break;
 
 	default:

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 

Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary

[Mike Frysinger]

On Fri, Dec 18, 2009 at 02:29, James Getzendanner wrote:
> From: Andy Getzendanner <james.getzendanner@students.olin.edu>
>
> This patch corrects the behavior of bfin_spi_setup() to clear bits LSBF,
> CPHA, and CPOL in the SPI control register when required by the desired
> SPI configuration.

why not always zero out ctl_reg then
    if (chip_info) {
    ...
    } else {
+        chip->ctl_reg = 0;

> The patch also modifies bfin_spi_setup() to
> set/clear the SIZE bit as appropriate.

ok, but this isnt the place to do it.  the size can be changed on a
per-transfer basis.  i have a patch locally to unify the transfer size
logic and i'll include this fix in the process.

> The control bits LSBF, CPHA, and CPOL are set by bfin_spi_setup() when
> appropriate, but the function appears to assume they are clear at entry
> and leaves them untouched when the desired configuration calls for them
> to be clear.  The SIZE bit is never set or cleared.
> bfin_spi_setup() is triggered in response to an ioctl which calls for
> the SPI configuration to be changed, but the settings controlled by
> LSBF, CPHA, and CPOL cannot be changed away from the setting implied by
> the value 1.  Additionally, the word length setting (controlled by SIZE)
> is not set correctly.

this info should be in the changelog, not below the --- ignore marker
-mike

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary

[Mike Frysinger]

On Fri, Dec 18, 2009 at 08:14, Mike Frysinger wrote:
> On Fri, Dec 18, 2009 at 02:29, James Getzendanner wrote:
>> From: Andy Getzendanner <james.getzendanner@students.olin.edu>
>> The patch also modifies bfin_spi_setup() to
>> set/clear the SIZE bit as appropriate.
>
> ok, but this isnt the place to do it.  the size can be changed on a
> per-transfer basis.  i have a patch locally to unify the transfer size
> logic and i'll include this fix in the process.

i lied, this isnt necessary.  it's already handled in the transfer code.
    cr = (read_CTRL(drv_data) & (~BIT_CTL_TIMOD));
    cr |= (width << 8);
    write_CTRL(drv_data, cr);

where width is CFG_SPI_WORDSIZE16 or CFG_SPI_WORDSIZE8 as dictated by
the transfer.
-mike

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary

[Grant Likely]

On Fri, Dec 18, 2009 at 10:34 AM, Mike Frysinger <vapier.adi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Dec 18, 2009 at 08:14, Mike Frysinger wrote:
>> On Fri, Dec 18, 2009 at 02:29, James Getzendanner wrote:
>>> From: Andy Getzendanner <james.getzendanner-9geRo0GdX4k6efY3Smnssodd74u8MsAO@public.gmane.org>
>>> The patch also modifies bfin_spi_setup() to
>>> set/clear the SIZE bit as appropriate.
>>
>> ok, but this isnt the place to do it.  the size can be changed on a
>> per-transfer basis.  i have a patch locally to unify the transfer size
>> logic and i'll include this fix in the process.
>
> i lied, this isnt necessary.  it's already handled in the transfer code.
>    cr = (read_CTRL(drv_data) & (~BIT_CTL_TIMOD));
>    cr |= (width << 8);
>    write_CTRL(drv_data, cr);
>
> where width is CFG_SPI_WORDSIZE16 or CFG_SPI_WORDSIZE8 as dictated by
> the transfer.
> -mike

Mike & James,

It's not clear to me from the discussion.  What is the status of this
patch, is it still needed?

also...

On Fri, Dec 18, 2009 at 12:29 AM, James Getzendanner
<James.Getzendanner-9geRo0GdX4k6efY3Smnssodd74u8MsAO@public.gmane.org> wrote:
> --- linux-2.6.32.1/drivers/spi/spi_bfin5xx.c.orig       2009-12-14 12:47:25.000000000 -0500
> +++ linux-2.6.32.1/drivers/spi/spi_bfin5xx.c    2009-12-18 00:27:10.000000000 -0500
> @@ -1051,10 +1051,16 @@ static int bfin_spi_setup(struct spi_dev
>        /* translate common spi framework into our register */
>        if (spi->mode & SPI_CPOL)
>                chip->ctl_reg |= CPOL;
> +       else
> +               chip->ctl_reg &= ~CPOL;
>        if (spi->mode & SPI_CPHA)
>                chip->ctl_reg |= CPHA;
> +       else
> +               chip->ctl_reg &= ~CPHA;
>        if (spi->mode & SPI_LSB_FIRST)
>                chip->ctl_reg |= LSBF;
> +       else
> +               chip->ctl_reg &= ~LSBF;
>        /* we dont support running in slave mode (yet?) */
>        chip->ctl_reg |= MSTR;

chip->ctl_reg &= ~(CPOL | CPHA | LSBF); before the set of if()
statements would be more concise.

g.

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

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev

Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary

[Mike Frysinger]

On Tue, Jan 19, 2010 at 11:12, Grant Likely wrote:
> On Fri, Dec 18, 2009 at 10:34 AM, Mike Frysinger wrote:
>> On Fri, Dec 18, 2009 at 08:14, Mike Frysinger wrote:
>>> On Fri, Dec 18, 2009 at 02:29, James Getzendanner wrote:
>>>> From: Andy Getzendanner <james.getzendanner@students.olin.edu>
>>>> The patch also modifies bfin_spi_setup() to
>>>> set/clear the SIZE bit as appropriate.
>>>
>>> ok, but this isnt the place to do it.  the size can be changed on a
>>> per-transfer basis.  i have a patch locally to unify the transfer size
>>> logic and i'll include this fix in the process.
>>
>> i lied, this isnt necessary.  it's already handled in the transfer code.
>>    cr = (read_CTRL(drv_data) & (~BIT_CTL_TIMOD));
>>    cr |= (width << 8);
>>    write_CTRL(drv_data, cr);
>>
>> where width is CFG_SPI_WORDSIZE16 or CFG_SPI_WORDSIZE8 as dictated by
>> the transfer.
>
> Mike & James,
>
> It's not clear to me from the discussion.  What is the status of this
> patch, is it still needed?

he pointed out valid issues in the Blackfin driver that i have fixed,
just not with his versions
-mike

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary

[Grant Likely]

On Tue, Jan 19, 2010 at 10:29 AM, Mike Frysinger <vapier.adi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Jan 19, 2010 at 11:12, Grant Likely wrote:
>> On Fri, Dec 18, 2009 at 10:34 AM, Mike Frysinger wrote:
>>> On Fri, Dec 18, 2009 at 08:14, Mike Frysinger wrote:
>>>> On Fri, Dec 18, 2009 at 02:29, James Getzendanner wrote:
>>>>> From: Andy Getzendanner <james.getzendanner-9geRo0GdX4k6efY3Smnssodd74u8MsAO@public.gmane.org>
>>>>> The patch also modifies bfin_spi_setup() to
>>>>> set/clear the SIZE bit as appropriate.
>>>>
>>>> ok, but this isnt the place to do it.  the size can be changed on a
>>>> per-transfer basis.  i have a patch locally to unify the transfer size
>>>> logic and i'll include this fix in the process.
>>>
>>> i lied, this isnt necessary.  it's already handled in the transfer code.
>>>    cr = (read_CTRL(drv_data) & (~BIT_CTL_TIMOD));
>>>    cr |= (width << 8);
>>>    write_CTRL(drv_data, cr);
>>>
>>> where width is CFG_SPI_WORDSIZE16 or CFG_SPI_WORDSIZE8 as dictated by
>>> the transfer.
>>
>> Mike & James,
>>
>> It's not clear to me from the discussion.  What is the status of this
>> patch, is it still needed?
>
> he pointed out valid issues in the Blackfin driver that i have fixed,
> just not with his versions

okay.  I'll drop the patch then.

g.

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

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev