[PATCH 00/15] dmaengine: dw-edma: HDMA support

[PATCH 00/15] dmaengine: dw-edma: HDMA support

[Gustavo Pimentel]

This patch series adds the HDMA support, as long the IP design has set
the compatible register map parameter, which allows compatibility at
some degree for the existing Synopsys DesignWare eDMA driver that is
already available on the Kernel.

The HDMA "Hyper-DMA" IP is an enhancement of the eDMA "embedded-DMA" IP.

This new improvement comes with a PCI DVSEC that allows to the driver
recognize and switch behavior if it's an eDMA or an HDMA, becoming
retrocompatible, in the absence of this DVSEC, the driver will assume
that is an eDMA IP.

It also adds the interleaved support, since it will be similar to the
current scatter-gather implementation.

As well fixes/improves some abnormal behaviors not detected before, such as:
 - crash on loading/unloading driver
 - memory space definition for the data area and for the linked list space
 - scatter-gather address calculation on 32 bits platforms
 - minor comment and variable reordering

Cc: Vinod Koul <vkoul@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org

Gustavo Pimentel (15):
  dmaengine: dw-edma: Add writeq() and readq() for 64 bits architectures
  dmaengine: dw-edma: Fix comments offset characters' alignment
  dmaengine: dw-edma: Add support for the HDMA feature
  PCI: Add pci_find_vsec_capability() to find a specific VSEC
  dmaengine: dw-edma: Add PCIe VSEC data retrieval support
  dmaengine: dw-edma: Add device_prep_interleave_dma() support
  dmaengine: dw-edma: Improve number of channels check
  dmaengine: dw-edma: Reorder variables to keep consistency
  dmaengine: dw-edma: Improve the linked list and data blocks definition
  dmaengine: dw-edma: Change linked list and data blocks offset and
    sizes
  dmaengine: dw-edma: Move struct dentry variable from static definition
    into dw_edma struct
  dmaengine: dw-edma: Fix crash on loading/unloading driver
  dmaengine: dw-edma: Change DMA abreviation from lower into upper case
  dmaengine: dw-edma: Revert fix scatter-gather address calculation
  dmaengine: dw-edma: Add pcim_iomap_table return checker

 drivers/dma/dw-edma/dw-edma-core.c       | 178 +++++++++++-------
 drivers/dma/dw-edma/dw-edma-core.h       |  37 ++--
 drivers/dma/dw-edma/dw-edma-pcie.c       | 275 +++++++++++++++++++++-------
 drivers/dma/dw-edma/dw-edma-v0-core.c    | 300 ++++++++++++++++++++++++-------
 drivers/dma/dw-edma/dw-edma-v0-core.h    |   2 +-
 drivers/dma/dw-edma/dw-edma-v0-debugfs.c |  77 ++++----
 drivers/dma/dw-edma/dw-edma-v0-debugfs.h |   4 +-
 drivers/dma/dw-edma/dw-edma-v0-regs.h    | 291 +++++++++++++++++++-----------
 drivers/pci/pci.c                        |  29 +++
 include/linux/pci.h                      |   1 +
 include/uapi/linux/pci_regs.h            |   5 +
 11 files changed, 844 insertions(+), 355 deletions(-)

-- 
2.7.4

