linux-kernel - Re: [PATCH v3 1/3] dmaengine: ptdma: Initial driver for the AMD PassThru DMA engine

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200124060058.GC2841@vkoul-mobl>
Date:   Fri, 24 Jan 2020 11:30:58 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Sanjay R Mehta <Sanju.Mehta@....com>
Cc:     gregkh@...uxfoundation.org, dan.j.williams@...el.com,
        Thomas.Lendacky@....com, Shyam-sundar.S-k@....com,
        Nehal-bakulchandra.Shah@....com, robh@...nel.org,
        mchehab+samsung@...nel.org, davem@...emloft.net,
        Jonathan.Cameron@...wei.com, linux-kernel@...r.kernel.org,
        dmaengine@...r.kernel.org, Gary R Hook <gary.hook@....com>
Subject: Re: [PATCH v3 1/3] dmaengine: ptdma: Initial driver for the AMD
 PassThru DMA engine

On 21-01-20, 03:04, Sanjay R Mehta wrote:
> From: Sanjay R Mehta <sanju.mehta@....com>
> 
> This device performs high-bandwidth memory-to-memory
> transfer operations.

Why is it called PassThru DMA engine?

>  obj-y += mediatek/
>  obj-y += qcom/
> diff --git a/drivers/dma/ptdma/Kconfig b/drivers/dma/ptdma/Kconfig
> new file mode 100644
> index 0000000..4ec259e
> --- /dev/null
> +++ b/drivers/dma/ptdma/Kconfig
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config AMD_PTDMA
> +	tristate  "AMD PassThru DMA Engine"
> +	depends on X86_64 && PCI

not using DMA_VIRTUAL_CHANNELS?

> +	help
> +	  Provides the support for AMD PassThru DMA Engine.

more help text please

> +++ b/drivers/dma/ptdma/ptdma-dev.c
> @@ -0,0 +1,387 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AMD Passthru DMA device driver
> + * -- Based on the CCP driver

What is ccp driver?

> + *
> + * Copyright (C) 2016,2019 Advanced Micro Devices, Inc.

2020..

> +#ifdef CONFIG_PM
> +static int pt_pci_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> +	return -ENOSYS;
> +}
> +
> +static int pt_pci_resume(struct pci_dev *pdev)
> +{
> +	return -ENOSYS;
> +}
> +#endif

please remove the dummy code, you can add these when you have support
for it

> +/* Bit masks */
> +#define	CMD_DESC_DW0_VAL		0x500012
> +#define	CMD_CONFIG_REQID		0x0
> +#define	CMD_CONFIG_VHB_EN		0x00000001
> +#define	CMD_QUEUE_PRIO			0x00000006
> +#define	CMD_TIMEOUT_DISABLE		0x00000000
> +#define	CMD_CLK_DYN_GATING_EN	0x1
> +#define	CMD_CLK_DYN_GATING_DIS	0x0
> +#define	CMD_CLK_HW_GATE_MODE	0x1
> +#define	CMD_CLK_SW_GATE_MODE	0x0
> +#define	CMD_CLK_GATE_ON_DELAY	0x1000
> +#define	CMD_CLK_GATE_CTL	0x0
> +#define	CMD_CLK_GATE_OFF_DELAY	0x1000

BIT and GENMASK for these...

All this adds handling of pt controller, I am not seeing dmaengine bits,
so please word the changelog accordingly and mention this adds based
bits and not dmaengine support (yet)

-- 
~Vinod