[PATCH v2 0/3] dma: imx-sdma: add the sdma engine to the imx8mq

[PATCH v2 0/3] dma: imx-sdma: add the sdma engine to the imx8mq

[Angus Ainslie (Purism)]

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

[PATCH v2 1/3] dma: imx-sdma: fix NULL pointer de-reference

[Angus Ainslie (Purism)]

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) <angus@akkea.ca>
---
 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

Re: [PATCH v2 1/3] dma: imx-sdma: fix NULL pointer de-reference

[Daniel Baluta]

On Sun, Jan 20, 2019 at 4:34 AM Angus Ainslie (Purism) <angus@akkea.ca> 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) <angus@akkea.ca>

Hi Angus,

I have already sent a fix for this:

https://patchwork.kernel.org/patch/10758203/

Re: [PATCH v2 1/3] dma: imx-sdma: fix NULL pointer de-reference

[Angus Ainslie]

On 2019-01-20 02:54, Daniel Baluta wrote:
> On Sun, Jan 20, 2019 at 4:34 AM Angus Ainslie (Purism) <angus@akkea.ca> 
> 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) <angus@akkea.ca>
> 
> 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

[PATCH v2 2/3] dma: imx-sdma: add clock ratio 1:1 check

[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) <angus@akkea.ca>
---
 .../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

Re: [PATCH v2 2/3] dma: imx-sdma: add clock ratio 1:1 check

[Daniel Baluta]

On Sun, Jan 20, 2019 at 4:32 AM Angus Ainslie (Purism) <angus@akkea.ca> 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) <angus@akkea.ca>
> ---
>  .../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
>

Re: [PATCH v2 2/3] dma: imx-sdma: add clock ratio 1:1 check

[Angus Ainslie]

Hi Daniel,

On 2019-01-20 02:58, Daniel Baluta wrote:
> On Sun, Jan 20, 2019 at 4:32 AM Angus Ainslie (Purism) <angus@akkea.ca> 
> 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 <yibin.gong@nxp.com>" as the code isn't what he signed off 
on.

>> 
>> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
>> ---
>>  .../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
>> 

Re: [PATCH v2 2/3] dma: imx-sdma: add clock ratio 1:1 check

[Daniel Baluta]

On Sun, Jan 20, 2019 at 4:38 PM Angus Ainslie <angus@akkea.ca> wrote:
>
> Hi Daniel,
>
> On 2019-01-20 02:58, Daniel Baluta wrote:
> > On Sun, Jan 20, 2019 at 4:32 AM Angus Ainslie (Purism) <angus@akkea.ca>
> > 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 <yibin.gong@nxp.com>" 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.

Re: [PATCH v2 2/3] dma: imx-sdma: add clock ratio 1:1 check

[Lucas Stach]

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) <angus@akkea.ca>
> ---
>  .../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;

Re: [PATCH v2 2/3] dma: imx-sdma: add clock ratio 1:1 check

[Angus Ainslie]

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) <angus@akkea.ca>
>> ---
>>  .../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;

[PATCH v2 3/3] imx8mq.dtsi: add the sdma nodes

[Angus Ainslie (Purism)]

Add the sdma nodes to the base devicetree for the imx8mq

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

[PATCH v3 0/5] dma: imx-sdma: add the sdma engine to the imx8mq

[Angus Ainslie (Purism)]

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

[PATCH v3 1/5] dma: imx-sdma: add clock ratio 1:1 check

[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 <yibin.gong@nxp.com>

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v3 1/5] dma: imx-sdma: add clock ratio 1:1 check

[Lucas Stach]

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 <yibin.gong@nxp.com>
> 
> > Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
>  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);
>  

[PATCH v3 2/5] dma: imx-sdma: add imx8mq sdma compatible parts

[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.

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

[PATCH v3 3/5] dt-bindings: dma: fsl-imx-sdma: add imx8mq compatible string

[Angus Ainslie (Purism)]

Add "fsl,imx8mq-sdma" to the list of accepted compatible strings.

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

[PATCH v3 4/5] dma: imx-sdma: add an index for imx8mq multi sdma devices

[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 <yibin.gong@nxp.com>

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v3 4/5] dma: imx-sdma: add an index for imx8mq multi sdma devices

[Lucas Stach]

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 <yibin.gong@nxp.com>

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) <angus@akkea.ca>
> ---
>  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)

Re: [PATCH v3 4/5] dma: imx-sdma: add an index for imx8mq multi sdma devices

[Angus Ainslie]

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 <yibin.gong@nxp.com>
> 
> 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) <angus@akkea.ca>
>> ---
>>  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)

Re: [PATCH v3 4/5] dma: imx-sdma: add an index for imx8mq multi sdma devices

[Vinod Koul]

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 <yibin.gong@nxp.com>
> > 
> > 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

[PATCH v3 5/5] imx8mq.dtsi: add the sdma nodes

[Angus Ainslie (Purism)]

