spi: atmel: convert to dma_request_slave_channel_compat()
PATCH 1/2 spi: atmel: convert to dma_request_slave_channel_compat()
Use generic DMA DT helper.
Platforms booting with or without DT populated are both supported.
Based on Ludovic Desroches <[email protected]> patchset "ARM: at91: move to generic DMA device tree binding"
--- drivers/spi/spi-atmel.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-)
rebased on linux-next next-20130531
drivers/spi/spi-atmel.c +27 -15
@@ -424,10 +424,15 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,return err; } -static bool filter (struct dma_chan *chan,void *slave) +static bool filter (struct dma_chan *chan,void *pdata) { -struct at_dma_slave *sl = slave; +struct atmel_spi_dma *sl_pdata = pdata; +struct at_dma_slave *sl; +if (!sl_pdata) +return false; + + sl = &sl_pdata->dma_slave;if (sl->dma_dev == chan->device->dev) { chan->private = sl;return true;
@@ -438,24 +443,31 @@ static bool filter(struct dma_chan *chan, void *slave)static int atmel_spi_configure_dma (struct atmel_spi *as) { -struct at_dma_slave *sdata = &as->dma.dma_slave;struct dma_slave_config slave_config; +struct device *dev = &as->pdev->dev;int err; -if (sdata && sdata->dma_dev) { -dma_cap_mask_t mask; +dma_cap_mask_t mask; +dma_cap_zero (mask); +dma_cap_set (DMA_SLAVE , mask); -/* Try to grab two DMA channels */ -dma_cap_zero (mask); -dma_cap_set (DMA_SLAVE , mask); - as->dma.chan_tx =dma_request_channel (mask, filter, sdata); -if (as->dma.chan_tx) - as->dma.chan_rx = -dma_request_channel (mask, filter, sdata); + as->dma.chan_tx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"tx" ); +if (!as->dma.chan_tx) { +dev_err (dev, +"DMA TX channel not available, SPI unable to use DMA\n" ); + err = -EBUSY ; + goto error; } -if (!as->dma.chan_rx || !as->dma.chan_tx) { -dev_err (&as->pdev->dev, -"DMA channel not available, SPI unable to use DMA\n" ); + + as->dma.chan_rx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"rx" ); + +if (!as->dma.chan_rx) { +dev_err (dev, +"DMA RX channel not available, SPI unable to use DMA\n" ); err = -EBUSY ; goto error; }
Signature · 1 line
1.7.10.4
PATCH 2/2 ARM: at91: dt: at91sam9x5: add SPI DMA client infos
--- arch/arm/boot/dts/at91sam9x5.dtsi | 6 ++++++ 1 file changed, 6 insertions(+)
rebased on linux-next next-20130531
plus Ludovic Desroches' patch "ARM: at91: dt: add header to define at_hdmac configuration" to get the AT91_DMA_CFG_PER_ID() macro
arch/arm/boot/dts/at91sam9x5.dtsi +6 -0
@@ -629,6 +629,9 @@ compatible = "atmel,at91rm9200-spi"; reg = <0xf0000000 0x100>; interrupts = <13 IRQ_TYPE_LEVEL_HIGH 3>; + dmas = <&dma0 1 AT91_DMA_CFG_PER_ID(1)>, + <&dma0 1 AT91_DMA_CFG_PER_ID(2)>; + dma-names = "tx", "rx"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_spi0>; status = "disabled";
@@ -640,6 +643,9 @@ compatible = "atmel,at91rm9200-spi"; reg = <0xf0004000 0x100>; interrupts = <14 IRQ_TYPE_LEVEL_HIGH 3>; + dmas = <&dma1 1 AT91_DMA_CFG_PER_ID(1)>, + <&dma1 1 AT91_DMA_CFG_PER_ID(2)>; + dma-names = "tx", "rx"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_spi1>; status = "disabled";
Signature · 1 line
1.7.10.4
Quoted 7 lines · Richard Genoud
Use generic DMA DT helper.
Platforms booting with or without DT populated are both supported.Based on Ludovic Desroches <[email protected]> patchset "ARM: at91: move to generic DMA device tree binding"
Signed-off-by Richard Genoud <[email protected]>
Ludo please take a look but this looks ok to me
Best Regards,
J.
Quoted 76 lines
--- drivers/spi/spi-atmel.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-)rebased on linux-next next-20130531
drivers/spi/spi-atmel.c +27
-15@@ -424,10 +424,15 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,return err; } -static bool filter (struct dma_chan *chan,void *slave) +static bool filter (struct dma_chan *chan,void *pdata) { -struct at_dma_slave *sl = slave; +struct atmel_spi_dma *sl_pdata = pdata; +struct at_dma_slave *sl; +if (!sl_pdata) +return false; + + sl = &sl_pdata->dma_slave;if (sl->dma_dev == chan->device->dev) { chan->private = sl;return true;@@ -438,24 +443,31 @@ static bool filter(struct dma_chan *chan, void *slave)static int atmel_spi_configure_dma (struct atmel_spi *as) { -struct at_dma_slave *sdata = &as->dma.dma_slave;struct dma_slave_config slave_config; +struct device *dev = &as->pdev->dev;int err; -if (sdata && sdata->dma_dev) { -dma_cap_mask_t mask; +dma_cap_mask_t mask; +dma_cap_zero (mask); +dma_cap_set (DMA_SLAVE , mask); -/* Try to grab two DMA channels */ -dma_cap_zero (mask); -dma_cap_set (DMA_SLAVE , mask); - as->dma.chan_tx =dma_request_channel (mask, filter, sdata); -if (as->dma.chan_tx) - as->dma.chan_rx = -dma_request_channel (mask, filter, sdata); + as->dma.chan_tx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"tx" ); +if (!as->dma.chan_tx) { +dev_err (dev, +"DMA TX channel not available, SPI unable to use DMA\n" ); + err = -EBUSY ; + goto error; } -if (!as->dma.chan_rx || !as->dma.chan_tx) { -dev_err (&as->pdev->dev, -"DMA channel not available, SPI unable to use DMA\n" ); + + as->dma.chan_rx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"rx" ); + +if (!as->dma.chan_rx) { +dev_err (dev, +"DMA RX channel not available, SPI unable to use DMA\n" ); err = -EBUSY ; goto error; }Signature · 1 line
1.7.10.4
PATCH 1/2 spi: atmel: convert to dma_request_slave_channel_compat()
In reply to Richard GenoudQuoted 7 lines · Richard Genoud
Use generic DMA DT helper.
Platforms booting with or without DT populated are both supported.Based on Ludovic Desroches <[email protected]> patchset "ARM: at91: move to generic DMA device tree binding"
Signed-off-by Richard Genoud <[email protected]>
Looks fine for me to so
One comment below
Quoted 16 lines
--- drivers/spi/spi-atmel.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-)rebased on linux-next next-20130531
drivers/spi/spi-atmel.c +1
-1@@ -424,10 +424,15 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,return err; } -static bool filter (struct dma_chan *chan,void *slave) +static bool filter (struct dma_chan *chan,void *pdata)
drivers/spi/spi-atmel.c static int atmel_spi_dma_slave_config(struct atmel_spi *as, · line 436
Quoted 10 lines
drivers/spi/spi-atmel.c +6
-1{ -struct at_dma_slave *sl = slave; +struct atmel_spi_dma *sl_pdata = pdata; +struct at_dma_slave *sl; +if (!sl_pdata) +return false; + + sl = &sl_pdata->dma_slave;if (sl->dma_dev == chan->device->dev) {
I am wondering if a null pointer dereference can happen here. In atmel_spi_configure_dma sdata was checked so maybe sl should be check here.
Quoted 50 lines
drivers/spi/spi-atmel.c +20
-13chan->private = sl;return true;@@ -438,24 +443,31 @@ static bool filter(struct dma_chan *chan, void *slave)static int atmel_spi_configure_dma (struct atmel_spi *as) { -struct at_dma_slave *sdata = &as->dma.dma_slave;struct dma_slave_config slave_config; +struct device *dev = &as->pdev->dev;int err; -if (sdata && sdata->dma_dev) { -dma_cap_mask_t mask; +dma_cap_mask_t mask; +dma_cap_zero (mask); +dma_cap_set (DMA_SLAVE , mask); -/* Try to grab two DMA channels */ -dma_cap_zero (mask); -dma_cap_set (DMA_SLAVE , mask); - as->dma.chan_tx =dma_request_channel (mask, filter, sdata); -if (as->dma.chan_tx) - as->dma.chan_rx = -dma_request_channel (mask, filter, sdata); + as->dma.chan_tx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"tx" ); +if (!as->dma.chan_tx) { +dev_err (dev, +"DMA TX channel not available, SPI unable to use DMA\n" ); + err = -EBUSY ; + goto error; } -if (!as->dma.chan_rx || !as->dma.chan_tx) { -dev_err (&as->pdev->dev, -"DMA channel not available, SPI unable to use DMA\n" ); + + as->dma.chan_rx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"rx" ); + +if (!as->dma.chan_rx) { +dev_err (dev, +"DMA RX channel not available, SPI unable to use DMA\n" ); err = -EBUSY ; goto error; }Signature · 1 line
1.7.10.4
A message that is not in the archive (sent off-list or not yet received): [email protected]
2013/6/3 Ludovic Desroches <[email protected]>:
Quoted 7 lines · Richard Genoud
Quoted 6 lines
Use generic DMA DT helper.
Platforms booting with or without DT populated are both supported.Based on Ludovic Desroches <[email protected]> patchset "ARM: at91: move to generic DMA device tree binding"
Quoted 7 lines
Quoted 1 line
Signed-off-by Richard Genoud <[email protected]>Looks fine for me to so
One comment below
Response below.
Quoted 21 lines
Quoted 21 lines
--- drivers/spi/spi-atmel.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-)rebased on linux-next next-20130531
drivers/spi/spi-atmel.c +3
-2@@ -424,10 +424,15 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,return err; } -static bool filter (struct dma_chan *chan,void *slave) +static bool filter (struct dma_chan *chan,void *pdata) { -struct at_dma_slave *sl = slave; +struct atmel_spi_dma *sl_pdata = pdata; +struct at_dma_slave *sl;
Quoted 9 lines
drivers/spi/spi-atmel.c static int atmel_spi_dma_slave_config(struct atmel_spi *as, · line 436
Quoted 5 lines
drivers/spi/spi-atmel.c +4
-0+if (!sl_pdata) +return false; + + sl = &sl_pdata->dma_slave;if (sl->dma_dev == chan->device->dev) {I am wondering if a null pointer dereference can happen here. In
atmel_spi_configure_dmasdata was checked so maybe sl should be check here.
sdata was checked, but there was no point checking it:
if as is not null, as->dma.dma_slave is defined ; so its address is not null. it's the same in the filter function now:
if sl_pdata is not null, sl_pdata->dma_slave is defined ; so its address is not null.
I added a check on sl_pdata in this patch, maybe a (BUG|WARN)_ON() would have been better, as it really should not happen.
Quoted 42 lines
Quoted 42 lines
drivers/spi/spi-atmel.c +18
-13chan->private = sl;return true;@@ -438,24 +443,31 @@ static bool filter(struct dma_chan *chan, void *slave)static int atmel_spi_configure_dma (struct atmel_spi *as) { -struct at_dma_slave *sdata = &as->dma.dma_slave;struct dma_slave_config slave_config; +struct device *dev = &as->pdev->dev;int err; -if (sdata && sdata->dma_dev) { -dma_cap_mask_t mask; +dma_cap_mask_t mask; +dma_cap_zero (mask); +dma_cap_set (DMA_SLAVE , mask); -/* Try to grab two DMA channels */ -dma_cap_zero (mask); -dma_cap_set (DMA_SLAVE , mask); - as->dma.chan_tx =dma_request_channel (mask, filter, sdata); -if (as->dma.chan_tx) - as->dma.chan_rx = -dma_request_channel (mask, filter, sdata); + as->dma.chan_tx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"tx" ); +if (!as->dma.chan_tx) { +dev_err (dev, +"DMA TX channel not available, SPI unable to use DMA\n" ); + err = -EBUSY ; + goto error; } -if (!as->dma.chan_rx || !as->dma.chan_tx) { -dev_err (&as->pdev->dev, -"DMA channel not available, SPI unable to use DMA\n" ); + + as->dma.chan_rx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"rx" ); + +if (!as->dma.chan_rx) {
Quoted 8 lines
Quoted 8 lines
drivers/spi/spi-atmel.c +2
-1+dev_err (dev, +"DMA RX channel not available, SPI unable to use DMA\n" ); err = -EBUSY ; goto error; } --1.7.10.4
Best regards,
Richard.
Mark Brown
maintainerQuoted 5 lines · Richard Genoud
Use generic DMA DT helper.
Platforms booting with or without DT populated are both supported.Based on Ludovic Desroches <[email protected]> patchset "ARM: at91: move to generic DMA device tree binding"
Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130603/6956c3f8/attachment.sig>
Quoted 6 lines · Richard Genoud
Use generic DMA DT helper.
Platforms booting with or without DT populated are both supported.Based on Ludovic Desroches <[email protected]> patchset "ARM: at91: move to generic DMA device tree binding"
Quoted 7 lines
Signed-off-by Richard Genoud <[email protected]>--- drivers/spi/spi-atmel.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-)rebased on linux-next next-20130531
Tested on at91sam9g25ek,
Based on linux-next, next-20130603
plus Ludovic Desroches' patch "ARM: at91: dt: add header to define at_hdmac configuration"
Quoted 68 lines
drivers/spi/spi-atmel.c +27
-15@@ -424,10 +424,15 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,return err; } -static bool filter (struct dma_chan *chan,void *slave) +static bool filter (struct dma_chan *chan,void *pdata) { -struct at_dma_slave *sl = slave; +struct atmel_spi_dma *sl_pdata = pdata; +struct at_dma_slave *sl; +if (!sl_pdata) +return false; + + sl = &sl_pdata->dma_slave;if (sl->dma_dev == chan->device->dev) { chan->private = sl;return true;@@ -438,24 +443,31 @@ static bool filter(struct dma_chan *chan, void *slave)static int atmel_spi_configure_dma (struct atmel_spi *as) { -struct at_dma_slave *sdata = &as->dma.dma_slave;struct dma_slave_config slave_config; +struct device *dev = &as->pdev->dev;int err; -if (sdata && sdata->dma_dev) { -dma_cap_mask_t mask; +dma_cap_mask_t mask; +dma_cap_zero (mask); +dma_cap_set (DMA_SLAVE , mask); -/* Try to grab two DMA channels */ -dma_cap_zero (mask); -dma_cap_set (DMA_SLAVE , mask); - as->dma.chan_tx =dma_request_channel (mask, filter, sdata); -if (as->dma.chan_tx) - as->dma.chan_rx = -dma_request_channel (mask, filter, sdata); + as->dma.chan_tx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"tx" ); +if (!as->dma.chan_tx) { +dev_err (dev, +"DMA TX channel not available, SPI unable to use DMA\n" ); + err = -EBUSY ; + goto error; } -if (!as->dma.chan_rx || !as->dma.chan_tx) { -dev_err (&as->pdev->dev, -"DMA channel not available, SPI unable to use DMA\n" ); + + as->dma.chan_rx =dma_request_slave_channel_compat (mask, filter, + &as->dma, + dev,"rx" ); + +if (!as->dma.chan_rx) { +dev_err (dev, +"DMA RX channel not available, SPI unable to use DMA\n" ); err = -EBUSY ; goto error; }