2015-12-09 08:18:48

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH for 4.4 0/2] DT/dmaengine: edma: Convert 16bit arrays to 32bit

Hi,

Based on the discussion regarding to (convert am33xx to use the new eDMA
bindings):
https://www.mail-archive.com/[email protected]/msg122117.html

This two patch will convert the new eDMA binding to not use 16bit arrays for
memcpy channel selection and for marking slots reserved.
The '/bits/ 16' seams to be causing confusion so it is probably better to just
use standard type for the arrays.

The new bindings for the eDMA is introduced for 4.4 and we do not have users of
it, which means that we can still change it w/o the risk of breaking anything
and we do not need to maintain the compatibility with 16bit arrays.

The changes in the eDMA driver is local to the DT parsing and should not
conflict with other changes (like the filter function mapping support). Hrm,
there might be trivial conflict in the include/linux/platform_data/edma.h with
the "dmaengine 'universal' API".

Tony, Arnd, Vinod: Can you agree on the practicalities on how these patches are
going to be handled? I would like to send the updated am33xx/am437x conversion
for 4.5 based on these changes.

Thanks and regards,
Peter
---
Peter Ujfalusi (2):
dmaengine: edma: DT: Change memcpy channel array from 16bit to 32bit
type
dmaengine: edma: DT: Change reserved slot array from 16bit to 32bit
type

Documentation/devicetree/bindings/dma/ti-edma.txt | 10 ++---
drivers/dma/edma.c | 53 +++++++++++++++--------
include/linux/platform_data/edma.h | 2 +-
3 files changed, 40 insertions(+), 25 deletions(-)

--
2.6.3


2015-12-09 08:18:56

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH for 4.4 1/2] dmaengine: edma: DT: Change memcpy channel array from 16bit to 32bit type