Add the sdma nodes to the base devicetree for the imx8mq

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v3 5/5] imx8mq.dtsi: add the sdma nodes

[Lucas Stach]

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) <angus@akkea.ca>
> ---
>  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>;

[PATCH v4 0/5] dmaengine: imx-sdma: add the sdma engine to the imx8mq

[Angus Ainslie (Purism)]

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

[PATCH v4 1/5] dmaengine: imx-sdma: add clock ratio 1:1 check

[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 <yibin.gong@nxp.com>

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v4 1/5] dmaengine: imx-sdma: add clock ratio 1:1 check

[Lucas Stach]

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 <yibin.gong@nxp.com>
> 
> > Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
>  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);
>  

[PATCH v4 2/5] dmaengine: imx-sdma: add imx8mq sdma compatible parts

[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.

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v4 2/5] dmaengine: imx-sdma: add imx8mq sdma compatible parts

[Lucas Stach]

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) <angus@akkea.ca>
> ---
>  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);

[PATCH v4 3/5] dt-bindings: dma: fsl-imx-sdma: add fsl,imx8mq to the accepted compatible node

[Angus Ainslie (Purism)]

The imx8mq is a slightly different variant on the imx7d

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v4 3/5] dt-bindings: dma: fsl-imx-sdma: add fsl,imx8mq to the accepted compatible node

[Lucas Stach]

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 <l.stach@pengutronix.de>

> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
>  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.

[PATCH v4 4/5] dma: imx-sdma: add a test for imx8mq multi sdma devices

[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) <angus@akkea.ca>
---
 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

Re: [PATCH v4 4/5] dma: imx-sdma: add a test for imx8mq multi sdma devices

[Lucas Stach]

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) <angus@akkea.ca>
> ---
>  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)

[PATCH v4 5/5] imx8mq.dtsi: add the sdma nodes

[Angus Ainslie (Purism)]

Add the sdma nodes to the base devicetree for the imx8mq

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v4 5/5] imx8mq.dtsi: add the sdma nodes

[Lucas Stach]

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) <angus@akkea.ca>

One nit below, with that fixed:

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

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>;

[PATCH v5 0/5] dmaengine: imx-sdma: add the sdma engine to the imx8mq

[Angus Ainslie (Purism)]

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

[PATCH v5 1/5] dmaengine: imx-sdma: add clock ratio 1:1 check

[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 <yibin.gong@nxp.com>

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v5 1/5] dmaengine: imx-sdma: add clock ratio 1:1 check

[Lucas Stach]

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 <yibin.gong@nxp.com>
> 
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  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);
>  

[PATCH v5 2/5] dmaengine: imx-sdma: add imx8mq sdma compatible parts

[Angus Ainslie (Purism)]

This is identical to the imx7d.

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v5 2/5] dmaengine: imx-sdma: add imx8mq sdma compatible parts

[Lucas Stach]

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) <angus@akkea.ca>
> ---
>  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);

[PATCH v5 3/5] dt-bindings: dma: fsl-imx-sdma: add fsl,imx8mq to the accepted compatible node

[Angus Ainslie (Purism)]

Add an fsl,imx8mq compatible string

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
 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

[PATCH v5 4/5] dmaengine: imx-sdma: add a test for imx8mq multi sdma devices

[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) <angus@akkea.ca>
---
 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

Re: [PATCH v5 4/5] dmaengine: imx-sdma: add a test for imx8mq multi sdma devices

[Lucas Stach]

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) <angus@akkea.ca>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  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)

[PATCH v5 5/5] dmaengine: imx-sdma: fix consistent dma test failures

[Angus Ainslie (Purism)]

Without the copy being aligned sdma1 fails ~10% of the time

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

[PATCH v6 0/3] dmaengine: imx-sdma: add the sdma engine to the imx8mq

[Angus Ainslie (Purism)]

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

[PATCH v6 1/3] dmaengine: imx-sdma: add clock ratio 1:1 check

[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 <yibin.gong@nxp.com>

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
 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

[PATCH v6 2/3] dmaengine: imx-sdma: add a test for imx8mq multi sdma devices

[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) <angus@akkea.ca>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
 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

Re: [PATCH v6 2/3] dmaengine: imx-sdma: add a test for imx8mq multi sdma devices

[Daniel Baluta]

On Mon, Jan 28, 2019 at 6:04 PM Angus Ainslie (Purism) <angus@akkea.ca> 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) <angus@akkea.ca>
> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

Thanks Angus for the patch!

Tested-by: Daniel Baluta <daniel.baluta@nxp.com>


> ---
>  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
>

[PATCH v6 3/3] dmaengine: imx-sdma: fix consistent dma test failures

[Angus Ainslie (Purism)]

Without the copy being aligned sdma1 fails ~10% of the time

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 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

Re: [PATCH v6 0/3] dmaengine: imx-sdma: add the sdma engine to the imx8mq

[Vinod Koul]

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