* [PATCH 2/2] ARM: at91: dt: at91sam9x5: add SPI DMA client infos
2013-05-31 15:01 [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat() Richard Genoud
@ 2013-05-31 15:02 ` Richard Genoud
2013-05-31 15:08 ` [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat() Jean-Christophe PLAGNIOL-VILLARD
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Richard Genoud @ 2013-05-31 15:02 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat()
2013-05-31 15:01 [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat() Richard Genoud
2013-05-31 15:02 ` [PATCH 2/2] ARM: at91: dt: at91sam9x5: add SPI DMA client infos Richard Genoud
@ 2013-05-31 15:08 ` Jean-Christophe PLAGNIOL-VILLARD
2013-06-03 8:37 ` Ludovic Desroches
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-31 15:08 UTC (permalink / raw)
To: linux-arm-kernel
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 <ludovic.desroches@atmel.com> patchset
> "ARM: at91: move to generic DMA device tree binding"
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat()
2013-05-31 15:01 [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat() Richard Genoud
2013-05-31 15:02 ` [PATCH 2/2] ARM: at91: dt: at91sam9x5: add SPI DMA client infos Richard Genoud
2013-05-31 15:08 ` [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat() Jean-Christophe PLAGNIOL-VILLARD
@ 2013-06-03 8:37 ` Ludovic Desroches
[not found] ` <51ac5610.87ebc20a.4198.ffffdc38SMTPIN_ADDED_BROKEN@mx.google.com>
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Ludovic Desroches @ 2013-06-03 8:37 UTC (permalink / raw)
To: linux-arm-kernel
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 <ludovic.desroches@atmel.com> patchset
> "ARM: at91: move to generic DMA device tree binding"
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Looks fine for me to so
Acked-by: Ludovic Desroches <ludovic.desroches@gmail.com>
One comment 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.
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread[parent not found: <51ac5610.87ebc20a.4198.ffffdc38SMTPIN_ADDED_BROKEN@mx.google.com>]
* [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat()
[not found] ` <51ac5610.87ebc20a.4198.ffffdc38SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2013-06-03 10:31 ` Richard Genoud
0 siblings, 0 replies; 7+ messages in thread
From: Richard Genoud @ 2013-06-03 10:31 UTC (permalink / raw)
To: linux-arm-kernel
2013/6/3 Ludovic Desroches <ludovic.desroches@atmel.com>:
> 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 <ludovic.desroches@atmel.com> patchset
>> "ARM: at91: move to generic DMA device tree binding"
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>
> Looks fine for me to so
> Acked-by: Ludovic Desroches <ludovic.desroches@gmail.com>
>
> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat()
2013-05-31 15:01 [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat() Richard Genoud
` (3 preceding siblings ...)
[not found] ` <51ac5610.87ebc20a.4198.ffffdc38SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2013-06-03 15:22 ` Mark Brown
2013-06-04 2:43 ` Yang Wenyou
5 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2013-06-03 15:22 UTC (permalink / raw)
To: linux-arm-kernel
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 <ludovic.desroches@atmel.com> 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>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat()
2013-05-31 15:01 [PATCH 1/2] spi: atmel: convert to dma_request_slave_channel_compat() Richard Genoud
` (4 preceding siblings ...)
2013-06-03 15:22 ` Mark Brown
@ 2013-06-04 2:43 ` Yang Wenyou
5 siblings, 0 replies; 7+ messages in thread
From: Yang Wenyou @ 2013-06-04 2:43 UTC (permalink / raw)
To: linux-arm-kernel
Richard Genoud wrote:
> Use generic DMA DT helper.
> Platforms booting with or without DT populated are both supported.
>
> Based on Ludovic Desroches <ludovic.desroches@atmel.com> patchset
> "ARM: at91: move to generic DMA device tree binding"
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
> 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<wenyou.yang@atmel.com>
> 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;
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread