[permalink] [raw]

Subject: Re: [PATCH 05/63] ARM: ux500: Populate the ux500-musb Device Tree entry

On Fri, May 3, 2013 at 4:31 PM, Lee Jones <[email protected]> wrote:

> This patch provides all the information to successfully probe() and
> correctly configure the ux500-musb device driver for DMA.
>
> Acked-by: Linus Walleij <[email protected]>
> Acked-by: Fabio Baltieri <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>

Applied to my ux500-devicetree branch.

Thanks!
Linus Walleij

2013-05-03 19:37:07

by Linus Walleij

[permalink] [raw]

Subject: Re: [PATCH 06/63] ARM: ux500: Pass DMA memcpy channels though Device Tree

On Fri, May 3, 2013 at 4:31 PM, Lee Jones <[email protected]> wrote:

> It's now possible to configure memcpy channels dynamically with DT.
> We're providing them despite the fact that they're the same as the
> hard-coded channels, as they will be removed once the u8500 platform
> goes DT-only.
>
> Signed-off-by: Lee Jones <[email protected]>

Applied to my ux500-devicetree branch.

Thanks!
Linus Walleij

2013-05-03 19:40:52

by Linus Walleij

[permalink] [raw]

Subject: Re: [PATCH 07/63] dmaengine: ste_dma40: Assign memcpy channels in the driver

On Fri, May 3, 2013 at 4:31 PM, Lee Jones <[email protected]> wrote:

> The channels reserved for memcpy are the same for all currently
> supported platforms. With this in mind, we can ease the platform
> data passing requirement by moving these assignments out from
> platform code and place them directly into the driver.
>
> Acked-by: Vinod Koul <[email protected]>
> Acked-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>

(...)
> -/* Reserved event lines for memcpy only */
> -static int dma40_memcpy_event[] = {
> - DB8500_DMA_MEMCPY_TX_0,
> - DB8500_DMA_MEMCPY_TX_1,
> - DB8500_DMA_MEMCPY_TX_2,
> - DB8500_DMA_MEMCPY_TX_3,
> - DB8500_DMA_MEMCPY_TX_4,
> - DB8500_DMA_MEMCPY_TX_5,
> -};
(...)
> +/* Reserved event lines for memcpy only. */
> +static int dma40_memcpy_channels[] = { 56, 57, 58, 59, 60 };

I'm still not OK with named numbers being replaced with hardcoded
numbers.

Apparently this is a hard review comment so I will go in and patch
it myself with a separate patch on top of this.

Yours,
Linus Walleij

2013-05-03 19:51:09

by Linus Walleij

[permalink] [raw]

On Fri, May 03, 2013 at 03:32:09PM +0100, Lee Jones wrote:
> Now we know the total of physical and logical channels, we may as
> well report them within the information log.
>
> Before:
> dma40 dma40.0: hardware revision: 3 @ 0x801c0000 with 8 physical channels
>
> After:
> <as above ...> and 256 logical channels
>
> Cc: Vinod Koul <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Per Forlin <[email protected]>
> Cc: Rabin Vincent <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
Acked-by: Vinod Koul <[email protected]>

--
~Vinod
> drivers/dma/ste_dma40.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index e5f0e3c..6d81f44 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -3213,8 +3213,9 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
>
> num_log_chans = num_phy_chans * D40_MAX_LOG_CHAN_PER_PHY;
>
> - dev_info(&pdev->dev, "hardware revision: %d @ 0x%x with %d physical channels\n",
> - rev, res->start, num_phy_chans);
> + dev_info(&pdev->dev,
> + "hardware rev: %d @ 0x%x with %d physical and %d logical channels\n",
> + rev, res->start, num_phy_chans, num_log_chans);
>
> base = kzalloc(ALIGN(sizeof(struct d40_base), 4) +
> (num_phy_chans + num_log_chans + ARRAY_SIZE(dma40_memcpy_channels)) *
> --
> 1.7.10.4
>

2013-05-23 11:31:16

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH 26/63] dmaengine: ste_dma40: Supply full Device Tree parsing support

On Fri, May 03, 2013 at 03:32:12PM +0100, Lee Jones wrote:
> Using the new DMA DT bindings and API, we can register the DMA40 driver
> as Device Tree capable. Now, when a client attempts to allocate a
> channel using the DMA DT bindings via its own node, we are able to parse
> the request and allocate a channel in the correct manner.
>
> Cc: Vinod Koul <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Per Forlin <[email protected]>
> Cc: Rabin Vincent <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> Reviewed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
Acked-by: Vinod Koul <[email protected]>

--
~Vinod
> ---
> .../devicetree/bindings/dma/ste-dma40.txt | 62 ++++++++++++++++++++
> drivers/dma/ste_dma40.c | 52 ++++++++++++++++
> 2 files changed, 114 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/ste-dma40.txt
>
> diff --git a/Documentation/devicetree/bindings/dma/ste-dma40.txt b/Documentation/devicetree/bindings/dma/ste-dma40.txt
> new file mode 100644
> index 0000000..2679a87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/ste-dma40.txt
> @@ -0,0 +1,62 @@
> +* DMA40 DMA Controller
> +
> +Required properties:
> +- compatible: "stericsson,dma40"
> +- reg: Address range of the DMAC registers
> +- reg-names: Names of the above areas to use during resource look-up
> +- interrupt: Should contain the DMAC interrupt number
> +- #dma-cells: must be <3>
> +
> +Optional properties:
> +- dma-channels: Number of channels supported by hardware - if not present
> + the driver will attempt to obtain the information from H/W
> +
> +Example:
> +
> + dma: dma-controller@801C0000 {
> + compatible = "stericsson,db8500-dma40", "stericsson,dma40";
> + reg = <0x801C0000 0x1000 0x40010000 0x800>;
> + reg-names = "base", "lcpa";
> + interrupt-parent = <&intc>;
> + interrupts = <0 25 0x4>;
> +
> + #dma-cells = <2>;
> + dma-channels = <8>;
> + };
> +
> +Clients
> +Required properties:
> +- dmas: Comma separated list of dma channel requests
> +- dma-names: Names of the aforementioned requested channels
> +
> +Each dmas request consists of 4 cells:
> + 1. A phandle pointing to the DMA controller
> + 2. Device Type
> + 3. The DMA request line number (only when 'use fixed channel' is set)
> + 4. A 32bit mask specifying; mode, direction and endianess [NB: This list will grow]
> + 0x00000001: Mode:
> + Logical channel when unset
> + Physical channel when set
> + 0x00000002: Direction:
> + Memory to Device when unset
> + Device to Memory when set
> + 0x00000004: Endianess:
> + Little endian when unset
> + Big endian when set
> + 0x00000008: Use fixed channel:
> + Use automatic channel selection when unset
> + Use DMA request line number when set
> +
> +Example:
> +
> + uart@80120000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x80120000 0x1000>;
> + interrupts = <0 11 0x4>;
> +
> + dmas = <&dma 13 0 0x2>, /* Logical - DevToMem */
> + <&dma 13 0 0x0>; /* Logical - MemToDev */
> + dma-names = "rx", "rx";
> +
> + status = "disabled";
> + };
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index d7c2397..b77d508 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -18,6 +18,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/err.h>
> #include <linux/of.h>
> +#include <linux/of_dma.h>
> #include <linux/amba/bus.h>
> #include <linux/regulator/consumer.h>
> #include <linux/platform_data/dma-ste-dma40.h>
> @@ -2419,6 +2420,50 @@ static void d40_set_prio_realtime(struct d40_chan *d40c)
> __d40_set_prio_rt(d40c, d40c->dma_cfg.dev_type, false);
> }
>
> +#define D40_DT_FLAGS_MODE(flags) ((flags >> 0) & 0x1)
> +#define D40_DT_FLAGS_DIR(flags) ((flags >> 1) & 0x1)
> +#define D40_DT_FLAGS_BIG_ENDIAN(flags) ((flags >> 2) & 0x1)
> +#define D40_DT_FLAGS_FIXED_CHAN(flags) ((flags >> 3) & 0x1)
> +
> +static struct dma_chan *d40_xlate(struct of_phandle_args *dma_spec,
> + struct of_dma *ofdma)
> +{
> + struct stedma40_chan_cfg cfg;
> + dma_cap_mask_t cap;
> + u32 flags;
> +
> + memset(&cfg, 0, sizeof(struct stedma40_chan_cfg));
> +
> + dma_cap_zero(cap);
> + dma_cap_set(DMA_SLAVE, cap);
> +
> + cfg.dev_type = dma_spec->args[0];
> + flags = dma_spec->args[2];
> +
> + switch (D40_DT_FLAGS_MODE(flags)) {
> + case 0: cfg.mode = STEDMA40_MODE_LOGICAL; break;
> + case 1: cfg.mode = STEDMA40_MODE_PHYSICAL; break;
> + }
> +
> + switch (D40_DT_FLAGS_DIR(flags)) {
> + case 0:
> + cfg.dir = STEDMA40_MEM_TO_PERIPH;
> + cfg.dst_info.big_endian = D40_DT_FLAGS_BIG_ENDIAN(flags);
> + break;
> + case 1:
> + cfg.dir = STEDMA40_PERIPH_TO_MEM;
> + cfg.src_info.big_endian = D40_DT_FLAGS_BIG_ENDIAN(flags);
> + break;
> + }
> +
> + if (D40_DT_FLAGS_FIXED_CHAN(flags)) {
> + cfg.phy_channel = dma_spec->args[1];
> + cfg.use_fixed_channel = true;
> + }
> +
> + return dma_request_channel(cap, stedma40_filter, &cfg);
> +}
> +
> /* DMA ENGINE functions */
> static int d40_alloc_chan_resources(struct dma_chan *chan)
> {
> @@ -3632,6 +3677,13 @@ static int __init d40_probe(struct platform_device *pdev)
>
> d40_hw_init(base);
>
> + if (np) {
> + err = of_dma_controller_register(np, d40_xlate, NULL);
> + if (err && err != -ENODEV)
> + dev_err(&pdev->dev,
> + "could not register of_dma_controller\n");
> + }
> +
> dev_info(base->dev, "initialized\n");
> return 0;
>
> --
> 1.7.10.4
>