[PATCH 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

[Gustavo Pimentel]

Add pci_find_vsec_capability() that crawls through the device config
space searching in all Vendor-Specific Extended Capabilities for a
particular capability ID.

Vendor-Specific Extended Capability (VSEC) is a PCIe capability (acts
like a wrapper) specified by PCI-SIG that allows the vendor to create
their own and specific capability in the device config space.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/pci.c             | 29 +++++++++++++++++++++++++++++
 include/linux/pci.h           |  1 +
 include/uapi/linux/pci_regs.h |  5 +++++
 3 files changed, 35 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6d4d5a2..235d0b2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -623,6 +623,35 @@ u64 pci_get_dsn(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_get_dsn);
 
+/**
+ * pci_find_vsec_capability - Find a vendor-specific extended capability
+ * @dev: PCI device to query
+ * @cap: vendor-specific capability id code
+ *
+ * Returns the address of the vendor-specific structure that matches the
+ * requested capability id code within the device's PCI configuration space
+ * or 0 if it does not find a match.
+ */
+int pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id)
+{
+	u32 header;
+	int vsec;
+
+	vsec = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VNDR);
+	while (vsec) {
+		if (pci_read_config_dword(dev, vsec + 0x4,
+					  &header) == PCIBIOS_SUCCESSFUL &&
+		    PCI_VSEC_CAP_ID(header) == vsec_cap_id)
+			break;
+
+		vsec = pci_find_next_ext_capability(dev, vsec,
+						    PCI_EXT_CAP_ID_VNDR);
+	}
+
+	return vsec;
+}
+EXPORT_SYMBOL_GPL(pci_find_vsec_capability);
+
 static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
 {
 	int rc, ttl = PCI_FIND_CAP_TTL;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 22207a7..effecb0 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1067,6 +1067,7 @@ int pci_find_capability(struct pci_dev *dev, int cap);
 int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
 int pci_find_ext_capability(struct pci_dev *dev, int cap);
 int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
+int pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id);
 int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
 int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
 struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index a95d55f..f5d17be 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -730,6 +730,11 @@
 #define PCI_EXT_CAP_DSN_SIZEOF	12
 #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
 
+/* Vendor-Specific Extended Capabilities */
+#define PCI_VSEC_CAP_ID(header)		(header & 0x0000ffff)
+#define PCI_VSEC_CAP_REV(header)	((header >> 16) & 0xf)
+#define PCI_VSEC_CAP_LEN(header)	((header >> 20) & 0xffc)
+
 /* Advanced Error Reporting */
 #define PCI_ERR_UNCOR_STATUS	4	/* Uncorrectable Error Status */
 #define  PCI_ERR_UNC_UND	0x00000001	/* Undefined */
-- 
2.7.4

Re: [PATCH 00/15] dmaengine: dw-edma: HDMA support

[Vinod Koul]

On 15-12-20, 18:30, Gustavo Pimentel wrote:
> This patch series adds the HDMA support, as long the IP design has set
> the compatible register map parameter, which allows compatibility at
> some degree for the existing Synopsys DesignWare eDMA driver that is
> already available on the Kernel.
> 
> The HDMA "Hyper-DMA" IP is an enhancement of the eDMA "embedded-DMA" IP.
> 
> This new improvement comes with a PCI DVSEC that allows to the driver
> recognize and switch behavior if it's an eDMA or an HDMA, becoming
> retrocompatible, in the absence of this DVSEC, the driver will assume
> that is an eDMA IP.
> 
> It also adds the interleaved support, since it will be similar to the
> current scatter-gather implementation.
> 
> As well fixes/improves some abnormal behaviors not detected before, such as:
>  - crash on loading/unloading driver
>  - memory space definition for the data area and for the linked list space
>  - scatter-gather address calculation on 32 bits platforms
>  - minor comment and variable reordering
> 
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: dmaengine@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> 
> Gustavo Pimentel (15):
>   dmaengine: dw-edma: Add writeq() and readq() for 64 bits architectures
>   dmaengine: dw-edma: Fix comments offset characters' alignment
>   dmaengine: dw-edma: Add support for the HDMA feature
>   PCI: Add pci_find_vsec_capability() to find a specific VSEC

Had this been picked up.. I see we have dependency on pci patch for this
series..