This change makes the DT file to be easier to read since the memcpy
channels array does not need the '/bits/ 16' to be specified, which might
confuse some people.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
Documentation/devicetree/bindings/dma/ti-edma.txt | 5 ++---
drivers/dma/edma.c | 22 ++++++++++------------
include/linux/platform_data/edma.h | 2 +-
3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt b/Documentation/devicetree/bindings/dma/ti-edma.txt
index d3d0a4fb1c73..ae8b8f1d6e69 100644
--- a/Documentation/devicetree/bindings/dma/ti-edma.txt
+++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
@@ -22,8 +22,7 @@ Required properties:
Optional properties:
- ti,hwmods: Name of the hwmods associated to the eDMA CC
- ti,edma-memcpy-channels: List of channels allocated to be used for memcpy, iow
- these channels will be SW triggered channels. The list must
- contain 16 bits numbers, see example.
+ these channels will be SW triggered channels. See example.
- ti,edma-reserved-slot-ranges: PaRAM slot ranges which should not be used by
the driver, they are allocated to be used by for example the
DSP. See example.
@@ -56,7 +55,7 @@ edma: edma@49000000 {
ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 7>, <&edma_tptc2 0>;

/* Channel 20 and 21 is allocated for memcpy */
- ti,edma-memcpy-channels = /bits/ 16 <20 21>;
+ ti,edma-memcpy-channels = <20 21>;
/* The following PaRAM slots are reserved: 35-45 and 100-110 */
ti,edma-reserved-slot-ranges = /bits/ 16 <35 10>,
/bits/ 16 <100 10>;
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 2af8f32bba0b..89fc17f3a6ff 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -1752,16 +1752,14 @@ static enum dma_status edma_tx_status(struct dma_chan *chan,
return ret;
}

-static bool edma_is_memcpy_channel(int ch_num, u16 *memcpy_channels)
+static bool edma_is_memcpy_channel(int ch_num, s32 *memcpy_channels)
{
- s16 *memcpy_ch = memcpy_channels;
-
if (!memcpy_channels)
return false;
- while (*memcpy_ch != -1) {
- if (*memcpy_ch == ch_num)
+ while (*memcpy_channels != -1) {
+ if (*memcpy_channels == ch_num)
return true;
- memcpy_ch++;
+ memcpy_channels++;
}
return false;
}
@@ -1775,7 +1773,7 @@ static void edma_dma_init(struct edma_cc *ecc, bool legacy_mode)
{
struct dma_device *s_ddev = &ecc->dma_slave;
struct dma_device *m_ddev = NULL;
- s16 *memcpy_channels = ecc->info->memcpy_channels;
+ s32 *memcpy_channels = ecc->info->memcpy_channels;
int i, j;

dma_cap_zero(s_ddev->cap_mask);
@@ -1996,16 +1994,16 @@ static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
prop = of_find_property(dev->of_node, "ti,edma-memcpy-channels", &sz);
if (prop) {
const char pname[] = "ti,edma-memcpy-channels";
- size_t nelm = sz / sizeof(s16);
- s16 *memcpy_ch;
+ size_t nelm = sz / sizeof(s32);
+ s32 *memcpy_ch;

- memcpy_ch = devm_kcalloc(dev, nelm + 1, sizeof(s16),
+ memcpy_ch = devm_kcalloc(dev, nelm + 1, sizeof(s32),
GFP_KERNEL);
if (!memcpy_ch)
return ERR_PTR(-ENOMEM);

- ret = of_property_read_u16_array(dev->of_node, pname,
- (u16 *)memcpy_ch, nelm);
+ ret = of_property_read_u32_array(dev->of_node, pname,
+ (u32 *)memcpy_ch, nelm);
if (ret)
return ERR_PTR(ret);

diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index 105700e62ea1..0a533f94438f 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -76,7 +76,7 @@ struct edma_soc_info {
struct edma_rsv_info *rsv;

/* List of channels allocated for memcpy, terminated with -1 */
- s16 *memcpy_channels;
+ s32 *memcpy_channels;

s8 (*queue_priority_mapping)[2];
const s16 (*xbar_chans)[2];
--
2.6.3

2015-12-09 08:19:08

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH for 4.4 2/2] dmaengine: edma: DT: Change reserved slot array from 16bit to 32bit type

This change makes the DT file to be easier to read since the reserved slots
array does not need the '/bits/ 16' to be specified, which might confuse
some people.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
Documentation/devicetree/bindings/dma/ti-edma.txt | 5 ++--
drivers/dma/edma.c | 31 ++++++++++++++++++-----
2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt b/Documentation/devicetree/bindings/dma/ti-edma.txt
index ae8b8f1d6e69..079b42a81d7c 100644
--- a/Documentation/devicetree/bindings/dma/ti-edma.txt
+++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
@@ -56,9 +56,8 @@ edma: edma@49000000 {

/* Channel 20 and 21 is allocated for memcpy */
ti,edma-memcpy-channels = <20 21>;
- /* The following PaRAM slots are reserved: 35-45 and 100-110 */
- ti,edma-reserved-slot-ranges = /bits/ 16 <35 10>,
- /bits/ 16 <100 10>;
+ /* The following PaRAM slots are reserved: 35-44 and 100-109 */
+ ti,edma-reserved-slot-ranges = <35 10>, <100 10>;
};

edma_tptc0: tptc@49800000 {
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 89fc17f3a6ff..a0f217349c07 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -2015,31 +2015,50 @@ static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
&sz);
if (prop) {
const char pname[] = "ti,edma-reserved-slot-ranges";
+ u32 (*tmp)[2];
s16 (*rsv_slots)[2];
- size_t nelm = sz / sizeof(*rsv_slots);
+ size_t nelm = sz / sizeof(*tmp);
struct edma_rsv_info *rsv_info;
+ int i;

if (!nelm)
return info;

+ tmp = kcalloc(nelm, sizeof(*tmp), GFP_KERNEL);
+ if (!tmp)
+ return ERR_PTR(-ENOMEM);
+
rsv_info = devm_kzalloc(dev, sizeof(*rsv_info), GFP_KERNEL);
- if (!rsv_info)
+ if (!rsv_info) {
+ kfree(tmp);
return ERR_PTR(-ENOMEM);
+ }

rsv_slots = devm_kcalloc(dev, nelm + 1, sizeof(*rsv_slots),
GFP_KERNEL);
- if (!rsv_slots)
+ if (!rsv_slots) {
+ kfree(tmp);
return ERR_PTR(-ENOMEM);
+ }

- ret = of_property_read_u16_array(dev->of_node, pname,
- (u16 *)rsv_slots, nelm * 2);
- if (ret)
+ ret = of_property_read_u32_array(dev->of_node, pname,
+ (u32 *)tmp, nelm * 2);
+ if (ret) {
+ kfree(tmp);
return ERR_PTR(ret);
+ }

+ for (i = 0; i < nelm; i++) {
+ rsv_slots[i][0] = tmp[i][0];
+ rsv_slots[i][1] = tmp[i][1];
+ }
rsv_slots[nelm][0] = -1;
rsv_slots[nelm][1] = -1;
+
info->rsv = rsv_info;
info->rsv->rsv_slots = (const s16 (*)[2])rsv_slots;
+
+ kfree(tmp);
}

return info;
--
2.6.3

2015-12-09 09:00:51

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH for 4.4 0/2] DT/dmaengine: edma: Convert 16bit arrays to 32bit

On Wednesday 09 December 2015 10:18:09 Peter Ujfalusi wrote:
> Hi,
>
> Based on the discussion regarding to (convert am33xx to use the new eDMA
> bindings):
> https://www.mail-archive.com/[email protected]/msg122117.html
>
> This two patch will convert the new eDMA binding to not use 16bit arrays for
> memcpy channel selection and for marking slots reserved.
> The '/bits/ 16' seams to be causing confusion so it is probably better to just
> use standard type for the arrays.
>
> The new bindings for the eDMA is introduced for 4.4 and we do not have users of
> it, which means that we can still change it w/o the risk of breaking anything
> and we do not need to maintain the compatibility with 16bit arrays.
>
> The changes in the eDMA driver is local to the DT parsing and should not
> conflict with other changes (like the filter function mapping support). Hrm,
> there might be trivial conflict in the include/linux/platform_data/edma.h with
> the "dmaengine 'universal' API".

Both patches

Acked-by: Arnd Bergmann <[email protected]>

> Tony, Arnd, Vinod: Can you agree on the practicalities on how these patches are
> going to be handled? I would like to send the updated am33xx/am437x conversion
> for 4.5 based on these changes.

I'd suggest you send a pull request with these two patches on top of 4.4-rc1,
and then either Vinod or I send it to Linus, and you base your other changes
on top of the same branch.

I think Vinod's tree is slightly more fitting for the 4.4 merge, but if he
prefers me to take them instead, I will.

Arnd

2015-12-09 20:02:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH for 4.4 1/2] dmaengine: edma: DT: Change memcpy channel array from 16bit to 32bit type

On Wed, Dec 09, 2015 at 10:18:10AM +0200, Peter Ujfalusi wrote:
> This change makes the DT file to be easier to read since the memcpy
> channels array does not need the '/bits/ 16' to be specified, which might
> confuse some people.

Why? I don't see the point of this change and plus you are breaking
compatibility with the change. There was little reason to do 16-bit
values to start with, but that's now the ABI.

Rob

> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/ti-edma.txt | 5 ++---
> drivers/dma/edma.c | 22 ++++++++++------------
> include/linux/platform_data/edma.h | 2 +-
> 3 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt b/Documentation/devicetree/bindings/dma/ti-edma.txt
> index d3d0a4fb1c73..ae8b8f1d6e69 100644
> --- a/Documentation/devicetree/bindings/dma/ti-edma.txt
> +++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
> @@ -22,8 +22,7 @@ Required properties:
> Optional properties:
> - ti,hwmods: Name of the hwmods associated to the eDMA CC
> - ti,edma-memcpy-channels: List of channels allocated to be used for memcpy, iow
> - these channels will be SW triggered channels. The list must
> - contain 16 bits numbers, see example.
> + these channels will be SW triggered channels. See example.
> - ti,edma-reserved-slot-ranges: PaRAM slot ranges which should not be used by
> the driver, they are allocated to be used by for example the
> DSP. See example.
> @@ -56,7 +55,7 @@ edma: edma@49000000 {
> ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 7>, <&edma_tptc2 0>;
>
> /* Channel 20 and 21 is allocated for memcpy */
> - ti,edma-memcpy-channels = /bits/ 16 <20 21>;
> + ti,edma-memcpy-channels = <20 21>;
> /* The following PaRAM slots are reserved: 35-45 and 100-110 */
> ti,edma-reserved-slot-ranges = /bits/ 16 <35 10>,
> /bits/ 16 <100 10>;
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 2af8f32bba0b..89fc17f3a6ff 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -1752,16 +1752,14 @@ static enum dma_status edma_tx_status(struct dma_chan *chan,
> return ret;
> }
>
> -static bool edma_is_memcpy_channel(int ch_num, u16 *memcpy_channels)
> +static bool edma_is_memcpy_channel(int ch_num, s32 *memcpy_channels)
> {
> - s16 *memcpy_ch = memcpy_channels;
> -
> if (!memcpy_channels)
> return false;
> - while (*memcpy_ch != -1) {
> - if (*memcpy_ch == ch_num)
> + while (*memcpy_channels != -1) {
> + if (*memcpy_channels == ch_num)
> return true;
> - memcpy_ch++;
> + memcpy_channels++;
> }
> return false;
> }
> @@ -1775,7 +1773,7 @@ static void edma_dma_init(struct edma_cc *ecc, bool legacy_mode)
> {
> struct dma_device *s_ddev = &ecc->dma_slave;
> struct dma_device *m_ddev = NULL;
> - s16 *memcpy_channels = ecc->info->memcpy_channels;
> + s32 *memcpy_channels = ecc->info->memcpy_channels;
> int i, j;
>
> dma_cap_zero(s_ddev->cap_mask);
> @@ -1996,16 +1994,16 @@ static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
> prop = of_find_property(dev->of_node, "ti,edma-memcpy-channels", &sz);
> if (prop) {
> const char pname[] = "ti,edma-memcpy-channels";
> - size_t nelm = sz / sizeof(s16);
> - s16 *memcpy_ch;
> + size_t nelm = sz / sizeof(s32);
> + s32 *memcpy_ch;
>
> - memcpy_ch = devm_kcalloc(dev, nelm + 1, sizeof(s16),
> + memcpy_ch = devm_kcalloc(dev, nelm + 1, sizeof(s32),
> GFP_KERNEL);
> if (!memcpy_ch)
> return ERR_PTR(-ENOMEM);
>
> - ret = of_property_read_u16_array(dev->of_node, pname,
> - (u16 *)memcpy_ch, nelm);
> + ret = of_property_read_u32_array(dev->of_node, pname,
> + (u32 *)memcpy_ch, nelm);
> if (ret)
> return ERR_PTR(ret);
>
> diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
> index 105700e62ea1..0a533f94438f 100644
> --- a/include/linux/platform_data/edma.h
> +++ b/include/linux/platform_data/edma.h
> @@ -76,7 +76,7 @@ struct edma_soc_info {
> struct edma_rsv_info *rsv;
>
> /* List of channels allocated for memcpy, terminated with -1 */
> - s16 *memcpy_channels;
> + s32 *memcpy_channels;
>
> s8 (*queue_priority_mapping)[2];
> const s16 (*xbar_chans)[2];
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-12-09 20:06:44

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH for 4.4 1/2] dmaengine: edma: DT: Change memcpy channel array from 16bit to 32bit type

On Wed, Dec 09, 2015 at 02:02:00PM -0600, Rob Herring wrote:
> On Wed, Dec 09, 2015 at 10:18:10AM +0200, Peter Ujfalusi wrote:
> > This change makes the DT file to be easier to read since the memcpy
> > channels array does not need the '/bits/ 16' to be specified, which might
> > confuse some people.
>
> Why? I don't see the point of this change and plus you are breaking
> compatibility with the change. There was little reason to do 16-bit
> values to start with, but that's now the ABI.

Okay, I now see the explanation in cover letter. That should be part of
this commit message.

Acked-by: Rob Herring <[email protected]>

>
> Rob
>
> > Signed-off-by: Peter Ujfalusi <[email protected]>
> > ---
> > Documentation/devicetree/bindings/dma/ti-edma.txt | 5 ++---
> > drivers/dma/edma.c | 22 ++++++++++------------
> > include/linux/platform_data/edma.h | 2 +-
> > 3 files changed, 13 insertions(+), 16 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt b/Documentation/devicetree/bindings/dma/ti-edma.txt
> > index d3d0a4fb1c73..ae8b8f1d6e69 100644
> > --- a/Documentation/devicetree/bindings/dma/ti-edma.txt
> > +++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
> > @@ -22,8 +22,7 @@ Required properties:
> > Optional properties:
> > - ti,hwmods: Name of the hwmods associated to the eDMA CC
> > - ti,edma-memcpy-channels: List of channels allocated to be used for memcpy, iow
> > - these channels will be SW triggered channels. The list must
> > - contain 16 bits numbers, see example.
> > + these channels will be SW triggered channels. See example.
> > - ti,edma-reserved-slot-ranges: PaRAM slot ranges which should not be used by
> > the driver, they are allocated to be used by for example the
> > DSP. See example.
> > @@ -56,7 +55,7 @@ edma: edma@49000000 {
> > ti,tptcs = <&edma_tptc0 7>, <&edma_tptc1 7>, <&edma_tptc2 0>;
> >
> > /* Channel 20 and 21 is allocated for memcpy */
> > - ti,edma-memcpy-channels = /bits/ 16 <20 21>;
> > + ti,edma-memcpy-channels = <20 21>;
> > /* The following PaRAM slots are reserved: 35-45 and 100-110 */
> > ti,edma-reserved-slot-ranges = /bits/ 16 <35 10>,
> > /bits/ 16 <100 10>;
> > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> > index 2af8f32bba0b..89fc17f3a6ff 100644
> > --- a/drivers/dma/edma.c
> > +++ b/drivers/dma/edma.c
> > @@ -1752,16 +1752,14 @@ static enum dma_status edma_tx_status(struct dma_chan *chan,
> > return ret;
> > }
> >
> > -static bool edma_is_memcpy_channel(int ch_num, u16 *memcpy_channels)
> > +static bool edma_is_memcpy_channel(int ch_num, s32 *memcpy_channels)
> > {
> > - s16 *memcpy_ch = memcpy_channels;
> > -
> > if (!memcpy_channels)
> > return false;
> > - while (*memcpy_ch != -1) {
> > - if (*memcpy_ch == ch_num)
> > + while (*memcpy_channels != -1) {
> > + if (*memcpy_channels == ch_num)
> > return true;
> > - memcpy_ch++;
> > + memcpy_channels++;
> > }
> > return false;
> > }
> > @@ -1775,7 +1773,7 @@ static void edma_dma_init(struct edma_cc *ecc, bool legacy_mode)
> > {
> > struct dma_device *s_ddev = &ecc->dma_slave;
> > struct dma_device *m_ddev = NULL;
> > - s16 *memcpy_channels = ecc->info->memcpy_channels;
> > + s32 *memcpy_channels = ecc->info->memcpy_channels;
> > int i, j;
> >
> > dma_cap_zero(s_ddev->cap_mask);
> > @@ -1996,16 +1994,16 @@ static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
> > prop = of_find_property(dev->of_node, "ti,edma-memcpy-channels", &sz);
> > if (prop) {
> > const char pname[] = "ti,edma-memcpy-channels";
> > - size_t nelm = sz / sizeof(s16);
> > - s16 *memcpy_ch;
> > + size_t nelm = sz / sizeof(s32);
> > + s32 *memcpy_ch;
> >
> > - memcpy_ch = devm_kcalloc(dev, nelm + 1, sizeof(s16),
> > + memcpy_ch = devm_kcalloc(dev, nelm + 1, sizeof(s32),
> > GFP_KERNEL);
> > if (!memcpy_ch)
> > return ERR_PTR(-ENOMEM);
> >
> > - ret = of_property_read_u16_array(dev->of_node, pname,
> > - (u16 *)memcpy_ch, nelm);
> > + ret = of_property_read_u32_array(dev->of_node, pname,
> > + (u32 *)memcpy_ch, nelm);
> > if (ret)
> > return ERR_PTR(ret);
> >
> > diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
> > index 105700e62ea1..0a533f94438f 100644
> > --- a/include/linux/platform_data/edma.h
> > +++ b/include/linux/platform_data/edma.h
> > @@ -76,7 +76,7 @@ struct edma_soc_info {
> > struct edma_rsv_info *rsv;
> >
> > /* List of channels allocated for memcpy, terminated with -1 */
> > - s16 *memcpy_channels;
> > + s32 *memcpy_channels;
> >
> > s8 (*queue_priority_mapping)[2];
> > const s16 (*xbar_chans)[2];
> > --
> > 2.6.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-12-09 20:08:17

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH for 4.4 2/2] dmaengine: edma: DT: Change reserved slot array from 16bit to 32bit type

On Wed, Dec 09, 2015 at 10:18:11AM +0200, Peter Ujfalusi wrote:
> This change makes the DT file to be easier to read since the reserved slots
> array does not need the '/bits/ 16' to be specified, which might confuse
> some people.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>

This too should have info on why you are breaking compatibility.

Acked-by: Rob Herring <[email protected]>

> ---
> Documentation/devicetree/bindings/dma/ti-edma.txt | 5 ++--
> drivers/dma/edma.c | 31 ++++++++++++++++++-----
> 2 files changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt b/Documentation/devicetree/bindings/dma/ti-edma.txt
> index ae8b8f1d6e69..079b42a81d7c 100644
> --- a/Documentation/devicetree/bindings/dma/ti-edma.txt
> +++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
> @@ -56,9 +56,8 @@ edma: edma@49000000 {
>
> /* Channel 20 and 21 is allocated for memcpy */
> ti,edma-memcpy-channels = <20 21>;
> - /* The following PaRAM slots are reserved: 35-45 and 100-110 */
> - ti,edma-reserved-slot-ranges = /bits/ 16 <35 10>,
> - /bits/ 16 <100 10>;
> + /* The following PaRAM slots are reserved: 35-44 and 100-109 */
> + ti,edma-reserved-slot-ranges = <35 10>, <100 10>;
> };
>
> edma_tptc0: tptc@49800000 {
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 89fc17f3a6ff..a0f217349c07 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -2015,31 +2015,50 @@ static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
> &sz);
> if (prop) {
> const char pname[] = "ti,edma-reserved-slot-ranges";
> + u32 (*tmp)[2];
> s16 (*rsv_slots)[2];
> - size_t nelm = sz / sizeof(*rsv_slots);
> + size_t nelm = sz / sizeof(*tmp);
> struct edma_rsv_info *rsv_info;
> + int i;
>
> if (!nelm)
> return info;
>
> + tmp = kcalloc(nelm, sizeof(*tmp), GFP_KERNEL);
> + if (!tmp)
> + return ERR_PTR(-ENOMEM);
> +
> rsv_info = devm_kzalloc(dev, sizeof(*rsv_info), GFP_KERNEL);
> - if (!rsv_info)
> + if (!rsv_info) {
> + kfree(tmp);
> return ERR_PTR(-ENOMEM);
> + }
>
> rsv_slots = devm_kcalloc(dev, nelm + 1, sizeof(*rsv_slots),
> GFP_KERNEL);
> - if (!rsv_slots)
> + if (!rsv_slots) {
> + kfree(tmp);
> return ERR_PTR(-ENOMEM);
> + }
>
> - ret = of_property_read_u16_array(dev->of_node, pname,
> - (u16 *)rsv_slots, nelm * 2);
> - if (ret)
> + ret = of_property_read_u32_array(dev->of_node, pname,
> + (u32 *)tmp, nelm * 2);
> + if (ret) {
> + kfree(tmp);
> return ERR_PTR(ret);
> + }
>
> + for (i = 0; i < nelm; i++) {
> + rsv_slots[i][0] = tmp[i][0];
> + rsv_slots[i][1] = tmp[i][1];
> + }
> rsv_slots[nelm][0] = -1;
> rsv_slots[nelm][1] = -1;
> +
> info->rsv = rsv_info;
> info->rsv->rsv_slots = (const s16 (*)[2])rsv_slots;
> +
> + kfree(tmp);
> }
>
> return info;
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-12-09 20:12:33

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH for 4.4 0/2] DT/dmaengine: edma: Convert 16bit arrays to 32bit

* Peter Ujfalusi <[email protected]> [151209 00:19]:
> Hi,
>
> Based on the discussion regarding to (convert am33xx to use the new eDMA
> bindings):
> https://www.mail-archive.com/[email protected]/msg122117.html
>
> This two patch will convert the new eDMA binding to not use 16bit arrays for
> memcpy channel selection and for marking slots reserved.
> The '/bits/ 16' seams to be causing confusion so it is probably better to just
> use standard type for the arrays.
>
> The new bindings for the eDMA is introduced for 4.4 and we do not have users of
> it, which means that we can still change it w/o the risk of breaking anything
> and we do not need to maintain the compatibility with 16bit arrays.
>
> The changes in the eDMA driver is local to the DT parsing and should not
> conflict with other changes (like the filter function mapping support). Hrm,
> there might be trivial conflict in the include/linux/platform_data/edma.h with
> the "dmaengine 'universal' API".
>
> Tony, Arnd, Vinod: Can you agree on the practicalities on how these patches are
> going to be handled? I would like to send the updated am33xx/am437x conversion
> for 4.5 based on these changes.

Yes this should go into v4.4 as discussed, otherwise it will be a mess.
For both, please feel free to add:

Acked-by: Tony Lindgren <[email protected]>

I suggest Vinod sets up an immutable branch against v4.4-rc1 with just these
two patches. Then it can get merged into whichever branch needs it, I
certainly will need it as most of my v4.5 branches are v4.4-rc1 or -rc2
based.

Then the immutable branch can be merged into v4.4 by Vinod or Arnd.

Regards,

Tony

2015-12-10 03:01:07

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH for 4.4 0/2] DT/dmaengine: edma: Convert 16bit arrays to 32bit

On Wed, Dec 09, 2015 at 12:12:27PM -0800, Tony Lindgren wrote:
> * Peter Ujfalusi <[email protected]> [151209 00:19]:
> > Hi,
> >
> > Based on the discussion regarding to (convert am33xx to use the new eDMA
> > bindings):
> > https://www.mail-archive.com/[email protected]/msg122117.html
> >
> > This two patch will convert the new eDMA binding to not use 16bit arrays for
> > memcpy channel selection and for marking slots reserved.
> > The '/bits/ 16' seams to be causing confusion so it is probably better to just
> > use standard type for the arrays.
> >
> > The new bindings for the eDMA is introduced for 4.4 and we do not have users of
> > it, which means that we can still change it w/o the risk of breaking anything
> > and we do not need to maintain the compatibility with 16bit arrays.
> >
> > The changes in the eDMA driver is local to the DT parsing and should not
> > conflict with other changes (like the filter function mapping support). Hrm,
> > there might be trivial conflict in the include/linux/platform_data/edma.h with
> > the "dmaengine 'universal' API".
> >
> > Tony, Arnd, Vinod: Can you agree on the practicalities on how these patches are
> > going to be handled? I would like to send the updated am33xx/am437x conversion
> > for 4.5 based on these changes.
>
> Yes this should go into v4.4 as discussed, otherwise it will be a mess.
> For both, please feel free to add:
>
> Acked-by: Tony Lindgren <[email protected]>
>
> I suggest Vinod sets up an immutable branch against v4.4-rc1 with just these
> two patches. Then it can get merged into whichever branch needs it, I
> certainly will need it as most of my v4.5 branches are v4.4-rc1 or -rc2
> based.
>
> Then the immutable branch can be merged into v4.4 by Vinod or Arnd.

Applied to fix/edma. This is based on -rc1 and will not rebase.

I am merging this to my fixes and will send to Linus in couple of days

Thanks
--
~Vinod