RE: [PATCH 0/3] davinci: spi: replace existing SPI driver

2010-06-09 Thread Nori, Sekhar
Hi Brian,

Can you please post this series to SPI development list
(spi-devel-gene...@lists.sourceforge.net) CCing the maintainers
David Brownell and Grant Likely?

Waiting for comments on this list before posting there will
result in lost time.

On Mon, Jun 07, 2010 at 20:39:05, Brian Niebuhr wrote:
>

[...]

> INTRODUCTION
>
> I have been working on a custom OMAP-L138 board that has multiple spi
> devices (seven) on one controller.  These devices have a wide range of
> transfer parameters (speed, phase, polarity, internal and gpio chip
> selects).  During my testing I found multiple errors in the davinci spi
> driver as a result of this complex setup.  The primary issues were:
>
> 1. There is a race condition due to the SPIBUF read busy-waits for slow
> devices
> 2. I found some DMA transfer length errors under some conditions
> 3. The chip select code caused extra byte transfers (with no chip
> select active) due to writes to SPIDAT1
> 4. Several issues prevented using multiple SPI devices, especially
> the DMA code, as disucussed previously on the davinci list.

This should be replicated in patch description for patch 1/3 as a record
of why the original driver was removed.

>
> The fixes to these problems were not simple.  I ended up making fairly
> large changes to the driver, and those changes are contained in these
> patches.  The full list of changes follows.
>
> CHANGE LIST
>
> 1. davinci_spi_chipelect() now performs both activation and deactivation
> of chip selects.  This lets spi_bitbang fully control chip
> select activation, as intended by the SPI API.
> 2. Chip select activation does not cause extra writes to the SPI bus
> 3. Chip select activation does not use SPIDEF for control.  This change
> will also allow for implementation of inverted (active high)
> chip selects in the future.
> 4. Added back gpio chip select capability from the old driver
> 5. Fixed prescale calculation for non-integer fractions of spi clock
> 6. Allow specification of SPI transfer parameters on a per-device
> (instead of per-controller) basis
> 7. Allow specification of polled, interrupt-based, or DMA operation on
> a per-device basis
> 8. Allow DMA with when more than one device is connected
> 9. Combined pio and dma txrx_bufs functions into one since they share
> large parts of their functionality, and to simplify item (8).
> 10. Use only SPIFMT0 to allow more than 4 devices
>
> TESTING
>
> I have tested the driver using a custom SPI stress test on my
> OMAP-L138-based board with three devices connected.  I have tested
> configurations with all three devices polled, all three interrupt-based,
> all three DMA, and a mixture.

This should be replicated in patch description for patch 2/3 as a record
of what the new driver provides.

>
> I have compiled with the davinci_all_defconfig, but I don't have EVMs
> for the other davinci platforms to test with.

DM6467, DM365 and OMAP-L137 EVMs have SPI devices as well. We can help
test on at least some of those. No need to wait for test results before
posting to SPI list though, it can happen in parallel to the review.

>
> SUMMARY
>
> This patch solves a lot of issues that should save a lot of people time
> down the road.  Since I posted the original patch I have had at least 5
> people contact me personally to get help applying the patch because SPI
> was broken on their boards.  I have heard back from at least 2 that the
> original patch worked for them.

You can include an Acked-by: from those developers. This will help build
the case for the new driver.

Thanks,
Sekhar

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


RE: [PATCH 0/3] davinci: spi: replace existing SPI driver

2010-06-10 Thread Sudhakar Rajashekhara
Hi,

On Mon, Jun 07, 2010 at 20:39:05, Brian Niebuhr wrote:
> NOTE
> 
> This patch requires the EDMA patch at:
> 
> http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2010-March/018022.html
> 
> which has since been reverted out of the davinci kernel.
> 
> 
> INTRODUCTION
> 
> I have been working on a custom OMAP-L138 board that has multiple spi
> devices (seven) on one controller.  These devices have a wide range of
> transfer parameters (speed, phase, polarity, internal and gpio chip
> selects).  During my testing I found multiple errors in the davinci spi
> driver as a result of this complex setup.  The primary issues were:
> 
> 1. There is a race condition due to the SPIBUF read busy-waits for slow
> devices
> 2. I found some DMA transfer length errors under some conditions
> 3. The chip select code caused extra byte transfers (with no chip
> select active) due to writes to SPIDAT1
> 4. Several issues prevented using multiple SPI devices, especially
> the DMA code, as disucussed previously on the davinci list.
> 
> The fixes to these problems were not simple.  I ended up making fairly
> large changes to the driver, and those changes are contained in these
> patches.  The full list of changes follows.
> 
> CHANGE LIST
> 
> 1. davinci_spi_chipelect() now performs both activation and deactivation
> of chip selects.  This lets spi_bitbang fully control chip
> select activation, as intended by the SPI API.
> 2. Chip select activation does not cause extra writes to the SPI bus
> 3. Chip select activation does not use SPIDEF for control.  This change
> will also allow for implementation of inverted (active high)
> chip selects in the future.
> 4. Added back gpio chip select capability from the old driver
> 5. Fixed prescale calculation for non-integer fractions of spi clock
> 6. Allow specification of SPI transfer parameters on a per-device
> (instead of per-controller) basis
> 7. Allow specification of polled, interrupt-based, or DMA operation on
> a per-device basis
> 8. Allow DMA with when more than one device is connected
> 9. Combined pio and dma txrx_bufs functions into one since they share
> large parts of their functionality, and to simplify item (8).
> 10. Use only SPIFMT0 to allow more than 4 devices
> 
> TESTING
> 
> I have tested the driver using a custom SPI stress test on my
> OMAP-L138-based board with three devices connected.  I have tested
> configurations with all three devices polled, all three interrupt-based,
> all three DMA, and a mixture.
> 
> I have compiled with the davinci_all_defconfig, but I don't have EVMs
> for the other davinci platforms to test with.
> 