>   dmaengine: dw-edma: Add PCIe VSEC data retrieval support
>   dmaengine: dw-edma: Add device_prep_interleave_dma() support
>   dmaengine: dw-edma: Improve number of channels check
>   dmaengine: dw-edma: Reorder variables to keep consistency
>   dmaengine: dw-edma: Improve the linked list and data blocks definition
>   dmaengine: dw-edma: Change linked list and data blocks offset and
>     sizes
>   dmaengine: dw-edma: Move struct dentry variable from static definition
>     into dw_edma struct
>   dmaengine: dw-edma: Fix crash on loading/unloading driver
>   dmaengine: dw-edma: Change DMA abreviation from lower into upper case
>   dmaengine: dw-edma: Revert fix scatter-gather address calculation
>   dmaengine: dw-edma: Add pcim_iomap_table return checker
> 
>  drivers/dma/dw-edma/dw-edma-core.c       | 178 +++++++++++-------
>  drivers/dma/dw-edma/dw-edma-core.h       |  37 ++--
>  drivers/dma/dw-edma/dw-edma-pcie.c       | 275 +++++++++++++++++++++-------
>  drivers/dma/dw-edma/dw-edma-v0-core.c    | 300 ++++++++++++++++++++++++-------
>  drivers/dma/dw-edma/dw-edma-v0-core.h    |   2 +-
>  drivers/dma/dw-edma/dw-edma-v0-debugfs.c |  77 ++++----
>  drivers/dma/dw-edma/dw-edma-v0-debugfs.h |   4 +-
>  drivers/dma/dw-edma/dw-edma-v0-regs.h    | 291 +++++++++++++++++++-----------
>  drivers/pci/pci.c                        |  29 +++
>  include/linux/pci.h                      |   1 +
>  include/uapi/linux/pci_regs.h            |   5 +
>  11 files changed, 844 insertions(+), 355 deletions(-)
> 
> -- 
> 2.7.4

-- 
~Vinod

Re: [PATCH 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

[Bjorn Helgaas]

[+cc Vinod, Dan, dmaengine]

On Tue, Dec 15, 2020 at 06:30:13PM +0100, Gustavo Pimentel wrote:
> Add pci_find_vsec_capability() that crawls through the device config
> space searching in all Vendor-Specific Extended Capabilities for a
> particular capability ID.
> 
> Vendor-Specific Extended Capability (VSEC) is a PCIe capability (acts
> like a wrapper) specified by PCI-SIG that allows the vendor to create
> their own and specific capability in the device config space.
> 
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

If you fix the below, feel free to add my

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Otherwise, I can take it myself.  But that will be an ordering issue
in the merge window if you merge the rest of the series via another
tree.

> ---
>  drivers/pci/pci.c             | 29 +++++++++++++++++++++++++++++
>  include/linux/pci.h           |  1 +
>  include/uapi/linux/pci_regs.h |  5 +++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6d4d5a2..235d0b2 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -623,6 +623,35 @@ u64 pci_get_dsn(struct pci_dev *dev)
>  }
>  EXPORT_SYMBOL_GPL(pci_get_dsn);
>  
> +/**
> + * pci_find_vsec_capability - Find a vendor-specific extended capability
> + * @dev: PCI device to query
> + * @cap: vendor-specific capability id code

s/id/ID/

> + *
> + * Returns the address of the vendor-specific structure that matches the
> + * requested capability id code within the device's PCI configuration space

s/id/ID/

> + * or 0 if it does not find a match.
> + */
> +int pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id)
> +{
> +	u32 header;
> +	int vsec;

  int vsec;
  u32 header;

since that's the order they're used.

> +
> +	vsec = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VNDR);
> +	while (vsec) {
> +		if (pci_read_config_dword(dev, vsec + 0x4,

s/0x4/PCI_VSEC_HDR/

> +					  &header) == PCIBIOS_SUCCESSFUL &&
> +		    PCI_VSEC_CAP_ID(header) == vsec_cap_id)
> +			break;

  return vsec;

> +
> +		vsec = pci_find_next_ext_capability(dev, vsec,
> +						    PCI_EXT_CAP_ID_VNDR);
> +	}
> +
> +	return vsec;

  return 0;

