2022-02-07 21:35:21

by Zong Li

Subject: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree

It currently assumes that there are always four channels, it would
cause the error if there is actually less than four channels. Change
that by getting number of channel from device tree.

For backwards-compatibility, it uses the default value (i.e. 4) when
there is no 'dma-channels' information in dts.

Signed-off-by: Zong Li <[email protected]>
---
drivers/dma/sf-pdma/sf-pdma.c | 21 ++++++++++++++-------
drivers/dma/sf-pdma/sf-pdma.h | 8 ++------
2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
index f12606aeff87..2ae10b61dfa1 100644
--- a/drivers/dma/sf-pdma/sf-pdma.c
+++ b/drivers/dma/sf-pdma/sf-pdma.c
@@ -482,9 +482,7 @@ static void sf_pdma_setup_chans(struct sf_pdma *pdma)
static int sf_pdma_probe(struct platform_device *pdev)
{
struct sf_pdma *pdma;
- struct sf_pdma_chan *chan;
struct resource *res;
- int len, chans;
int ret;
const enum dma_slave_buswidth widths =
DMA_SLAVE_BUSWIDTH_1_BYTE | DMA_SLAVE_BUSWIDTH_2_BYTES |
@@ -492,13 +490,21 @@ static int sf_pdma_probe(struct platform_device *pdev)
DMA_SLAVE_BUSWIDTH_16_BYTES | DMA_SLAVE_BUSWIDTH_32_BYTES |
DMA_SLAVE_BUSWIDTH_64_BYTES;

- chans = PDMA_NR_CH;
- len = sizeof(*pdma) + sizeof(*chan) * chans;
- pdma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
+ pdma = devm_kzalloc(&pdev->dev, sizeof(*pdma), GFP_KERNEL);
if (!pdma)
return -ENOMEM;

- pdma->n_chans = chans;
+ ret = of_property_read_u32(pdev->dev.of_node, "dma-channels",
+ &pdma->n_chans);
+ if (ret) {
+ dev_notice(&pdev->dev, "set number of channels to default value: 4\n");
+ pdma->n_chans = PDMA_MAX_NR_CH;
+ }
+
+ if (pdma->n_chans > PDMA_MAX_NR_CH) {
+ dev_err(&pdev->dev, "the number of channels exceeds the maximum\n");
+ return -EINVAL;
+ }

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
pdma->membase = devm_ioremap_resource(&pdev->dev, res);
@@ -556,7 +562,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
struct sf_pdma_chan *ch;
int i;

- for (i = 0; i < PDMA_NR_CH; i++) {
+ for (i = 0; i < pdma->n_chans; i++) {
ch = &pdma->chans[i];

devm_free_irq(&pdev->dev, ch->txirq, ch);
@@ -574,6 +580,7 @@ static int sf_pdma_remove(struct platform_device *pdev)

static const struct of_device_id sf_pdma_dt_ids[] = {
{ .compatible = "sifive,fu540-c000-pdma" },
+ { .compatible = "sifive,pdma0" },
{},
};
MODULE_DEVICE_TABLE(of, sf_pdma_dt_ids);
diff --git a/drivers/dma/sf-pdma/sf-pdma.h b/drivers/dma/sf-pdma/sf-pdma.h
index 0c20167b097d..8127d792f639 100644
--- a/drivers/dma/sf-pdma/sf-pdma.h
+++ b/drivers/dma/sf-pdma/sf-pdma.h
@@ -22,11 +22,7 @@
#include "../dmaengine.h"
#include "../virt-dma.h"

-#define PDMA_NR_CH 4
-
-#if (PDMA_NR_CH != 4)
-#error "Please define PDMA_NR_CH to 4"
-#endif
+#define PDMA_MAX_NR_CH 4

#define PDMA_BASE_ADDR 0x3000000
#define PDMA_CHAN_OFFSET 0x1000
@@ -118,7 +114,7 @@ struct sf_pdma {
void __iomem *membase;
void __iomem *mappedbase;
u32 n_chans;
- struct sf_pdma_chan chans[PDMA_NR_CH];
+ struct sf_pdma_chan chans[PDMA_MAX_NR_CH];
};

#endif /* _SF_PDMA_H */
--
2.31.1

2022-02-15 13:59:18

by Vinod Koul

Subject: Re: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree

On 07-02-22, 14:30, Zong Li wrote:
> It currently assumes that there are always four channels, it would
> cause the error if there is actually less than four channels. Change
> that by getting number of channel from device tree.
>
> For backwards-compatibility, it uses the default value (i.e. 4) when
> there is no 'dma-channels' information in dts.
>
> Signed-off-by: Zong Li <[email protected]>
> ---
> drivers/dma/sf-pdma/sf-pdma.c | 21 ++++++++++++++-------
> drivers/dma/sf-pdma/sf-pdma.h | 8 ++------
> 2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
> index f12606aeff87..2ae10b61dfa1 100644
> --- a/drivers/dma/sf-pdma/sf-pdma.c
> +++ b/drivers/dma/sf-pdma/sf-pdma.c
> @@ -482,9 +482,7 @@ static void sf_pdma_setup_chans(struct sf_pdma *pdma)
> static int sf_pdma_probe(struct platform_device *pdev)
> {
> struct sf_pdma *pdma;
> - struct sf_pdma_chan *chan;
> struct resource *res;
> - int len, chans;
> int ret;
> const enum dma_slave_buswidth widths =
> DMA_SLAVE_BUSWIDTH_1_BYTE | DMA_SLAVE_BUSWIDTH_2_BYTES |
> @@ -492,13 +490,21 @@ static int sf_pdma_probe(struct platform_device *pdev)
> DMA_SLAVE_BUSWIDTH_16_BYTES | DMA_SLAVE_BUSWIDTH_32_BYTES |
> DMA_SLAVE_BUSWIDTH_64_BYTES;
>
> - chans = PDMA_NR_CH;
> - len = sizeof(*pdma) + sizeof(*chan) * chans;
> - pdma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
> + pdma = devm_kzalloc(&pdev->dev, sizeof(*pdma), GFP_KERNEL);
> if (!pdma)
> return -ENOMEM;
>
> - pdma->n_chans = chans;
> + ret = of_property_read_u32(pdev->dev.of_node, "dma-channels",
> + &pdma->n_chans);
> + if (ret) {
> + dev_notice(&pdev->dev, "set number of channels to default value: 4\n");

This is useful for only debug i think, so dev_dbg perhaps

> + pdma->n_chans = PDMA_MAX_NR_CH;
> + }
> +
> + if (pdma->n_chans > PDMA_MAX_NR_CH) {
> + dev_err(&pdev->dev, "the number of channels exceeds the maximum\n");
> + return -EINVAL;
> + }
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> pdma->membase = devm_ioremap_resource(&pdev->dev, res);
> @@ -556,7 +562,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
> struct sf_pdma_chan *ch;
> int i;
>
> - for (i = 0; i < PDMA_NR_CH; i++) {
> + for (i = 0; i < pdma->n_chans; i++) {
> ch = &pdma->chans[i];
>
> devm_free_irq(&pdev->dev, ch->txirq, ch);
> @@ -574,6 +580,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
>
> static const struct of_device_id sf_pdma_dt_ids[] = {
> { .compatible = "sifive,fu540-c000-pdma" },
> + { .compatible = "sifive,pdma0" },
> {},
> };
> MODULE_DEVICE_TABLE(of, sf_pdma_dt_ids);
> diff --git a/drivers/dma/sf-pdma/sf-pdma.h b/drivers/dma/sf-pdma/sf-pdma.h
> index 0c20167b097d..8127d792f639 100644
> --- a/drivers/dma/sf-pdma/sf-pdma.h
> +++ b/drivers/dma/sf-pdma/sf-pdma.h
> @@ -22,11 +22,7 @@
> #include "../dmaengine.h"
> #include "../virt-dma.h"
>
> -#define PDMA_NR_CH 4
> -
> -#if (PDMA_NR_CH != 4)
> -#error "Please define PDMA_NR_CH to 4"
> -#endif
> +#define PDMA_MAX_NR_CH 4
>
> #define PDMA_BASE_ADDR 0x3000000
> #define PDMA_CHAN_OFFSET 0x1000
> @@ -118,7 +114,7 @@ struct sf_pdma {
> void __iomem *membase;
> void __iomem *mappedbase;
> u32 n_chans;
> - struct sf_pdma_chan chans[PDMA_NR_CH];
> + struct sf_pdma_chan chans[PDMA_MAX_NR_CH];

why waste memory allocating max, we know number of channels in probe,
why not allocate runtime?

--
~Vinod

2022-02-16 07:41:33

by Zong Li

Subject: Re: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree

On Tue, Feb 15, 2022 at 8:06 PM Vinod Koul <[email protected]> wrote:
>
> On 07-02-22, 14:30, Zong Li wrote:
> > It currently assumes that there are always four channels, it would
> > cause the error if there is actually less than four channels. Change
> > that by getting number of channel from device tree.
> >
> > For backwards-compatibility, it uses the default value (i.e. 4) when
> > there is no 'dma-channels' information in dts.
> >
> > Signed-off-by: Zong Li <[email protected]>
> > ---
> > drivers/dma/sf-pdma/sf-pdma.c | 21 ++++++++++++++-------
> > drivers/dma/sf-pdma/sf-pdma.h | 8 ++------
> > 2 files changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
> > index f12606aeff87..2ae10b61dfa1 100644
> > --- a/drivers/dma/sf-pdma/sf-pdma.c
> > +++ b/drivers/dma/sf-pdma/sf-pdma.c
> > @@ -482,9 +482,7 @@ static void sf_pdma_setup_chans(struct sf_pdma *pdma)
> > static int sf_pdma_probe(struct platform_device *pdev)
> > {
> > struct sf_pdma *pdma;
> > - struct sf_pdma_chan *chan;
> > struct resource *res;
> > - int len, chans;
> > int ret;
> > const enum dma_slave_buswidth widths =
> > DMA_SLAVE_BUSWIDTH_1_BYTE | DMA_SLAVE_BUSWIDTH_2_BYTES |
> > @@ -492,13 +490,21 @@ static int sf_pdma_probe(struct platform_device *pdev)
> > DMA_SLAVE_BUSWIDTH_16_BYTES | DMA_SLAVE_BUSWIDTH_32_BYTES |
> > DMA_SLAVE_BUSWIDTH_64_BYTES;
> >
> > - chans = PDMA_NR_CH;
> > - len = sizeof(*pdma) + sizeof(*chan) * chans;
> > - pdma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
> > + pdma = devm_kzalloc(&pdev->dev, sizeof(*pdma), GFP_KERNEL);
> > if (!pdma)
> > return -ENOMEM;
> >
> > - pdma->n_chans = chans;
> > + ret = of_property_read_u32(pdev->dev.of_node, "dma-channels",
> > + &pdma->n_chans);
> > + if (ret) {
> > + dev_notice(&pdev->dev, "set number of channels to default value: 4\n");
>
> This is useful for only debug i think, so dev_dbg perhaps
>

Thanks for your suggestion, let me change it in the next version.

> > + pdma->n_chans = PDMA_MAX_NR_CH;
> > + }
> > +
> > + if (pdma->n_chans > PDMA_MAX_NR_CH) {
> > + dev_err(&pdev->dev, "the number of channels exceeds the maximum\n");
> > + return -EINVAL;
> > + }
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > pdma->membase = devm_ioremap_resource(&pdev->dev, res);
> > @@ -556,7 +562,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
> > struct sf_pdma_chan *ch;
> > int i;
> >
> > - for (i = 0; i < PDMA_NR_CH; i++) {
> > + for (i = 0; i < pdma->n_chans; i++) {
> > ch = &pdma->chans[i];
> >
> > devm_free_irq(&pdev->dev, ch->txirq, ch);
> > @@ -574,6 +580,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
> >
> > static const struct of_device_id sf_pdma_dt_ids[] = {
> > { .compatible = "sifive,fu540-c000-pdma" },
> > + { .compatible = "sifive,pdma0" },
> > {},
> > };
> > MODULE_DEVICE_TABLE(of, sf_pdma_dt_ids);
> > diff --git a/drivers/dma/sf-pdma/sf-pdma.h b/drivers/dma/sf-pdma/sf-pdma.h
> > index 0c20167b097d..8127d792f639 100644
> > --- a/drivers/dma/sf-pdma/sf-pdma.h
> > +++ b/drivers/dma/sf-pdma/sf-pdma.h
> > @@ -22,11 +22,7 @@
> > #include "../dmaengine.h"
> > #include "../virt-dma.h"
> >
> > -#define PDMA_NR_CH 4
> > -
> > -#if (PDMA_NR_CH != 4)
> > -#error "Please define PDMA_NR_CH to 4"
> > -#endif
> > +#define PDMA_MAX_NR_CH 4
> >
> > #define PDMA_BASE_ADDR 0x3000000
> > #define PDMA_CHAN_OFFSET 0x1000
> > @@ -118,7 +114,7 @@ struct sf_pdma {
> > void __iomem *membase;
> > void __iomem *mappedbase;
> > u32 n_chans;
> > - struct sf_pdma_chan chans[PDMA_NR_CH];
> > + struct sf_pdma_chan chans[PDMA_MAX_NR_CH];
>
> why waste memory allocating max, we know number of channels in probe,
> why not allocate runtime?
>

I kept it there because I'd like to do minimum change in this patch
set. You're right, let me change it in the next version.

> --
> ~Vinod

2022-03-04 04:07:10

by Palmer Dabbelt

Subject: Re: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree

On Tue, 15 Feb 2022 22:52:14 PST (-0800), [email protected] wrote:
> On Tue, Feb 15, 2022 at 8:06 PM Vinod Koul <[email protected]> wrote:
>>
>> On 07-02-22, 14:30, Zong Li wrote:
>> > It currently assumes that there are always four channels, it would
>> > cause the error if there is actually less than four channels. Change
>> > that by getting number of channel from device tree.
>> >
>> > For backwards-compatibility, it uses the default value (i.e. 4) when
>> > there is no 'dma-channels' information in dts.
>> >
>> > Signed-off-by: Zong Li <[email protected]>
>> > ---
>> > drivers/dma/sf-pdma/sf-pdma.c | 21 ++++++++++++++-------
>> > drivers/dma/sf-pdma/sf-pdma.h | 8 ++------
>> > 2 files changed, 16 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
>> > index f12606aeff87..2ae10b61dfa1 100644
>> > --- a/drivers/dma/sf-pdma/sf-pdma.c
>> > +++ b/drivers/dma/sf-pdma/sf-pdma.c
>> > @@ -482,9 +482,7 @@ static void sf_pdma_setup_chans(struct sf_pdma *pdma)
>> > static int sf_pdma_probe(struct platform_device *pdev)
>> > {
>> > struct sf_pdma *pdma;
>> > - struct sf_pdma_chan *chan;
>> > struct resource *res;
>> > - int len, chans;
>> > int ret;
>> > const enum dma_slave_buswidth widths =
>> > DMA_SLAVE_BUSWIDTH_1_BYTE | DMA_SLAVE_BUSWIDTH_2_BYTES |
>> > @@ -492,13 +490,21 @@ static int sf_pdma_probe(struct platform_device *pdev)
>> > DMA_SLAVE_BUSWIDTH_16_BYTES | DMA_SLAVE_BUSWIDTH_32_BYTES |
>> > DMA_SLAVE_BUSWIDTH_64_BYTES;
>> >
>> > - chans = PDMA_NR_CH;
>> > - len = sizeof(*pdma) + sizeof(*chan) * chans;
>> > - pdma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
>> > + pdma = devm_kzalloc(&pdev->dev, sizeof(*pdma), GFP_KERNEL);
>> > if (!pdma)
>> > return -ENOMEM;
>> >
>> > - pdma->n_chans = chans;
>> > + ret = of_property_read_u32(pdev->dev.of_node, "dma-channels",
>> > + &pdma->n_chans);
>> > + if (ret) {
>> > + dev_notice(&pdev->dev, "set number of channels to default value: 4\n");
>>
>> This is useful for only debug i think, so dev_dbg perhaps
>>
>
> Thanks for your suggestion, let me change it in the next version.

Not sure if I'm missing something, but I don't see a v6. I'm going to
assume that one will be sent, but the suggested changes look minor
enough so

Acked-by: Palmer Dabbelt <[email protected]>

LMK if you guys were expecting this to go in via the RISC-V tree,
otherwise I'll assume this aimed at the dmaengine tree. Probably best to keep
all three together, so feel free to take the DTS updates as well -- having some
shared tag never hurts, but the DTs don't move that much so any conflicts
should be straight-forward to just fix at merge time.

Thanks!

>> > + pdma->n_chans = PDMA_MAX_NR_CH;
>> > + }
>> > +
>> > + if (pdma->n_chans > PDMA_MAX_NR_CH) {
>> > + dev_err(&pdev->dev, "the number of channels exceeds the maximum\n");
>> > + return -EINVAL;
>> > + }
>> >
>> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > pdma->membase = devm_ioremap_resource(&pdev->dev, res);
>> > @@ -556,7 +562,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
>> > struct sf_pdma_chan *ch;
>> > int i;
>> >
>> > - for (i = 0; i < PDMA_NR_CH; i++) {
>> > + for (i = 0; i < pdma->n_chans; i++) {
>> > ch = &pdma->chans[i];
>> >
>> > devm_free_irq(&pdev->dev, ch->txirq, ch);
>> > @@ -574,6 +580,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
>> >
>> > static const struct of_device_id sf_pdma_dt_ids[] = {
>> > { .compatible = "sifive,fu540-c000-pdma" },
>> > + { .compatible = "sifive,pdma0" },
>> > {},
>> > };
>> > MODULE_DEVICE_TABLE(of, sf_pdma_dt_ids);
>> > diff --git a/drivers/dma/sf-pdma/sf-pdma.h b/drivers/dma/sf-pdma/sf-pdma.h
>> > index 0c20167b097d..8127d792f639 100644
>> > --- a/drivers/dma/sf-pdma/sf-pdma.h
>> > +++ b/drivers/dma/sf-pdma/sf-pdma.h
>> > @@ -22,11 +22,7 @@
>> > #include "../dmaengine.h"
>> > #include "../virt-dma.h"
>> >
>> > -#define PDMA_NR_CH 4
>> > -
>> > -#if (PDMA_NR_CH != 4)
>> > -#error "Please define PDMA_NR_CH to 4"
>> > -#endif
>> > +#define PDMA_MAX_NR_CH 4
>> >
>> > #define PDMA_BASE_ADDR 0x3000000
>> > #define PDMA_CHAN_OFFSET 0x1000
>> > @@ -118,7 +114,7 @@ struct sf_pdma {
>> > void __iomem *membase;
>> > void __iomem *mappedbase;
>> > u32 n_chans;
>> > - struct sf_pdma_chan chans[PDMA_NR_CH];
>> > + struct sf_pdma_chan chans[PDMA_MAX_NR_CH];
>>
>> why waste memory allocating max, we know number of channels in probe,
>> why not allocate runtime?
>>
>
> I kept it there because I'd like to do minimum change in this patch
> set. You're right, let me change it in the next version.
>
>> --
>> ~Vinod

2022-03-04 18:04:10

by Zong Li

Subject: Re: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree

On Fri, Mar 4, 2022 at 4:43 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Tue, 15 Feb 2022 22:52:14 PST (-0800), [email protected] wrote:
> > On Tue, Feb 15, 2022 at 8:06 PM Vinod Koul <[email protected]> wrote:
> >>
> >> On 07-02-22, 14:30, Zong Li wrote:
> >> > It currently assumes that there are always four channels, it would
> >> > cause the error if there is actually less than four channels. Change
> >> > that by getting number of channel from device tree.
> >> >
> >> > For backwards-compatibility, it uses the default value (i.e. 4) when
> >> > there is no 'dma-channels' information in dts.
> >> >
> >> > Signed-off-by: Zong Li <[email protected]>
> >> > ---
> >> > drivers/dma/sf-pdma/sf-pdma.c | 21 ++++++++++++++-------
> >> > drivers/dma/sf-pdma/sf-pdma.h | 8 ++------
> >> > 2 files changed, 16 insertions(+), 13 deletions(-)
> >> >
> >> > diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
> >> > index f12606aeff87..2ae10b61dfa1 100644
> >> > --- a/drivers/dma/sf-pdma/sf-pdma.c
> >> > +++ b/drivers/dma/sf-pdma/sf-pdma.c
> >> > @@ -482,9 +482,7 @@ static void sf_pdma_setup_chans(struct sf_pdma *pdma)
> >> > static int sf_pdma_probe(struct platform_device *pdev)
> >> > {
> >> > struct sf_pdma *pdma;
> >> > - struct sf_pdma_chan *chan;
> >> > struct resource *res;
> >> > - int len, chans;
> >> > int ret;
> >> > const enum dma_slave_buswidth widths =
> >> > DMA_SLAVE_BUSWIDTH_1_BYTE | DMA_SLAVE_BUSWIDTH_2_BYTES |
> >> > @@ -492,13 +490,21 @@ static int sf_pdma_probe(struct platform_device *pdev)
> >> > DMA_SLAVE_BUSWIDTH_16_BYTES | DMA_SLAVE_BUSWIDTH_32_BYTES |
> >> > DMA_SLAVE_BUSWIDTH_64_BYTES;
> >> >
> >> > - chans = PDMA_NR_CH;
> >> > - len = sizeof(*pdma) + sizeof(*chan) * chans;
> >> > - pdma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
> >> > + pdma = devm_kzalloc(&pdev->dev, sizeof(*pdma), GFP_KERNEL);
> >> > if (!pdma)
> >> > return -ENOMEM;
> >> >
> >> > - pdma->n_chans = chans;
> >> > + ret = of_property_read_u32(pdev->dev.of_node, "dma-channels",
> >> > + &pdma->n_chans);
> >> > + if (ret) {
> >> > + dev_notice(&pdev->dev, "set number of channels to default value: 4\n");
> >>
> >> This is useful for only debug i think, so dev_dbg perhaps
> >>
> >
> > Thanks for your suggestion, let me change it in the next version.
>
> Not sure if I'm missing something, but I don't see a v6. I'm going to
> assume that one will be sent, but the suggested changes look minor
> enough so
>

I have been sending the v6 patchset, thank you for the review.

> Acked-by: Palmer Dabbelt <[email protected]>
>
> LMK if you guys were expecting this to go in via the RISC-V tree,
> otherwise I'll assume this aimed at the dmaengine tree. Probably best to keep
> all three together, so feel free to take the DTS updates as well -- having some
> shared tag never hurts, but the DTs don't move that much so any conflicts
> should be straight-forward to just fix at merge time.
>

Yes, if the v6 version is good enough and you could pick them into
RISC-V tree, I'd appreciate that. Thanks!

> Thanks!
>
> >> > + pdma->n_chans = PDMA_MAX_NR_CH;
> >> > + }
> >> > +
> >> > + if (pdma->n_chans > PDMA_MAX_NR_CH) {
> >> > + dev_err(&pdev->dev, "the number of channels exceeds the maximum\n");
> >> > + return -EINVAL;
> >> > + }
> >> >
> >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> > pdma->membase = devm_ioremap_resource(&pdev->dev, res);
> >> > @@ -556,7 +562,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
> >> > struct sf_pdma_chan *ch;
> >> > int i;
> >> >
> >> > - for (i = 0; i < PDMA_NR_CH; i++) {
> >> > + for (i = 0; i < pdma->n_chans; i++) {
> >> > ch = &pdma->chans[i];
> >> >
> >> > devm_free_irq(&pdev->dev, ch->txirq, ch);
> >> > @@ -574,6 +580,7 @@ static int sf_pdma_remove(struct platform_device *pdev)
> >> >
> >> > static const struct of_device_id sf_pdma_dt_ids[] = {
> >> > { .compatible = "sifive,fu540-c000-pdma" },
> >> > + { .compatible = "sifive,pdma0" },
> >> > {},
> >> > };
> >> > MODULE_DEVICE_TABLE(of, sf_pdma_dt_ids);
> >> > diff --git a/drivers/dma/sf-pdma/sf-pdma.h b/drivers/dma/sf-pdma/sf-pdma.h
> >> > index 0c20167b097d..8127d792f639 100644
> >> > --- a/drivers/dma/sf-pdma/sf-pdma.h
> >> > +++ b/drivers/dma/sf-pdma/sf-pdma.h
> >> > @@ -22,11 +22,7 @@
> >> > #include "../dmaengine.h"
> >> > #include "../virt-dma.h"
> >> >
> >> > -#define PDMA_NR_CH 4
> >> > -
> >> > -#if (PDMA_NR_CH != 4)
> >> > -#error "Please define PDMA_NR_CH to 4"
> >> > -#endif
> >> > +#define PDMA_MAX_NR_CH 4
> >> >
> >> > #define PDMA_BASE_ADDR 0x3000000
> >> > #define PDMA_CHAN_OFFSET 0x1000
> >> > @@ -118,7 +114,7 @@ struct sf_pdma {
> >> > void __iomem *membase;
> >> > void __iomem *mappedbase;
> >> > u32 n_chans;
> >> > - struct sf_pdma_chan chans[PDMA_NR_CH];
> >> > + struct sf_pdma_chan chans[PDMA_MAX_NR_CH];
> >>
> >> why waste memory allocating max, we know number of channels in probe,
> >> why not allocate runtime?
> >>
> >
> > I kept it there because I'd like to do minimum change in this patch
> > set. You're right, let me change it in the next version.
> >
> >> --
> >> ~Vinod