Re: [PATCH v6 4/6] dmaengine: ae4dma: Register AE4DMA using pt_dmaengine_register

[Basavaraj Natikar <bnatikar@amd.com>]

On 10/23/2024 11:39 AM, Vinod Koul wrote:
> On 22-10-24, 10:53, Basavaraj Natikar wrote:
>> On 10/21/2024 11:02 PM, Vinod Koul wrote:
>>> On 09-09-24, 18:09, Basavaraj Natikar wrote:
>>>> Use the pt_dmaengine_register function to register a AE4DMA DMA engine.
>>>>
>>>> Reviewed-by: Raju Rangoju <Raju.Rangoju@amd.com>
>>>> Reviewed-by: Philipp Stanner <pstanner@redhat.com>
>>>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>>>> ---
>>>>    drivers/dma/amd/ae4dma/ae4dma-dev.c     |  51 +----------
>>>>    drivers/dma/amd/ae4dma/ae4dma-pci.c     |   1 +
>>>>    drivers/dma/amd/ae4dma/ae4dma.h         |   3 +
>>>>    drivers/dma/amd/ptdma/ptdma-dmaengine.c | 114 +++++++++++++++++++++++-
>>>>    drivers/dma/amd/ptdma/ptdma.h           |   3 +
>>>>    5 files changed, 123 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/drivers/dma/amd/ae4dma/ae4dma-dev.c b/drivers/dma/amd/ae4dma/ae4dma-dev.c
>>>> index f0b3a3763adc..cd84b502265e 100644
>>>> --- a/drivers/dma/amd/ae4dma/ae4dma-dev.c
>>>> +++ b/drivers/dma/amd/ae4dma/ae4dma-dev.c
>>>> @@ -14,53 +14,6 @@ static unsigned int max_hw_q = 1;
>>>>    module_param(max_hw_q, uint, 0444);
>>>>    MODULE_PARM_DESC(max_hw_q, "max hw queues supported by engine (any non-zero value, default: 1)");
>>>> -static char *ae4_error_codes[] = {
>>>> -	"",
>>>> -	"ERR 01: INVALID HEADER DW0",
>>>> -	"ERR 02: INVALID STATUS",
>>>> -	"ERR 03: INVALID LENGTH - 4 BYTE ALIGNMENT",
>>>> -	"ERR 04: INVALID SRC ADDR - 4 BYTE ALIGNMENT",
>>>> -	"ERR 05: INVALID DST ADDR - 4 BYTE ALIGNMENT",
>>>> -	"ERR 06: INVALID ALIGNMENT",
>>>> -	"ERR 07: INVALID DESCRIPTOR",
>>>> -};
>>>> -
>>>> -static void ae4_log_error(struct pt_device *d, int e)
>>>> -{
>>>> -	/* ERR 01 - 07 represents Invalid AE4 errors */
>>>> -	if (e <= 7)
>>>> -		dev_info(d->dev, "AE4DMA error: %s (0x%x)\n", ae4_error_codes[e], e);
>>>> -	/* ERR 08 - 15 represents Invalid Descriptor errors */
>>>> -	else if (e > 7 && e <= 15)
>>>> -		dev_info(d->dev, "AE4DMA error: %s (0x%x)\n", "INVALID DESCRIPTOR", e);
>>>> -	/* ERR 16 - 31 represents Firmware errors */
>>>> -	else if (e > 15 && e <= 31)
>>>> -		dev_info(d->dev, "AE4DMA error: %s (0x%x)\n", "FIRMWARE ERROR", e);
>>>> -	/* ERR 32 - 63 represents Fatal errors */
>>>> -	else if (e > 31 && e <= 63)
>>>> -		dev_info(d->dev, "AE4DMA error: %s (0x%x)\n", "FATAL ERROR", e);
>>>> -	/* ERR 64 - 255 represents PTE errors */
>>>> -	else if (e > 63 && e <= 255)
>>>> -		dev_info(d->dev, "AE4DMA error: %s (0x%x)\n", "PTE ERROR", e);
>>>> -	else
>>>> -		dev_info(d->dev, "Unknown AE4DMA error");
>>>> -}
>>>> -
>>>> -static void ae4_check_status_error(struct ae4_cmd_queue *ae4cmd_q, int idx)
>>>> -{
>>>> -	struct pt_cmd_queue *cmd_q = &ae4cmd_q->cmd_q;
>>>> -	struct ae4dma_desc desc;
>>>> -	u8 status;
>>>> -
>>>> -	memcpy(&desc, &cmd_q->qbase[idx], sizeof(struct ae4dma_desc));
>>>> -	status = desc.dw1.status;
>>>> -	if (status && status != AE4_DESC_COMPLETED) {
>>>> -		cmd_q->cmd_error = desc.dw1.err_code;
>>>> -		if (cmd_q->cmd_error)
>>>> -			ae4_log_error(cmd_q->pt, cmd_q->cmd_error);
>>>> -	}
>>>> -}
>>> why is this getting moved?
>> To avoid circular dependency between PTDMA and AE4DMA, we are reusing PTDMA code to prevent
>> duplication, as AE4DMA depends on PTDMA.
> Okay please move the code first and then do changes, that makes it easier to
> review and follow

Sure Vinod, I sent changes in V7.

Thanks,
--
Basavaraj

>