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]>
---
drivers/spi/spi-atmel.c | 42 +++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 15 deletions(-)
rebased on linux-next next-20130531
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 31cfc87..ea1ec00 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -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;
}
--
1.7.10.4
Signed-off-by: Richard Genoud <[email protected]>
---
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
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index af91599..a5942d0 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -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";
--
1.7.10.4
On 17:01 Fri 31 May , Richard Genoud wrote:
> 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.
> ---
> drivers/spi/spi-atmel.c | 42 +++++++++++++++++++++++++++---------------
> 1 file changed, 27 insertions(+), 15 deletions(-)
>
> rebased on linux-next next-20130531
>
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 31cfc87..ea1ec00 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -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;
> }
> --
> 1.7.10.4
>
2013/6/3 Ludovic Desroches <[email protected]>:
> On Fri, May 31, 2013 at 05:01:59PM +0200, Richard Genoud wrote:
>> 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
> Acked-by: Ludovic Desroches <[email protected]>
>
> One comment below
>
Response below.
>> ---
>> drivers/spi/spi-atmel.c | 42 +++++++++++++++++++++++++++---------------
>> 1 file changed, 27 insertions(+), 15 deletions(-)
>>
>> rebased on linux-next next-20130531
>>
>> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
>> index 31cfc87..ea1ec00 100644
>> --- a/drivers/spi/spi-atmel.c
>> +++ b/drivers/spi/spi-atmel.c
>> @@ -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) {
>
> 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.
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.
>> 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;
>> }
>> --
>> 1.7.10.4
>>
Best regards,
Richard.
On Fri, May 31, 2013 at 05:01:59PM +0200, Richard Genoud wrote:
> 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.
Richard Genoud wrote:
> 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]>
> ---
> 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"
Tested-by: Wenyou Yang<[email protected]>
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 31cfc87..ea1ec00 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -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;
> }
>