> +}
> +EXPORT_SYMBOL_GPL(pci_find_vsec_capability);
> +
>  static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
>  {
>  	int rc, ttl = PCI_FIND_CAP_TTL;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 22207a7..effecb0 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1067,6 +1067,7 @@ int pci_find_capability(struct pci_dev *dev, int cap);
>  int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
>  int pci_find_ext_capability(struct pci_dev *dev, int cap);
>  int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
> +int pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id);
>  int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
>  int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
>  struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index a95d55f..f5d17be 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -730,6 +730,11 @@
>  #define PCI_EXT_CAP_DSN_SIZEOF	12
>  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
>  
> +/* Vendor-Specific Extended Capabilities */
> +#define PCI_VSEC_CAP_ID(header)		(header & 0x0000ffff)
> +#define PCI_VSEC_CAP_REV(header)	((header >> 16) & 0xf)
> +#define PCI_VSEC_CAP_LEN(header)	((header >> 20) & 0xffc)

Please put these next to the existing PCI_VSEC_HDR.

Why does PCI_VSEC_CAP_LEN mask with 0xffc instead of 0xfff?  I don't
see anything in the spec about VSEC Length having to be a multiple of
4 (PCIe r5.0, sec 7.9.5.2).

But you don't use this anyway, so I'd just drop it (and
PCI_VSEC_CAP_REV) altogether.

>  /* Advanced Error Reporting */
>  #define PCI_ERR_UNCOR_STATUS	4	/* Uncorrectable Error Status */
>  #define  PCI_ERR_UNC_UND	0x00000001	/* Undefined */
> -- 
> 2.7.4
> 

Re: [PATCH 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

[Dan Williams]

[ add Ben ]

On Mon, Feb 1, 2021 at 2:39 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+cc Vinod, Dan, dmaengine]
>
> On Tue, Dec 15, 2020 at 06:30:13PM +0100, Gustavo Pimentel wrote:
> > Add pci_find_vsec_capability() that crawls through the device config
> > space searching in all Vendor-Specific Extended Capabilities for a
> > particular capability ID.
> >
> > Vendor-Specific Extended Capability (VSEC) is a PCIe capability (acts
> > like a wrapper) specified by PCI-SIG that allows the vendor to create
> > their own and specific capability in the device config space.
> >
> > Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>
> If you fix the below, feel free to add my
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Otherwise, I can take it myself.  But that will be an ordering issue
> in the merge window if you merge the rest of the series via another
> tree.

I wonder if this warrants and if you'd be willing to stand up a stable
branch for just this commit for concerned parties to integrate,
because CXL development should adopt it as well.

