diff mbox

[8/8] spi: sirf: make DMA transfer mode optional

Message ID 1409648557-5470-4-git-send-email-Barry.Song@csr.com (mailing list archive)
State New, archived
Headers show

Commit Message

Barry Song Sept. 2, 2014, 9:02 a.m. UTC
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.

Signed-off-by: Qipan Li <Qipan.Li@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/spi/Kconfig    | 10 ++++++++++
 drivers/spi/spi-sirf.c | 11 ++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

Comments

Mark Brown Sept. 4, 2014, 10:41 p.m. UTC | #1
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.
Barry Song Sept. 5, 2014, 8:14 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Sept. 6, 2014, 1:55 p.m. UTC | #3
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.
Barry Song Sept. 7, 2014, 12:55 a.m. UTC | #4
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



--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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,