linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dmaengine: fsl-qdma: fixed the source/destination descriptior format
@ 2019-04-19  8:46 Peng Ma
  2019-04-26 11:50 ` Vinod Koul
  0 siblings, 1 reply; 5+ messages in thread
From: Peng Ma @ 2019-04-19  8:46 UTC (permalink / raw)
  To: dan.j.williams, vkoul; +Cc: leoyang.li, dmaengine, linux-kernel, Peng Ma

CMD of Source/Destination descriptior format should be lower of
struct fsl_qdma_engine number data address.

Signed-off-by: Peng Ma <peng.ma@nxp.com>
---
 drivers/dma/fsl-qdma.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
index aa1d0ae..542765a 100644
--- a/drivers/dma/fsl-qdma.c
+++ b/drivers/dma/fsl-qdma.c
@@ -113,6 +113,7 @@
 /* Field definition for Descriptor offset */
 #define QDMA_CCDF_STATUS		20
 #define QDMA_CCDF_OFFSET		20
+#define QDMA_SDDF_CMD(x)		(((u64)(x)) << 32)
 
 /* Field definition for safe loop count*/
 #define FSL_QDMA_HALT_COUNT		1500
@@ -214,6 +215,12 @@ struct fsl_qdma_engine {
 
 };
 
+static inline void
+qdma_sddf_set_cmd(struct fsl_qdma_format *sddf, u32 val)
+{
+	sddf->data = QDMA_SDDF_CMD(val);
+}
+
 static inline u64
 qdma_ccdf_addr_get64(const struct fsl_qdma_format *ccdf)
 {
@@ -341,6 +348,7 @@ static void fsl_qdma_free_chan_resources(struct dma_chan *chan)
 static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
 				      dma_addr_t dst, dma_addr_t src, u32 len)
 {
+	u32 cmd;
 	struct fsl_qdma_format *sdf, *ddf;
 	struct fsl_qdma_format *ccdf, *csgf_desc, *csgf_src, *csgf_dest;
 
@@ -353,6 +361,7 @@ static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
 
 	memset(fsl_comp->virt_addr, 0, FSL_QDMA_COMMAND_BUFFER_SIZE);
 	memset(fsl_comp->desc_virt_addr, 0, FSL_QDMA_DESCRIPTOR_BUFFER_SIZE);
+
 	/* Head Command Descriptor(Frame Descriptor) */
 	qdma_desc_addr_set64(ccdf, fsl_comp->bus_addr + 16);
 	qdma_ccdf_set_format(ccdf, qdma_ccdf_get_offset(ccdf));
@@ -369,14 +378,14 @@ static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
 	/* This entry is the last entry. */
 	qdma_csgf_set_f(csgf_dest, len);
 	/* Descriptor Buffer */
-	sdf->data =
-		cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
-			    FSL_QDMA_CMD_RWTTYPE_OFFSET);
-	ddf->data =
-		cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
-			    FSL_QDMA_CMD_RWTTYPE_OFFSET);
-	ddf->data |=
-		cpu_to_le64(FSL_QDMA_CMD_LWC << FSL_QDMA_CMD_LWC_OFFSET);
+	cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
+			  FSL_QDMA_CMD_RWTTYPE_OFFSET);
+	qdma_sddf_set_cmd(sdf, cmd);
+
+	cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
+			  FSL_QDMA_CMD_RWTTYPE_OFFSET);
+	cmd |= cpu_to_le32(FSL_QDMA_CMD_LWC << FSL_QDMA_CMD_LWC_OFFSET);
+	qdma_sddf_set_cmd(ddf, cmd);
 }
 
 /*
@@ -701,10 +710,8 @@ static irqreturn_t fsl_qdma_error_handler(int irq, void *dev_id)
 
 	intr = qdma_readl(fsl_qdma, status + FSL_QDMA_DEDR);
 
-	if (intr) {
+	if (intr)
 		dev_err(fsl_qdma->dma_dev.dev, "DMA transaction error!\n");
-		return IRQ_NONE;
-	}
 
 	qdma_writel(fsl_qdma, FSL_QDMA_DEDR_CLEAR, status + FSL_QDMA_DEDR);
 	return IRQ_HANDLED;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] dmaengine: fsl-qdma: fixed the source/destination descriptior format
  2019-04-19  8:46 [PATCH] dmaengine: fsl-qdma: fixed the source/destination descriptior format Peng Ma
@ 2019-04-26 11:50 ` Vinod Koul
  2019-04-28  2:00   ` [EXT] " Peng Ma
  0 siblings, 1 reply; 5+ messages in thread
From: Vinod Koul @ 2019-04-26 11:50 UTC (permalink / raw)
  To: Peng Ma; +Cc: dan.j.williams, leoyang.li, dmaengine, linux-kernel

On 19-04-19, 08:46, Peng Ma wrote:
> CMD of Source/Destination descriptior format should be lower of

s/descriptior/descriptor

> struct fsl_qdma_engine number data address.
> 
> Signed-off-by: Peng Ma <peng.ma@nxp.com>
> ---
>  drivers/dma/fsl-qdma.c |   29 ++++++++++++++++++-----------
>  1 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
> index aa1d0ae..542765a 100644
> --- a/drivers/dma/fsl-qdma.c
> +++ b/drivers/dma/fsl-qdma.c
> @@ -113,6 +113,7 @@
>  /* Field definition for Descriptor offset */
>  #define QDMA_CCDF_STATUS		20
>  #define QDMA_CCDF_OFFSET		20
> +#define QDMA_SDDF_CMD(x)		(((u64)(x)) << 32)
>  
>  /* Field definition for safe loop count*/
>  #define FSL_QDMA_HALT_COUNT		1500
> @@ -214,6 +215,12 @@ struct fsl_qdma_engine {
>  
>  };
>  
> +static inline void
> +qdma_sddf_set_cmd(struct fsl_qdma_format *sddf, u32 val)
> +{
> +	sddf->data = QDMA_SDDF_CMD(val);
> +}
> +
>  static inline u64
>  qdma_ccdf_addr_get64(const struct fsl_qdma_format *ccdf)
>  {
> @@ -341,6 +348,7 @@ static void fsl_qdma_free_chan_resources(struct dma_chan *chan)
>  static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
>  				      dma_addr_t dst, dma_addr_t src, u32 len)
>  {
> +	u32 cmd;
>  	struct fsl_qdma_format *sdf, *ddf;
>  	struct fsl_qdma_format *ccdf, *csgf_desc, *csgf_src, *csgf_dest;
>  
> @@ -353,6 +361,7 @@ static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
>  
>  	memset(fsl_comp->virt_addr, 0, FSL_QDMA_COMMAND_BUFFER_SIZE);
>  	memset(fsl_comp->desc_virt_addr, 0, FSL_QDMA_DESCRIPTOR_BUFFER_SIZE);
> +
>  	/* Head Command Descriptor(Frame Descriptor) */
>  	qdma_desc_addr_set64(ccdf, fsl_comp->bus_addr + 16);
>  	qdma_ccdf_set_format(ccdf, qdma_ccdf_get_offset(ccdf));
> @@ -369,14 +378,14 @@ static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
>  	/* This entry is the last entry. */
>  	qdma_csgf_set_f(csgf_dest, len);
>  	/* Descriptor Buffer */
> -	sdf->data =
> -		cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
> -			    FSL_QDMA_CMD_RWTTYPE_OFFSET);
> -	ddf->data =
> -		cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
> -			    FSL_QDMA_CMD_RWTTYPE_OFFSET);
> -	ddf->data |=
> -		cpu_to_le64(FSL_QDMA_CMD_LWC << FSL_QDMA_CMD_LWC_OFFSET);
> +	cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
> +			  FSL_QDMA_CMD_RWTTYPE_OFFSET);
> +	qdma_sddf_set_cmd(sdf, cmd);
> +
> +	cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
> +			  FSL_QDMA_CMD_RWTTYPE_OFFSET);
> +	cmd |= cpu_to_le32(FSL_QDMA_CMD_LWC << FSL_QDMA_CMD_LWC_OFFSET);
> +	qdma_sddf_set_cmd(ddf, cmd);
>  }
>  
>  /*
> @@ -701,10 +710,8 @@ static irqreturn_t fsl_qdma_error_handler(int irq, void *dev_id)
>  
>  	intr = qdma_readl(fsl_qdma, status + FSL_QDMA_DEDR);
>  
> -	if (intr) {
> +	if (intr)
>  		dev_err(fsl_qdma->dma_dev.dev, "DMA transaction error!\n");
> -		return IRQ_NONE;
> -	}

this seems unrelated can you explain?

>  
>  	qdma_writel(fsl_qdma, FSL_QDMA_DEDR_CLEAR, status + FSL_QDMA_DEDR);
>  	return IRQ_HANDLED;
> -- 
> 1.7.1

-- 
~Vinod

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [EXT] Re: [PATCH] dmaengine: fsl-qdma: fixed the source/destination descriptior format
  2019-04-26 11:50 ` Vinod Koul
