Determine the number of DMA channels by 'dma-channels' property
Subsystems (6)
v5 cover Determine the number of DMA channels by 'dma-channels' property
The PDMA driver currently assumes there are four channels by default, it might cause the error if there is actually less than four channels. Change that by getting number of channel dynamically from device tree. For backwards-compatible, it uses the default value (i.e. 4) when there is no 'dma-channels' information in dts.
This patch set contains the dts and dt-bindings change.
Changed in v5:
- Rebase on tag v5.17-rc3
- Fix typo in dt-bindings and commit message
- Add PDMA versioning scheme for compatible
Changed in v4:
- Remove cflags of debug use reported-by: kernel test robot <[email protected]>
Changed in v3:
- Fix allocating wrong size
- Return error if 'dma-channels' is larger than maximum
Changed in v2:
- Rebase on tag v5.16
- Use 4 as default value of dma-channels
Zong Li (3):
dt-bindings: Add dma-channels property and modify compatible
riscv: dts: Add dma-channels property and modify compatible
dmaengine: sf-pdma: Get number of channel by device tree
.../bindings/dma/sifive,fu540-c000-pdma.yaml | 19 +++++++++++++++-- .../boot/dts/microchip/microchip-mpfs.dtsi | 3 ++- arch/riscv/boot/dts/sifive/fu540-c000.dtsi | 3 ++- drivers/dma/sf-pdma/sf-pdma.c | 21 ++++++++++++------- drivers/dma/sf-pdma/sf-pdma.h | 8 ++----- 5 files changed, 37 insertions(+), 17 deletions(-)
Signature · 7 lines
2.31.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
PATCH v5 1/3 dt-bindings: Add dma-channels property and modify compatible
Add dma-channels property, then we can determine how many channels there by device tree, rather than statically defining it in PDMA driver. In addition, we also modify the compatible for PDMA versioning scheme.
--- .../bindings/dma/sifive,fu540-c000-pdma.yaml | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml +17 -2
@@ -25,7 +25,15 @@ description: | properties: compatible: items: - - const: sifive,fu540-c000-pdma + - enum: + - sifive,fu540-c000-pdma + - const: sifive,pdma0 + description: + Should be "sifive,<chip>-pdma" and "sifive,pdma<version>". + Supported compatible strings are - + "sifive,fu540-c000-pdma" for the SiFive PDMA v0 as integrated onto the + SiFive FU540 chip resp and "sifive,pdma0" for the SiFive PDMA v0 IP block + with no chip integration tweaks. reg: maxItems: 1
@@ -34,6 +42,12 @@ properties: minItems: 1 maxItems: 8 + dma-channels: + description: For backwards-compatibility, the default value is 4 + minimum: 1 + maximum: 4 + default: 4 + '#dma-cells': const: 1
@@ -48,8 +62,9 @@ additionalProperties: false examples: - | dma@3000000 { - compatible = "sifive,fu540-c000-pdma"; + compatible = "sifive,fu540-c000-pdma", "sifive,pdma0"; reg = <0x3000000 0x8000>; + dma-channels = <4>; interrupts = <23>, <24>, <25>, <26>, <27>, <28>, <29>, <30>; #dma-cells = <1>; };
Signature · 7 lines
2.31.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v5 1/3] dt-bindings: Add dma-channels property and modify compatible
Quoted 10 lines · Zong Li
Add dma-channels property, then we can determine how many channels there by device tree, rather than statically defining it in PDMA driver. In addition, we also modify the compatible for PDMA versioning scheme.
Signed-off-by Zong Li <[email protected]>Suggested-by Palmer Dabbelt <[email protected]>--- .../bindings/dma/sifive,fu540-c000-pdma.yaml | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v5 1/3] dt-bindings: Add dma-channels property and modify compatible
In reply to Zong LiQuoted 45 lines · On Sun, 06 Feb 2022 22:30:38 PST (-0800), [email protected] wrote:
Add dma-channels property, then we can determine how many channels there by device tree, rather than statically defining it in PDMA driver. In addition, we also modify the compatible for PDMA versioning scheme.
Signed-off-by Zong Li <[email protected]>Suggested-by Palmer Dabbelt <[email protected]>--- .../bindings/dma/sifive,fu540-c000-pdma.yaml | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml +15
-1@@ -25,7 +25,15 @@ description: | properties: compatible: items: - - const: sifive,fu540-c000-pdma + - enum: + - sifive,fu540-c000-pdma + - const: sifive,pdma0 + description: + Should be "sifive,<chip>-pdma" and "sifive,pdma<version>". + Supported compatible strings are - + "sifive,fu540-c000-pdma" for the SiFive PDMA v0 as integrated onto the + SiFive FU540 chip resp and "sifive,pdma0" for the SiFive PDMA v0 IP block + with no chip integration tweaks. reg: maxItems: 1@@ -34,6 +42,12 @@ properties: minItems: 1 maxItems: 8 + dma-channels: + description: For backwards-compatibility, the default value is 4 + minimum: 1 + maximum: 4 + default: 4 + '#dma-cells': const: 1
Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml additionalProperties: false · line 70
Quoted 10 lines
Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml +2
-1@@ -48,8 +62,9 @@ additionalProperties: false examples: - | dma@3000000 { - compatible = "sifive,fu540-c000-pdma"; + compatible = "sifive,fu540-c000-pdma", "sifive,pdma0"; reg = <0x3000000 0x8000>; + dma-channels = <4>; interrupts = <23>, <24>, <25>, <26>, <27>, <28>, <29>, <30>; #dma-cells = <1>; };
PATCH v5 2/3 riscv: dts: Add dma-channels property and modify compatible
In reply to Zong LiAdd dma-channels property, then we can determine how many channels there by device tree, in addition, we add the pdma versioning scheme for compatible.
--- arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 3 ++- arch/riscv/boot/dts/sifive/fu540-c000.dtsi | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +2 -1
@@ -187,11 +187,12 @@ plic: interrupt-controller@c000000 { }; dma@3000000 { - compatible = "sifive,fu540-c000-pdma"; + compatible = "sifive,fu540-c000-pdma", "sifive,pdma0"; reg = <0x0 0x3000000 0x0 0x8000>; interrupt-parent = <&plic>; interrupts = <23>, <24>, <25>, <26>, <27>, <28>, <29>, <30>; + dma-channels = <4>; #dma-cells = <1>; };
arch/riscv/boot/dts/sifive/fu540-c000.dtsi +2 -1
@@ -168,11 +168,12 @@ uart0: serial@10010000 { status = "disabled"; }; dma: dma@3000000 { - compatible = "sifive,fu540-c000-pdma"; + compatible = "sifive,fu540-c000-pdma", "sifive,pdma0"; reg = <0x0 0x3000000 0x0 0x8000>; interrupt-parent = <&plic0>; interrupts = <23>, <24>, <25>, <26>, <27>, <28>, <29>, <30>; + dma-channels = <4>; #dma-cells = <1>; }; uart1: serial@10011000 {
Signature · 7 lines
2.31.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
PATCH v5 3/3 dmaengine: sf-pdma: Get number of channel by device tree
In reply to Zong LiIt 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.
--- drivers/dma/sf-pdma/sf-pdma.c | 21 ++++++++++++++------- drivers/dma/sf-pdma/sf-pdma.h | 8 ++------ 2 files changed, 16 insertions(+), 13 deletions(-)
drivers/dma/sf-pdma/sf-pdma.c +14 -7
@@ -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);
drivers/dma/sf-pdma/sf-pdma.h +2 -6
@@ -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 */
Signature · 7 lines
2.31.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Vinod Koul
maintainerRe: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree
Quoted 33 lines · Zong Li
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(-)drivers/dma/sf-pdma/sf-pdma.c +0
-4@@ -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;
drivers/dma/sf-pdma/sf-pdma.c static int sf_pdma_probe(struct platform_device *pdev) · line 500
Quoted 10 lines
drivers/dma/sf-pdma/sf-pdma.c +5
-2- 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
Quoted 45 lines
drivers/dma/sf-pdma/sf-pdma.c +9
-1+ 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);drivers/dma/sf-pdma/sf-pdma.h +1
-5@@ -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
drivers/dma/sf-pdma/sf-pdma.h struct sf_pdma { · line 117
Quoted 5 lines
drivers/dma/sf-pdma/sf-pdma.h +1
-1@@ -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?
Signature · 1 line
~Vinod
Re: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree
Quoted 40 lines · Vinod Koul
Quoted 38 lines · Zong Li
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(-)drivers/dma/sf-pdma/sf-pdma.c +1
-5@@ -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 ;
Quoted 8 lines
drivers/dma/sf-pdma/sf-pdma.c static int sf_pdma_probe(struct platform_device *pdev) · line 500
Quoted 5 lines
drivers/dma/sf-pdma/sf-pdma.c +4
-1- 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_dbgperhaps
Thanks for your suggestion, let me change it in the next version.
Quoted 42 lines
Quoted 42 lines
drivers/dma/sf-pdma/sf-pdma.c +9
-1+ 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);drivers/dma/sf-pdma/sf-pdma.h +1
-5@@ -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
Quoted 12 lines
drivers/dma/sf-pdma/sf-pdma.h struct sf_pdma { · line 117
Quoted 8 lines
drivers/dma/sf-pdma/sf-pdma.h +1
-1#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.
Quoted 2 lines
--
~Vinod
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree
Quoted 50 lines · On Tue, 15 Feb 2022 22:52:14 PST (-0800), [email protected] wrote:
Quoted 40 lines · Vinod Koul
Quoted 38 lines · Zong Li
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(-)drivers/dma/sf-pdma/sf-pdma.c +1
-5@@ -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 ;Quoted 8 lines
drivers/dma/sf-pdma/sf-pdma.c static int sf_pdma_probe(struct platform_device *pdev) · line 500
Quoted 5 lines
drivers/dma/sf-pdma/sf-pdma.c +4
-1- 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_dbgperhaps
Quoted 1 line
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
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!
Quoted 60 lines
Quoted 42 lines
Quoted 42 lines
drivers/dma/sf-pdma/sf-pdma.c +9
-1+ 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);drivers/dma/sf-pdma/sf-pdma.h +1
-5@@ -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 4Quoted 12 lines
drivers/dma/sf-pdma/sf-pdma.h struct sf_pdma { · line 117
Quoted 8 lines
drivers/dma/sf-pdma/sf-pdma.h +1
-1#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.
Quoted 2 lines
--
~Vinod
Re: [PATCH v5 3/3] dmaengine: sf-pdma: Get number of channel by device tree
Quoted 52 lines · Palmer Dabbelt
Quoted 50 lines · On Tue, 15 Feb 2022 22:52:14 PST (-0800), [email protected] wrote:
Quoted 48 lines · Vinod Koul
Quoted 33 lines · Zong Li
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(-)drivers/dma/sf-pdma/sf-pdma.c +0
-4@@ -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;drivers/dma/sf-pdma/sf-pdma.c static int sf_pdma_probe(struct platform_device *pdev) · line 500
Quoted 10 lines
drivers/dma/sf-pdma/sf-pdma.c +5
-2- 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_dbgperhaps
Quoted 6 lines
Quoted 1 line
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.
Quoted 8 lines
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!
Quoted 62 lines
Thanks!
Quoted 60 lines
Quoted 42 lines
Quoted 42 lines
drivers/dma/sf-pdma/sf-pdma.c +9
-1+ 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);drivers/dma/sf-pdma/sf-pdma.h +1
-5@@ -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 4Quoted 12 lines
drivers/dma/sf-pdma/sf-pdma.h struct sf_pdma { · line 117
Quoted 8 lines
drivers/dma/sf-pdma/sf-pdma.h +1
-1#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.
Quoted 2 lines
--
~Vinod