| Message ID | 20230317113238.142970-7-mie@igel.co.jp (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Introduce a test for continuous transfer | expand |
> pci@vger.kernel.org > 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@igel.co.jp> > --- > 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. > > #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
On 2023/03/17 23:47, Frank Li wrote: >> pci@vger.kernel.org >> 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@igel.co.jp> >> --- >> 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@samsung.com/ > >> #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.
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> #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
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@igel.co.jp> --- drivers/misc/pci_endpoint_test.c | 42 +------------------------------- 1 file changed, 1 insertion(+), 41 deletions(-)