Re: [PATCHv3 2/4] dmaengine: dw_dmac: move to generic DMA binding
On Saturday 16 February 2013, Andy Shevchenko wrote:
> > @@ -1836,6 +1825,12 @@ static int dw_probe(struct platform_device *pdev)
> >
> > dma_async_device_register(>dma);
> >
> > + if (pdev->dev.of_node)
> > + err = of_dma_controller_register(pdev->dev.of_node,
> > +dw_dma_xlate, dw);
> > + if (err && err != -ENODEV)
> > + dev_err(>dev, "could not register
> > of_dma_controller\n");
>
> I believe we may make it as
> if (...of_node) {
> err = ...register();
> if (err...)
> dev_err();
> }
I thing the two are equivalent because we only get to the first if()
when err is 0. However, I agree that your version is a bit clearer,
so I'll change it.
> > --- a/drivers/dma/dw_dmac_regs.h
> > +++ b/drivers/dma/dw_dmac_regs.h
>
> > @@ -211,9 +212,15 @@ struct dw_dma_chan {
> > /* hardware configuration */
> > unsigned intblock_size;
> > boolnollp;
> > + unsigned intrequest_line;
> > + struct dw_dma_slave slave;
> > +
>
> Do we really need an extra empty line here?
No, that was an accident.
> > /* configuration passed via DMA_SLAVE_CONFIG */
> > struct dma_slave_config dma_sconfig;
> > +
> > + /* backlink to dw_dma */
> > + struct dw_dma *dw;
>
> Seems it's not needed and came from rebase?
Probably. It certainly was not intentional.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 2/4] dmaengine: dw_dmac: move to generic DMA binding
On Sun, Feb 17, 2013 at 12:21 AM, Arnd Bergmann wrote:
> The original device tree binding for this driver, from Viresh Kumar
> unfortunately conflicted with the generic DMA binding, and did not allow
> to completely seperate slave device configuration from the controller.
>
> This is an attempt to replace it with an implementation of the generic
> binding, but it is currently completely untested, because I do not have
> any hardware with this particular controller.
>
> The patch applies on top of the slave-dma tree, which contains both the base
> support for the generic DMA binding, as well as the earlier attempt from
> Viresh. Both of these are currently not merged upstream however.
>
> This version incorporates feedback from Viresh Kumar, Andy Shevchenko
> and Russell King.
Sorry, few comments below.
After addressing them take my Acked-by: Andy Shevchenko
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1836,6 +1825,12 @@ static int dw_probe(struct platform_device *pdev)
>
> dma_async_device_register(>dma);
>
> + if (pdev->dev.of_node)
> + err = of_dma_controller_register(pdev->dev.of_node,
> +dw_dma_xlate, dw);
> + if (err && err != -ENODEV)
> + dev_err(>dev, "could not register of_dma_controller\n");
I believe we may make it as
if (...of_node) {
err = ...register();
if (err...)
dev_err();
}
> --- a/drivers/dma/dw_dmac_regs.h
> +++ b/drivers/dma/dw_dmac_regs.h
> @@ -211,9 +212,15 @@ struct dw_dma_chan {
> /* hardware configuration */
> unsigned intblock_size;
> boolnollp;
> + unsigned intrequest_line;
> + struct dw_dma_slave slave;
> +
Do we really need an extra empty line here?
>
> /* configuration passed via DMA_SLAVE_CONFIG */
> struct dma_slave_config dma_sconfig;
> +
> + /* backlink to dw_dma */
> + struct dw_dma *dw;
Seems it's not needed and came from rebase?
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHv3 2/4] dmaengine: dw_dmac: move to generic DMA binding
The original device tree binding for this driver, from Viresh Kumar
unfortunately conflicted with the generic DMA binding, and did not allow
to completely seperate slave device configuration from the controller.
This is an attempt to replace it with an implementation of the generic
binding, but it is currently completely untested, because I do not have
any hardware with this particular controller.
The patch applies on top of the slave-dma tree, which contains both the base
support for the generic DMA binding, as well as the earlier attempt from
Viresh. Both of these are currently not merged upstream however.
This version incorporates feedback from Viresh Kumar, Andy Shevchenko
and Russell King.
Signed-off-by: Arnd Bergmann
Acked: Viresh Kumar
Cc: Andy Shevchenko
Cc: Vinod Koul
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-arm-ker...@lists.infradead.org
---
Documentation/devicetree/bindings/dma/snps-dma.txt | 70 +-
drivers/dma/dw_dmac.c | 142 ++---
drivers/dma/dw_dmac_regs.h | 11 +-
include/linux/dw_dmac.h| 5 -
4 files changed, 112 insertions(+), 116 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt
b/Documentation/devicetree/bindings/dma/snps-dma.txt
index 5bb3dfb..d58675e 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -3,59 +3,61 @@
Required properties:
- compatible: "snps,dma-spear1340"
- reg: Address range of the DMAC registers
-- interrupt-parent: Should be the phandle for the interrupt controller
- that services interrupts for this device
- interrupt: Should contain the DMAC interrupt number
-- nr_channels: Number of channels supported by hardware
-- is_private: The device channels should be marked as private and not for by
the
- general purpose DMA channel allocator. False if not passed.
+- dma-channels: Number of channels supported by hardware
+- dma-requests: Number of DMA request lines supported, up to 16
+- dma-masters: Number of AHB masters supported by the controller
+- #dma-cells: must be <3>
- chan_allocation_order: order of allocation of channel, 0 (default):
ascending,
1: descending
- chan_priority: priority of channels. 0 (default): increase from chan 0->n, 1:
increase from chan n->0
- block_size: Maximum block size supported by the controller
-- nr_masters: Number of AHB masters supported by the controller
- data_width: Maximum data width supported by hardware per AHB master
(0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
-- slave_info:
- - bus_id: name of this device channel, not just a device name since
- devices may have more than one channel e.g. "foo_tx". For using the
- dw_generic_filter(), slave drivers must pass exactly this string as
- param to filter function.
- - cfg_hi: Platform-specific initializer for the CFG_HI register
- - cfg_lo: Platform-specific initializer for the CFG_LO register
- - src_master: src master for transfers on allocated channel.
- - dst_master: dest master for transfers on allocated channel.
+
+
+Optional properties:
+- interrupt-parent: Should be the phandle for the interrupt controller
+ that services interrupts for this device
+- is_private: The device channels should be marked as private and not for by
the
+ general purpose DMA channel allocator. False if not passed.
Example:
- dma@fc00 {
+ dmahost: dma@fc00 {
compatible = "snps,dma-spear1340";
reg = <0xfc00 0x1000>;
interrupt-parent = <>;
interrupts = <12>;
- nr_channels = <8>;
+ dma-channels = <8>;
+ dma-requests = <16>;
+ dma-masters = <2>;
+ #dma-cells = <3>;
chan_allocation_order = <1>;
chan_priority = <1>;
block_size = <0xfff>;
- nr_masters = <2>;
data_width = <3 3 0 0>;
+ };
- slave_info {
- uart0-tx {
- bus_id = "uart0-tx";
- cfg_hi = <0x4000>; /* 0x8 << 11 */
- cfg_lo = <0>;
- src_master = <0>;
- dst_master = <1>;
- };
- spi0-tx {
- bus_id = "spi0-tx";
- cfg_hi = <0x2000>; /* 0x4 << 11 */
- cfg_lo = <0>;
- src_master = <0>;
- dst_master = <0>;
- };
- };
+DMA clients connected to the Designware DMA controller must use the format
+described in the dma.txt file, using a four-cell specifier for each channel.
+The four cells in order
[PATCHv3 2/4] dmaengine: dw_dmac: move to generic DMA binding
The original device tree binding for this driver, from Viresh Kumar
unfortunately conflicted with the generic DMA binding, and did not allow
to completely seperate slave device configuration from the controller.
This is an attempt to replace it with an implementation of the generic
binding, but it is currently completely untested, because I do not have
any hardware with this particular controller.
The patch applies on top of the slave-dma tree, which contains both the base
support for the generic DMA binding, as well as the earlier attempt from
Viresh. Both of these are currently not merged upstream however.
This version incorporates feedback from Viresh Kumar, Andy Shevchenko
and Russell King.
Signed-off-by: Arnd Bergmann a...@arndb.de
Acked: Viresh Kumar viresh.ku...@linaro.org
Cc: Andy Shevchenko andriy.shevche...@linux.intel.com
Cc: Vinod Koul vinod.k...@linux.intel.com
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-arm-ker...@lists.infradead.org
---
Documentation/devicetree/bindings/dma/snps-dma.txt | 70 +-
drivers/dma/dw_dmac.c | 142 ++---
drivers/dma/dw_dmac_regs.h | 11 +-
include/linux/dw_dmac.h| 5 -
4 files changed, 112 insertions(+), 116 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt
b/Documentation/devicetree/bindings/dma/snps-dma.txt
index 5bb3dfb..d58675e 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -3,59 +3,61 @@
Required properties:
- compatible: snps,dma-spear1340
- reg: Address range of the DMAC registers
-- interrupt-parent: Should be the phandle for the interrupt controller
- that services interrupts for this device
- interrupt: Should contain the DMAC interrupt number
-- nr_channels: Number of channels supported by hardware
-- is_private: The device channels should be marked as private and not for by
the
- general purpose DMA channel allocator. False if not passed.
+- dma-channels: Number of channels supported by hardware
+- dma-requests: Number of DMA request lines supported, up to 16
+- dma-masters: Number of AHB masters supported by the controller
+- #dma-cells: must be 3
- chan_allocation_order: order of allocation of channel, 0 (default):
ascending,
1: descending
- chan_priority: priority of channels. 0 (default): increase from chan 0-n, 1:
increase from chan n-0
- block_size: Maximum block size supported by the controller
-- nr_masters: Number of AHB masters supported by the controller
- data_width: Maximum data width supported by hardware per AHB master
(0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
-- slave_info:
- - bus_id: name of this device channel, not just a device name since
- devices may have more than one channel e.g. foo_tx. For using the
- dw_generic_filter(), slave drivers must pass exactly this string as
- param to filter function.
- - cfg_hi: Platform-specific initializer for the CFG_HI register
- - cfg_lo: Platform-specific initializer for the CFG_LO register
- - src_master: src master for transfers on allocated channel.
- - dst_master: dest master for transfers on allocated channel.
+
+
+Optional properties:
+- interrupt-parent: Should be the phandle for the interrupt controller
+ that services interrupts for this device
+- is_private: The device channels should be marked as private and not for by
the
+ general purpose DMA channel allocator. False if not passed.
Example:
- dma@fc00 {
+ dmahost: dma@fc00 {
compatible = snps,dma-spear1340;
reg = 0xfc00 0x1000;
interrupt-parent = vic1;
interrupts = 12;
- nr_channels = 8;
+ dma-channels = 8;
+ dma-requests = 16;
+ dma-masters = 2;
+ #dma-cells = 3;
chan_allocation_order = 1;
chan_priority = 1;
block_size = 0xfff;
- nr_masters = 2;
data_width = 3 3 0 0;
+ };
- slave_info {
- uart0-tx {
- bus_id = uart0-tx;
- cfg_hi = 0x4000; /* 0x8 11 */
- cfg_lo = 0;
- src_master = 0;
- dst_master = 1;
- };
- spi0-tx {
- bus_id = spi0-tx;
- cfg_hi = 0x2000; /* 0x4 11 */
- cfg_lo = 0;
- src_master = 0;
- dst_master = 0;
- };
- };
+DMA clients connected to the Designware DMA controller must use the format
+described in the dma.txt file, using a four-cell specifier for
Re: [PATCHv3 2/4] dmaengine: dw_dmac: move to generic DMA binding
On Sun, Feb 17, 2013 at 12:21 AM, Arnd Bergmann a...@arndb.de wrote:
The original device tree binding for this driver, from Viresh Kumar
unfortunately conflicted with the generic DMA binding, and did not allow
to completely seperate slave device configuration from the controller.
This is an attempt to replace it with an implementation of the generic
binding, but it is currently completely untested, because I do not have
any hardware with this particular controller.
The patch applies on top of the slave-dma tree, which contains both the base
support for the generic DMA binding, as well as the earlier attempt from
Viresh. Both of these are currently not merged upstream however.
This version incorporates feedback from Viresh Kumar, Andy Shevchenko
and Russell King.
Sorry, few comments below.
After addressing them take my Acked-by: Andy Shevchenko
andriy.shevche...@linux.intel.com
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1836,6 +1825,12 @@ static int dw_probe(struct platform_device *pdev)
dma_async_device_register(dw-dma);
+ if (pdev-dev.of_node)
+ err = of_dma_controller_register(pdev-dev.of_node,
+dw_dma_xlate, dw);
+ if (err err != -ENODEV)
+ dev_err(pdev-dev, could not register of_dma_controller\n);
I believe we may make it as
if (...of_node) {
err = ...register();
if (err...)
dev_err();
}
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -211,9 +212,15 @@ struct dw_dma_chan {
/* hardware configuration */
unsigned intblock_size;
boolnollp;
+ unsigned intrequest_line;
+ struct dw_dma_slave slave;
+
Do we really need an extra empty line here?
/* configuration passed via DMA_SLAVE_CONFIG */
struct dma_slave_config dma_sconfig;
+
+ /* backlink to dw_dma */
+ struct dw_dma *dw;
Seems it's not needed and came from rebase?
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 2/4] dmaengine: dw_dmac: move to generic DMA binding
On Saturday 16 February 2013, Andy Shevchenko wrote:
@@ -1836,6 +1825,12 @@ static int dw_probe(struct platform_device *pdev)
dma_async_device_register(dw-dma);
+ if (pdev-dev.of_node)
+ err = of_dma_controller_register(pdev-dev.of_node,
+dw_dma_xlate, dw);
+ if (err err != -ENODEV)
+ dev_err(pdev-dev, could not register
of_dma_controller\n);
I believe we may make it as
if (...of_node) {
err = ...register();
if (err...)
dev_err();
}
I thing the two are equivalent because we only get to the first if()
when err is 0. However, I agree that your version is a bit clearer,
so I'll change it.
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -211,9 +212,15 @@ struct dw_dma_chan {
/* hardware configuration */
unsigned intblock_size;
boolnollp;
+ unsigned intrequest_line;
+ struct dw_dma_slave slave;
+
Do we really need an extra empty line here?
No, that was an accident.
/* configuration passed via DMA_SLAVE_CONFIG */
struct dma_slave_config dma_sconfig;
+
+ /* backlink to dw_dma */
+ struct dw_dma *dw;
Seems it's not needed and came from rebase?
Probably. It certainly was not intentional.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