>
> > ---
> >  drivers/pci/pci.c             | 29 +++++++++++++++++++++++++++++
> >  include/linux/pci.h           |  1 +
> >  include/uapi/linux/pci_regs.h |  5 +++++
> >  3 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 6d4d5a2..235d0b2 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -623,6 +623,35 @@ u64 pci_get_dsn(struct pci_dev *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(pci_get_dsn);
> >
> > +/**
> > + * pci_find_vsec_capability - Find a vendor-specific extended capability
> > + * @dev: PCI device to query
> > + * @cap: vendor-specific capability id code
>
> s/id/ID/
>
> > + *
> > + * Returns the address of the vendor-specific structure that matches the
> > + * requested capability id code within the device's PCI configuration space
>
> s/id/ID/
>
> > + * or 0 if it does not find a match.
> > + */
> > +int pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id)
> > +{
> > +     u32 header;
> > +     int vsec;
>
>   int vsec;
>   u32 header;
>
> since that's the order they're used.
>
> > +
> > +     vsec = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VNDR);
> > +     while (vsec) {
> > +             if (pci_read_config_dword(dev, vsec + 0x4,
>
> s/0x4/PCI_VSEC_HDR/
>
> > +                                       &header) == PCIBIOS_SUCCESSFUL &&
> > +                 PCI_VSEC_CAP_ID(header) == vsec_cap_id)
> > +                     break;
>
>   return vsec;
>
> > +
> > +             vsec = pci_find_next_ext_capability(dev, vsec,
> > +                                                 PCI_EXT_CAP_ID_VNDR);
> > +     }
> > +
> > +     return vsec;
>
>   return 0;
>
> > +}
> > +EXPORT_SYMBOL_GPL(pci_find_vsec_capability);
> > +
> >  static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
> >  {
> >       int rc, ttl = PCI_FIND_CAP_TTL;
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 22207a7..effecb0 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1067,6 +1067,7 @@ int pci_find_capability(struct pci_dev *dev, int cap);
> >  int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
> >  int pci_find_ext_capability(struct pci_dev *dev, int cap);
> >  int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
> > +int pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id);
> >  int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
> >  int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
> >  struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index a95d55f..f5d17be 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -730,6 +730,11 @@
> >  #define PCI_EXT_CAP_DSN_SIZEOF       12
> >  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
> >
> > +/* Vendor-Specific Extended Capabilities */
> > +#define PCI_VSEC_CAP_ID(header)              (header & 0x0000ffff)
> > +#define PCI_VSEC_CAP_REV(header)     ((header >> 16) & 0xf)
> > +#define PCI_VSEC_CAP_LEN(header)     ((header >> 20) & 0xffc)
>
> Please put these next to the existing PCI_VSEC_HDR.
>
> Why does PCI_VSEC_CAP_LEN mask with 0xffc instead of 0xfff?  I don't
> see anything in the spec about VSEC Length having to be a multiple of
> 4 (PCIe r5.0, sec 7.9.5.2).
>
> But you don't use this anyway, so I'd just drop it (and
> PCI_VSEC_CAP_REV) altogether.
>
> >  /* Advanced Error Reporting */
> >  #define PCI_ERR_UNCOR_STATUS 4       /* Uncorrectable Error Status */
> >  #define  PCI_ERR_UNC_UND     0x00000001      /* Undefined */
> > --
> > 2.7.4
> >