I was testing this series on OMAP-L137, OMAP-L138 and DM365 EVMs and I
found that on all these EVMS, Interrupt mode has problems. Driver works
fine in DMA and POLL modes. Some problems which I had observed with the
old driver are gone now.

Regards,
Sudhakar


___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


RE: [PATCH 0/3] davinci: spi: replace existing SPI driver

2010-06-14 Thread Brian Niebuhr
> > I have compiled with the davinci_all_defconfig, but I don't 
> have EVMs
> > for the other davinci platforms to test with.
> > 
> 
> I was testing this series on OMAP-L137, OMAP-L138 and DM365 EVMs and I
> found that on all these EVMS, Interrupt mode has problems. 
> Driver works
> fine in DMA and POLL modes. Some problems which I had 
> observed with the
> old driver are gone now.

Do you have any idea what errors you're seeing in interrupt mode?  I
went and retested interrupt mode with several devices on my custom board
and it still appeared to be working fine.  It may be an interaction with
code outside the driver because I'm running the identical driver code
but I'm not running the latest kernel from the davinci git repository.
I'm trying to figure out if I can get my DA850 EVM back into a state
where I can debug this.  Thanks for any hints you can give!  

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


RE: [PATCH 0/3] davinci: spi: replace existing SPI driver

2010-06-14 Thread Brian Niebuhr
> Can you please post this series to SPI development list
> (spi-devel-gene...@lists.sourceforge.net) CCing the maintainers
> David Brownell and Grant Likely?

Done
 
> > INTRODUCTION
> >
> > I have been working on a custom OMAP-L138 board that has 
> multiple spi
> > devices (seven) on one controller.  These devices have a 
> wide range of
> > transfer parameters (speed, phase, polarity, internal and gpio chip
> > selects).  During my testing I found multiple errors in the 
> davinci spi
> > driver as a result of this complex setup.  The primary issues were:
> >
> > 1. There is a race condition due to the SPIBUF read 
> busy-waits for slow
> > devices
> > 2. I found some DMA transfer length errors under some conditions
> > 3. The chip select code caused extra byte transfers (with no chip
> > select active) due to writes to SPIDAT1
> > 4. Several issues prevented using multiple SPI devices, especially
> > the DMA code, as disucussed previously on the davinci list.
> 
> This should be replicated in patch description for patch 1/3 
> as a record
> of why the original driver was removed.

Will do.
 
> > The fixes to these problems were not simple.  I ended up 
> making fairly
> > large changes to the driver, and those changes are 
> contained in these
> > patches.  The full list of changes follows.
> >
> > CHANGE LIST
> >
> > 1. davinci_spi_chipelect() now performs both activation and 
> deactivation
> > of chip selects.  This lets spi_bitbang fully control chip
> > select activation, as intended by the SPI API.
> > 2. Chip select activation does not cause extra writes to the SPI bus
> > 3. Chip select activation does not use SPIDEF for control.  
> This change
> > will also allow for implementation of inverted (active high)
> > chip selects in the future.
> > 4. Added back gpio chip select capability from the old driver
> > 5. Fixed prescale calculation for non-integer fractions of spi clock
> > 6. Allow specification of SPI transfer parameters on a per-device
> > (instead of per-controller) basis
> > 7. Allow specification of polled, interrupt-based, or DMA 
> operation on
> > a per-device basis
> > 8. Allow DMA with when more than one device is connected
> > 9. Combined pio and dma txrx_bufs functions into one since 
> they share
> > large parts of their functionality, and to simplify 
> item (8).
> > 10. Use only SPIFMT0 to allow more than 4 devices
> >
> > TESTING
> >
> > I have tested the driver using a custom SPI stress test on my
> > OMAP-L138-based board with three devices connected.  I have tested
> > configurations with all three devices polled, all three 
> interrupt-based,
> > all three DMA, and a mixture.
> 
> This should be replicated in patch description for patch 2/3 
> as a record
> of what the new driver provides.

