Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <2a4e0f94-4766-8db1-c648-e72b1f7924fa@igel.co.jp>
Date:   Tue, 4 Apr 2023 19:11:55 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.9.0
Subject: Re: [EXT] [RFC PATCH 06/11] misc: pci_endpoint_test: Use a common
 header file between endpoint driver
Content-Language: en-US
To:     Frank Li <frank.li@nxp.com>,
        Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc:     Vinod Koul <vkoul@kernel.org>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= <kw@linux.com>,
        Manivannan Sadhasivam <mani@kernel.org>,
        Kishon Vijay Abraham I <kishon@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Kunihiko Hayashi <hayashi.kunihiko@socionext.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
References: <20230317113238.142970-1-mie@igel.co.jp>
 <20230317113238.142970-7-mie@igel.co.jp>
 <AM6PR04MB4838D8F8AF23C61048BDFB9788BD9@AM6PR04MB4838.eurprd04.prod.outlook.com>
From:   Shunsuke Mie <mie@igel.co.jp>
In-Reply-To: <AM6PR04MB4838D8F8AF23C61048BDFB9788BD9@AM6PR04MB4838.eurprd04.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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.