Re: [PATCH 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

[Ben Widawsky]

On 21-02-01 15:24:45, Dan Williams wrote:
> [ add Ben ]
> 
> On Mon, Feb 1, 2021 at 2:39 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > [+cc Vinod, Dan, dmaengine]
> >
> > On Tue, Dec 15, 2020 at 06:30:13PM +0100, Gustavo Pimentel wrote:
> > > Add pci_find_vsec_capability() that crawls through the device config
> > > space searching in all Vendor-Specific Extended Capabilities for a
> > > particular capability ID.
> > >
> > > Vendor-Specific Extended Capability (VSEC) is a PCIe capability (acts
> > > like a wrapper) specified by PCI-SIG that allows the vendor to create
> > > their own and specific capability in the device config space.
> > >
> > > Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> >
> > If you fix the below, feel free to add my
> >
> > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> >
> > Otherwise, I can take it myself.  But that will be an ordering issue
> > in the merge window if you merge the rest of the series via another
> > tree.
> 
> I wonder if this warrants and if you'd be willing to stand up a stable
> branch for just this commit for concerned parties to integrate,
> because CXL development should adopt it as well.
> 

Yeah, can we add DVSEC too please?

> >
> > > ---
> > >  drivers/pci/pci.c             | 29 +++++++++++++++++++++++++++++
> > >  include/linux/pci.h           |  1 +
> > >  include/uapi/linux/pci_regs.h |  5 +++++
> > >  3 files changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index 6d4d5a2..235d0b2 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -623,6 +623,35 @@ u64 pci_get_dsn(struct pci_dev *dev)
> > >  }
> > >  EXPORT_SYMBOL_GPL(pci_get_dsn);
> > >
> > > +/**
> > > + * pci_find_vsec_capability - Find a vendor-specific extended capability
> > > + * @dev: PCI device to query
> > > + * @cap: vendor-specific capability id code
> >
> > s/id/ID/
> >
> > > + *
> > > + * Returns the address of the vendor-specific structure that matches the
> > > + * requested capability id code within the device's PCI configuration space
> >
> > s/id/ID/
> >
> > > + * or 0 if it does not find a match.
> > > + */
> > > +int pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id)
> > > +{
> > > +     u32 header;
> > > +     int vsec;
> >
> >   int vsec;
> >   u32 header;
> >
> > since that's the order they're used.
> >
> > > +
> > > +     vsec = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VNDR);
> > > +     while (vsec) {
> > > +             if (pci_read_config_dword(dev, vsec + 0x4,
> >
> > s/0x4/PCI_VSEC_HDR/
> >
> > > +                                       &header) == PCIBIOS_SUCCESSFUL &&
> > > +                 PCI_VSEC_CAP_ID(header) == vsec_cap_id)
> > > +                     break;
> >
> >   return vsec;
> >
> > > +
> > > +             vsec = pci_find_next_ext_capability(dev, vsec,
> > > +                                                 PCI_EXT_CAP_ID_VNDR);
> > > +     }
> > > +
> > > +     return vsec;
> >
> >   return 0;
> >
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_find_vsec_capability);
> > > +
> > >  static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
> > >  {
> > >       int rc, ttl = PCI_FIND_CAP_TTL;
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index 22207a7..effecb0 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -1067,6 +1067,7 @@ int pci_find_capability(struct pci_dev *dev, int cap);
> > >  int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
> > >  int pci_find_ext_capability(struct pci_dev *dev, int cap);
> > >  int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
> > > +int pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id);
> > >  int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
> > >  int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
> > >  struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
> > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > > index a95d55f..f5d17be 100644
> > > --- a/include/uapi/linux/pci_regs.h
> > > +++ b/include/uapi/linux/pci_regs.h
> > > @@ -730,6 +730,11 @@
> > >  #define PCI_EXT_CAP_DSN_SIZEOF       12
> > >  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
> > >
> > > +/* Vendor-Specific Extended Capabilities */
> > > +#define PCI_VSEC_CAP_ID(header)              (header & 0x0000ffff)
> > > +#define PCI_VSEC_CAP_REV(header)     ((header >> 16) & 0xf)
> > > +#define PCI_VSEC_CAP_LEN(header)     ((header >> 20) & 0xffc)
> >
> > Please put these next to the existing PCI_VSEC_HDR.
> >
> > Why does PCI_VSEC_CAP_LEN mask with 0xffc instead of 0xfff?  I don't
> > see anything in the spec about VSEC Length having to be a multiple of
> > 4 (PCIe r5.0, sec 7.9.5.2).
> >
> > But you don't use this anyway, so I'd just drop it (and
> > PCI_VSEC_CAP_REV) altogether.
> >
> > >  /* Advanced Error Reporting */
> > >  #define PCI_ERR_UNCOR_STATUS 4       /* Uncorrectable Error Status */
> > >  #define  PCI_ERR_UNC_UND     0x00000001      /* Undefined */
> > > --
> > > 2.7.4
> > >

RE: [PATCH 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

[Gustavo Pimentel]

On Mon, Feb 1, 2021 at 22:39:20, Bjorn Helgaas <helgaas@kernel.org> 
wrote:

Hi Bjorn,

> > +/**
> > + * pci_find_vsec_capability - Find a vendor-specific extended capability
> > + * @dev: PCI device to query
> > + * @cap: vendor-specific capability id code
> 
> s/id/ID/

I will do it for all the requested changes.

> >  
> > +/* Vendor-Specific Extended Capabilities */
> > +#define PCI_VSEC_CAP_ID(header)		(header & 0x0000ffff)
> > +#define PCI_VSEC_CAP_REV(header)	((header >> 16) & 0xf)
> > +#define PCI_VSEC_CAP_LEN(header)	((header >> 20) & 0xffc)
> 
> Please put these next to the existing PCI_VSEC_HDR.

I will move it next to HDR. The 0xffc was a typo, thanks for noticing it.
About the PCI_VSEC_CAP_REV and PCI_VSEC_CAP_LEN macros, I will be using 
on dw_edma_pcie_get_vsec_dma_data() from dw-edma-pcie.c has a validation 
of the right DVSEC.

I will send a version 2 with those fixes.

-Gustavo