Will do.

> > I have compiled with the davinci_all_defconfig, but I don't 
> have EVMs
> > for the other davinci platforms to test with.
> 
> DM6467, DM365 and OMAP-L137 EVMs have SPI devices as well. We can help
> test on at least some of those. No need to wait for test 
> results before
> posting to SPI list though, it can happen in parallel to the review.

Ok, thanks.

> > SUMMARY
> >
> > This patch solves a lot of issues that should save a lot of 
> people time
> > down the road.  Since I posted the original patch I have 
> had at least 5
> > people contact me personally to get help applying the patch 
> because SPI
> > was broken on their boards.  I have heard back from at 
> least 2 that the
> > original patch worked for them.
> 
> You can include an Acked-by: from those developers. This will 
> help build
> the case for the new driver.

I'll see if I still have the emails.

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


RE: [PATCH 0/3] davinci: spi: replace existing SPI driver

2010-06-15 Thread Marc-Andre Chenier
   1. RE: [PATCH 0/3] davinci: spi: replace existing SPI driver
  (Brian

Brian,
Just wanted to say that I'm glad you posted the patch here to fixe many bugs in 
the Davinci SPI driver to the Davinci GIT kernel tree.  I am using the Davinci 
GIT kernel but with your equivalent SPI patches from the Arago Git kernel and 
it works pretty well for my cases.  Only tested the polling mode with a 
OMAP-L138 EVM. Thanks.




This email and any files transmitted with it are confidential and intended 
solely for the use of the individual or entity to whom they are addressed. If 
you have received this email in error please notify the system manager. 
 
Afin de combattre les pourriels, DALSA ne retournera plus d'avis d'echec 
d'envoi pour les adresses invalides. Ce courriel ainsi que ses pieces jointes 
sont strictement reserves a l'usage de la ou du destinataire et peut contenir 
de l'information privilegiee et confidentielle. Si vous avez recu cette 
correspondance par erreur, veuillez supprimer le message. Merci.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


RE: [PATCH 0/3] davinci: spi: replace existing SPI driver

2010-06-15 Thread Sudhakar Rajashekhara
Hi,

On Tue, Jun 15, 2010 at 00:43:06, Brian Niebuhr wrote:
> > > I have compiled with the davinci_all_defconfig, but I don't 
> > have EVMs
> > > for the other davinci platforms to test with.
> > > 
> > 
> > I was testing this series on OMAP-L137, OMAP-L138 and DM365 EVMs and I
> > found that on all these EVMS, Interrupt mode has problems. 
> > Driver works
> > fine in DMA and POLL modes. Some problems which I had 
> > observed with the
> > old driver are gone now.
> 
> Do you have any idea what errors you're seeing in interrupt mode?  I
> went and retested interrupt mode with several devices on my custom board
> and it still appeared to be working fine.  It may be an interaction with
> code outside the driver because I'm running the identical driver code
> but I'm not running the latest kernel from the davinci git repository.
> I'm trying to figure out if I can get my DA850 EVM back into a state
> where I can debug this.  Thanks for any hints you can give!  
> 

You may be knowing that on OMAP-L138 EVM, we have M25P64 Flash on SPI. When
the driver is configured in Interrupt mode, kernel booting just hangs. I
noticed that the SPI probe function is not coming out of spi_bitbang_start()
call.

Thanks,
Sudhakar


___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


RE: [PATCH 0/3] davinci: spi: replace existing SPI driver

2010-06-15 Thread Nori, Sekhar
Hi Marc-Andre,

On Tue, Jun 15, 2010 at 16:14:18, Marc-Andre Chenier wrote:
>1. RE: [PATCH 0/3] davinci: spi: replace existing SPI driver
>   (Brian
>
> Brian,
> Just wanted to say that I'm glad you posted the patch here to fixe many bugs 
> in the Davinci SPI driver to the Davinci GIT kernel tree.  I am using the 
> Davinci GIT kernel but with your equivalent SPI patches from the Arago Git 
> kernel and it works pretty well for my cases.  Only tested the polling mode 
> with a OMAP-L138 EVM. Thanks.
>

Can you please formally ack Brian's patches posted on the SPI mailing list
using:

Acked-by: Name 

This will make it easier for the work to get accepted upstream.

If you are not subscribed to the SPI mailing list, please use this link:

http://post.gmane.org/post.php?group=gmane.linux.kernel.spi.devel&followup=5595

Brian,

Can you please keep the DaVinci list copied on the posts to SPI list? This
will make it easier for folks on this list to ack your patches.

Thanks,
Sekhar

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source