* [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary @ 2009-12-18 7:29 James Getzendanner [not found] ` <DA80C3E6C10BE94E8FCACEB239C2FEE46645317E77-64WhM8za9n95vsLK/AIMcA@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: James Getzendanner @ 2009-12-18 7:29 UTC (permalink / raw) To: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <DA80C3E6C10BE94E8FCACEB239C2FEE46645317E77-64WhM8za9n95vsLK/AIMcA@public.gmane.org>]
* Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary [not found] ` <DA80C3E6C10BE94E8FCACEB239C2FEE46645317E77-64WhM8za9n95vsLK/AIMcA@public.gmane.org> @ 2009-12-18 13:14 ` Mike Frysinger [not found] ` <8bd0f97a0912180514k42f32162jc850f4b4abbc3f7a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Mike Frysinger @ 2009-12-18 13:14 UTC (permalink / raw) To: James Getzendanner Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <8bd0f97a0912180514k42f32162jc850f4b4abbc3f7a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary [not found] ` <8bd0f97a0912180514k42f32162jc850f4b4abbc3f7a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2009-12-18 17:34 ` Mike Frysinger [not found] ` <8bd0f97a0912180934s3d2c70b7uf65d09dc016c1faa-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Mike Frysinger @ 2009-12-18 17:34 UTC (permalink / raw) To: James Getzendanner Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <8bd0f97a0912180934s3d2c70b7uf65d09dc016c1faa-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary [not found] ` <8bd0f97a0912180934s3d2c70b7uf65d09dc016c1faa-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2010-01-19 16:12 ` Grant Likely [not found] ` <fa686aa41001190812l28cf29fs5471c1022d25deee-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Grant Likely @ 2010-01-19 16:12 UTC (permalink / raw) To: Mike Frysinger Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b, James Getzendanner, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <fa686aa41001190812l28cf29fs5471c1022d25deee-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary [not found] ` <fa686aa41001190812l28cf29fs5471c1022d25deee-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2010-01-19 17:29 ` Mike Frysinger [not found] ` <8bd0f97a1001190929w78624e95pa3f32e5147c2617e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Mike Frysinger @ 2010-01-19 17:29 UTC (permalink / raw) To: Grant Likely Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b, James Getzendanner, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <8bd0f97a1001190929w78624e95pa3f32e5147c2617e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [Uclinux-dist-devel] [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary [not found] ` <8bd0f97a1001190929w78624e95pa3f32e5147c2617e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2010-01-19 17:46 ` Grant Likely 0 siblings, 0 replies; 6+ messages in thread From: Grant Likely @ 2010-01-19 17:46 UTC (permalink / raw) To: Mike Frysinger Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b, James Getzendanner, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-19 17:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-12-18 7:29 [PATCH] Clear control bits in drivers/spi/spi_bfin5xx.c as necessary James Getzendanner [not found] ` <DA80C3E6C10BE94E8FCACEB239C2FEE46645317E77-64WhM8za9n95vsLK/AIMcA@public.gmane.org> 2009-12-18 13:14 ` [Uclinux-dist-devel] " Mike Frysinger [not found] ` <8bd0f97a0912180514k42f32162jc850f4b4abbc3f7a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2009-12-18 17:34 ` Mike Frysinger [not found] ` <8bd0f97a0912180934s3d2c70b7uf65d09dc016c1faa-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2010-01-19 16:12 ` Grant Likely [not found] ` <fa686aa41001190812l28cf29fs5471c1022d25deee-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2010-01-19 17:29 ` Mike Frysinger [not found] ` <8bd0f97a1001190929w78624e95pa3f32e5147c2617e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2010-01-19 17:46 ` Grant Likely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).