@ 2019-04-28  2:00   ` Peng Ma
  2019-04-29  5:15     ` Vinod Koul
  0 siblings, 1 reply; 5+ messages in thread
From: Peng Ma @ 2019-04-28  2:00 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, Leo Li, dmaengine, linux-kernel

Hi Vinod,

Thanks your comments.
Please see my comments inline.

Best Regards,
Peng

>-----Original Message-----
>From: Vinod Koul <vkoul@kernel.org>
>Sent: 2019年4月26日 19:51
>To: Peng Ma <peng.ma@nxp.com>
>Cc: dan.j.williams@intel.com; Leo Li <leoyang.li@nxp.com>;
>dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: [EXT] Re: [PATCH] dmaengine: fsl-qdma: fixed the source/destination
>descriptior format
>
>Caution: EXT Email
>
>On 19-04-19, 08:46, Peng Ma wrote:
>> CMD of Source/Destination descriptior format should be lower of
>
>s/descriptior/descriptor
>
[Peng Ma] Got it.
>> struct fsl_qdma_engine number data address.
>>
>> Signed-off-by: Peng Ma <peng.ma@nxp.com>
>> ---
>>  drivers/dma/fsl-qdma.c |   29 ++++++++++++++++++-----------
>>  1 files changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c index
>> aa1d0ae..542765a 100644
>> --- a/drivers/dma/fsl-qdma.c
>> +++ b/drivers/dma/fsl-qdma.c
>> @@ -113,6 +113,7 @@
>>  /* Field definition for Descriptor offset */
>>  #define QDMA_CCDF_STATUS             20
>>  #define QDMA_CCDF_OFFSET             20
>> +#define QDMA_SDDF_CMD(x)             (((u64)(x)) << 32)
>>
>>  /* Field definition for safe loop count*/
>>  #define FSL_QDMA_HALT_COUNT          1500
>> @@ -214,6 +215,12 @@ struct fsl_qdma_engine {
>>
>>  };
>>
>> +static inline void
>> +qdma_sddf_set_cmd(struct fsl_qdma_format *sddf, u32 val) {
>> +     sddf->data = QDMA_SDDF_CMD(val); }
>> +
>>  static inline u64
>>  qdma_ccdf_addr_get64(const struct fsl_qdma_format *ccdf)  { @@
>-341,6
>> +348,7 @@ static void fsl_qdma_free_chan_resources(struct dma_chan
>> *chan)  static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp
>*fsl_comp,
>>                                     dma_addr_t dst, dma_addr_t
>src,
>> u32 len)  {
>> +     u32 cmd;
>>       struct fsl_qdma_format *sdf, *ddf;
>>       struct fsl_qdma_format *ccdf, *csgf_desc, *csgf_src, *csgf_dest;
>>
>> @@ -353,6 +361,7 @@ static void fsl_qdma_comp_fill_memcpy(struct
>> fsl_qdma_comp *fsl_comp,
>>
>>       memset(fsl_comp->virt_addr, 0,
>FSL_QDMA_COMMAND_BUFFER_SIZE);
>>       memset(fsl_comp->desc_virt_addr, 0,
>> FSL_QDMA_DESCRIPTOR_BUFFER_SIZE);
>> +
>>       /* Head Command Descriptor(Frame Descriptor) */
>>       qdma_desc_addr_set64(ccdf, fsl_comp->bus_addr + 16);
>>       qdma_ccdf_set_format(ccdf, qdma_ccdf_get_offset(ccdf)); @@
>> -369,14 +378,14 @@ static void fsl_qdma_comp_fill_memcpy(struct
>fsl_qdma_comp *fsl_comp,
>>       /* This entry is the last entry. */
>>       qdma_csgf_set_f(csgf_dest, len);
>>       /* Descriptor Buffer */
>> -     sdf->data =
>> -             cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
>> -                         FSL_QDMA_CMD_RWTTYPE_OFFSET);
>> -     ddf->data =
>> -             cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
>> -                         FSL_QDMA_CMD_RWTTYPE_OFFSET);
>> -     ddf->data |=
>> -             cpu_to_le64(FSL_QDMA_CMD_LWC <<
>FSL_QDMA_CMD_LWC_OFFSET);
>> +     cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
>> +                       FSL_QDMA_CMD_RWTTYPE_OFFSET);
>> +     qdma_sddf_set_cmd(sdf, cmd);
>> +
>> +     cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
>> +                       FSL_QDMA_CMD_RWTTYPE_OFFSET);
>> +     cmd |= cpu_to_le32(FSL_QDMA_CMD_LWC <<
>FSL_QDMA_CMD_LWC_OFFSET);
>> +     qdma_sddf_set_cmd(ddf, cmd);
>>  }
>>
>>  /*
>> @@ -701,10 +710,8 @@ static irqreturn_t fsl_qdma_error_handler(int
>> irq, void *dev_id)
>>
>>       intr = qdma_readl(fsl_qdma, status + FSL_QDMA_DEDR);
>>
>> -     if (intr) {
>> +     if (intr)
>>               dev_err(fsl_qdma->dma_dev.dev, "DMA transaction
>error!\n");
>> -             return IRQ_NONE;
>> -     }
>
>this seems unrelated can you explain?
>
[Peng Ma] This is an added improvement. When an error occurs we should clean the error reg then to return.
I forgot to explain it on comments. Should I add this changed to the comments?
>>
>>       qdma_writel(fsl_qdma, FSL_QDMA_DEDR_CLEAR, status +
>FSL_QDMA_DEDR);
>>       return IRQ_HANDLED;
>> --
>> 1.7.1
>
>--
>~Vinod

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [EXT] Re: [PATCH] dmaengine: fsl-qdma: fixed the source/destination descriptior format
  2019-04-28  2:00   ` [EXT] " Peng Ma
@ 2019-04-29  5:15     ` Vinod Koul
  2019-04-29  6:28       ` Peng Ma
  0 siblings, 1 reply; 5+ messages in thread
From: Vinod Koul @ 2019-04-29  5:15 UTC (permalink / raw)
  To: Peng Ma; +Cc: dan.j.williams, Leo Li, dmaengine, linux-kernel

On 28-04-19, 02:00, Peng Ma wrote:
> Hi Vinod,
> 
> Thanks your comments.
> Please see my comments inline.
> 
> Best Regards,
> Peng
> 
> >-----Original Message-----
> >From: Vinod Koul <vkoul@kernel.org>
> >Sent: 2019年4月26日 19:51
> >To: Peng Ma <peng.ma@nxp.com>
> >Cc: dan.j.williams@intel.com; Leo Li <leoyang.li@nxp.com>;
> >dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org
> >Subject: [EXT] Re: [PATCH] dmaengine: fsl-qdma: fixed the source/destination
> >descriptior format
> >
> >Caution: EXT Email
> >
> >On 19-04-19, 08:46, Peng Ma wrote:
> >> CMD of Source/Destination descriptior format should be lower of
> >
> >s/descriptior/descriptor
> >
> [Peng Ma] Got it.
> >> struct fsl_qdma_engine number data address.
> >>
> >> Signed-off-by: Peng Ma <peng.ma@nxp.com>
> >> ---
> >>  drivers/dma/fsl-qdma.c |   29 ++++++++++++++++++-----------
> >>  1 files changed, 18 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c index
> >> aa1d0ae..542765a 100644
> >> --- a/drivers/dma/fsl-qdma.c
> >> +++ b/drivers/dma/fsl-qdma.c
> >> @@ -113,6 +113,7 @@
> >>  /* Field definition for Descriptor offset */
> >>  #define QDMA_CCDF_STATUS             20
> >>  #define QDMA_CCDF_OFFSET             20
> >> +#define QDMA_SDDF_CMD(x)             (((u64)(x)) << 32)
> >>
> >>  /* Field definition for safe loop count*/
> >>  #define FSL_QDMA_HALT_COUNT          1500
> >> @@ -214,6 +215,12 @@ struct fsl_qdma_engine {
> >>
> >>  };
> >>
> >> +static inline void
> >> +qdma_sddf_set_cmd(struct fsl_qdma_format *sddf, u32 val) {
> >> +     sddf->data = QDMA_SDDF_CMD(val); }
> >> +
> >>  static inline u64
> >>  qdma_ccdf_addr_get64(const struct fsl_qdma_format *ccdf)  { @@
> >-341,6
> >> +348,7 @@ static void fsl_qdma_free_chan_resources(struct dma_chan
> >> *chan)  static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp
> >*fsl_comp,
> >>                                     dma_addr_t dst, dma_addr_t
> >src,
> >> u32 len)  {
> >> +     u32 cmd;
> >>       struct fsl_qdma_format *sdf, *ddf;
> >>       struct fsl_qdma_format *ccdf, *csgf_desc, *csgf_src, *csgf_dest;
> >>
> >> @@ -353,6 +361,7 @@ static void fsl_qdma_comp_fill_memcpy(struct
> >> fsl_qdma_comp *fsl_comp,
> >>
> >>       memset(fsl_comp->virt_addr, 0,
> >FSL_QDMA_COMMAND_BUFFER_SIZE);
> >>       memset(fsl_comp->desc_virt_addr, 0,
> >> FSL_QDMA_DESCRIPTOR_BUFFER_SIZE);
> >> +
> >>       /* Head Command Descriptor(Frame Descriptor) */
> >>       qdma_desc_addr_set64(ccdf, fsl_comp->bus_addr + 16);
> >>       qdma_ccdf_set_format(ccdf, qdma_ccdf_get_offset(ccdf)); @@
> >> -369,14 +378,14 @@ static void fsl_qdma_comp_fill_memcpy(struct
> >fsl_qdma_comp *fsl_comp,
> >>       /* This entry is the last entry. */
> >>       qdma_csgf_set_f(csgf_dest, len);
> >>       /* Descriptor Buffer */
> >> -     sdf->data =
> >> -             cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
> >> -                         FSL_QDMA_CMD_RWTTYPE_OFFSET);
> >> -     ddf->data =
> >> -             cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
> >> -                         FSL_QDMA_CMD_RWTTYPE_OFFSET);
> >> -     ddf->data |=
> >> -             cpu_to_le64(FSL_QDMA_CMD_LWC <<
> >FSL_QDMA_CMD_LWC_OFFSET);
> >> +     cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
> >> +                       FSL_QDMA_CMD_RWTTYPE_OFFSET);
> >> +     qdma_sddf_set_cmd(sdf, cmd);
> >> +
> >> +     cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
> >> +                       FSL_QDMA_CMD_RWTTYPE_OFFSET);
> >> +     cmd |= cpu_to_le32(FSL_QDMA_CMD_LWC <<
> >FSL_QDMA_CMD_LWC_OFFSET);
> >> +     qdma_sddf_set_cmd(ddf, cmd);
> >>  }
> >>
> >>  /*
> >> @@ -701,10 +710,8 @@ static irqreturn_t fsl_qdma_error_handler(int
> >> irq, void *dev_id)
> >>
> >>       intr = qdma_readl(fsl_qdma, status + FSL_QDMA_DEDR);
> >>
> >> -     if (intr) {
> >> +     if (intr)
> >>               dev_err(fsl_qdma->dma_dev.dev, "DMA transaction
> >error!\n");
> >> -             return IRQ_NONE;
> >> -     }
> >
> >this seems unrelated can you explain?
> >
> [Peng Ma] This is an added improvement. When an error occurs we should clean the error reg then to return.
> I forgot to explain it on comments. Should I add this changed to the comments?

Yes and you should make it a separate patch. A patch should do only 1
thing!

-- 
~Vinod

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [EXT] Re: [PATCH] dmaengine: fsl-qdma: fixed the source/destination descriptior format
  2019-04-29  5:15     ` Vinod Koul
@ 2019-04-29  6:28       ` Peng Ma
  0 siblings, 0 replies; 5+ messages in thread
From: Peng Ma @ 2019-04-29  6:28 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, Leo Li, dmaengine, linux-kernel



>-----Original Message-----
>From: Vinod Koul <vkoul@kernel.org>
>Sent: 2019年4月29日 13:16
>To: Peng Ma <peng.ma@nxp.com>
>Cc: dan.j.williams@intel.com; Leo Li <leoyang.li@nxp.com>;
>dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: Re: [EXT] Re: [PATCH] dmaengine: fsl-qdma: fixed the
>source/destination descriptior format
>
>Caution: EXT Email
>
>On 28-04-19, 02:00, Peng Ma wrote:
>> Hi Vinod,
>>
>> Thanks your comments.
>> Please see my comments inline.
>>
>> Best Regards,
>> Peng
>>
>> >-----Original Message-----
>> >From: Vinod Koul <vkoul@kernel.org>
>> >Sent: 2019年4月26日 19:51
>> >To: Peng Ma <peng.ma@nxp.com>
>> >Cc: dan.j.williams@intel.com; Leo Li <leoyang.li@nxp.com>;
>> >dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org
>> >Subject: [EXT] Re: [PATCH] dmaengine: fsl-qdma: fixed the
>> >source/destination descriptior format
>> >
>> >Caution: EXT Email
>> >
>> >On 19-04-19, 08:46, Peng Ma wrote:
>> >> CMD of Source/Destination descriptior format should be lower of
>> >
>> >s/descriptior/descriptor
>> >
>> [Peng Ma] Got it.
>> >> struct fsl_qdma_engine number data address.
>> >>
>> >> Signed-off-by: Peng Ma <peng.ma@nxp.com>
>> >> ---
>> >>  drivers/dma/fsl-qdma.c |   29 ++++++++++++++++++-----------
>> >>  1 files changed, 18 insertions(+), 11 deletions(-)
>> >>
>> >> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c index
>> >> aa1d0ae..542765a 100644
>> >> --- a/drivers/dma/fsl-qdma.c
>> >> +++ b/drivers/dma/fsl-qdma.c
>> >> @@ -113,6 +113,7 @@
>> >>  /* Field definition for Descriptor offset */
>> >>  #define QDMA_CCDF_STATUS             20
>> >>  #define QDMA_CCDF_OFFSET             20
>> >> +#define QDMA_SDDF_CMD(x)             (((u64)(x)) << 32)
>> >>
>> >>  /* Field definition for safe loop count*/
>> >>  #define FSL_QDMA_HALT_COUNT          1500
>> >> @@ -214,6 +215,12 @@ struct fsl_qdma_engine {
>> >>
>> >>  };
>> >>
>> >> +static inline void
>> >> +qdma_sddf_set_cmd(struct fsl_qdma_format *sddf, u32 val) {
>> >> +     sddf->data = QDMA_SDDF_CMD(val); }
>> >> +
>> >>  static inline u64
>> >>  qdma_ccdf_addr_get64(const struct fsl_qdma_format *ccdf)  { @@
>> >-341,6
>> >> +348,7 @@ static void fsl_qdma_free_chan_resources(struct dma_chan
>> >> *chan)  static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp
>> >*fsl_comp,
>> >>                                     dma_addr_t dst,
>dma_addr_t
>> >src,
>> >> u32 len)  {
>> >> +     u32 cmd;
>> >>       struct fsl_qdma_format *sdf, *ddf;
>> >>       struct fsl_qdma_format *ccdf, *csgf_desc, *csgf_src,
>> >> *csgf_dest;
>> >>
>> >> @@ -353,6 +361,7 @@ static void fsl_qdma_comp_fill_memcpy(struct
>> >> fsl_qdma_comp *fsl_comp,
>> >>
>> >>       memset(fsl_comp->virt_addr, 0,
>> >FSL_QDMA_COMMAND_BUFFER_SIZE);
>> >>       memset(fsl_comp->desc_virt_addr, 0,
>> >> FSL_QDMA_DESCRIPTOR_BUFFER_SIZE);
>> >> +
>> >>       /* Head Command Descriptor(Frame Descriptor) */
>> >>       qdma_desc_addr_set64(ccdf, fsl_comp->bus_addr + 16);
>> >>       qdma_ccdf_set_format(ccdf, qdma_ccdf_get_offset(ccdf)); @@
>> >> -369,14 +378,14 @@ static void fsl_qdma_comp_fill_memcpy(struct
>> >fsl_qdma_comp *fsl_comp,
>> >>       /* This entry is the last entry. */
>> >>       qdma_csgf_set_f(csgf_dest, len);
>> >>       /* Descriptor Buffer */
>> >> -     sdf->data =
>> >> -             cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
>> >> -                         FSL_QDMA_CMD_RWTTYPE_OFFSET);
>> >> -     ddf->data =
>> >> -             cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
>> >> -                         FSL_QDMA_CMD_RWTTYPE_OFFSET);
>> >> -     ddf->data |=
>> >> -             cpu_to_le64(FSL_QDMA_CMD_LWC <<
>> >FSL_QDMA_CMD_LWC_OFFSET);
>> >> +     cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
>> >> +                       FSL_QDMA_CMD_RWTTYPE_OFFSET);
>> >> +     qdma_sddf_set_cmd(sdf, cmd);
>> >> +
>> >> +     cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
>> >> +                       FSL_QDMA_CMD_RWTTYPE_OFFSET);
>> >> +     cmd |= cpu_to_le32(FSL_QDMA_CMD_LWC <<
>> >FSL_QDMA_CMD_LWC_OFFSET);
>> >> +     qdma_sddf_set_cmd(ddf, cmd);
>> >>  }
>> >>
>> >>  /*
>> >> @@ -701,10 +710,8 @@ static irqreturn_t fsl_qdma_error_handler(int
>> >> irq, void *dev_id)
>> >>
>> >>       intr = qdma_readl(fsl_qdma, status + FSL_QDMA_DEDR);
>> >>
>> >> -     if (intr) {
>> >> +     if (intr)
>> >>               dev_err(fsl_qdma->dma_dev.dev, "DMA transaction
>> >error!\n");
>> >> -             return IRQ_NONE;
>> >> -     }
>> >
>> >this seems unrelated can you explain?
>> >
>> [Peng Ma] This is an added improvement. When an error occurs we should
>clean the error reg then to return.
>> I forgot to explain it on comments. Should I add this changed to the
>comments?
>
>Yes and you should make it a separate patch. A patch should do only 1 thing!
>
[Peng Ma] OK, Got it, thanks.
Best Regards,
Peng
>--
>~Vinod

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-04-29  6:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-19  8:46 [PATCH] dmaengine: fsl-qdma: fixed the source/destination descriptior format Peng Ma
2019-04-26 11:50 ` Vinod Koul
2019-04-28  2:00   ` [EXT] " Peng Ma
2019-04-29  5:15     ` Vinod Koul
2019-04-29  6:28       ` Peng Ma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).