[PATCH v2 08/10] PCI: Add FMT and TYPE definition for TLP header

[Jacky Chou]

Hi Bjorn,

Thank you for your reply.

> >  include/uapi/linux/pci_regs.h | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> 
> I don't think these definitions are relevant to uapi users, so they could go in
> drivers/pci/pci.h, similar to the existing PCIE_MSG_* definitions.

Agreed.
I will move them to drivers/pci/pci.h and rename them in next version.

> 
> Please follow the style of PCIE_MSG_*, including the brief spec citations and
> /* */ comments.
> 
> Not sure we need *all* of these; it looks like you only use:
> 
>   PCI_TLP_TYPE_CFG0_RD
>   PCI_TLP_TYPE_CFG0_WR
>   PCI_TLP_TYPE_CFG1_RD
>   PCI_TLP_TYPE_CFG1_WR
>   PCI_TLP_FMT_3DW_NO_DATA
>   PCI_TLP_FMT_3DW_DATA
>

Yes, I just use these in our PCIe RC driver.
I will delete the others that are not used in next version.

> > diff --git a/include/uapi/linux/pci_regs.h
> > b/include/uapi/linux/pci_regs.h index a3a3e942dedf..700b915e00f5
> > 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -1230,4 +1230,36 @@
> >  #define PCI_DVSEC_CXL_PORT_CTL				0x0c
> >  #define PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR		0x00000001
> >
> > +/* Fmt[2:0] encoding for TLP Header */
> > +#define PCI_TLP_FMT_3DW_NO_DATA		0x0  // 3DW header, no data
> > +#define PCI_TLP_FMT_4DW_NO_DATA		0x1  // 4DW header, no data
> > +#define PCI_TLP_FMT_3DW_DATA		0x2  // 3DW header, with data
> > +#define PCI_TLP_FMT_4DW_DATA		0x3  // 4DW header, with data
> > +#define PCI_TLP_FMT_PREFIX		0x4  // Prefix header
> > +
> > +/* Type[4:0] encoding for TLP Header */
> > +#define PCI_TLP_TYPE_MEM_RD		0x00 // Memory Read Request
> > +#define PCI_TLP_TYPE_MEM_RDLK		0x01 // Memory Read Lock
> Request
> > +#define PCI_TLP_TYPE_MEM_WR		0x00 // Memory Write Request
> (Fmt must be with data)
> > +#define PCI_TLP_TYPE_IO_RD		0x02 // IO Read Request
> > +#define PCI_TLP_TYPE_IO_WR		0x02 // IO Write Request (Fmt must be
> with data)
> > +#define PCI_TLP_TYPE_CFG0_RD		0x04 // Config Type 0 Read
> Request
> > +#define PCI_TLP_TYPE_CFG0_WR		0x04 // Config Type 0 Write
> Request (Fmt must be with data)
> > +#define PCI_TLP_TYPE_CFG1_RD		0x05 // Config Type 1 Read
> Request
> > +#define PCI_TLP_TYPE_CFG1_WR		0x05 // Config Type 1 Write
> Request (Fmt must be with data)
> > +#define PCI_TLP_TYPE_MSG		0x10 // Message Request (see routing
> field)
> > +#define PCI_TLP_TYPE_MSGD		0x11 // Message Request with Data
> (see routing field)
> > +#define PCI_TLP_TYPE_CPL		0x0A // Completion without Data
> > +#define PCI_TLP_TYPE_CPLD		0x0A // Completion with Data (Fmt
> must be with data)
> > +#define PCI_TLP_TYPE_CPLLCK		0x0B // Completion Locked
> > +#define PCI_TLP_TYPE_CPLDLCK		0x0B // Completion with Data
> Locked (Fmt must be with data)
> > +#define PCI_TLP_TYPE_FETCH_ADD		0x0C // Fetch and Add AtomicOp
> Request
> > +#define PCI_TLP_TYPE_SWAP		0x0D // Unconditional Swap AtomicOp
> Request
> > +#define PCI_TLP_TYPE_CMP_SWAP		0x0E // Compare and Swap
> AtomicOp Request
> > +#define PCI_TLP_TYPE_LOCAL_PREFIX	0x00 // Local TLP Prefix (Fmt =
> 0x4)
> > +#define PCI_TLP_TYPE_E2E_PREFIX		0x10 // End-to-End TLP Prefix (Fmt
> = 0x4)
> > +
> > +/* Macro to combine Fmt and Type into the 8-bit field */
> > +#define PCIE_TLP_FMT_TYPE(fmt, type)   (((fmt) << 5) | ((type) & 0x1F))
> 
> This looks like it might be controller-specific and could go in pcie-aspeed.c.

Agreed.

Thanks,
Jacky