* [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id @ 2019-01-22 13:20 Baolin Wang 2019-01-22 13:20 ` [PATCH 2/3] dmaengine: sprd: Add new DMA engine translation function Baolin Wang ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Baolin Wang @ 2019-01-22 13:20 UTC (permalink / raw) To: vkoul, robh+dt, mark.rutland Cc: devicetree, eric.long, arnd, baolin.wang, zhang.lyra, broonie, linux-kernel, dmaengine, arm, olof, orsonzhai, dan.j.williams, linux-arm-kernel The DMA engine clients can trigger DMA engine automatically by setting the corresponding hardware slave id for the DMA engine. Thus add one cell to present the hardware slave id for DMA clients. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- Documentation/devicetree/bindings/dma/sprd-dma.txt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/dma/sprd-dma.txt b/Documentation/devicetree/bindings/dma/sprd-dma.txt index 7a10fea..7812cf0 100644 --- a/Documentation/devicetree/bindings/dma/sprd-dma.txt +++ b/Documentation/devicetree/bindings/dma/sprd-dma.txt @@ -6,8 +6,8 @@ Required properties: - compatible: Should be "sprd,sc9860-dma". - reg: Should contain DMA registers location and length. - interrupts: Should contain one interrupt shared by all channel. -- #dma-cells: must be <1>. Used to represent the number of integer - cells in the dmas property of client device. +- #dma-cells: must be <2>. Used to represent the channel id and slave id + of integer cells in the dmas property of client device. - #dma-channels : Number of DMA channels supported. Should be 32. - clock-names: Should contain the clock of the DMA controller. - clocks: Should contain a clock specifier for each entry in clock-names. @@ -28,14 +28,16 @@ apdma: dma-controller@20100000 { Client: DMA clients connected to the Spreadtrum DMA controller must use the format -described in the dma.txt file, using a two-cell specifier for each channel. -The two cells in order are: +described in the dma.txt file, using a three-cell specifier for each channel. +The three cells in order are: 1. A phandle pointing to the DMA controller. 2. The channel id. +3. The hardware slave id which is used for clients to trigger DMA engine +automatically. spi0: spi@70a00000{ ... dma-names = "rx_chn", "tx_chn"; - dmas = <&apdma 11>, <&apdma 12>; + dmas = <&apdma 11 11>, <&apdma 12 12>; ... }; -- 1.7.9.5 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] dmaengine: sprd: Add new DMA engine translation function 2019-01-22 13:20 [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id Baolin Wang @ 2019-01-22 13:20 ` Baolin Wang 2019-01-22 13:20 ` [PATCH 3/3] arm64: dts: sprd: Change 2 cells to provide DMA controller specific information Baolin Wang 2019-01-30 16:51 ` [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id Arnd Bergmann 2 siblings, 0 replies; 16+ messages in thread From: Baolin Wang @ 2019-01-22 13:20 UTC (permalink / raw) To: vkoul, robh+dt, mark.rutland Cc: devicetree, eric.long, arnd, baolin.wang, zhang.lyra, broonie, linux-kernel, dmaengine, arm, olof, orsonzhai, dan.j.williams, linux-arm-kernel Add new DMA engine translation function to get the hardware slave id of the corresponding DMA engine channel. Meanwhile we do not need to set default slave id in sprd_dma_alloc_chan_resources(), remove it. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- drivers/dma/sprd-dma.c | 49 ++++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c index e2f0167..7d180bb 100644 --- a/drivers/dma/sprd-dma.c +++ b/drivers/dma/sprd-dma.c @@ -204,9 +204,9 @@ struct sprd_dma_dev { struct sprd_dma_chn channels[0]; }; -static bool sprd_dma_filter_fn(struct dma_chan *chan, void *param); -static struct of_dma_filter_info sprd_dma_info = { - .filter_fn = sprd_dma_filter_fn, +struct sprd_dma_filter_param { + u32 chn_id; + u32 slave_id; }; static inline struct sprd_dma_chn *to_sprd_dma_chan(struct dma_chan *c) @@ -580,15 +580,7 @@ static irqreturn_t dma_irq_handle(int irq, void *dev_id) static int sprd_dma_alloc_chan_resources(struct dma_chan *chan) { - struct sprd_dma_chn *schan = to_sprd_dma_chan(chan); - int ret; - - ret = pm_runtime_get_sync(chan->device->dev); - if (ret < 0) - return ret; - - schan->dev_id = SPRD_DMA_SOFTWARE_UID; - return 0; + return pm_runtime_get_sync(chan->device->dev); } static void sprd_dma_free_chan_resources(struct dma_chan *chan) @@ -1022,12 +1014,31 @@ static bool sprd_dma_filter_fn(struct dma_chan *chan, void *param) { struct sprd_dma_chn *schan = to_sprd_dma_chan(chan); struct sprd_dma_dev *sdev = to_sprd_dma_dev(&schan->vc.chan); - u32 req = *(u32 *)param; + struct sprd_dma_filter_param *sparam = param; - if (req < sdev->total_chns) - return req == schan->chn_num + 1; - else - return false; + if (sparam->chn_id < sdev->total_chns && + sparam->chn_id == schan->chn_num + 1) { + schan->dev_id = sparam->slave_id; + return true; + } + + return false; +} + +static struct dma_chan *sprd_dma_xlate(struct of_phandle_args *dma_spec, + struct of_dma *ofdma) +{ + struct sprd_dma_dev *sdev = ofdma->of_dma_data; + dma_cap_mask_t mask = sdev->dma_dev.cap_mask; + struct sprd_dma_filter_param param; + + if (dma_spec->args_count != 2) + return NULL; + + param.chn_id = dma_spec->args[0]; + param.slave_id = dma_spec->args[1]; + + return dma_request_channel(mask, sprd_dma_filter_fn, ¶m); } static int sprd_dma_probe(struct platform_device *pdev) @@ -1133,9 +1144,7 @@ static int sprd_dma_probe(struct platform_device *pdev) goto err_register; } - sprd_dma_info.dma_cap = sdev->dma_dev.cap_mask; - ret = of_dma_controller_register(np, of_dma_simple_xlate, - &sprd_dma_info); + ret = of_dma_controller_register(np, sprd_dma_xlate, sdev); if (ret) goto err_of_register; -- 1.7.9.5 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] arm64: dts: sprd: Change 2 cells to provide DMA controller specific information 2019-01-22 13:20 [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id Baolin Wang 2019-01-22 13:20 ` [PATCH 2/3] dmaengine: sprd: Add new DMA engine translation function Baolin Wang @ 2019-01-22 13:20 ` Baolin Wang 2019-01-30 16:51 ` [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id Arnd Bergmann 2 siblings, 0 replies; 16+ messages in thread From: Baolin Wang @ 2019-01-22 13:20 UTC (permalink / raw) To: vkoul, robh+dt, mark.rutland Cc: devicetree, eric.long, arnd, baolin.wang, zhang.lyra, broonie, linux-kernel, dmaengine, arm, olof, orsonzhai, dan.j.williams, linux-arm-kernel Change to use 2 cells to provide the channel id and slave id for client. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- arch/arm64/boot/dts/sprd/whale2.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/sprd/whale2.dtsi b/arch/arm64/boot/dts/sprd/whale2.dtsi index 34b6ca0..dbd61f8 100644 --- a/arch/arm64/boot/dts/sprd/whale2.dtsi +++ b/arch/arm64/boot/dts/sprd/whale2.dtsi @@ -117,7 +117,7 @@ compatible = "sprd,sc9860-dma"; reg = <0 0x20100000 0 0x4000>; interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>; - #dma-cells = <1>; + #dma-cells = <2>; #dma-channels = <32>; clock-names = "enable"; clocks = <&apahb_gate CLK_DMA_EB>; @@ -273,7 +273,7 @@ agcp_dma: dma-controller@41580000 { compatible = "sprd,sc9860-dma"; reg = <0 0x41580000 0 0x4000>; - #dma-cells = <1>; + #dma-cells = <2>; #dma-channels = <32>; clock-names = "enable", "ashb_eb"; clocks = <&agcp_gate CLK_AGCP_DMAAP_EB>, -- 1.7.9.5 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-01-22 13:20 [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id Baolin Wang 2019-01-22 13:20 ` [PATCH 2/3] dmaengine: sprd: Add new DMA engine translation function Baolin Wang 2019-01-22 13:20 ` [PATCH 3/3] arm64: dts: sprd: Change 2 cells to provide DMA controller specific information Baolin Wang @ 2019-01-30 16:51 ` Arnd Bergmann 2019-02-01 11:53 ` Baolin Wang 2 siblings, 1 reply; 16+ messages in thread From: Arnd Bergmann @ 2019-01-30 16:51 UTC (permalink / raw) To: Baolin Wang Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > The DMA engine clients can trigger DMA engine automatically by setting > the corresponding hardware slave id for the DMA engine. Thus add one > cell to present the hardware slave id for DMA clients. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > --- > Documentation/devicetree/bindings/dma/sprd-dma.txt | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/dma/sprd-dma.txt b/Documentation/devicetree/bindings/dma/sprd-dma.txt > index 7a10fea..7812cf0 100644 > --- a/Documentation/devicetree/bindings/dma/sprd-dma.txt > +++ b/Documentation/devicetree/bindings/dma/sprd-dma.txt > @@ -6,8 +6,8 @@ Required properties: > - compatible: Should be "sprd,sc9860-dma". > - reg: Should contain DMA registers location and length. > - interrupts: Should contain one interrupt shared by all channel. > -- #dma-cells: must be <1>. Used to represent the number of integer > - cells in the dmas property of client device. > +- #dma-cells: must be <2>. Used to represent the channel id and slave id > + of integer cells in the dmas property of client device. > - #dma-channels : Number of DMA channels supported. Should be 32. > - clock-names: Should contain the clock of the DMA controller. > - clocks: Should contain a clock specifier for each entry in clock-names. > @@ -28,14 +28,16 @@ apdma: dma-controller@20100000 { > > Client: > DMA clients connected to the Spreadtrum DMA controller must use the format > -described in the dma.txt file, using a two-cell specifier for each channel. > -The two cells in order are: > +described in the dma.txt file, using a three-cell specifier for each channel. > +The three cells in order are: > 1. A phandle pointing to the DMA controller. > 2. The channel id. > +3. The hardware slave id which is used for clients to trigger DMA engine > +automatically. I notice that this is an incompatible binding change. Is that necessary? If the current code works, I'd suggest allowing both #dma-cells=<2> and <3>, and then implementing both in the driver. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-01-30 16:51 ` [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id Arnd Bergmann @ 2019-02-01 11:53 ` Baolin Wang 2019-02-12 8:24 ` Baolin Wang 0 siblings, 1 reply; 16+ messages in thread From: Baolin Wang @ 2019-02-01 11:53 UTC (permalink / raw) To: Arnd Bergmann Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM Hi Arnd, On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > The DMA engine clients can trigger DMA engine automatically by setting > > the corresponding hardware slave id for the DMA engine. Thus add one > > cell to present the hardware slave id for DMA clients. > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > --- > > Documentation/devicetree/bindings/dma/sprd-dma.txt | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/dma/sprd-dma.txt b/Documentation/devicetree/bindings/dma/sprd-dma.txt > > index 7a10fea..7812cf0 100644 > > --- a/Documentation/devicetree/bindings/dma/sprd-dma.txt > > +++ b/Documentation/devicetree/bindings/dma/sprd-dma.txt > > @@ -6,8 +6,8 @@ Required properties: > > - compatible: Should be "sprd,sc9860-dma". > > - reg: Should contain DMA registers location and length. > > - interrupts: Should contain one interrupt shared by all channel. > > -- #dma-cells: must be <1>. Used to represent the number of integer > > - cells in the dmas property of client device. > > +- #dma-cells: must be <2>. Used to represent the channel id and slave id > > + of integer cells in the dmas property of client device. > > - #dma-channels : Number of DMA channels supported. Should be 32. > > - clock-names: Should contain the clock of the DMA controller. > > - clocks: Should contain a clock specifier for each entry in clock-names. > > @@ -28,14 +28,16 @@ apdma: dma-controller@20100000 { > > > > Client: > > DMA clients connected to the Spreadtrum DMA controller must use the format > > -described in the dma.txt file, using a two-cell specifier for each channel. > > -The two cells in order are: > > +described in the dma.txt file, using a three-cell specifier for each channel. > > +The three cells in order are: > > 1. A phandle pointing to the DMA controller. > > 2. The channel id. > > +3. The hardware slave id which is used for clients to trigger DMA engine > > +automatically. > > I notice that this is an incompatible binding change. Is that necessary? > If the current code works, I'd suggest allowing both #dma-cells=<2> > and <3>, and then implementing both in the driver. Yes, this is necessary. Yes, current code can work, but the problem is that the DMA clients must add one property (something like "sprd,slave-id") to specify the slave id. So considering this, we want to change the dma-cells to 2, including dma channel and dma slave id, which can avoid introducing some similar properties for DMA clients. Now there are no DMA clients in mainline for Spreadtrum platform, and we want to upstream our first DMA clients: SPI controller. So no other drivers need to change when we changing dma cells. Thanks. -- Baolin Wang Best Regards _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-01 11:53 ` Baolin Wang @ 2019-02-12 8:24 ` Baolin Wang 2019-02-18 10:31 ` Arnd Bergmann 0 siblings, 1 reply; 16+ messages in thread From: Baolin Wang @ 2019-02-12 8:24 UTC (permalink / raw) To: Arnd Bergmann Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM Hi Arnd, On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > Hi Arnd, > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > The DMA engine clients can trigger DMA engine automatically by setting > > > the corresponding hardware slave id for the DMA engine. Thus add one > > > cell to present the hardware slave id for DMA clients. > > > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > > --- > > > Documentation/devicetree/bindings/dma/sprd-dma.txt | 12 +++++++----- > > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/dma/sprd-dma.txt b/Documentation/devicetree/bindings/dma/sprd-dma.txt > > > index 7a10fea..7812cf0 100644 > > > --- a/Documentation/devicetree/bindings/dma/sprd-dma.txt > > > +++ b/Documentation/devicetree/bindings/dma/sprd-dma.txt > > > @@ -6,8 +6,8 @@ Required properties: > > > - compatible: Should be "sprd,sc9860-dma". > > > - reg: Should contain DMA registers location and length. > > > - interrupts: Should contain one interrupt shared by all channel. > > > -- #dma-cells: must be <1>. Used to represent the number of integer > > > - cells in the dmas property of client device. > > > +- #dma-cells: must be <2>. Used to represent the channel id and slave id > > > + of integer cells in the dmas property of client device. > > > - #dma-channels : Number of DMA channels supported. Should be 32. > > > - clock-names: Should contain the clock of the DMA controller. > > > - clocks: Should contain a clock specifier for each entry in clock-names. > > > @@ -28,14 +28,16 @@ apdma: dma-controller@20100000 { > > > > > > Client: > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > -The two cells in order are: > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > +The three cells in order are: > > > 1. A phandle pointing to the DMA controller. > > > 2. The channel id. > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > +automatically. > > > > I notice that this is an incompatible binding change. Is that necessary? > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > and <3>, and then implementing both in the driver. > > Yes, this is necessary. > > Yes, current code can work, but the problem is that the DMA clients > must add one property (something like "sprd,slave-id") to specify the > slave id. So considering this, we want to change the dma-cells to 2, > including dma channel and dma slave id, which can avoid introducing > some similar properties for DMA clients. > > Now there are no DMA clients in mainline for Spreadtrum platform, and > we want to upstream our first DMA clients: SPI controller. So no other > drivers need to change when we changing dma cells. Thanks. Do you have any other concerns about this patch set? If not, I think Vinod can apply this patch set. Thanks. -- Baolin Wang Best Regards _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-12 8:24 ` Baolin Wang @ 2019-02-18 10:31 ` Arnd Bergmann 2019-02-18 10:52 ` Baolin Wang 0 siblings, 1 reply; 16+ messages in thread From: Arnd Bergmann @ 2019-02-18 10:31 UTC (permalink / raw) To: Baolin Wang Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote: > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > Client: > > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > > -The two cells in order are: > > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > > +The three cells in order are: > > > > 1. A phandle pointing to the DMA controller. > > > > 2. The channel id. > > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > > +automatically. > > > > > > I notice that this is an incompatible binding change. Is that necessary? > > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > > and <3>, and then implementing both in the driver. > > > > Yes, this is necessary. > > > > Yes, current code can work, but the problem is that the DMA clients > > must add one property (something like "sprd,slave-id") to specify the > > slave id. So considering this, we want to change the dma-cells to 2, > > including dma channel and dma slave id, which can avoid introducing > > some similar properties for DMA clients. > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and > > we want to upstream our first DMA clients: SPI controller. So no other > > drivers need to change when we changing dma cells. Thanks. > > Do you have any other concerns about this patch set? If not, I think > Vinod can apply this patch set. Thanks. Sorry for the late reply. Yes, this makes sense since there are no existing users then. For the DT changes going through the dmaengine tree Acked-by: Arnd Bergmann <arnd@arndb.de> One more question, to make sure we don't need to edit it again: Why do you need both a 'channel id' and a 'slave id' here? Is this a strict hardware requirement for your DMA engine? In many other designs, only a DMA request line number needs to be described, and I think this would correspond to what you call the 'hardware slave id' in your documentation. Does each request line here correspond to a fixed channel id as well, or can a channel be shared between multiple slave devices? Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-18 10:31 ` Arnd Bergmann @ 2019-02-18 10:52 ` Baolin Wang 2019-02-18 12:23 ` Arnd Bergmann 0 siblings, 1 reply; 16+ messages in thread From: Baolin Wang @ 2019-02-18 10:52 UTC (permalink / raw) To: Arnd Bergmann Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM Hi Arnd, On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > > > Client: > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > > > -The two cells in order are: > > > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > > > +The three cells in order are: > > > > > 1. A phandle pointing to the DMA controller. > > > > > 2. The channel id. > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > > > +automatically. > > > > > > > > I notice that this is an incompatible binding change. Is that necessary? > > > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > > > and <3>, and then implementing both in the driver. > > > > > > Yes, this is necessary. > > > > > > Yes, current code can work, but the problem is that the DMA clients > > > must add one property (something like "sprd,slave-id") to specify the > > > slave id. So considering this, we want to change the dma-cells to 2, > > > including dma channel and dma slave id, which can avoid introducing > > > some similar properties for DMA clients. > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and > > > we want to upstream our first DMA clients: SPI controller. So no other > > > drivers need to change when we changing dma cells. Thanks. > > > > Do you have any other concerns about this patch set? If not, I think > > Vinod can apply this patch set. Thanks. > > Sorry for the late reply. Yes, this makes sense since there are no > existing users then. For the DT changes going through the dmaengine > tree > > Acked-by: Arnd Bergmann <arnd@arndb.de> Thanks for your reviewing. > > One more question, to make sure we don't need to edit it again: > Why do you need both a 'channel id' and a 'slave id' here? Is this > a strict hardware requirement for your DMA engine? In many > other designs, only a DMA request line number needs to > be described, and I think this would correspond to what you > call the 'hardware slave id' in your documentation. I try to explain why we need the slave id. For our DMA engine driver, we have software request mode and hardware request mode. For software request mode, the DMA engine driver need trigger DMA to start transfer manually. But for hardware request mode, we just set one unique slave id corresponding to the slave hardware to the DMA engine, then the slave hardware can trigger DMA automatically. And the slave id is not always same with the channel id according to the SoC design, so we add one cell to specify the slave id. > > Does each request line here correspond to a fixed channel > id as well, or can a channel be shared between multiple Yes, each request line corresponds to a fixed channel id. > slave devices? > > Arnd -- Baolin Wang Best Regards _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-18 10:52 ` Baolin Wang @ 2019-02-18 12:23 ` Arnd Bergmann 2019-02-19 3:14 ` Baolin Wang 0 siblings, 1 reply; 16+ messages in thread From: Arnd Bergmann @ 2019-02-18 12:23 UTC (permalink / raw) To: Baolin Wang Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote: > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > > > > > Client: > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > > > > -The two cells in order are: > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > > > > +The three cells in order are: > > > > > > 1. A phandle pointing to the DMA controller. > > > > > > 2. The channel id. > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > > > > +automatically. > > > > > > > > > > I notice that this is an incompatible binding change. Is that necessary? > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > > > > and <3>, and then implementing both in the driver. > > > > > > > > Yes, this is necessary. > > > > > > > > Yes, current code can work, but the problem is that the DMA clients > > > > must add one property (something like "sprd,slave-id") to specify the > > > > slave id. So considering this, we want to change the dma-cells to 2, > > > > including dma channel and dma slave id, which can avoid introducing > > > > some similar properties for DMA clients. > > > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and > > > > we want to upstream our first DMA clients: SPI controller. So no other > > > > drivers need to change when we changing dma cells. Thanks. > > > > > > Do you have any other concerns about this patch set? If not, I think > > > Vinod can apply this patch set. Thanks. > > > > Sorry for the late reply. Yes, this makes sense since there are no > > existing users then. For the DT changes going through the dmaengine > > tree > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > Thanks for your reviewing. > > > > > One more question, to make sure we don't need to edit it again: > > Why do you need both a 'channel id' and a 'slave id' here? Is this > > a strict hardware requirement for your DMA engine? In many > > other designs, only a DMA request line number needs to > > be described, and I think this would correspond to what you > > call the 'hardware slave id' in your documentation. > > I try to explain why we need the slave id. > > For our DMA engine driver, we have software request mode and hardware > request mode. For software request mode, the DMA engine driver need > trigger DMA to start transfer manually. But for hardware request mode, > we just set one unique slave id corresponding to the slave hardware to > the DMA engine, then the slave hardware can trigger DMA automatically. > And the slave id is not always same with the channel id according to > the SoC design, so we add one cell to specify the slave id. I did understand the need for a slave-id, I was instead wondering about the channel-id number. On many SoCs, all channels are equal, and you just have to pick one of those with the right capabilities for a particular slave. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-18 12:23 ` Arnd Bergmann @ 2019-02-19 3:14 ` Baolin Wang 2019-02-19 9:30 ` Geert Uytterhoeven 0 siblings, 1 reply; 16+ messages in thread From: Baolin Wang @ 2019-02-19 3:14 UTC (permalink / raw) To: Arnd Bergmann Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > > > > > > > Client: > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > > > > > -The two cells in order are: > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > > > > > +The three cells in order are: > > > > > > > 1. A phandle pointing to the DMA controller. > > > > > > > 2. The channel id. > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > > > > > +automatically. > > > > > > > > > > > > I notice that this is an incompatible binding change. Is that necessary? > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > > > > > and <3>, and then implementing both in the driver. > > > > > > > > > > Yes, this is necessary. > > > > > > > > > > Yes, current code can work, but the problem is that the DMA clients > > > > > must add one property (something like "sprd,slave-id") to specify the > > > > > slave id. So considering this, we want to change the dma-cells to 2, > > > > > including dma channel and dma slave id, which can avoid introducing > > > > > some similar properties for DMA clients. > > > > > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and > > > > > we want to upstream our first DMA clients: SPI controller. So no other > > > > > drivers need to change when we changing dma cells. Thanks. > > > > > > > > Do you have any other concerns about this patch set? If not, I think > > > > Vinod can apply this patch set. Thanks. > > > > > > Sorry for the late reply. Yes, this makes sense since there are no > > > existing users then. For the DT changes going through the dmaengine > > > tree > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > Thanks for your reviewing. > > > > > > > > One more question, to make sure we don't need to edit it again: > > > Why do you need both a 'channel id' and a 'slave id' here? Is this > > > a strict hardware requirement for your DMA engine? In many > > > other designs, only a DMA request line number needs to > > > be described, and I think this would correspond to what you > > > call the 'hardware slave id' in your documentation. > > > > I try to explain why we need the slave id. > > > > For our DMA engine driver, we have software request mode and hardware > > request mode. For software request mode, the DMA engine driver need > > trigger DMA to start transfer manually. But for hardware request mode, > > we just set one unique slave id corresponding to the slave hardware to > > the DMA engine, then the slave hardware can trigger DMA automatically. > > And the slave id is not always same with the channel id according to > > the SoC design, so we add one cell to specify the slave id. > > I did understand the need for a slave-id, I was instead wondering about > the channel-id number. On many SoCs, all channels are equal, and you > just have to pick one of those with the right capabilities for a particular > slave. Yes, all channels are equal. We just set a unique slave id for the channel for a particular slave. For example, the SPI slave can use channel 10 for tx transfer by setting slave id 11, or it also can use channel 9 for tx transfer by setting same slave id 11. -- Baolin Wang Best Regards _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-19 3:14 ` Baolin Wang @ 2019-02-19 9:30 ` Geert Uytterhoeven 2019-02-19 9:49 ` Baolin Wang 0 siblings, 1 reply; 16+ messages in thread From: Geert Uytterhoeven @ 2019-02-19 9:30 UTC (permalink / raw) To: Baolin Wang Cc: Mark Rutland, DTML, eric.long, arm-soc, Arnd Bergmann, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM Hi Baolin, On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote: > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > > > > > > > > > Client: > > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > > > > > > -The two cells in order are: > > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > > > > > > +The three cells in order are: > > > > > > > > 1. A phandle pointing to the DMA controller. > > > > > > > > 2. The channel id. > > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > > > > > > +automatically. > > > > > > > > > > > > > > I notice that this is an incompatible binding change. Is that necessary? > > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > > > > > > and <3>, and then implementing both in the driver. > > > > > > > > > > > > Yes, this is necessary. > > > > > > > > > > > > Yes, current code can work, but the problem is that the DMA clients > > > > > > must add one property (something like "sprd,slave-id") to specify the > > > > > > slave id. So considering this, we want to change the dma-cells to 2, > > > > > > including dma channel and dma slave id, which can avoid introducing > > > > > > some similar properties for DMA clients. > > > > > > > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and > > > > > > we want to upstream our first DMA clients: SPI controller. So no other > > > > > > drivers need to change when we changing dma cells. Thanks. > > > > > > > > > > Do you have any other concerns about this patch set? If not, I think > > > > > Vinod can apply this patch set. Thanks. > > > > > > > > Sorry for the late reply. Yes, this makes sense since there are no > > > > existing users then. For the DT changes going through the dmaengine > > > > tree > > > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > > Thanks for your reviewing. > > > > > > > > > > > One more question, to make sure we don't need to edit it again: > > > > Why do you need both a 'channel id' and a 'slave id' here? Is this > > > > a strict hardware requirement for your DMA engine? In many > > > > other designs, only a DMA request line number needs to > > > > be described, and I think this would correspond to what you > > > > call the 'hardware slave id' in your documentation. > > > > > > I try to explain why we need the slave id. > > > > > > For our DMA engine driver, we have software request mode and hardware > > > request mode. For software request mode, the DMA engine driver need > > > trigger DMA to start transfer manually. But for hardware request mode, > > > we just set one unique slave id corresponding to the slave hardware to > > > the DMA engine, then the slave hardware can trigger DMA automatically. > > > And the slave id is not always same with the channel id according to > > > the SoC design, so we add one cell to specify the slave id. > > > > I did understand the need for a slave-id, I was instead wondering about > > the channel-id number. On many SoCs, all channels are equal, and you > > just have to pick one of those with the right capabilities for a particular > > slave. > > Yes, all channels are equal. We just set a unique slave id for the > channel for a particular slave. For example, the SPI slave can use > channel 10 for tx transfer by setting slave id 11, or it also can use > channel 9 for tx transfer by setting same slave id 11. So the channel selection is software policy, not hardware description, and thus doesn't belong in DT? Can't the DMA engine driver allocate channels dynamically, removing the need to specify this in DT? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-19 9:30 ` Geert Uytterhoeven @ 2019-02-19 9:49 ` Baolin Wang 2019-02-19 12:20 ` Vinod Koul 0 siblings, 1 reply; 16+ messages in thread From: Baolin Wang @ 2019-02-19 9:49 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Mark Rutland, DTML, eric.long, arm-soc, Arnd Bergmann, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM Hi Geert, On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Baolin, > > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote: > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > > > > > > > > > > > Client: > > > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > > > > > > > -The two cells in order are: > > > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > > > > > > > +The three cells in order are: > > > > > > > > > 1. A phandle pointing to the DMA controller. > > > > > > > > > 2. The channel id. > > > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > > > > > > > +automatically. > > > > > > > > > > > > > > > > I notice that this is an incompatible binding change. Is that necessary? > > > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > > > > > > > and <3>, and then implementing both in the driver. > > > > > > > > > > > > > > Yes, this is necessary. > > > > > > > > > > > > > > Yes, current code can work, but the problem is that the DMA clients > > > > > > > must add one property (something like "sprd,slave-id") to specify the > > > > > > > slave id. So considering this, we want to change the dma-cells to 2, > > > > > > > including dma channel and dma slave id, which can avoid introducing > > > > > > > some similar properties for DMA clients. > > > > > > > > > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and > > > > > > > we want to upstream our first DMA clients: SPI controller. So no other > > > > > > > drivers need to change when we changing dma cells. Thanks. > > > > > > > > > > > > Do you have any other concerns about this patch set? If not, I think > > > > > > Vinod can apply this patch set. Thanks. > > > > > > > > > > Sorry for the late reply. Yes, this makes sense since there are no > > > > > existing users then. For the DT changes going through the dmaengine > > > > > tree > > > > > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > > > > Thanks for your reviewing. > > > > > > > > > > > > > > One more question, to make sure we don't need to edit it again: > > > > > Why do you need both a 'channel id' and a 'slave id' here? Is this > > > > > a strict hardware requirement for your DMA engine? In many > > > > > other designs, only a DMA request line number needs to > > > > > be described, and I think this would correspond to what you > > > > > call the 'hardware slave id' in your documentation. > > > > > > > > I try to explain why we need the slave id. > > > > > > > > For our DMA engine driver, we have software request mode and hardware > > > > request mode. For software request mode, the DMA engine driver need > > > > trigger DMA to start transfer manually. But for hardware request mode, > > > > we just set one unique slave id corresponding to the slave hardware to > > > > the DMA engine, then the slave hardware can trigger DMA automatically. > > > > And the slave id is not always same with the channel id according to > > > > the SoC design, so we add one cell to specify the slave id. > > > > > > I did understand the need for a slave-id, I was instead wondering about > > > the channel-id number. On many SoCs, all channels are equal, and you > > > just have to pick one of those with the right capabilities for a particular > > > slave. > > > > Yes, all channels are equal. We just set a unique slave id for the > > channel for a particular slave. For example, the SPI slave can use > > channel 10 for tx transfer by setting slave id 11, or it also can use > > channel 9 for tx transfer by setting same slave id 11. > > So the channel selection is software policy, not hardware description, and > thus doesn't belong in DT? > > Can't the DMA engine driver allocate channels dynamically, removing the > need to specify this in DT? In theory we can do as you suggested. But we still want to manage/assign the DMA channel resources manually for one SoC, we can make sure some important DMA slaves (such as audio) can request a DMA channel at runtime firstly, another benefit is that it is easy to debug since we can easily know which channel is assigned for this slave. -- Baolin Wang Best Regards _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-19 9:49 ` Baolin Wang @ 2019-02-19 12:20 ` Vinod Koul 2019-02-20 3:12 ` Baolin Wang 0 siblings, 1 reply; 16+ messages in thread From: Vinod Koul @ 2019-02-19 12:20 UTC (permalink / raw) To: Baolin Wang Cc: Mark Rutland, DTML, eric.long, arm-soc, Arnd Bergmann, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Rob Herring, Geert Uytterhoeven, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM On 19-02-19, 17:49, Baolin Wang wrote: > Hi Geert, > > On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > Hi Baolin, > > > > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > Client: > > > > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > > > > > > > > -The two cells in order are: > > > > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > > > > > > > > +The three cells in order are: > > > > > > > > > > 1. A phandle pointing to the DMA controller. > > > > > > > > > > 2. The channel id. > > > > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > > > > > > > > +automatically. > > > > > > > > > > > > > > > > > > I notice that this is an incompatible binding change. Is that necessary? > > > > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > > > > > > > > and <3>, and then implementing both in the driver. > > > > > > > > > > > > > > > > Yes, this is necessary. > > > > > > > > > > > > > > > > Yes, current code can work, but the problem is that the DMA clients > > > > > > > > must add one property (something like "sprd,slave-id") to specify the > > > > > > > > slave id. So considering this, we want to change the dma-cells to 2, > > > > > > > > including dma channel and dma slave id, which can avoid introducing > > > > > > > > some similar properties for DMA clients. > > > > > > > > > > > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and > > > > > > > > we want to upstream our first DMA clients: SPI controller. So no other > > > > > > > > drivers need to change when we changing dma cells. Thanks. > > > > > > > > > > > > > > Do you have any other concerns about this patch set? If not, I think > > > > > > > Vinod can apply this patch set. Thanks. > > > > > > > > > > > > Sorry for the late reply. Yes, this makes sense since there are no > > > > > > existing users then. For the DT changes going through the dmaengine > > > > > > tree > > > > > > > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > > > > > > Thanks for your reviewing. > > > > > > > > > > > > > > > > > One more question, to make sure we don't need to edit it again: > > > > > > Why do you need both a 'channel id' and a 'slave id' here? Is this > > > > > > a strict hardware requirement for your DMA engine? In many > > > > > > other designs, only a DMA request line number needs to > > > > > > be described, and I think this would correspond to what you > > > > > > call the 'hardware slave id' in your documentation. > > > > > > > > > > I try to explain why we need the slave id. > > > > > > > > > > For our DMA engine driver, we have software request mode and hardware > > > > > request mode. For software request mode, the DMA engine driver need > > > > > trigger DMA to start transfer manually. But for hardware request mode, > > > > > we just set one unique slave id corresponding to the slave hardware to > > > > > the DMA engine, then the slave hardware can trigger DMA automatically. > > > > > And the slave id is not always same with the channel id according to > > > > > the SoC design, so we add one cell to specify the slave id. > > > > > > > > I did understand the need for a slave-id, I was instead wondering about > > > > the channel-id number. On many SoCs, all channels are equal, and you > > > > just have to pick one of those with the right capabilities for a particular > > > > slave. > > > > > > Yes, all channels are equal. We just set a unique slave id for the > > > channel for a particular slave. For example, the SPI slave can use > > > channel 10 for tx transfer by setting slave id 11, or it also can use > > > channel 9 for tx transfer by setting same slave id 11. > > > > So the channel selection is software policy, not hardware description, and > > thus doesn't belong in DT? > > > > Can't the DMA engine driver allocate channels dynamically, removing the > > need to specify this in DT? > > In theory we can do as you suggested. But we still want to > manage/assign the DMA channel resources manually for one SoC, we can > make sure some important DMA slaves (such as audio) can request a DMA > channel at runtime firstly, another benefit is that it is easy to > debug since we can easily know which channel is assigned for this > slave. Are you suggesting that you have more users than channels available? I dont think it is hard to debug if channels are dynamic in nature (for example we can print channel number and you know which one are you talking about, fwiw i have worked on a such a system where we grabbed the free channel) -- ~Vinod _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-19 12:20 ` Vinod Koul @ 2019-02-20 3:12 ` Baolin Wang 2019-02-20 9:07 ` Arnd Bergmann 0 siblings, 1 reply; 16+ messages in thread From: Baolin Wang @ 2019-02-20 3:12 UTC (permalink / raw) To: Vinod Koul Cc: Mark Rutland, DTML, eric.long, arm-soc, Arnd Bergmann, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Rob Herring, Geert Uytterhoeven, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM On Tue, 19 Feb 2019 at 20:20, Vinod Koul <vkoul@kernel.org> wrote: > > On 19-02-19, 17:49, Baolin Wang wrote: > > Hi Geert, > > > > On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > > > Hi Baolin, > > > > > > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > > > > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > > > Client: > > > > > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format > > > > > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel. > > > > > > > > > > > -The two cells in order are: > > > > > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel. > > > > > > > > > > > +The three cells in order are: > > > > > > > > > > > 1. A phandle pointing to the DMA controller. > > > > > > > > > > > 2. The channel id. > > > > > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine > > > > > > > > > > > +automatically. > > > > > > > > > > > > > > > > > > > > I notice that this is an incompatible binding change. Is that necessary? > > > > > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2> > > > > > > > > > > and <3>, and then implementing both in the driver. > > > > > > > > > > > > > > > > > > Yes, this is necessary. > > > > > > > > > > > > > > > > > > Yes, current code can work, but the problem is that the DMA clients > > > > > > > > > must add one property (something like "sprd,slave-id") to specify the > > > > > > > > > slave id. So considering this, we want to change the dma-cells to 2, > > > > > > > > > including dma channel and dma slave id, which can avoid introducing > > > > > > > > > some similar properties for DMA clients. > > > > > > > > > > > > > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and > > > > > > > > > we want to upstream our first DMA clients: SPI controller. So no other > > > > > > > > > drivers need to change when we changing dma cells. Thanks. > > > > > > > > > > > > > > > > Do you have any other concerns about this patch set? If not, I think > > > > > > > > Vinod can apply this patch set. Thanks. > > > > > > > > > > > > > > Sorry for the late reply. Yes, this makes sense since there are no > > > > > > > existing users then. For the DT changes going through the dmaengine > > > > > > > tree > > > > > > > > > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > > > > > > > > Thanks for your reviewing. > > > > > > > > > > > > > > > > > > > > One more question, to make sure we don't need to edit it again: > > > > > > > Why do you need both a 'channel id' and a 'slave id' here? Is this > > > > > > > a strict hardware requirement for your DMA engine? In many > > > > > > > other designs, only a DMA request line number needs to > > > > > > > be described, and I think this would correspond to what you > > > > > > > call the 'hardware slave id' in your documentation. > > > > > > > > > > > > I try to explain why we need the slave id. > > > > > > > > > > > > For our DMA engine driver, we have software request mode and hardware > > > > > > request mode. For software request mode, the DMA engine driver need > > > > > > trigger DMA to start transfer manually. But for hardware request mode, > > > > > > we just set one unique slave id corresponding to the slave hardware to > > > > > > the DMA engine, then the slave hardware can trigger DMA automatically. > > > > > > And the slave id is not always same with the channel id according to > > > > > > the SoC design, so we add one cell to specify the slave id. > > > > > > > > > > I did understand the need for a slave-id, I was instead wondering about > > > > > the channel-id number. On many SoCs, all channels are equal, and you > > > > > just have to pick one of those with the right capabilities for a particular > > > > > slave. > > > > > > > > Yes, all channels are equal. We just set a unique slave id for the > > > > channel for a particular slave. For example, the SPI slave can use > > > > channel 10 for tx transfer by setting slave id 11, or it also can use > > > > channel 9 for tx transfer by setting same slave id 11. > > > > > > So the channel selection is software policy, not hardware description, and > > > thus doesn't belong in DT? > > > > > > Can't the DMA engine driver allocate channels dynamically, removing the > > > need to specify this in DT? > > > > In theory we can do as you suggested. But we still want to > > manage/assign the DMA channel resources manually for one SoC, we can > > make sure some important DMA slaves (such as audio) can request a DMA > > channel at runtime firstly, another benefit is that it is easy to > > debug since we can easily know which channel is assigned for this > > slave. > > Are you suggesting that you have more users than channels available? Until now we have not met this issue, but we can not make sure if this can happen in future. Moreover DMA channel resources are belonging to the DMA engine's hardware resources, I think it should be described in DT like many other drivers did. > I dont think it is hard to debug if channels are dynamic in > nature (for example we can print channel number and you know which one > are you talking about, fwiw i have worked on a such a system where we > grabbed the free channel) > > -- > ~Vinod -- Baolin Wang Best Regards _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-20 3:12 ` Baolin Wang @ 2019-02-20 9:07 ` Arnd Bergmann 2019-02-20 11:29 ` Baolin Wang 0 siblings, 1 reply; 16+ messages in thread From: Arnd Bergmann @ 2019-02-20 9:07 UTC (permalink / raw) To: Baolin Wang Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Geert Uytterhoeven, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM On Wed, Feb 20, 2019 at 4:13 AM Baolin Wang <baolin.wang@linaro.org> wrote: > On Tue, 19 Feb 2019 at 20:20, Vinod Koul <vkoul@kernel.org> wrote: > > On 19-02-19, 17:49, Baolin Wang wrote: > > > On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > I did understand the need for a slave-id, I was instead wondering about > > > > > > the channel-id number. On many SoCs, all channels are equal, and you > > > > > > just have to pick one of those with the right capabilities for a particular > > > > > > slave. > > > > > > > > > > Yes, all channels are equal. We just set a unique slave id for the > > > > > channel for a particular slave. For example, the SPI slave can use > > > > > channel 10 for tx transfer by setting slave id 11, or it also can use > > > > > channel 9 for tx transfer by setting same slave id 11. > > > > > > > > So the channel selection is software policy, not hardware description, and > > > > thus doesn't belong in DT? > > > > > > > > Can't the DMA engine driver allocate channels dynamically, removing the > > > > need to specify this in DT? > > > > > > In theory we can do as you suggested. But we still want to > > > manage/assign the DMA channel resources manually for one SoC, we can > > > make sure some important DMA slaves (such as audio) can request a DMA > > > channel at runtime firstly, another benefit is that it is easy to > > > debug since we can easily know which channel is assigned for this > > > slave. > > > > Are you suggesting that you have more users than channels available? > > Until now we have not met this issue, but we can not make sure if this > can happen in future. Moreover DMA channel resources are belonging to > the DMA engine's hardware resources, I think it should be described in > DT like many other drivers did. As far as I can tell, most platforms do not describe the assignment of resources in DT for dma engines, the numbers in there are meant to describe whatever is fixed, and most platforms should do it that way. The naming is sometimes a bit confusing, as a dma request line assignment can be called a slave-id or a channel-id depending whose documentation you read. The drivers/dma/virt-dma.c implementation is used in some cases to represent request numbers as virtual channels, so a dmaengine driver can allow one dma_request_chan() per slave, and then assign the hardware channels dynamically while doing a transfer, rather than having a fixed channel assignment when we first ask for a channel. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id 2019-02-20 9:07 ` Arnd Bergmann @ 2019-02-20 11:29 ` Baolin Wang 0 siblings, 0 replies; 16+ messages in thread From: Baolin Wang @ 2019-02-20 11:29 UTC (permalink / raw) To: Arnd Bergmann Cc: Mark Rutland, DTML, eric.long, arm-soc, Lyra Zhang, Mark Brown, Linux Kernel Mailing List, dmaengine, Vinod Koul, Rob Herring, Geert Uytterhoeven, Olof Johansson, Orson Zhai, Dan Williams, Linux ARM On Wed, 20 Feb 2019 at 17:08, Arnd Bergmann <arnd@arndb.de> wrote: > > On Wed, Feb 20, 2019 at 4:13 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > On Tue, 19 Feb 2019 at 20:20, Vinod Koul <vkoul@kernel.org> wrote: > > > On 19-02-19, 17:49, Baolin Wang wrote: > > > > On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > I did understand the need for a slave-id, I was instead wondering about > > > > > > > the channel-id number. On many SoCs, all channels are equal, and you > > > > > > > just have to pick one of those with the right capabilities for a particular > > > > > > > slave. > > > > > > > > > > > > Yes, all channels are equal. We just set a unique slave id for the > > > > > > channel for a particular slave. For example, the SPI slave can use > > > > > > channel 10 for tx transfer by setting slave id 11, or it also can use > > > > > > channel 9 for tx transfer by setting same slave id 11. > > > > > > > > > > So the channel selection is software policy, not hardware description, and > > > > > thus doesn't belong in DT? > > > > > > > > > > Can't the DMA engine driver allocate channels dynamically, removing the > > > > > need to specify this in DT? > > > > > > > > In theory we can do as you suggested. But we still want to > > > > manage/assign the DMA channel resources manually for one SoC, we can > > > > make sure some important DMA slaves (such as audio) can request a DMA > > > > channel at runtime firstly, another benefit is that it is easy to > > > > debug since we can easily know which channel is assigned for this > > > > slave. > > > > > > Are you suggesting that you have more users than channels available? > > > > Until now we have not met this issue, but we can not make sure if this > > can happen in future. Moreover DMA channel resources are belonging to > > the DMA engine's hardware resources, I think it should be described in > > DT like many other drivers did. > > As far as I can tell, most platforms do not describe the assignment > of resources in DT for dma engines, the numbers in there are meant to > describe whatever is fixed, and most platforms should do it that way. > > The naming is sometimes a bit confusing, as a dma request line > assignment can be called a slave-id or a channel-id depending whose > documentation you read. The drivers/dma/virt-dma.c implementation > is used in some cases to represent request numbers as virtual > channels, so a dmaengine driver can allow one dma_request_chan() > per slave, and then assign the hardware channels dynamically > while doing a transfer, rather than having a fixed channel assignment > when we first ask for a channel. Okay, sounds reasonable to me, and I think no issues will happen if we assign channels dynamically after some slave usages' investigation. I will remove channel id from DT in next version. Thanks for all your help. -- Baolin Wang Best Regards _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-02-20 11:29 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-22 13:20 [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id Baolin Wang 2019-01-22 13:20 ` [PATCH 2/3] dmaengine: sprd: Add new DMA engine translation function Baolin Wang 2019-01-22 13:20 ` [PATCH 3/3] arm64: dts: sprd: Change 2 cells to provide DMA controller specific information Baolin Wang 2019-01-30 16:51 ` [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id Arnd Bergmann 2019-02-01 11:53 ` Baolin Wang 2019-02-12 8:24 ` Baolin Wang 2019-02-18 10:31 ` Arnd Bergmann 2019-02-18 10:52 ` Baolin Wang 2019-02-18 12:23 ` Arnd Bergmann 2019-02-19 3:14 ` Baolin Wang 2019-02-19 9:30 ` Geert Uytterhoeven 2019-02-19 9:49 ` Baolin Wang 2019-02-19 12:20 ` Vinod Koul 2019-02-20 3:12 ` Baolin Wang 2019-02-20 9:07 ` Arnd Bergmann 2019-02-20 11:29 ` Baolin Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).