| Message ID | 1409648557-5470-4-git-send-email-Barry.Song@csr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 02, 2014 at 05:02:37PM +0800, Barry Song wrote: > From: Qipan Li <Qipan.Li@csr.com> > > some customers want a pure PIO transfer mode even though DMA mode is > supported. here we give them a Kconfig option, but still keep the DMA > enabled in default. This doesn't feel like something for the standard kernel - can you explain the benefit the customers think they're getting here? It seems like either this is something we should have as a more general option, perhaps controllable at runtime or there's likely to be some bug or other problem that's being worked around here.
2014-09-05 6:41 GMT+08:00 Mark Brown <broonie@kernel.org>: > On Tue, Sep 02, 2014 at 05:02:37PM +0800, Barry Song wrote: >> From: Qipan Li <Qipan.Li@csr.com> >> >> some customers want a pure PIO transfer mode even though DMA mode is >> supported. here we give them a Kconfig option, but still keep the DMA >> enabled in default. > > This doesn't feel like something for the standard kernel - can you > explain the benefit the customers think they're getting here? It seems > like either this is something we should have as a more general option, > perhaps controllable at runtime or there's likely to be some bug or > other problem that's being worked around here. i have no strong technical reason for this as the benefit is not always technical. you know....the commit log has provided the reason for this patch. the difficulty for me is maintaining the difference between mainline and local codes, but if we have to maintain two copies, we can. -barry
On Fri, Sep 05, 2014 at 04:14:17PM +0800, Barry Song wrote: > i have no strong technical reason for this as the benefit is not > always technical. you know....the commit log has provided the reason > for this patch. > the difficulty for me is maintaining the difference between mainline > and local codes, but if we have to maintain two copies, we can. OK, well I think that's best though it would be better if we were able to understand what benefit your customers believe they are seeing here and hopefully address it in a better way. Doing this sort of thing with Kconfig options is not great, especially when you start trying to build kernels that work with more than one board.
On 14-9-6 ??9:55, "Mark Brown" <broonie@kernel.org> wrote: >On Fri, Sep 05, 2014 at 04:14:17PM +0800, Barry Song wrote: > >> i have no strong technical reason for this as the benefit is not >> always technical. you know....the commit log has provided the reason >> for this patch. >> the difficulty for me is maintaining the difference between mainline >> and local codes, but if we have to maintain two copies, we can. > >OK, well I think that's best though it would be better if we were able >to understand what benefit your customers believe they are seeing here >and hopefully address it in a better way. Doing this sort of thing with >Kconfig options is not great, especially when you start trying to build >kernels that work with more than one board. my goal is doubling-check this with customers, if there is no problem, drop this one as it is not a valid technical solution. -barry
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 62e2242..0f84bd8 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -477,6 +477,16 @@ config SPI_SIRF help SPI driver for CSR SiRFprimaII SoCs +config SPI_SIRF_DMA + bool "CSR SiRFprimaII/AtlasVI SPI DMA mode support" + depends on SPI_SIRF + default y + help + CSR SPI driver support DMA and PIO mode, select it let SPI driver support DMA mode + deselect it let SPI controller in PIO mode. + In CSR SPI driver distinguish the config to decide what transfer mode use. + Default configure value is y, if want pure PIO mode deselect it will be OK. + config SPI_SUN4I tristate "Allwinner A10 SoCs SPI controller" depends on ARCH_SUNXI || COMPILE_TEST diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c index 67d07b2..f934159 100644 --- a/drivers/spi/spi-sirf.c +++ b/drivers/spi/spi-sirf.c @@ -333,6 +333,7 @@ static void spi_sirfsoc_cmd_transfer(struct spi_device *spi, sspi->left_rx_word -= t->len; } +#ifdef CONFIG_SPI_SIRF_DMA static void spi_sirfsoc_dma_transfer(struct spi_device *spi, struct spi_transfer *t) { @@ -407,6 +408,7 @@ static void spi_sirfsoc_dma_transfer(struct spi_device *spi, if (sspi->left_tx_word >= SIRFSOC_SPI_DAT_FRM_LEN_MAX) writel(0, sspi->base + SIRFSOC_SPI_TX_RX_EN); } +#endif static void spi_sirfsoc_pio_transfer(struct spi_device *spi, struct spi_transfer *t) @@ -474,8 +476,10 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t) */ if (sspi->tx_by_cmd) spi_sirfsoc_cmd_transfer(spi, t); +#ifdef CONFIG_SPI_SIRF_DMA else if (IS_DMA_VALID(t)) spi_sirfsoc_dma_transfer(spi, t); +#endif else spi_sirfsoc_pio_transfer(spi, t); @@ -526,6 +530,11 @@ spi_sirfsoc_setup_transfer(struct spi_device *spi, struct spi_transfer *t) u32 regval; u32 txfifo_ctrl, rxfifo_ctrl; u32 fifo_size = SIRFSOC_SPI_FIFO_SIZE / 4; +#ifdef CONFIG_SPI_SIRF_DMA + int spi_use_dma = 1; +#else + int spi_use_dma = 0; +#endif sspi = spi_master_get_devdata(spi->master); @@ -610,7 +619,7 @@ spi_sirfsoc_setup_transfer(struct spi_device *spi, struct spi_transfer *t) regval |= SIRFSOC_SPI_CS_IO_MODE; writel(regval, sspi->base + SIRFSOC_SPI_CTRL); - if (IS_DMA_VALID(t)) { + if (spi_use_dma && IS_DMA_VALID(t)) { /* Enable DMA mode for RX, TX */ writel(0, sspi->base + SIRFSOC_SPI_TX_DMA_IO_CTRL); writel(SIRFSOC_SPI_RX_DMA_FLUSH,