RE: [PATCH 0/3] davinci: spi: replace existing SPI driver
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
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
> > 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
> 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
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
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
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