Add sdma support for the imx8mq
Changes since V1
Fixed to build against v5.0-rc2
Angus Ainslie (Purism) (3):
dma: imx-sdma: fix NULL pointer de-reference
dma: imx-sdma: add clock ratio 1:1 check
imx8mq.dtsi: add the sdma nodes
.../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 31 ++++++++++++++++
drivers/dma/imx-sdma.c | 37 +++++++++++++------
3 files changed, 57 insertions(+), 12 deletions(-)
--
2.17.1
Add the sdma nodes to the base devicetree for the imx8mq
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 31 +++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index c0402375e7c1..0b9a9b5ae7b7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -336,6 +336,19 @@
clocks = <&clk IMX8MQ_CLK_WDOG3_ROOT>;
status = "disabled";
};
+
+ sdma2: sdma@302c0000 {
+ compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+ reg = <0x302c0000 0x10000>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
+ <&clk IMX8MQ_CLK_SDMA2_ROOT>;
+ clock-names = "ipg", "ahb";
+ #dma-cells = <3>;
+ fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+ fsl,ratio-1-1;
+ status = "disabled";
+ };
};
bus@30400000 { /* AIPS2 */
@@ -370,6 +383,8 @@
clocks = <&clk IMX8MQ_CLK_UART3_ROOT>,
<&clk IMX8MQ_CLK_UART3_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 26 4 0>, <&sdma1 27 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -381,6 +396,8 @@
clocks = <&clk IMX8MQ_CLK_UART2_ROOT>,
<&clk IMX8MQ_CLK_UART2_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 24 4 0>, <&sdma1 25 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -432,6 +449,8 @@
clocks = <&clk IMX8MQ_CLK_UART4_ROOT>,
<&clk IMX8MQ_CLK_UART4_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 28 4 0>, <&sdma1 29 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -465,6 +484,18 @@
status = "disabled";
};
+ sdma1: sdma@30bd0000 {
+ compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+ reg = <0x30bd0000 0x10000>;
+ interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
+ <&clk IMX8MQ_CLK_SDMA1_ROOT>;
+ clock-names = "ipg", "ahb";
+ #dma-cells = <3>;
+ fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+ status = "disabled";
+ };
+
fec1: ethernet@30be0000 {
compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
reg = <0x30be0000 0x10000>;
--
2.17.1
On the imx8mq I get NULL pointer de-deference errors if the device
isn't passed in during allocation.
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 86708fb9bda1..0b3a67ff8e82 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -677,7 +677,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
int ret;
unsigned long flags;
- buf_virt = dma_alloc_coherent(NULL, size, &buf_phys, GFP_KERNEL);
+ buf_virt = dma_alloc_coherent(sdma->dev, size, &buf_phys, GFP_KERNEL);
if (!buf_virt) {
return -ENOMEM;
}
@@ -696,7 +696,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
- dma_free_coherent(NULL, size, buf_virt, buf_phys);
+ dma_free_coherent(sdma->dev, size, buf_virt, buf_phys);
return ret;
}
@@ -1182,8 +1182,8 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
{
int ret = -EBUSY;
- sdma->bd0 = dma_alloc_coherent(NULL, PAGE_SIZE, &sdma->bd0_phys,
- GFP_NOWAIT);
+ sdma->bd0 = dma_alloc_coherent(sdma->dev, PAGE_SIZE, &sdma->bd0_phys,
+ GFP_NOWAIT);
if (!sdma->bd0) {
ret = -ENOMEM;
goto out;
@@ -1205,8 +1205,8 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
int ret = 0;
- desc->bd = dma_alloc_coherent(NULL, bd_size, &desc->bd_phys,
- GFP_NOWAIT);
+ desc->bd = dma_alloc_coherent(desc->sdmac->sdma->dev, bd_size,
+ &desc->bd_phys, GFP_NOWAIT);
if (!desc->bd) {
ret = -ENOMEM;
goto out;
@@ -1219,7 +1219,8 @@ static void sdma_free_bd(struct sdma_desc *desc)
{
u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
- dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
+ dma_free_coherent(desc->sdmac->sdma->dev, bd_size, desc->bd,
+ desc->bd_phys);
}
static void sdma_desc_free(struct virt_dma_desc *vd)
@@ -1842,7 +1843,7 @@ static int sdma_init(struct sdma_engine *sdma)
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
- sdma->channel_control = dma_alloc_coherent(NULL,
+ sdma->channel_control = dma_alloc_coherent(sdma->dev,
MAX_DMA_CHANNELS * sizeof (struct sdma_channel_control) +
sizeof(struct sdma_context_data),
&ccb_phys, GFP_KERNEL);
--
2.17.1
On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.
based on NXP commit MLK-16841-1
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
.../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
drivers/dma/imx-sdma.c | 20 +++++++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index 3c9a57a8443b..17544c1820b7 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -67,6 +67,7 @@ Optional properties:
reg is the GPR register offset.
shift is the bit position inside the GPR register.
val is the value of the bit (0 or 1).
+- fsl,ratio-1-1: AHB/SDMA core clock ration 1:1, 2:1 without this.
Examples:
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b3a67ff8e82..65dada21d3c1 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
unsigned int irq;
dma_addr_t bd0_phys;
struct sdma_buffer_descriptor *bd0;
+ /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+ bool clk_ratio;
};
static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
/* Set bits of CONFIG register with dynamic context switching */
- if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
- writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+ if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
+ if (sdma->clk_ratio)
+ reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
+ else
+ reg = SDMA_H_CONFIG_CSM;
+
+ writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+ }
return ret;
}
@@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
/* Set bits of CONFIG register but with static context switching */
- /* FIXME: Check whether to set ACR bit depending on clock ratios */
- writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+ if (sdma->clk_ratio)
+ writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+ else
+ writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
@@ -1975,6 +1985,8 @@ static int sdma_probe(struct platform_device *pdev)
if (!sdma)
return -ENOMEM;
+ sdma->clk_ratio = of_property_read_bool(np, "fsl,ratio-1-1");
+
spin_lock_init(&sdma->channel_0_lock);
sdma->dev = &pdev->dev;
--
2.17.1
On Sun, Jan 20, 2019 at 4:34 AM Angus Ainslie (Purism) <[email protected]> wrote:
>
> On the imx8mq I get NULL pointer de-deference errors if the device
> isn't passed in during allocation.
>
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
Hi Angus,
I have already sent a fix for this:
https://patchwork.kernel.org/patch/10758203/
On Sun, Jan 20, 2019 at 4:32 AM Angus Ainslie (Purism) <[email protected]> wrote:
>
> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> to 500Mhz, so use 1:1 instead.
>
> based on NXP commit MLK-16841-1
Hi Angus,
Thanks for doing this!
I'm not sure specifying the MLK here helps. I think it would be better
to somehow add the original Signed-off-by and mention that the commit
was pulled from NXP linux-imx tree.
>
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> .../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
> drivers/dma/imx-sdma.c | 20 +++++++++++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> index 3c9a57a8443b..17544c1820b7 100644
> --- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> +++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> @@ -67,6 +67,7 @@ Optional properties:
> reg is the GPR register offset.
> shift is the bit position inside the GPR register.
> val is the value of the bit (0 or 1).
> +- fsl,ratio-1-1: AHB/SDMA core clock ration 1:1, 2:1 without this.
>
> Examples:
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 0b3a67ff8e82..65dada21d3c1 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -440,6 +440,8 @@ struct sdma_engine {
> unsigned int irq;
> dma_addr_t bd0_phys;
> struct sdma_buffer_descriptor *bd0;
> + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> + bool clk_ratio;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
> dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>
> /* Set bits of CONFIG register with dynamic context switching */
> - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
> - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
> + if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
> + if (sdma->clk_ratio)
> + reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
> + else
> + reg = SDMA_H_CONFIG_CSM;
> +
> + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
> + }
>
> return ret;
> }
> @@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
> writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>
> /* Set bits of CONFIG register but with static context switching */
> - /* FIXME: Check whether to set ACR bit depending on clock ratios */
> - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
> + if (sdma->clk_ratio)
> + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
> + else
> + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
> writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
> @@ -1975,6 +1985,8 @@ static int sdma_probe(struct platform_device *pdev)
> if (!sdma)
> return -ENOMEM;
>
> + sdma->clk_ratio = of_property_read_bool(np, "fsl,ratio-1-1");
> +
> spin_lock_init(&sdma->channel_0_lock);
>
> sdma->dev = &pdev->dev;
> --
> 2.17.1
>
Hi Daniel,
On 2019-01-20 02:58, Daniel Baluta wrote:
> On Sun, Jan 20, 2019 at 4:32 AM Angus Ainslie (Purism) <[email protected]>
> wrote:
>>
>> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
>> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
>> to 500Mhz, so use 1:1 instead.
>>
>> based on NXP commit MLK-16841-1
>
> Hi Angus,
>
> Thanks for doing this!
>
> I'm not sure specifying the MLK here helps. I think it would be better
> to somehow add the original Signed-off-by and mention that the commit
> was pulled from NXP linux-imx tree.
If it was exactly the same patch I would have but as I added the lines
in
sdma_run_channel0 I didn't think it would be right to put signed off by
"Robin Gong <[email protected]>" as the code isn't what he signed off
on.
>>
>> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
>> ---
>> .../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
>> drivers/dma/imx-sdma.c | 20
>> +++++++++++++++----
>> 2 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
>> b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
>> index 3c9a57a8443b..17544c1820b7 100644
>> --- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
>> +++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
>> @@ -67,6 +67,7 @@ Optional properties:
>> reg is the GPR register offset.
>> shift is the bit position inside the GPR register.
>> val is the value of the bit (0 or 1).
>> +- fsl,ratio-1-1: AHB/SDMA core clock ration 1:1, 2:1 without this.
>>
>> Examples:
>>
>> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
>> index 0b3a67ff8e82..65dada21d3c1 100644
>> --- a/drivers/dma/imx-sdma.c
>> +++ b/drivers/dma/imx-sdma.c
>> @@ -440,6 +440,8 @@ struct sdma_engine {
>> unsigned int irq;
>> dma_addr_t bd0_phys;
>> struct sdma_buffer_descriptor *bd0;
>> + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
>> + bool clk_ratio;
>> };
>>
>> static int sdma_config_write(struct dma_chan *chan,
>> @@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine
>> *sdma)
>> dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>>
>> /* Set bits of CONFIG register with dynamic context switching
>> */
>> - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
>> - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs +
>> SDMA_H_CONFIG);
>> + if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
>> + if (sdma->clk_ratio)
>> + reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
>> + else
>> + reg = SDMA_H_CONFIG_CSM;
>> +
>> + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
>> + }
>>
This is the code that I added out of an over abundance of prudence.
Angus
>> return ret;
>> }
>> @@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
>> writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>>
>> /* Set bits of CONFIG register but with static context
>> switching */
>> - /* FIXME: Check whether to set ACR bit depending on clock
>> ratios */
>> - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>> + if (sdma->clk_ratio)
>> + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs +
>> SDMA_H_CONFIG);
>> + else
>> + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>>
>> writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>>
>> @@ -1975,6 +1985,8 @@ static int sdma_probe(struct platform_device
>> *pdev)
>> if (!sdma)
>> return -ENOMEM;
>>
>> + sdma->clk_ratio = of_property_read_bool(np, "fsl,ratio-1-1");
>> +
>> spin_lock_init(&sdma->channel_0_lock);
>>
>> sdma->dev = &pdev->dev;
>> --
>> 2.17.1
>>
On 2019-01-20 02:54, Daniel Baluta wrote:
> On Sun, Jan 20, 2019 at 4:34 AM Angus Ainslie (Purism) <[email protected]>
> wrote:
>>
>> On the imx8mq I get NULL pointer de-deference errors if the device
>> isn't passed in during allocation.
>>
>> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
>
> Hi Angus,
>
> I have already sent a fix for this:
>
> https://patchwork.kernel.org/patch/10758203/
Sorry, I missed that. I'll drop it for V3.
Angus
On Sun, Jan 20, 2019 at 4:38 PM Angus Ainslie <[email protected]> wrote:
>
> Hi Daniel,
>
> On 2019-01-20 02:58, Daniel Baluta wrote:
> > On Sun, Jan 20, 2019 at 4:32 AM Angus Ainslie (Purism) <[email protected]>
> > wrote:
> >>
> >> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> >> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> >> to 500Mhz, so use 1:1 instead.
> >>
> >> based on NXP commit MLK-16841-1
> >
> > Hi Angus,
> >
> > Thanks for doing this!
> >
> > I'm not sure specifying the MLK here helps. I think it would be better
> > to somehow add the original Signed-off-by and mention that the commit
> > was pulled from NXP linux-imx tree.
>
> If it was exactly the same patch I would have but as I added the lines
> in
> sdma_run_channel0 I didn't think it would be right to put signed off by
> "Robin Gong <[email protected]>" as the code isn't what he signed off
> on.
I see your point. I often encounter this scenario when sending patches.
I think the best solution would be to mention in the commit message
the author of the initial patch which you based your work on.
Hi Angus,
Am Samstag, den 19.01.2019, 19:31 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> to 500Mhz, so use 1:1 instead.
>
> based on NXP commit MLK-16841-1
>
> > Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> .../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
> drivers/dma/imx-sdma.c | 20 +++++++++++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> index 3c9a57a8443b..17544c1820b7 100644
> --- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> +++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> @@ -67,6 +67,7 @@ Optional properties:
> reg is the GPR register offset.
> shift is the bit position inside the GPR register.
> val is the value of the bit (0 or 1).
> +- fsl,ratio-1-1: AHB/SDMA core clock ration 1:1, 2:1 without this.
Why does this need a separate DT property? Shouldn't the driver be able
to infer this from the clock rates going into the SDMA hardware?
Regards,
Lucas
> Examples:
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 0b3a67ff8e82..65dada21d3c1 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -440,6 +440,8 @@ struct sdma_engine {
> > > unsigned int irq;
> > > dma_addr_t bd0_phys;
> > > struct sdma_buffer_descriptor *bd0;
> > + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> > > + bool clk_ratio;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
> > dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>
> > /* Set bits of CONFIG register with dynamic context switching */
> > - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
> > - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
> > + if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
> > + if (sdma->clk_ratio)
> > + reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
> > + else
> > + reg = SDMA_H_CONFIG_CSM;
> +
> > + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
> > + }
>
> > return ret;
> }
> @@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
> > writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>
> > /* Set bits of CONFIG register but with static context switching */
> > - /* FIXME: Check whether to set ACR bit depending on clock ratios */
> > - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
> > + if (sdma->clk_ratio)
> > + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
> > + else
> > + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
> > writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
> @@ -1975,6 +1985,8 @@ static int sdma_probe(struct platform_device *pdev)
> > if (!sdma)
> > return -ENOMEM;
>
> > + sdma->clk_ratio = of_property_read_bool(np, "fsl,ratio-1-1");
> +
> > spin_lock_init(&sdma->channel_0_lock);
>
> > sdma->dev = &pdev->dev;
Hi Lucas,
On 2019-01-21 02:17, Lucas Stach wrote:
> Hi Angus,
>
> Am Samstag, den 19.01.2019, 19:31 -0700 schrieb Angus Ainslie (Purism):
>> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
>> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
>> to 500Mhz, so use 1:1 instead.
>>
>> based on NXP commit MLK-16841-1
>>
>> > Signed-off-by: Angus Ainslie (Purism) <[email protected]>
>> ---
>> .../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
>> drivers/dma/imx-sdma.c | 20
>> +++++++++++++++----
>> 2 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
>> b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
>> index 3c9a57a8443b..17544c1820b7 100644
>> --- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
>> +++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
>> @@ -67,6 +67,7 @@ Optional properties:
>> reg is the GPR register offset.
>> shift is the bit position inside the GPR register.
>> val is the value of the bit (0 or 1).
>> +- fsl,ratio-1-1: AHB/SDMA core clock ration 1:1, 2:1 without this.
>
> Why does this need a separate DT property? Shouldn't the driver be able
> to infer this from the clock rates going into the SDMA hardware?
>
That's probably a better way to do it then setting the property can't
get missed. I'll address it in v3.
Angus
> Regards,
> Lucas
>
>> Examples:
>>
>> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
>> index 0b3a67ff8e82..65dada21d3c1 100644
>> --- a/drivers/dma/imx-sdma.c
>> +++ b/drivers/dma/imx-sdma.c
>> @@ -440,6 +440,8 @@ struct sdma_engine {
>> > > unsigned int irq;
>> > > dma_addr_t bd0_phys;
>> > > struct sdma_buffer_descriptor *bd0;
>> > + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
>> > > + bool clk_ratio;
>> };
>>
>> static int sdma_config_write(struct dma_chan *chan,
>> @@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine
>> *sdma)
>> > dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>>
>> > /* Set bits of CONFIG register with dynamic context switching */
>> > - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
>> > - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
>> > + if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
>> > + if (sdma->clk_ratio)
>> > + reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
>> > + else
>> > + reg = SDMA_H_CONFIG_CSM;
>> +
>> > + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
>> > + }
>>
>> > return ret;
>> }
>> @@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
>> > writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>>
>> > /* Set bits of CONFIG register but with static context switching */
>> > - /* FIXME: Check whether to set ACR bit depending on clock ratios */
>> > - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>> > + if (sdma->clk_ratio)
>> > + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
>> > + else
>> > + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>>
>> > writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>>
>> @@ -1975,6 +1985,8 @@ static int sdma_probe(struct platform_device
>> *pdev)
>> > if (!sdma)
>> > return -ENOMEM;
>>
>> > + sdma->clk_ratio = of_property_read_bool(np, "fsl,ratio-1-1");
>> +
>> > spin_lock_init(&sdma->channel_0_lock);
>>
>> > sdma->dev = &pdev->dev;
Add sdma support for the imx8mq
Changes since V2
Dropped device tree bindings and added clock ratio check.
Fixed bug where incorrect sdma device was selected.
Added new compatible string for "fsl,imx8mq-sdma"
Changes since V1
Fixed to build against v5.0-rc2
Angus Ainslie (Purism) (5):
dma: imx-sdma: add clock ratio 1:1 check
dma: imx-sdma: add imx8mq sdma compatible parts
dt-bindings: dma: fsl-imx-sdma: add imx8mq compatible string
dma: imx-sdma: add an index for imx8mq multi sdma devices
imx8mq.dtsi: add the sdma nodes
.../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 31 +++++++++++++
drivers/dma/imx-sdma.c | 45 +++++++++++++++++--
include/linux/platform_data/dma-imx.h | 1 +
4 files changed, 74 insertions(+), 4 deletions(-)
--
2.17.1
Add the sdma nodes to the base devicetree for the imx8mq
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 31 +++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index c0402375e7c1..4397992fd021 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -336,6 +336,19 @@
clocks = <&clk IMX8MQ_CLK_WDOG3_ROOT>;
status = "disabled";
};
+
+ sdma2: sdma@302c0000 {
+ compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+ reg = <0x302c0000 0x10000>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
+ <&clk IMX8MQ_CLK_SDMA2_ROOT>;
+ clock-names = "ipg", "ahb";
+ #dma-cells = <3>;
+ fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+ fsl,ratio-1-1;
+ status = "disabled";
+ };
};
bus@30400000 { /* AIPS2 */
@@ -370,6 +383,8 @@
clocks = <&clk IMX8MQ_CLK_UART3_ROOT>,
<&clk IMX8MQ_CLK_UART3_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 26 4 0>, <&sdma1 27 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -381,6 +396,8 @@
clocks = <&clk IMX8MQ_CLK_UART2_ROOT>,
<&clk IMX8MQ_CLK_UART2_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 24 4 0>, <&sdma1 25 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -432,6 +449,8 @@
clocks = <&clk IMX8MQ_CLK_UART4_ROOT>,
<&clk IMX8MQ_CLK_UART4_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 28 4 0>, <&sdma1 29 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -465,6 +484,18 @@
status = "disabled";
};
+ sdma1: sdma@30bd0000 {
+ compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+ reg = <0x30bd0000 0x10000>;
+ interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
+ <&clk IMX8MQ_CLK_AHB>;
+ clock-names = "ipg", "ahb";
+ #dma-cells = <3>;
+ fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+ status = "disabled";
+ };
+
fec1: ethernet@30be0000 {
compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
reg = <0x30be0000 0x10000>;
--
2.17.1
On i.mx8mq, there are two sdma instances, and the common dma framework
will get a channel dynamically from any available sdma instance whether
it's the first sdma device or the second sdma device. Some IPs like
SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
the correct sdma device, use an index to match.
Based on MLK-16104-2 by Robin Gong <[email protected]>
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 12 ++++++++++++
include/linux/platform_data/dma-imx.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 2e691b1cd0eb..bf3752a6a64f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -442,6 +442,7 @@ struct sdma_engine {
struct sdma_buffer_descriptor *bd0;
/* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
bool clk_ratio;
+ int idx;
};
static int sdma_config_write(struct dma_chan *chan,
@@ -606,6 +607,8 @@ static const struct of_device_id sdma_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, sdma_dt_ids);
+static int sdma_dev_idx;
+
#define SDMA_H_CONFIG_DSPDMA BIT(12) /* indicates if the DSPDMA is used */
#define SDMA_H_CONFIG_RTD_PINS BIT(11) /* indicates if Real-Time Debug pins are enabled */
#define SDMA_H_CONFIG_ACR BIT(4) /* indicates if AHB freq /core freq = 2 or 1 */
@@ -1934,6 +1937,11 @@ static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
if (!imx_dma_is_general_purpose(chan))
return false;
+ /* return false if it's not the right device */
+ if ((sdmac->sdma->drvdata == &sdma_imx8mq)
+ && (sdmac->sdma->idx != data->idx))
+ return false;
+
sdmac->data = *data;
chan->private = &sdmac->data;
@@ -1961,6 +1969,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
* be set to sdmac->event_id1.
*/
data.dma_request2 = 0;
+ data.idx = sdma->idx;
return dma_request_channel(mask, sdma_filter_fn, &data);
}
@@ -2149,6 +2158,9 @@ static int sdma_probe(struct platform_device *pdev)
of_node_put(spba_bus);
}
+ /* There maybe multi sdma devices such as i.mx8mq */
+ sdma->idx = sdma_dev_idx++;
+
return 0;
err_register:
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index 7d964e787299..843faf081282 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -55,6 +55,7 @@ struct imx_dma_data {
int dma_request2; /* secondary DMA request line */
enum sdma_peripheral_type peripheral_type;
int priority;
+ int idx;
};
static inline int imx_dma_is_ipu(struct dma_chan *chan)
--
2.17.1
Add "fsl,imx8mq-sdma" to the list of accepted compatible strings.
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index 3c9a57a8443b..9d8bbac27d8b 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -9,6 +9,7 @@ Required properties:
"fsl,imx53-sdma"
"fsl,imx6q-sdma"
"fsl,imx7d-sdma"
+ "fsl,imx8mq-sdma"
The -to variants should be preferred since they allow to determine the
correct ROM script addresses needed for the driver to work without additional
firmware.
--
2.17.1
This is identical to the imx7d data structures but we need to
be able to differentiate that the imx8mq has 2 sdma controllers.
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 531a9d8b032a..2e691b1cd0eb 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -556,6 +556,12 @@ static struct sdma_driver_data sdma_imx7d = {
.script_addrs = &sdma_script_imx7d,
};
+static struct sdma_driver_data sdma_imx8mq = {
+ .chnenbl0 = SDMA_CHNENBL0_IMX35,
+ .num_events = 48,
+ .script_addrs = &sdma_script_imx7d,
+};
+
static const struct platform_device_id sdma_devtypes[] = {
{
.name = "imx25-sdma",
@@ -578,6 +584,9 @@ static const struct platform_device_id sdma_devtypes[] = {
}, {
.name = "imx7d-sdma",
.driver_data = (unsigned long)&sdma_imx7d,
+ }, {
+ .name = "imx8mq-sdma",
+ .driver_data = (unsigned long)&sdma_imx8mq,
}, {
/* sentinel */
}
@@ -592,6 +601,7 @@ static const struct of_device_id sdma_dt_ids[] = {
{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
{ .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, },
{ .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, },
+ { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx8mq, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, sdma_dt_ids);
--
2.17.1
On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.
Based on NXP commit MLK-16841-1 by Robin Gong <[email protected]>
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b3a67ff8e82..531a9d8b032a 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
unsigned int irq;
dma_addr_t bd0_phys;
struct sdma_buffer_descriptor *bd0;
+ /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+ bool clk_ratio;
};
static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
/* Set bits of CONFIG register with dynamic context switching */
- if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
- writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+ if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
+ if (sdma->clk_ratio)
+ reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
+ else
+ reg = SDMA_H_CONFIG_CSM;
+
+ writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+ }
return ret;
}
@@ -1840,6 +1848,11 @@ static int sdma_init(struct sdma_engine *sdma)
if (ret)
goto disable_clk_ipg;
+ if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
+ sdma->clk_ratio = 1;
+ else
+ sdma->clk_ratio = 0;
+
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
@@ -1880,8 +1893,10 @@ static int sdma_init(struct sdma_engine *sdma)
writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
/* Set bits of CONFIG register but with static context switching */
- /* FIXME: Check whether to set ACR bit depending on clock ratios */
- writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+ if (sdma->clk_ratio)
+ writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+ else
+ writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
--
2.17.1
Hi Angus,
Am Mittwoch, den 23.01.2019, 08:23 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8mq, there are two sdma instances, and the common dma framework
> will get a channel dynamically from any available sdma instance whether
> it's the first sdma device or the second sdma device. Some IPs like
> SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
> the correct sdma device, use an index to match.
>
> Based on MLK-16104-2 by Robin Gong <[email protected]>
This relies on the probe order of the devices (which should be treated
as random) for the match to find the right device. This is not
acceptable upstream.
The DT "dmas" property already specifies the correct SDMA device to use
for a consumer, so the filter function should really match the OF node
of the SDMA device with the node specified in the dmas phandle in order
to pick the right SDMA engine.
Regards,
Lucas
>
> > Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 12 ++++++++++++
> include/linux/platform_data/dma-imx.h | 1 +
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 2e691b1cd0eb..bf3752a6a64f 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -442,6 +442,7 @@ struct sdma_engine {
> > > struct sdma_buffer_descriptor *bd0;
> > /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> > > bool clk_ratio;
> > > + int idx;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -606,6 +607,8 @@ static const struct of_device_id sdma_dt_ids[] = {
> };
> MODULE_DEVICE_TABLE(of, sdma_dt_ids);
>
> +static int sdma_dev_idx;
> +
> > #define SDMA_H_CONFIG_DSPDMA BIT(12) /* indicates if the DSPDMA is used */
> > #define SDMA_H_CONFIG_RTD_PINS BIT(11) /* indicates if Real-Time Debug pins are enabled */
> > #define SDMA_H_CONFIG_ACR BIT(4) /* indicates if AHB freq /core freq = 2 or 1 */
> @@ -1934,6 +1937,11 @@ static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
> > if (!imx_dma_is_general_purpose(chan))
> > return false;
>
> > + /* return false if it's not the right device */
> > + if ((sdmac->sdma->drvdata == &sdma_imx8mq)
> > + && (sdmac->sdma->idx != data->idx))
> > + return false;
> +
> > sdmac->data = *data;
> > chan->private = &sdmac->data;
>
> @@ -1961,6 +1969,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
> > * be set to sdmac->event_id1.
> > */
> > data.dma_request2 = 0;
> > + data.idx = sdma->idx;
>
> > return dma_request_channel(mask, sdma_filter_fn, &data);
> }
> @@ -2149,6 +2158,9 @@ static int sdma_probe(struct platform_device *pdev)
> > of_node_put(spba_bus);
> > }
>
> > + /* There maybe multi sdma devices such as i.mx8mq */
> > + sdma->idx = sdma_dev_idx++;
> +
> > return 0;
>
> err_register:
> diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
> index 7d964e787299..843faf081282 100644
> --- a/include/linux/platform_data/dma-imx.h
> +++ b/include/linux/platform_data/dma-imx.h
> @@ -55,6 +55,7 @@ struct imx_dma_data {
> > int dma_request2; /* secondary DMA request line */
> > enum sdma_peripheral_type peripheral_type;
> > int priority;
> > + int idx;
> };
>
> static inline int imx_dma_is_ipu(struct dma_chan *chan)
On 2019-01-23 08:31, Lucas Stach wrote:
> Hi Angus,
>
> Am Mittwoch, den 23.01.2019, 08:23 -0700 schrieb Angus Ainslie
> (Purism):
>> On i.mx8mq, there are two sdma instances, and the common dma framework
>> will get a channel dynamically from any available sdma instance
>> whether
>> it's the first sdma device or the second sdma device. Some IPs like
>> SAI only work with sdma2 not sdma1. To make sure the sdma channel is
>> from
>> the correct sdma device, use an index to match.
>>
>> Based on MLK-16104-2 by Robin Gong <[email protected]>
>
> This relies on the probe order of the devices (which should be treated
> as random) for the match to find the right device. This is not
> acceptable upstream.
>
> The DT "dmas" property already specifies the correct SDMA device to use
> for a consumer, so the filter function should really match the OF node
> of the SDMA device with the node specified in the dmas phandle in order
> to pick the right SDMA engine.
>
Thanks Lucas, I'll fix it for rev 4
> Regards,
> Lucas
>
>>
>> > Signed-off-by: Angus Ainslie (Purism) <[email protected]>
>> ---
>> drivers/dma/imx-sdma.c | 12 ++++++++++++
>> include/linux/platform_data/dma-imx.h | 1 +
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
>> index 2e691b1cd0eb..bf3752a6a64f 100644
>> --- a/drivers/dma/imx-sdma.c
>> +++ b/drivers/dma/imx-sdma.c
>> @@ -442,6 +442,7 @@ struct sdma_engine {
>> > > struct sdma_buffer_descriptor *bd0;
>> > /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
>> > > bool clk_ratio;
>> > > + int idx;
>> };
>>
>> static int sdma_config_write(struct dma_chan *chan,
>> @@ -606,6 +607,8 @@ static const struct of_device_id sdma_dt_ids[] = {
>> };
>> MODULE_DEVICE_TABLE(of, sdma_dt_ids);
>>
>> +static int sdma_dev_idx;
>> +
>> > #define SDMA_H_CONFIG_DSPDMA BIT(12) /* indicates if the DSPDMA is used */
>> > #define SDMA_H_CONFIG_RTD_PINS BIT(11) /* indicates if Real-Time Debug pins are enabled */
>> > #define SDMA_H_CONFIG_ACR BIT(4) /* indicates if AHB freq /core freq = 2 or 1 */
>> @@ -1934,6 +1937,11 @@ static bool sdma_filter_fn(struct dma_chan
>> *chan, void *fn_param)
>> > if (!imx_dma_is_general_purpose(chan))
>> > return false;
>>
>> > + /* return false if it's not the right device */
>> > + if ((sdmac->sdma->drvdata == &sdma_imx8mq)
>> > + && (sdmac->sdma->idx != data->idx))
>> > + return false;
>> +
>> > sdmac->data = *data;
>> > chan->private = &sdmac->data;
>>
>> @@ -1961,6 +1969,7 @@ static struct dma_chan *sdma_xlate(struct
>> of_phandle_args *dma_spec,
>> > * be set to sdmac->event_id1.
>> > */
>> > data.dma_request2 = 0;
>> > + data.idx = sdma->idx;
>>
>> > return dma_request_channel(mask, sdma_filter_fn, &data);
>> }
>> @@ -2149,6 +2158,9 @@ static int sdma_probe(struct platform_device
>> *pdev)
>> > of_node_put(spba_bus);
>> > }
>>
>> > + /* There maybe multi sdma devices such as i.mx8mq */
>> > + sdma->idx = sdma_dev_idx++;
>> +
>> > return 0;
>>
>> err_register:
>> diff --git a/include/linux/platform_data/dma-imx.h
>> b/include/linux/platform_data/dma-imx.h
>> index 7d964e787299..843faf081282 100644
>> --- a/include/linux/platform_data/dma-imx.h
>> +++ b/include/linux/platform_data/dma-imx.h
>> @@ -55,6 +55,7 @@ struct imx_dma_data {
>> > int dma_request2; /* secondary DMA request line */
>> > enum sdma_peripheral_type peripheral_type;
>> > int priority;
>> > + int idx;
>> };
>>
>> static inline int imx_dma_is_ipu(struct dma_chan *chan)
Am Mittwoch, den 23.01.2019, 08:23 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> to 500Mhz, so use 1:1 instead.
>
> > Based on NXP commit MLK-16841-1 by Robin Gong <[email protected]>
>
> > Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 0b3a67ff8e82..531a9d8b032a 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -440,6 +440,8 @@ struct sdma_engine {
> > > unsigned int irq;
> > > dma_addr_t bd0_phys;
> > > struct sdma_buffer_descriptor *bd0;
> > + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> > > + bool clk_ratio;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
> > dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>
> > /* Set bits of CONFIG register with dynamic context switching */
> > - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
> > - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
> + if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
If the ACR bit gets set in sdma_init(), do we ever end up in this code
path? From a quick glance it seems we might wrongfully skip the CSM
enable here.
> + if (sdma->clk_ratio)
> > + reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
> > + else
> + reg = SDMA_H_CONFIG_CSM;
That's a personal style preference, but I would write this as:
reg = SDMA_H_CONFIG_CSM;
if (sdma->clk_ratio);
reg |= SDMA_H_CONFIG_ACR;
> +
> > + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
> > + }
>
> > return ret;
> }
> @@ -1840,6 +1848,11 @@ static int sdma_init(struct sdma_engine *sdma)
> > if (ret)
> > goto disable_clk_ipg;
>
> > + if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
> > + sdma->clk_ratio = 1;
> > + else
> + sdma->clk_ratio = 0;
sdma is zeroed at allocation, so the else path here isn't necessary.
> +
> > /* Be sure SDMA has not started yet */
> > writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
>
> @@ -1880,8 +1893,10 @@ static int sdma_init(struct sdma_engine *sdma)
> > writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>
> > /* Set bits of CONFIG register but with static context switching */
> > - /* FIXME: Check whether to set ACR bit depending on clock ratios */
> > - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
> > + if (sdma->clk_ratio)
> > + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
> > + else
> > + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
> > writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
On 23-01-19, 08:42, Angus Ainslie wrote:
> On 2019-01-23 08:31, Lucas Stach wrote:
> > Hi Angus,
> >
> > Am Mittwoch, den 23.01.2019, 08:23 -0700 schrieb Angus Ainslie (Purism):
> > > On i.mx8mq, there are two sdma instances, and the common dma framework
> > > will get a channel dynamically from any available sdma instance
> > > whether
> > > it's the first sdma device or the second sdma device. Some IPs like
> > > SAI only work with sdma2 not sdma1. To make sure the sdma channel is
> > > from
> > > the correct sdma device, use an index to match.
> > >
> > > Based on MLK-16104-2 by Robin Gong <[email protected]>
> >
> > This relies on the probe order of the devices (which should be treated
> > as random) for the match to find the right device. This is not
> > acceptable upstream.
> >
> > The DT "dmas" property already specifies the correct SDMA device to use
> > for a consumer, so the filter function should really match the OF node
> > of the SDMA device with the node specified in the dmas phandle in order
> > to pick the right SDMA engine.
> >
>
> Thanks Lucas, I'll fix it for rev 4
And fix the subsystem name, it is 'dmaengine: ...' not 'dma: ...'. git
log of the subsystem should give you good clue of the tags to be used..
Thanks
--
~Vinod
Am Mittwoch, den 23.01.2019, 08:23 -0700 schrieb Angus Ainslie (Purism):
> Add the sdma nodes to the base devicetree for the imx8mq
>
> > Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 31 +++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index c0402375e7c1..4397992fd021 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -336,6 +336,19 @@
> > clocks = <&clk IMX8MQ_CLK_WDOG3_ROOT>;
> > status = "disabled";
> > };
> +
> > > + sdma2: sdma@302c0000 {
> > + compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
> > + reg = <0x302c0000 0x10000>;
> > + interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
> > + <&clk IMX8MQ_CLK_SDMA2_ROOT>;
> > + clock-names = "ipg", "ahb";
> > + #dma-cells = <3>;
> > + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> + fsl,ratio-1-1;
Property does not exist anymore.
> + status = "disabled";
SoC peripherals that have no outside connection and thus don't depend
on any board level configuration should always be enabled. Please drop
this status property.
> + };
> > };
>
> > bus@30400000 { /* AIPS2 */
> @@ -370,6 +383,8 @@
> > clocks = <&clk IMX8MQ_CLK_UART3_ROOT>,
> > <&clk IMX8MQ_CLK_UART3_ROOT>;
> > clock-names = "ipg", "per";
> > + dmas = <&sdma1 26 4 0>, <&sdma1 27 4 0>;
> > + dma-names = "rx", "tx";
> > status = "disabled";
> > };
>
> @@ -381,6 +396,8 @@
> > clocks = <&clk IMX8MQ_CLK_UART2_ROOT>,
> > <&clk IMX8MQ_CLK_UART2_ROOT>;
> > clock-names = "ipg", "per";
> > + dmas = <&sdma1 24 4 0>, <&sdma1 25 4 0>;
> > + dma-names = "rx", "tx";
> > status = "disabled";
> > };
>
> @@ -432,6 +449,8 @@
> > clocks = <&clk IMX8MQ_CLK_UART4_ROOT>,
> > <&clk IMX8MQ_CLK_UART4_ROOT>;
> > clock-names = "ipg", "per";
> > + dmas = <&sdma1 28 4 0>, <&sdma1 29 4 0>;
> > + dma-names = "rx", "tx";
> > status = "disabled";
> > };
>
> @@ -465,6 +484,18 @@
> > status = "disabled";
> > };
>
> > > + sdma1: sdma@30bd0000 {
> > + compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
> > + reg = <0x30bd0000 0x10000>;
> > + interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
> > + <&clk IMX8MQ_CLK_AHB>;
> > + clock-names = "ipg", "ahb";
> > + #dma-cells = <3>;
> > + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> + status = "disabled";
Same comment as above, this node should be enabled by default.
Regards,
Lucas
> + };
> +
> > > fec1: ethernet@30be0000 {
> > compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
> > reg = <0x30be0000 0x10000>;
This is identical to the imx7d data structures but we need to
be able to differentiate that the imx8mq has 2 sdma controllers.
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 5e5ef0b5a973..ec6fb48f387a 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -556,6 +556,12 @@ static struct sdma_driver_data sdma_imx7d = {
.script_addrs = &sdma_script_imx7d,
};
+static struct sdma_driver_data sdma_imx8mq = {
+ .chnenbl0 = SDMA_CHNENBL0_IMX35,
+ .num_events = 48,
+ .script_addrs = &sdma_script_imx7d,
+};
+
static const struct platform_device_id sdma_devtypes[] = {
{
.name = "imx25-sdma",
@@ -578,6 +584,9 @@ static const struct platform_device_id sdma_devtypes[] = {
}, {
.name = "imx7d-sdma",
.driver_data = (unsigned long)&sdma_imx7d,
+ }, {
+ .name = "imx8mq-sdma",
+ .driver_data = (unsigned long)&sdma_imx8mq,
}, {
/* sentinel */
}
@@ -592,6 +601,7 @@ static const struct of_device_id sdma_dt_ids[] = {
{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
{ .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, },
{ .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, },
+ { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx8mq, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, sdma_dt_ids);
--
2.17.1
Add the sdma nodes to the base devicetree for the imx8mq
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 28 +++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index c0402375e7c1..f0cd3675ead0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -336,6 +336,17 @@
clocks = <&clk IMX8MQ_CLK_WDOG3_ROOT>;
status = "disabled";
};
+
+ sdma2: sdma@302c0000 {
+ compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+ reg = <0x302c0000 0x10000>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
+ <&clk IMX8MQ_CLK_SDMA2_ROOT>;
+ clock-names = "ipg", "ahb";
+ #dma-cells = <3>;
+ fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+ };
};
bus@30400000 { /* AIPS2 */
@@ -370,6 +381,8 @@
clocks = <&clk IMX8MQ_CLK_UART3_ROOT>,
<&clk IMX8MQ_CLK_UART3_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 26 4 0>, <&sdma1 27 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -381,6 +394,8 @@
clocks = <&clk IMX8MQ_CLK_UART2_ROOT>,
<&clk IMX8MQ_CLK_UART2_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 24 4 0>, <&sdma1 25 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -432,6 +447,8 @@
clocks = <&clk IMX8MQ_CLK_UART4_ROOT>,
<&clk IMX8MQ_CLK_UART4_ROOT>;
clock-names = "ipg", "per";
+ dmas = <&sdma1 28 4 0>, <&sdma1 29 4 0>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -465,6 +482,17 @@
status = "disabled";
};
+ sdma1: sdma@30bd0000 {
+ compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+ reg = <0x30bd0000 0x10000>;
+ interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
+ <&clk IMX8MQ_CLK_AHB>;
+ clock-names = "ipg", "ahb";
+ #dma-cells = <3>;
+ fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+ };
+
fec1: ethernet@30be0000 {
compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
reg = <0x30be0000 0x10000>;
--
2.17.1
On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.
Based on NXP commit MLK-16841-1 by Robin Gong <[email protected]>
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b3a67ff8e82..5e5ef0b5a973 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
unsigned int irq;
dma_addr_t bd0_phys;
struct sdma_buffer_descriptor *bd0;
+ /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+ bool clk_ratio;
};
static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
/* Set bits of CONFIG register with dynamic context switching */
- if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
- writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+ if ((readl(sdma->regs + SDMA_H_CONFIG) & ~SDMA_H_CONFIG_ACR) == 0) {
+ reg = SDMA_H_CONFIG_CSM;
+
+ if (sdma->clk_ratio)
+ reg |= SDMA_H_CONFIG_ACR;
+
+ writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+ }
return ret;
}
@@ -1840,6 +1848,9 @@ static int sdma_init(struct sdma_engine *sdma)
if (ret)
goto disable_clk_ipg;
+ if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
+ sdma->clk_ratio = 1;
+
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
@@ -1880,8 +1891,10 @@ static int sdma_init(struct sdma_engine *sdma)
writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
/* Set bits of CONFIG register but with static context switching */
- /* FIXME: Check whether to set ACR bit depending on clock ratios */
- writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+ if (sdma->clk_ratio)
+ writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+ else
+ writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
--
2.17.1
On i.mx8mq, there are two sdma instances, and the common dma framework
will get a channel dynamically from any available sdma instance whether
it's the first sdma device or the second sdma device. Some IPs like
SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
the correct sdma device, use the phandle to match.
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 8 ++++++++
include/linux/platform_data/dma-imx.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index ec6fb48f387a..88e112a4aabf 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1927,11 +1927,17 @@ static int sdma_init(struct sdma_engine *sdma)
static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_engine *sdma = sdmac->sdma;
struct imx_dma_data *data = fn_param;
if (!imx_dma_is_general_purpose(chan))
return false;
+ /* return false if it's not the right device */
+ if ((sdmac->sdma->drvdata == &sdma_imx8mq)
+ && (sdma->dev->of_node->phandle != data->phandle))
+ return false;
+
sdmac->data = *data;
chan->private = &sdmac->data;
@@ -1942,6 +1948,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma)
{
struct sdma_engine *sdma = ofdma->of_dma_data;
+ struct device_node *np = ofdma->of_node;
dma_cap_mask_t mask = sdma->dma_device.cap_mask;
struct imx_dma_data data;
@@ -1959,6 +1966,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
* be set to sdmac->event_id1.
*/
data.dma_request2 = 0;
+ data.phandle = np->phandle;
return dma_request_channel(mask, sdma_filter_fn, &data);
}
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index 7d964e787299..eeb5b73ae542 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -55,6 +55,7 @@ struct imx_dma_data {
int dma_request2; /* secondary DMA request line */
enum sdma_peripheral_type peripheral_type;
int priority;
+ int phandle;
};
static inline int imx_dma_is_ipu(struct dma_chan *chan)
--
2.17.1
The imx8mq is a slightly different variant on the imx7d
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index 3c9a57a8443b..9d8bbac27d8b 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -9,6 +9,7 @@ Required properties:
"fsl,imx53-sdma"
"fsl,imx6q-sdma"
"fsl,imx7d-sdma"
+ "fsl,imx8mq-sdma"
The -to variants should be preferred since they allow to determine the
correct ROM script addresses needed for the driver to work without additional
firmware.
--
2.17.1
Add sdma support for the imx8mq
Changes since V3
Builds against 5.0-rc3.
Dropped sdma device index matching in favour of phandles.
Corrected subsytem name to dmaengine
Devicetree fixes.
Fixed SDMA_H_CONFIG register bug.
Changes since V2
Dropped device tree bindings and added clock ratio check.
Fixed bug where incorrect sdma device was selected.
Added new compatible string for "fsl,imx8mq-sdma"
Changes since V1
Fixed to build against v5.0-rc2
Angus Ainslie (Purism) (5):
dmaengine: imx-sdma: add clock ratio 1:1 check
dmaengine: imx-sdma: add imx8mq sdma compatible parts
dt-bindings: dma: fsl-imx-sdma: add fsl,imx8mq to the accepted
compatible node
dma: imx-sdma: add a test for imx8mq multi sdma devices
imx8mq.dtsi: add the sdma nodes
.../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 28 +++++++++++++
drivers/dma/imx-sdma.c | 39 +++++++++++++++++--
include/linux/platform_data/dma-imx.h | 1 +
4 files changed, 65 insertions(+), 4 deletions(-)
--
2.17.1
Am Donnerstag, den 24.01.2019, 19:55 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8mq, there are two sdma instances, and the common dma framework
> will get a channel dynamically from any available sdma instance whether
> it's the first sdma device or the second sdma device. Some IPs like
> SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
> the correct sdma device, use the phandle to match.
>
> > Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 8 ++++++++
> include/linux/platform_data/dma-imx.h | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index ec6fb48f387a..88e112a4aabf 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1927,11 +1927,17 @@ static int sdma_init(struct sdma_engine *sdma)
> static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
> {
> > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > + struct sdma_engine *sdma = sdmac->sdma;
> > struct imx_dma_data *data = fn_param;
>
> > if (!imx_dma_is_general_purpose(chan))
> > return false;
>
> > + /* return false if it's not the right device */
> + if ((sdmac->sdma->drvdata == &sdma_imx8mq)
Why do this check only on i.MX8M? This is completely generic and can
and should be done for every SDMA engine with a OF node.
Also why use the phandle to match this instead of the of_node pointer
directly?
Regards,
Lucas
> + && (sdma->dev->of_node->phandle != data->phandle))
> > + return false;
> +
> > sdmac->data = *data;
> > chan->private = &sdmac->data;
>
> @@ -1942,6 +1948,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
> > struct of_dma *ofdma)
> {
> > struct sdma_engine *sdma = ofdma->of_dma_data;
> > + struct device_node *np = ofdma->of_node;
> > dma_cap_mask_t mask = sdma->dma_device.cap_mask;
> > struct imx_dma_data data;
>
> @@ -1959,6 +1966,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
> > * be set to sdmac->event_id1.
> > */
> > data.dma_request2 = 0;
> > + data.phandle = np->phandle;
>
> > return dma_request_channel(mask, sdma_filter_fn, &data);
> }
> diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
> index 7d964e787299..eeb5b73ae542 100644
> --- a/include/linux/platform_data/dma-imx.h
> +++ b/include/linux/platform_data/dma-imx.h
> @@ -55,6 +55,7 @@ struct imx_dma_data {
> > int dma_request2; /* secondary DMA request line */
> > enum sdma_peripheral_type peripheral_type;
> > int priority;
> > + int phandle;
> };
>
> static inline int imx_dma_is_ipu(struct dma_chan *chan)
Am Donnerstag, den 24.01.2019, 19:55 -0700 schrieb Angus Ainslie (Purism):
> This is identical to the imx7d data structures but we need to
> be able to differentiate that the imx8mq has 2 sdma controllers.
No, we don't, see comment on patch 4/5. This can reuse the imx7d data.
Regards,
Lucas
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 5e5ef0b5a973..ec6fb48f387a 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -556,6 +556,12 @@ static struct sdma_driver_data sdma_imx7d = {
> > .script_addrs = &sdma_script_imx7d,
> };
>
> +static struct sdma_driver_data sdma_imx8mq = {
> > + .chnenbl0 = SDMA_CHNENBL0_IMX35,
> > + .num_events = 48,
> > + .script_addrs = &sdma_script_imx7d,
> +};
> +
> static const struct platform_device_id sdma_devtypes[] = {
> > {
> > .name = "imx25-sdma",
> @@ -578,6 +584,9 @@ static const struct platform_device_id sdma_devtypes[] = {
> > }, {
> > .name = "imx7d-sdma",
> > .driver_data = (unsigned long)&sdma_imx7d,
> > + }, {
> > + .name = "imx8mq-sdma",
> > + .driver_data = (unsigned long)&sdma_imx8mq,
> > }, {
> > /* sentinel */
> > }
> @@ -592,6 +601,7 @@ static const struct of_device_id sdma_dt_ids[] = {
> > { .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
> > { .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, },
> > { .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, },
> > + { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx8mq, },
> > { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, sdma_dt_ids);
Am Donnerstag, den 24.01.2019, 19:55 -0700 schrieb Angus Ainslie (Purism):
> Add the sdma nodes to the base devicetree for the imx8mq
>
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
One nit below, with that fixed:
Reviewed-by: Lucas Stach <[email protected]>
You might need to split this patch out from the series and send it to
Shawn separately after the dmaengine changes have been accepted.
regards,
Lucas
> ---
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 28 +++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index c0402375e7c1..f0cd3675ead0 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -336,6 +336,17 @@
> > clocks = <&clk IMX8MQ_CLK_WDOG3_ROOT>;
> > status = "disabled";
> > };
> +
> > > + sdma2: sdma@302c0000 {
> > + compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
> > + reg = <0x302c0000 0x10000>;
> > + interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
> + <&clk IMX8MQ_CLK_SDMA2_ROOT>;
Some spaces to align the second clock reference as in the rest of this
file, please.
> + clock-names = "ipg", "ahb";
> > + #dma-cells = <3>;
> > + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> > + };
> > };
>
> > bus@30400000 { /* AIPS2 */
> @@ -370,6 +381,8 @@
> > clocks = <&clk IMX8MQ_CLK_UART3_ROOT>,
> > <&clk IMX8MQ_CLK_UART3_ROOT>;
> > clock-names = "ipg", "per";
> > + dmas = <&sdma1 26 4 0>, <&sdma1 27 4 0>;
> > + dma-names = "rx", "tx";
> > status = "disabled";
> > };
>
> @@ -381,6 +394,8 @@
> > clocks = <&clk IMX8MQ_CLK_UART2_ROOT>,
> > <&clk IMX8MQ_CLK_UART2_ROOT>;
> > clock-names = "ipg", "per";
> > + dmas = <&sdma1 24 4 0>, <&sdma1 25 4 0>;
> > + dma-names = "rx", "tx";
> > status = "disabled";
> > };
>
> @@ -432,6 +447,8 @@
> > clocks = <&clk IMX8MQ_CLK_UART4_ROOT>,
> > <&clk IMX8MQ_CLK_UART4_ROOT>;
> > clock-names = "ipg", "per";
> > + dmas = <&sdma1 28 4 0>, <&sdma1 29 4 0>;
> > + dma-names = "rx", "tx";
> > status = "disabled";
> > };
>
> @@ -465,6 +482,17 @@
> > status = "disabled";
> > };
>
> > > + sdma1: sdma@30bd0000 {
> > + compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
> > + reg = <0x30bd0000 0x10000>;
> > + interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
> > + <&clk IMX8MQ_CLK_AHB>;
> > + clock-names = "ipg", "ahb";
> > + #dma-cells = <3>;
> > + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> > + };
> +
> > > fec1: ethernet@30be0000 {
> > compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
> > reg = <0x30be0000 0x10000>;
Am Donnerstag, den 24.01.2019, 19:55 -0700 schrieb Angus Ainslie (Purism):
> The imx8mq is a slightly different variant on the imx7d
Not really AFAICS, but it's good to document the used compatibles
anyways, so:
Reviewed-by: Lucas Stach <[email protected]>
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> index 3c9a57a8443b..9d8bbac27d8b 100644
> --- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> +++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> @@ -9,6 +9,7 @@ Required properties:
> "fsl,imx53-sdma"
> "fsl,imx6q-sdma"
> "fsl,imx7d-sdma"
> + "fsl,imx8mq-sdma"
> The -to variants should be preferred since they allow to determine the
> correct ROM script addresses needed for the driver to work without additional
> firmware.
Am Donnerstag, den 24.01.2019, 19:55 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> to 500Mhz, so use 1:1 instead.
>
> > Based on NXP commit MLK-16841-1 by Robin Gong <[email protected]>
>
> > Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 0b3a67ff8e82..5e5ef0b5a973 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -440,6 +440,8 @@ struct sdma_engine {
> > > unsigned int irq;
> > > dma_addr_t bd0_phys;
> > > struct sdma_buffer_descriptor *bd0;
> > + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> > > + bool clk_ratio;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
> > dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>
> > /* Set bits of CONFIG register with dynamic context switching */
> > - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
> > - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
> + if ((readl(sdma->regs + SDMA_H_CONFIG) & ~SDMA_H_CONFIG_ACR) == 0) {
The intention of this code was probably to set the CSM bit if they
weren't set before, so masking out individual bits from the register
and risking to skip this when one of the other bits was set doesn't
seem right.
I guess the whole block can be simplified to:
reg = readl(sdma->regs + SDMA_H_CONFIG);
if ((reg & SDMA_H_CONFIG_CSM) != SDMA_H_CONFIG_CSM)
reg |= SDMA_H_CONFIG_CSM;
writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
Regards,
Lucas
> + reg = SDMA_H_CONFIG_CSM;
> +
> > + if (sdma->clk_ratio)
> > + reg |= SDMA_H_CONFIG_ACR;
> +
> > + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
> > + }
>
> > return ret;
> }
> @@ -1840,6 +1848,9 @@ static int sdma_init(struct sdma_engine *sdma)
> > if (ret)
> > goto disable_clk_ipg;
>
> > + if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
> > + sdma->clk_ratio = 1;
> +
> > /* Be sure SDMA has not started yet */
> > writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
>
> @@ -1880,8 +1891,10 @@ static int sdma_init(struct sdma_engine *sdma)
> > writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>
> > /* Set bits of CONFIG register but with static context switching */
> > - /* FIXME: Check whether to set ACR bit depending on clock ratios */
> > - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
> > + if (sdma->clk_ratio)
> > + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
> > + else
> > + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
> > writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
Without the copy being aligned sdma1 fails ~10% of the time
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index d5f86becf59e..88910ec09568 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -2118,6 +2118,7 @@ static int sdma_probe(struct platform_device *pdev)
sdma->dma_device.device_prep_dma_memcpy = sdma_prep_memcpy;
sdma->dma_device.device_issue_pending = sdma_issue_pending;
sdma->dma_device.dev->dma_parms = &sdma->dma_parms;
+ sdma->dma_device.copy_align = 2;
dma_set_max_seg_size(sdma->dma_device.dev, SDMA_BD_MAX_CNT);
platform_set_drvdata(pdev, sdma);
--
2.17.1
On i.mx8mq, there are two sdma instances, and the common dma framework
will get a channel dynamically from any available sdma instance whether
it's the first sdma device or the second sdma device. Some IPs like
SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
the correct sdma device, use the node pointer to match.
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 6 ++++++
include/linux/platform_data/dma-imx.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index c4db4fe6bcc9..d5f86becf59e 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1918,11 +1918,16 @@ static int sdma_init(struct sdma_engine *sdma)
static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_engine *sdma = sdmac->sdma;
struct imx_dma_data *data = fn_param;
if (!imx_dma_is_general_purpose(chan))
return false;
+ /* return false if it's not the right device */
+ if (sdma->dev->of_node != data->of_node)
+ return false;
+
sdmac->data = *data;
chan->private = &sdmac->data;
@@ -1950,6 +1955,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
* be set to sdmac->event_id1.
*/
data.dma_request2 = 0;
+ data.of_node = ofdma->of_node;
return dma_request_channel(mask, sdma_filter_fn, &data);
}
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index 7d964e787299..9daea8d42a10 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -55,6 +55,7 @@ struct imx_dma_data {
int dma_request2; /* secondary DMA request line */
enum sdma_peripheral_type peripheral_type;
int priority;
+ struct device_node *of_node;
};
static inline int imx_dma_is_ipu(struct dma_chan *chan)
--
2.17.1
On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.
Based on NXP commit MLK-16841-1 by Robin Gong <[email protected]>
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b3a67ff8e82..757fad2fbfae 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
unsigned int irq;
dma_addr_t bd0_phys;
struct sdma_buffer_descriptor *bd0;
+ /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+ bool clk_ratio;
};
static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,11 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
/* Set bits of CONFIG register with dynamic context switching */
- if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
- writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+ reg = readl(sdma->regs + SDMA_H_CONFIG);
+ if ((reg & SDMA_H_CONFIG_CSM) == 0) {
+ reg |= SDMA_H_CONFIG_CSM;
+ writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+ }
return ret;
}
@@ -1840,6 +1845,9 @@ static int sdma_init(struct sdma_engine *sdma)
if (ret)
goto disable_clk_ipg;
+ if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
+ sdma->clk_ratio = 1;
+
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
@@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
/* Set bits of CONFIG register but with static context switching */
- /* FIXME: Check whether to set ACR bit depending on clock ratios */
- writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+ if (sdma->clk_ratio)
+ writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+ else
+ writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
--
2.17.1
Add an fsl,imx8mq compatible string
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
---
Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index 3c9a57a8443b..9d8bbac27d8b 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -9,6 +9,7 @@ Required properties:
"fsl,imx53-sdma"
"fsl,imx6q-sdma"
"fsl,imx7d-sdma"
+ "fsl,imx8mq-sdma"
The -to variants should be preferred since they allow to determine the
correct ROM script addresses needed for the driver to work without additional
firmware.
--
2.17.1
This is identical to the imx7d.
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 757fad2fbfae..c4db4fe6bcc9 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -578,6 +578,9 @@ static const struct platform_device_id sdma_devtypes[] = {
}, {
.name = "imx7d-sdma",
.driver_data = (unsigned long)&sdma_imx7d,
+ }, {
+ .name = "imx8mq-sdma",
+ .driver_data = (unsigned long)&sdma_imx7d,
}, {
/* sentinel */
}
@@ -592,6 +595,7 @@ static const struct of_device_id sdma_dt_ids[] = {
{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
{ .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, },
{ .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, },
+ { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx7d, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, sdma_dt_ids);
--
2.17.1
Add sdma support for the imx8mq
Changes since V4
Builds against 5.0-rc4.
Dropped device tree additions, will send a separate patch.
Fixed sdma1 alignement.
Dropped phandles in favour of of_nodes.
Refactored SDMA_H_CONFIG register fix.
Changes since V3
Builds against 5.0-rc3.
Dropped sdma device index matching in favour of phandles.
Corrected subsytem name to dmaengine
Devicetree fixes.
Fixed SDMA_H_CONFIG register bug.
Changes since V2
Dropped device tree bindings and added clock ratio check.
Fixed bug where incorrect sdma device was selected.
Added new compatible string for "fsl,imx8mq-sdma"
Changes since V1
Fixed to build against v5.0-rc2
Angus Ainslie (Purism) (5):
dmaengine: imx-sdma: add clock ratio 1:1 check
dmaengine: imx-sdma: add imx8mq sdma compatible parts
dt-bindings: dma: fsl-imx-sdma: add fsl,imx8mq to the accepted
compatible node
dmaengine: imx-sdma: add a test for imx8mq multi sdma devices
dmaengine: imx-sdma: fix consistent dma test failures
.../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
drivers/dma/imx-sdma.c | 29 ++++++++++++++++---
include/linux/platform_data/dma-imx.h | 1 +
3 files changed, 27 insertions(+), 4 deletions(-)
--
2.17.1
Am Sonntag, den 27.01.2019, 23:08 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> to 500Mhz, so use 1:1 instead.
>
> Based on NXP commit MLK-16841-1 by Robin Gong <[email protected]>
>
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 0b3a67ff8e82..757fad2fbfae 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -440,6 +440,8 @@ struct sdma_engine {
> unsigned int irq;
> dma_addr_t bd0_phys;
> struct sdma_buffer_descriptor *bd0;
> + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> + bool clk_ratio;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -662,8 +664,11 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
> dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>
> /* Set bits of CONFIG register with dynamic context switching */
> - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
> - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
> + reg = readl(sdma->regs + SDMA_H_CONFIG);
> + if ((reg & SDMA_H_CONFIG_CSM) == 0) {
> + reg |= SDMA_H_CONFIG_CSM;
> + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
> + }
>
> return ret;
> }
> @@ -1840,6 +1845,9 @@ static int sdma_init(struct sdma_engine *sdma)
> if (ret)
> goto disable_clk_ipg;
>
> + if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
> + sdma->clk_ratio = 1;
> +
> /* Be sure SDMA has not started yet */
> writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
>
> @@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
> writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>
> /* Set bits of CONFIG register but with static context switching */
> - /* FIXME: Check whether to set ACR bit depending on clock ratios */
> - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
> + if (sdma->clk_ratio)
> + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
> + else
> + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
> writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
Am Sonntag, den 27.01.2019, 23:08 -0700 schrieb Angus Ainslie (Purism):
> This is identical to the imx7d.
So it can be dropped and the i.MX8M DT should just specify the
"fsl,imx7d-sdma" as a fallback compatible for the SDMA codes.
If both the imx8m and imx7d compatible are present in the DT, we can
introduce a more specific matchic when we actually need it. No need to
pollute the code with this from the start.
Regards,
Lucas
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 757fad2fbfae..c4db4fe6bcc9 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -578,6 +578,9 @@ static const struct platform_device_id
> sdma_devtypes[] = {
> }, {
> .name = "imx7d-sdma",
> .driver_data = (unsigned long)&sdma_imx7d,
> + }, {
> + .name = "imx8mq-sdma",
> + .driver_data = (unsigned long)&sdma_imx7d,
> }, {
> /* sentinel */
> }
> @@ -592,6 +595,7 @@ static const struct of_device_id sdma_dt_ids[] =
> {
> { .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
> { .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, },
> { .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, },
> + { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx7d, },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, sdma_dt_ids);
Am Sonntag, den 27.01.2019, 23:08 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8mq, there are two sdma instances, and the common dma framework
> will get a channel dynamically from any available sdma instance whether
> it's the first sdma device or the second sdma device. Some IPs like
> SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
> the correct sdma device, use the node pointer to match.
>
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 6 ++++++
> include/linux/platform_data/dma-imx.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index c4db4fe6bcc9..d5f86becf59e 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1918,11 +1918,16 @@ static int sdma_init(struct sdma_engine *sdma)
> static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
> {
> struct sdma_channel *sdmac = to_sdma_chan(chan);
> + struct sdma_engine *sdma = sdmac->sdma;
> struct imx_dma_data *data = fn_param;
>
> if (!imx_dma_is_general_purpose(chan))
> return false;
>
> + /* return false if it's not the right device */
> + if (sdma->dev->of_node != data->of_node)
> + return false;
> +
> sdmac->data = *data;
> chan->private = &sdmac->data;
>
> @@ -1950,6 +1955,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
> * be set to sdmac->event_id1.
> */
> data.dma_request2 = 0;
> + data.of_node = ofdma->of_node;
>
> return dma_request_channel(mask, sdma_filter_fn, &data);
> }
> diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
> index 7d964e787299..9daea8d42a10 100644
> --- a/include/linux/platform_data/dma-imx.h
> +++ b/include/linux/platform_data/dma-imx.h
> @@ -55,6 +55,7 @@ struct imx_dma_data {
> int dma_request2; /* secondary DMA request line */
> enum sdma_peripheral_type peripheral_type;
> int priority;
> + struct device_node *of_node;
> };
>
> static inline int imx_dma_is_ipu(struct dma_chan *chan)
Without the copy being aligned sdma1 fails ~10% of the time
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
---
drivers/dma/imx-sdma.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index d2fae53be689..e7d4d8390813 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -2114,6 +2114,7 @@ static int sdma_probe(struct platform_device *pdev)
sdma->dma_device.device_prep_dma_memcpy = sdma_prep_memcpy;
sdma->dma_device.device_issue_pending = sdma_issue_pending;
sdma->dma_device.dev->dma_parms = &sdma->dma_parms;
+ sdma->dma_device.copy_align = 2;
dma_set_max_seg_size(sdma->dma_device.dev, SDMA_BD_MAX_CNT);
platform_set_drvdata(pdev, sdma);
--
2.17.1
Add sdma support for the imx8mq
Changes since V5
Dropped device tree bindings.
Added reviewed by tags.
Changes since V4
Builds against 5.0-rc4.
Dropped device tree additions, will send a separate patch.
Fixed sdma1 alignement.
Dropped phandles in favour of of_nodes.
Refactored SDMA_H_CONFIG register fix.
Changes since V3
Builds against 5.0-rc3.
Dropped sdma device index matching in favour of phandles.
Corrected subsytem name to dmaengine
Devicetree fixes.
Fixed SDMA_H_CONFIG register bug.
Changes since V2
Dropped device tree bindings and added clock ratio check.
Fixed bug where incorrect sdma device was selected.
Added new compatible string for "fsl,imx8mq-sdma"
Changes since V1
Fixed to build against v5.0-rc2
Angus Ainslie (Purism) (3):
dmaengine: imx-sdma: add clock ratio 1:1 check
dmaengine: imx-sdma: add a test for imx8mq multi sdma devices
dmaengine: imx-sdma: fix consistent dma test failures
drivers/dma/imx-sdma.c | 25 +++++++++++++++++++++----
include/linux/platform_data/dma-imx.h | 1 +
2 files changed, 22 insertions(+), 4 deletions(-)
--
2.17.1
On i.mx8mq, there are two sdma instances, and the common dma framework
will get a channel dynamically from any available sdma instance whether
it's the first sdma device or the second sdma device. Some IPs like
SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
the correct sdma device, use the node pointer to match.
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
---
drivers/dma/imx-sdma.c | 6 ++++++
include/linux/platform_data/dma-imx.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 757fad2fbfae..d2fae53be689 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1914,11 +1914,16 @@ static int sdma_init(struct sdma_engine *sdma)
static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_engine *sdma = sdmac->sdma;
struct imx_dma_data *data = fn_param;
if (!imx_dma_is_general_purpose(chan))
return false;
+ /* return false if it's not the right device */
+ if (sdma->dev->of_node != data->of_node)
+ return false;
+
sdmac->data = *data;
chan->private = &sdmac->data;
@@ -1946,6 +1951,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
* be set to sdmac->event_id1.
*/
data.dma_request2 = 0;
+ data.of_node = ofdma->of_node;
return dma_request_channel(mask, sdma_filter_fn, &data);
}
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index 7d964e787299..9daea8d42a10 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -55,6 +55,7 @@ struct imx_dma_data {
int dma_request2; /* secondary DMA request line */
enum sdma_peripheral_type peripheral_type;
int priority;
+ struct device_node *of_node;
};
static inline int imx_dma_is_ipu(struct dma_chan *chan)
--
2.17.1
On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.
Based on NXP commit MLK-16841-1 by Robin Gong <[email protected]>
Signed-off-by: Angus Ainslie (Purism) <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
---
drivers/dma/imx-sdma.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b3a67ff8e82..757fad2fbfae 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
unsigned int irq;
dma_addr_t bd0_phys;
struct sdma_buffer_descriptor *bd0;
+ /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+ bool clk_ratio;
};
static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,11 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
/* Set bits of CONFIG register with dynamic context switching */
- if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
- writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+ reg = readl(sdma->regs + SDMA_H_CONFIG);
+ if ((reg & SDMA_H_CONFIG_CSM) == 0) {
+ reg |= SDMA_H_CONFIG_CSM;
+ writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+ }
return ret;
}
@@ -1840,6 +1845,9 @@ static int sdma_init(struct sdma_engine *sdma)
if (ret)
goto disable_clk_ipg;
+ if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
+ sdma->clk_ratio = 1;
+
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
@@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
/* Set bits of CONFIG register but with static context switching */
- /* FIXME: Check whether to set ACR bit depending on clock ratios */
- writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+ if (sdma->clk_ratio)
+ writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+ else
+ writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
--
2.17.1
On Mon, Jan 28, 2019 at 6:04 PM Angus Ainslie (Purism) <[email protected]> wrote:
>
> On i.mx8mq, there are two sdma instances, and the common dma framework
> will get a channel dynamically from any available sdma instance whether
> it's the first sdma device or the second sdma device. Some IPs like
> SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
> the correct sdma device, use the node pointer to match.
>
> Signed-off-by: Angus Ainslie (Purism) <[email protected]>
> Reviewed-by: Lucas Stach <[email protected]>
Thanks Angus for the patch!
Tested-by: Daniel Baluta <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 6 ++++++
> include/linux/platform_data/dma-imx.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 757fad2fbfae..d2fae53be689 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1914,11 +1914,16 @@ static int sdma_init(struct sdma_engine *sdma)
> static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
> {
> struct sdma_channel *sdmac = to_sdma_chan(chan);
> + struct sdma_engine *sdma = sdmac->sdma;
> struct imx_dma_data *data = fn_param;
>
> if (!imx_dma_is_general_purpose(chan))
> return false;
>
> + /* return false if it's not the right device */
> + if (sdma->dev->of_node != data->of_node)
> + return false;
> +
> sdmac->data = *data;
> chan->private = &sdmac->data;
>
> @@ -1946,6 +1951,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
> * be set to sdmac->event_id1.
> */
> data.dma_request2 = 0;
> + data.of_node = ofdma->of_node;
>
> return dma_request_channel(mask, sdma_filter_fn, &data);
> }
> diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
> index 7d964e787299..9daea8d42a10 100644
> --- a/include/linux/platform_data/dma-imx.h
> +++ b/include/linux/platform_data/dma-imx.h
> @@ -55,6 +55,7 @@ struct imx_dma_data {
> int dma_request2; /* secondary DMA request line */
> enum sdma_peripheral_type peripheral_type;
> int priority;
> + struct device_node *of_node;
> };
>
> static inline int imx_dma_is_ipu(struct dma_chan *chan)
> --
> 2.17.1
>
On 28-01-19, 09:03, Angus Ainslie (Purism) wrote:
> Add sdma support for the imx8mq
>
> Changes since V5
>
> Dropped device tree bindings.
> Added reviewed by tags.
Applied, thanks
--
~Vinod