Re: [EXT] [RFC PATCH 06/11] misc: pci_endpoint_test: Use a common header file between endpoint driver

[Shunsuke Mie]

On 2023/03/17 23:47, Frank Li wrote:
> > pci@xxxxxxxxxxxxxxx
> > Subject: [EXT] [RFC PATCH 06/11] misc: pci_endpoint_test: Use a common
> > header file between endpoint driver
> >
> > Caution: EXT Email
> >
> > Duplicated definitions between pci-epf-test and pci_endpoint_test are
> > already moved to a header file. Remove the common definitions and include
> > the header file. In addition, the separate register address writes were
> > combined into a single write.
> >
> > Signed-off-by: Shunsuke Mie <mie@xxxxxxxxxx>
> > ---
> >   drivers/misc/pci_endpoint_test.c | 42 +-------------------------------
> >   1 file changed, 1 insertion(+), 41 deletions(-)
> >
> > diff --git a/drivers/misc/pci_endpoint_test.c
> > b/drivers/misc/pci_endpoint_test.c
> > index 55733dee95ad..d4a42e9ab86a 100644
> > --- a/drivers/misc/pci_endpoint_test.c
> > +++ b/drivers/misc/pci_endpoint_test.c
> > @@ -22,52 +22,12 @@
> >   #include <linux/pci_ids.h>
> >
> >   #include <linux/pci_regs.h>
> > +#include <linux/pci-epf-test.h>
> Pci-epf-test.h was only used by these two files.
>
> Actually, I think move  drivers/misc/pci_endpoint_test.c to under drivers/pci/endpoint/functions/
> And shared one private header is more reasonable.
> These two files should be stay together because tight coupling.

I agree that the shared header is not reasonable. However, it seems 
difficult to move pci_endpoint_test.c

because it is not an endpoint function driver. Furthermore, since 
Kselftest adaption [1] is being worked

on, I'd like to reconsider how we can apply the Kselftest patch.

[1] 
https://lore.kernel.org/all/20221007053934.5188-1-aman1.gupta@xxxxxxxxxxx/

> >   #include <uapi/linux/pcitest.h>
> >
> >   #define DRV_MODULE_NAME                                "pci-endpoint-test"
> >
> > -#define IRQ_TYPE_UNDEFINED                     -1
> > -#define IRQ_TYPE_LEGACY                                0
> > -#define IRQ_TYPE_MSI                           1
> > -#define IRQ_TYPE_MSIX                          2
> > -
> > -#define PCI_ENDPOINT_TEST_MAGIC                        0x0
> > -
> > -#define PCI_ENDPOINT_TEST_COMMAND              0x4
> > -#define COMMAND_RAISE_LEGACY_IRQ               BIT(0)
> > -#define COMMAND_RAISE_MSI_IRQ                  BIT(1)
> > -#define COMMAND_RAISE_MSIX_IRQ                 BIT(2)
> > -#define COMMAND_READ                           BIT(3)
> > -#define COMMAND_WRITE                          BIT(4)
> > -#define COMMAND_COPY                           BIT(5)
> > -
> > -#define PCI_ENDPOINT_TEST_STATUS               0x8
> > -#define STATUS_READ_SUCCESS                    BIT(0)
> > -#define STATUS_READ_FAIL                       BIT(1)
> > -#define STATUS_WRITE_SUCCESS                   BIT(2)
> > -#define STATUS_WRITE_FAIL                      BIT(3)
> > -#define STATUS_COPY_SUCCESS                    BIT(4)
> > -#define STATUS_COPY_FAIL                       BIT(5)
> > -#define STATUS_IRQ_RAISED                      BIT(6)
> > -#define STATUS_SRC_ADDR_INVALID                        BIT(7)
> > -#define STATUS_DST_ADDR_INVALID                        BIT(8)
> > -
> > -#define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR       0x0c
> > -#define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR       0x10
> > -
> > -#define PCI_ENDPOINT_TEST_LOWER_DST_ADDR       0x14
> > -#define PCI_ENDPOINT_TEST_UPPER_DST_ADDR       0x18
> > -
> > -#define PCI_ENDPOINT_TEST_SIZE                 0x1c
> > -#define PCI_ENDPOINT_TEST_CHECKSUM             0x20
> > -
> > -#define PCI_ENDPOINT_TEST_IRQ_TYPE             0x24
> > -#define PCI_ENDPOINT_TEST_IRQ_NUMBER           0x28
> > -
> > -#define PCI_ENDPOINT_TEST_FLAGS                        0x2c
> > -#define FLAG_USE_DMA                           BIT(0)
> > -
> >   #define PCI_DEVICE_ID_TI_AM654                 0xb00c
> >   #define PCI_DEVICE_ID_TI_J7200                 0xb00f
> >   #define PCI_DEVICE_ID_TI_AM64                  0xb010
> > --
> > 2.25.1

Best,

Shunsuke.