The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: "Verma, Devendra" <devverma@amd.com>
Cc: mani@kernel.org, vkoul@kernel.org, frank.li@kernel.org,
	dmaengine@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, michal.simek@amd.com,
	Koichiro Den <den@valinux.co.jp>
Subject: Re: [PATCH v1 1/3] dmaengine: dw-edma: Make Xilinx Macros Dev Name Agnostic
Date: Thu, 23 Jul 2026 10:36:21 -0500	[thread overview]
Message-ID: <amI09ZcfhX4SEPIH@SMW015318> (raw)
In-Reply-To: <58a5b255-778e-4b57-870a-ef5ec1bcb72e@amd.com>

On Thu, Jul 23, 2026 at 03:53:13PM +0530, Verma, Devendra wrote:
>
>
> On 22-Jul-26 20:21, Frank Li wrote:
> > On Wed, Jul 22, 2026 at 04:46:38PM +0530, Devendra K Verma wrote:
> > > Xilinx specific macros for MDB device can be reused for the
> > > Xilinx supported other similar IP such as CPM6.
> > > Renamed the Xilinx specific macros in a way that can be
> > > reused for Xilinx supported upcoming IP, CPM6.
> > > Naming is in accordance with the naming done for Synopsys macros.
> > >
> > > Signed-off-by: Devendra K Verma <devverma@amd.com>
> > > ---
> >
> > I don't think it is necessary to remove MDB. You may mention somewhere
> > CMP6 compatible with MDB. It is quite common in driver.
> >
> > The major concerns is that if a incompatible devices appear, such as CMP7,
> > which have special registers, need add _MDB_ back.
> >
> > Frank
> >
>
> The MDB was used cause at that time only Xilinx MDB was the IP to go
> public.
> I agree with your suggestion but the idea used here is that, for the
> common functionalities use the names which are affiliated to vendor
> that is Xilinx and can be used across the IP having the similarity.
> In case specific functionality is required for MDB or CPM6 then
> it can be associated using vendor + device name, that way it is easy
> to identify which ones are common and specific to the IP.
>
> Currently, if any functionality added for MDB ONLY then it would be
> assumed to be supported on Xilinx CPM6 as well.

Rename when this case really appear.

Frank

