| Message ID | 1315852165-32402-5-git-send-email-thomas.abraham@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 12, 2011 at 11:59 PM, Thomas Abraham <thomas.abraham@linaro.org> wrote: > For PL330 dma controllers instantiated from device tree, the channel > lookup is based on phandle of the dma controller and dma request id > specified by the client node. During probe, the private data of each > channel of the controller is set to point to the device node of the > dma controller. The 'chan_id' of the each channel is used as the > dma request id. > > Client driver requesting dma channels specify the phandle of the > dma controller and the request id. The pl330 filter function > converts the phandle to the device node pointer and matches that > with channel's private data. If a match is found, the request id > from the client node and the 'chan_id' of the channel is matched. > A channel is found if both the values match. > > Cc: Jassi Brar <jassisinghbrar@gmail.com> > Cc: Boojin Kim <boojin.kim@samsung.com> > Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> > Reviewed-by: Rob Herring <rob.herring@calxeda.com> > --- Acked-by: Jassi Brar <jassisinghbrar@gmail.com> thanks -jassi
On Mon, Sep 12, 2011 at 11:59:23PM +0530, Thomas Abraham wrote: > For PL330 dma controllers instantiated from device tree, the channel > lookup is based on phandle of the dma controller and dma request id > specified by the client node. During probe, the private data of each > channel of the controller is set to point to the device node of the > dma controller. The 'chan_id' of the each channel is used as the > dma request id. > > Client driver requesting dma channels specify the phandle of the > dma controller and the request id. The pl330 filter function > converts the phandle to the device node pointer and matches that > with channel's private data. If a match is found, the request id > from the client node and the 'chan_id' of the channel is matched. > A channel is found if both the values match. > > Cc: Jassi Brar <jassisinghbrar@gmail.com> > Cc: Boojin Kim <boojin.kim@samsung.com> > Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> > Reviewed-by: Rob Herring <rob.herring@calxeda.com> > --- > .../devicetree/bindings/dma/arm-pl330.txt | 29 +++++++++++++++++ > drivers/dma/pl330.c | 33 +++++++++++++++++-- > 2 files changed, 58 insertions(+), 4 deletions(-) > create mode 100644 Documentation/devicetree/bindings/dma/arm-pl330.txt > > diff --git a/Documentation/devicetree/bindings/dma/arm-pl330.txt b/Documentation/devicetree/bindings/dma/arm-pl330.txt > new file mode 100644 > index 0000000..05b007d > --- /dev/null > +++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt > @@ -0,0 +1,29 @@ > +* ARM PrimeCell PL330 DMA Controller > + > +The ARM PrimeCell PL330 DMA controller can move blocks of memory contents > +between memory and peripherals or memory to memory. > + > +Required properties: > + - compatible: should include both "arm,pl330" and "arm,primecell". > + - reg: physical base address of the controller and length of memory mapped > + region. > + - interrupts: interrupt number to the cpu. > + > +Example: (from Samsung's Exynos4 processor dtsi file) > + > + pdma0: pdma@12680000 { > + compatible = "arm,pl330", "arm,primecell"; > + reg = <0x12680000 0x1000>; > + interrupts = <99>; > + }; > + > +Client drivers (device nodes requiring dma transfers from dev-to-mem or > +mem-to-dev) should specify the DMA channel numbers using a two-value pair > +as shown below. > + > + [property name] = <[phandle of the dma controller] [dma request id]>; > + > + where 'dma request id' is the dma request number which is connected > + to the client controller. > + > + Example: tx-dma-channel = <&pdma0 12>; Looks good to me. You may want to specify that the property name should be in the form: <name>-dma-channel just to enforce a bit of convention on the users. > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c > index 992bf82..7a4ebf1 100644 > --- a/drivers/dma/pl330.c > +++ b/drivers/dma/pl330.c > @@ -19,6 +19,7 @@ > #include <linux/amba/pl330.h> > #include <linux/pm_runtime.h> > #include <linux/scatterlist.h> > +#include <linux/of.h> > > #define NR_DEFAULT_DESC 16 > > @@ -277,6 +278,20 @@ bool pl330_filter(struct dma_chan *chan, void *param) > if (chan->device->dev->driver != &pl330_driver.drv) > return false; > > +#ifdef CONFIG_OF > + if (chan->device->dev->of_node) { > + const __be32 *prop_value; > + phandle phandle; > + struct device_node *node; > + > + prop_value = ((struct property *)param)->value; > + phandle = be32_to_cpup(prop_value++); > + node = of_find_node_by_phandle(phandle); > + return ((chan->private == node) && > + (chan->chan_id == be32_to_cpup(prop_value))); Don't open code this. There is already a function to decode a phandle property. of_parse_phandle() should do the trick. Otherwise looks good to me. g. > + } > +#endif > + > peri_id = chan->private; > return *peri_id == (unsigned)param; > } > @@ -855,12 +870,17 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > INIT_LIST_HEAD(&pd->channels); > > /* Initialize channel parameters */ > - num_chan = max(pdat ? pdat->nr_valid_peri : 0, (u8)pi->pcfg.num_chan); > + num_chan = max(pdat ? pdat->nr_valid_peri : (u8)pi->pcfg.num_peri, > + (u8)pi->pcfg.num_chan); > pdmac->peripherals = kzalloc(num_chan * sizeof(*pch), GFP_KERNEL); > > for (i = 0; i < num_chan; i++) { > pch = &pdmac->peripherals[i]; > - pch->chan.private = pdat ? &pdat->peri_id[i] : NULL; > + if (!adev->dev.of_node) > + pch->chan.private = pdat ? &pdat->peri_id[i] : NULL; > + else > + pch->chan.private = adev->dev.of_node; > + > INIT_LIST_HEAD(&pch->work_list); > spin_lock_init(&pch->lock); > pch->pl330_chid = NULL; > @@ -874,10 +894,15 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > } > > pd->dev = &adev->dev; > - if (pdat) > + if (pdat) { > pd->cap_mask = pdat->cap_mask; > - else > + } else { > dma_cap_set(DMA_MEMCPY, pd->cap_mask); > + if (pi->pcfg.num_peri) { > + dma_cap_set(DMA_SLAVE, pd->cap_mask); > + dma_cap_set(DMA_CYCLIC, pd->cap_mask); > + } > + } > > pd->device_alloc_chan_resources = pl330_alloc_chan_resources; > pd->device_free_chan_resources = pl330_free_chan_resources; > -- > 1.6.6.rc2 >
Hi Grant, On 14 September 2011 21:54, Grant Likely <grant.likely@secretlab.ca> wrote: > On Mon, Sep 12, 2011 at 11:59:23PM +0530, Thomas Abraham wrote: >> For PL330 dma controllers instantiated from device tree, the channel >> lookup is based on phandle of the dma controller and dma request id >> specified by the client node. During probe, the private data of each >> channel of the controller is set to point to the device node of the >> dma controller. The 'chan_id' of the each channel is used as the >> dma request id. >> >> Client driver requesting dma channels specify the phandle of the >> dma controller and the request id. The pl330 filter function >> converts the phandle to the device node pointer and matches that >> with channel's private data. If a match is found, the request id >> from the client node and the 'chan_id' of the channel is matched. >> A channel is found if both the values match. >> >> Cc: Jassi Brar <jassisinghbrar@gmail.com> >> Cc: Boojin Kim <boojin.kim@samsung.com> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> >> Reviewed-by: Rob Herring <rob.herring@calxeda.com> >> --- >> .../devicetree/bindings/dma/arm-pl330.txt | 29 +++++++++++++++++ >> drivers/dma/pl330.c | 33 +++++++++++++++++-- >> 2 files changed, 58 insertions(+), 4 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/dma/arm-pl330.txt >> >> diff --git a/Documentation/devicetree/bindings/dma/arm-pl330.txt b/Documentation/devicetree/bindings/dma/arm-pl330.txt >> new file mode 100644 >> index 0000000..05b007d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt >> @@ -0,0 +1,29 @@ >> +* ARM PrimeCell PL330 DMA Controller >> + >> +The ARM PrimeCell PL330 DMA controller can move blocks of memory contents >> +between memory and peripherals or memory to memory. >> + >> +Required properties: >> + - compatible: should include both "arm,pl330" and "arm,primecell". >> + - reg: physical base address of the controller and length of memory mapped >> + region. >> + - interrupts: interrupt number to the cpu. >> + >> +Example: (from Samsung's Exynos4 processor dtsi file) >> + >> + pdma0: pdma@12680000 { >> + compatible = "arm,pl330", "arm,primecell"; >> + reg = <0x12680000 0x1000>; >> + interrupts = <99>; >> + }; >> + >> +Client drivers (device nodes requiring dma transfers from dev-to-mem or >> +mem-to-dev) should specify the DMA channel numbers using a two-value pair >> +as shown below. >> + >> + [property name] = <[phandle of the dma controller] [dma request id]>; >> + >> + where 'dma request id' is the dma request number which is connected >> + to the client controller. >> + >> + Example: tx-dma-channel = <&pdma0 12>; > > Looks good to me. You may want to specify that the property name > should be in the form: <name>-dma-channel just to enforce a bit of > convention on the users. Ok. This will be added as a suggestion for property name specifying the dma channel. > >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c >> index 992bf82..7a4ebf1 100644 >> --- a/drivers/dma/pl330.c >> +++ b/drivers/dma/pl330.c >> @@ -19,6 +19,7 @@ >> #include <linux/amba/pl330.h> >> #include <linux/pm_runtime.h> >> #include <linux/scatterlist.h> >> +#include <linux/of.h> >> >> #define NR_DEFAULT_DESC 16 >> >> @@ -277,6 +278,20 @@ bool pl330_filter(struct dma_chan *chan, void *param) >> if (chan->device->dev->driver != &pl330_driver.drv) >> return false; >> >> +#ifdef CONFIG_OF >> + if (chan->device->dev->of_node) { >> + const __be32 *prop_value; >> + phandle phandle; >> + struct device_node *node; >> + >> + prop_value = ((struct property *)param)->value; >> + phandle = be32_to_cpup(prop_value++); >> + node = of_find_node_by_phandle(phandle); >> + return ((chan->private == node) && >> + (chan->chan_id == be32_to_cpup(prop_value))); > > Don't open code this. There is already a function to decode a phandle > property. of_parse_phandle() should do the trick. The parameter to this function 'void *param' is already pointing to a 'struct property'. That 'struct property' has a value of type <[phandle of dma controller] [channel id]>. Since the property is already known, the of_get_property() call within the of_parse_phandle() would complicate the above code. And, of_parse_phandle requires a 'property name' inside the dma client device node, which the above code fragment does not know about (property names are defined by client drivers). So, I would prefer to continue with the above implementation. > > Otherwise looks good to me. > > g. Thanks for your review. Regards, Thomas. [...]
diff --git a/Documentation/devicetree/bindings/dma/arm-pl330.txt b/Documentation/devicetree/bindings/dma/arm-pl330.txt new file mode 100644 index 0000000..05b007d --- /dev/null +++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt @@ -0,0 +1,29 @@ +* ARM PrimeCell PL330 DMA Controller + +The ARM PrimeCell PL330 DMA controller can move blocks of memory contents +between memory and peripherals or memory to memory. + +Required properties: + - compatible: should include both "arm,pl330" and "arm,primecell". + - reg: physical base address of the controller and length of memory mapped + region. + - interrupts: interrupt number to the cpu. + +Example: (from Samsung's Exynos4 processor dtsi file) + + pdma0: pdma@12680000 { + compatible = "arm,pl330", "arm,primecell"; + reg = <0x12680000 0x1000>; + interrupts = <99>; + }; + +Client drivers (device nodes requiring dma transfers from dev-to-mem or +mem-to-dev) should specify the DMA channel numbers using a two-value pair +as shown below. + + [property name] = <[phandle of the dma controller] [dma request id]>; + + where 'dma request id' is the dma request number which is connected + to the client controller. + + Example: tx-dma-channel = <&pdma0 12>; diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index 992bf82..7a4ebf1 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -19,6 +19,7 @@ #include <linux/amba/pl330.h> #include <linux/pm_runtime.h> #include <linux/scatterlist.h> +#include <linux/of.h> #define NR_DEFAULT_DESC 16 @@ -277,6 +278,20 @@ bool pl330_filter(struct dma_chan *chan, void *param) if (chan->device->dev->driver != &pl330_driver.drv) return false; +#ifdef CONFIG_OF + if (chan->device->dev->of_node) { + const __be32 *prop_value; + phandle phandle; + struct device_node *node; + + prop_value = ((struct property *)param)->value; + phandle = be32_to_cpup(prop_value++); + node = of_find_node_by_phandle(phandle); + return ((chan->private == node) && + (chan->chan_id == be32_to_cpup(prop_value))); + } +#endif + peri_id = chan->private; return *peri_id == (unsigned)param; } @@ -855,12 +870,17 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) INIT_LIST_HEAD(&pd->channels); /* Initialize channel parameters */ - num_chan = max(pdat ? pdat->nr_valid_peri : 0, (u8)pi->pcfg.num_chan); + num_chan = max(pdat ? pdat->nr_valid_peri : (u8)pi->pcfg.num_peri, + (u8)pi->pcfg.num_chan); pdmac->peripherals = kzalloc(num_chan * sizeof(*pch), GFP_KERNEL); for (i = 0; i < num_chan; i++) { pch = &pdmac->peripherals[i]; - pch->chan.private = pdat ? &pdat->peri_id[i] : NULL; + if (!adev->dev.of_node) + pch->chan.private = pdat ? &pdat->peri_id[i] : NULL; + else + pch->chan.private = adev->dev.of_node; + INIT_LIST_HEAD(&pch->work_list); spin_lock_init(&pch->lock); pch->pl330_chid = NULL; @@ -874,10 +894,15 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) } pd->dev = &adev->dev; - if (pdat) + if (pdat) { pd->cap_mask = pdat->cap_mask; - else + } else { dma_cap_set(DMA_MEMCPY, pd->cap_mask); + if (pi->pcfg.num_peri) { + dma_cap_set(DMA_SLAVE, pd->cap_mask); + dma_cap_set(DMA_CYCLIC, pd->cap_mask); + } + } pd->device_alloc_chan_resources = pl330_alloc_chan_resources; pd->device_free_chan_resources = pl330_free_chan_resources;