> To remove the confusion
> extra comment would be required to inform which functionality is
> supported for which IP. That is why the removal of MDB appoach was taken
> to reduce the confusion and work. The first approach looked clean.
>
> Please let me know your thougths.
>
> -Devendra
>
> > >   drivers/dma/dw-edma/dw-edma-pcie.c | 58 +++++++++++++++---------------
> > >   1 file changed, 29 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > > index 791c46e8ae4c..ec5e057a0f11 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > > @@ -29,21 +29,21 @@
> > >   #define PCI_DEVICE_ID_XILINX_B054		0xb054
> > >   #define PCI_DEVICE_ID_XILINX_B00F		0xb00f
> > >
> > > -#define DW_PCIE_XILINX_MDB_VSEC_DMA_ID		0x6
> > > -#define DW_PCIE_XILINX_MDB_VSEC_ID		0x20
> > > -#define DW_PCIE_XILINX_MDB_VSEC_DMA_BAR		GENMASK(10, 8)
> > > -#define DW_PCIE_XILINX_MDB_VSEC_DMA_MAP		GENMASK(2, 0)
> > > -#define DW_PCIE_XILINX_MDB_VSEC_DMA_WR_CH	GENMASK(9, 0)
> > > -#define DW_PCIE_XILINX_MDB_VSEC_DMA_RD_CH	GENMASK(25, 16)
> > > -
> > > -#define DW_PCIE_XILINX_MDB_DEVMEM_OFF_REG_HIGH	0xc
> > > -#define DW_PCIE_XILINX_MDB_DEVMEM_OFF_REG_LOW	0x8
> > > -#define DW_PCIE_XILINX_MDB_INVALID_ADDR		(~0ULL)
> > > -
> > > -#define DW_PCIE_XILINX_MDB_LL_OFF_GAP		0x200000
> > > -#define DW_PCIE_XILINX_MDB_LL_SIZE		0x800
> > > -#define DW_PCIE_XILINX_MDB_DT_OFF_GAP		0x100000
> > > -#define DW_PCIE_XILINX_MDB_DT_SIZE		0x800
> > > +#define DW_PCIE_XILINX_VSEC_DMA_ID		0x6
> > > +#define DW_PCIE_XILINX_VSEC_ID			0x20
> > > +#define DW_PCIE_XILINX_VSEC_DMA_BAR		GENMASK(10, 8)
> > > +#define DW_PCIE_XILINX_VSEC_DMA_MAP		GENMASK(2, 0)
> > > +#define DW_PCIE_XILINX_VSEC_DMA_WR_CH		GENMASK(9, 0)
> > > +#define DW_PCIE_XILINX_VSEC_DMA_RD_CH		GENMASK(25, 16)
> > > +
> > > +#define DW_PCIE_XILINX_DEVMEM_OFF_REG_HIGH	0xc
> > > +#define DW_PCIE_XILINX_DEVMEM_OFF_REG_LOW	0x8
> > > +#define DW_PCIE_XILINX_INVALID_ADDR		(~0ULL)
> > > +
> > > +#define DW_PCIE_XILINX_LL_OFF_GAP		0x200000
> > > +#define DW_PCIE_XILINX_LL_SIZE			0x800
> > > +#define DW_PCIE_XILINX_DT_OFF_GAP		0x100000
> > > +#define DW_PCIE_XILINX_DT_SIZE			0x800
> > >
> > >   #define DW_BLOCK(a, b, c) \
> > >   	{ \
> > > @@ -258,10 +258,10 @@ static void dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
> > >   	u16 vsec;
> > >   	u64 off;
> > >
> > > -	pdata->devmem_phys_off = DW_PCIE_XILINX_MDB_INVALID_ADDR;
> > > +	pdata->devmem_phys_off = DW_PCIE_XILINX_INVALID_ADDR;
> > >
> > >   	vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_XILINX,
> > > -					DW_PCIE_XILINX_MDB_VSEC_DMA_ID);
> > > +					DW_PCIE_XILINX_VSEC_DMA_ID);
> > >   	if (!vsec)
> > >   		return;
> > >
> > > @@ -272,18 +272,18 @@ static void dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
> > >
> > >   	pci_dbg(pdev, "Detected Xilinx PCIe Vendor-Specific Extended Capability DMA\n");
> > >   	pci_read_config_dword(pdev, vsec + 0x8, &val);
> > > -	map = FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_MAP, val);
> > > +	map = FIELD_GET(DW_PCIE_XILINX_VSEC_DMA_MAP, val);
> > >   	if (map != EDMA_MF_HDMA_NATIVE)
> > >   		return;
> > >
> > >   	pdata->mf = map;
> > > -	pdata->rg.bar = FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_BAR, val);
> > > +	pdata->rg.bar = FIELD_GET(DW_PCIE_XILINX_VSEC_DMA_BAR, val);
> > >
> > >   	pci_read_config_dword(pdev, vsec + 0xc, &val);
> > >   	pdata->wr_ch_cnt = min(pdata->wr_ch_cnt,
> > > -			       FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_WR_CH, val));
> > > +			       FIELD_GET(DW_PCIE_XILINX_VSEC_DMA_WR_CH, val));
> > >   	pdata->rd_ch_cnt = min(pdata->rd_ch_cnt,
> > > -			       FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_RD_CH, val));
> > > +			       FIELD_GET(DW_PCIE_XILINX_VSEC_DMA_RD_CH, val));
> > >
> > >   	pci_read_config_dword(pdev, vsec + 0x14, &val);
> > >   	off = val;
> > > @@ -293,16 +293,16 @@ static void dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
> > >   	pdata->rg.off = off;
> > >
> > >   	vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_XILINX,
> > > -					DW_PCIE_XILINX_MDB_VSEC_ID);
> > > +					DW_PCIE_XILINX_VSEC_ID);
> > >   	if (!vsec)
> > >   		return;
> > >
> > >   	pci_read_config_dword(pdev,
> > > -			      vsec + DW_PCIE_XILINX_MDB_DEVMEM_OFF_REG_HIGH,
> > > +			      vsec + DW_PCIE_XILINX_DEVMEM_OFF_REG_HIGH,
> > >   			      &val);
> > >   	off = val;
> > >   	pci_read_config_dword(pdev,
> > > -			      vsec + DW_PCIE_XILINX_MDB_DEVMEM_OFF_REG_LOW,
> > > +			      vsec + DW_PCIE_XILINX_DEVMEM_OFF_REG_LOW,
> > >   			      &val);
> > >   	off <<= 32;
> > >   	off |= val;
> > > @@ -360,7 +360,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> > >   		 * address use the non-LL mode or simple mode supported by
> > >   		 * the HDMA IP.
> > >   		 */
> > > -		if (vsec_data->devmem_phys_off == DW_PCIE_XILINX_MDB_INVALID_ADDR)
> > > +		if (vsec_data->devmem_phys_off == DW_PCIE_XILINX_INVALID_ADDR)
> > >   			non_ll = true;
> > >
> > >   		/*
> > > @@ -370,10 +370,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> > >   		 */
> > >   		if (!non_ll)
> > >   			dw_edma_set_chan_region_offset(vsec_data, BAR_2, 0,
> > > -						       DW_PCIE_XILINX_MDB_LL_OFF_GAP,
> > > -						       DW_PCIE_XILINX_MDB_LL_SIZE,
> > > -						       DW_PCIE_XILINX_MDB_DT_OFF_GAP,
> > > -						       DW_PCIE_XILINX_MDB_DT_SIZE);
> > > +						       DW_PCIE_XILINX_LL_OFF_GAP,
> > > +						       DW_PCIE_XILINX_LL_SIZE,
> > > +						       DW_PCIE_XILINX_DT_OFF_GAP,
> > > +						       DW_PCIE_XILINX_DT_SIZE);
> > >   	}
> > >
> > >   	/* Mapping PCI BAR regions */
> > > --
> > > 2.43.0
> > >
>

  parent reply	other threads:[~2026-07-23 15:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 11:16 [PATCH v1 0/3] CPM6 Channel Separation Support Devendra K Verma
2026-07-22 11:16 ` [PATCH v1 1/3] dmaengine: dw-edma: Make Xilinx Macros Dev Name Agnostic Devendra K Verma
2026-07-22 14:51   ` Frank Li
     [not found]     ` <58a5b255-778e-4b57-870a-ef5ec1bcb72e@amd.com>
2026-07-23 15:36       ` Frank Li [this message]
2026-07-22 11:16 ` [PATCH v1 2/3] dmaengine: dw-edma: Enable Chan Separation via VSEC Devendra K Verma
2026-07-22 15:00   ` Frank Li
2026-07-22 11:16 ` [PATCH v1 3/3] dmaengine: dw-edma: Add changes to support Channel Separation Devendra K Verma
2026-07-22 15:22   ` Frank Li
     [not found]     ` <c4d9ef2e-3bfa-4dfc-b943-1b42e04e9cf1@amd.com>
2026-07-23 15:28       ` Frank Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amI09ZcfhX4SEPIH@SMW015318 \
    --to=frank.li@oss.nxp.com \
    --cc=den@valinux.co.jp \
    --cc=devverma@amd.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=frank.li@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=michal.simek@amd.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox