> -----Original Message----- > From: 'Manivannan Sadhasivam' > [mailto:manivannan.sadhasivam@xxxxxxxxxx] > Sent: 14 February 2023 11:47 > To: Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics > <aman1.gupta@xxxxxxxxxxx> > Cc: 'Shuah Khan' <skhan@xxxxxxxxxxxxxxxxxxx>; shradha.t@xxxxxxxxxxx; > pankaj.dubey@xxxxxxxxxxx; kishon@xxxxxx; lpieralisi@xxxxxxxxxx; > kw@xxxxxxxxx; shuah@xxxxxxxxxx; 'Bjorn Helgaas' <helgaas@xxxxxxxxxx>; > linux-pci@xxxxxxxxxxxxxxx; linux-kselftest@xxxxxxxxxxxxxxx; 'Padmanabhan > Rajanbabu' <p.rajanbabu@xxxxxxxxxxx> > Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for PCI endpoint > driver test > > On Tue, Dec 27, 2022 at 10:45:26AM +0530, Aman Gupta/FDS SW > /SSIR/Engineer/Samsung Electronics wrote: > > > > > > > -----Original Message----- > > > From: Shuah Khan [mailto:skhan@xxxxxxxxxxxxxxxxxxx] > > > Sent: 23 December 2022 22:01 > > > To: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > > Cc: Aman Gupta <aman1.gupta@xxxxxxxxxxx>; > shradha.t@xxxxxxxxxxx; > > > pankaj.dubey@xxxxxxxxxxx; kishon@xxxxxx; lpieralisi@xxxxxxxxxx; > > > kw@xxxxxxxxx; shuah@xxxxxxxxxx; Bjorn Helgaas <helgaas@xxxxxxxxxx>; > > > linux-pci@xxxxxxxxxxxxxxx; linux-kselftest@xxxxxxxxxxxxxxx; > > > Padmanabhan Rajanbabu <p.rajanbabu@xxxxxxxxxxx>; Shuah Khan > > > <skhan@xxxxxxxxxxxxxxxxxxx> > > > Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for > > > PCI endpoint driver test > > > > > > On 12/23/22 08:02, Manivannan Sadhasivam wrote: > > > > On Thu, Dec 22, 2022 at 10:49:48AM -0700, Shuah Khan wrote: > > > >> On 12/22/22 10:45, Manivannan Sadhasivam wrote: > > > >>> On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote: > > > >>>> On 10/6/22 23:39, Aman Gupta wrote: > > > >>>>> This patch enables the support to perform selftest on PCIe > > > >>>>> endpoint driver present in the system. The following tests are > > > >>>>> currently performed by the selftest utility > > > >>>>> > > > >>>>> 1. BAR Tests (BAR0 to BAR5) > > > >>>>> 2. MSI Interrupt Tests (MSI1 to MSI32) 3. Read Tests (For 1, > > > >>>>> 1024, 1025, 1024000, 1024001 Bytes) 4. Write Tests (For 1, > > > >>>>> 1024, 1025, 1024000, 1024001 Bytes) 5. Copy Tests (For 1, > > > >>>>> 1024, 1025, 1024000, > > > >>>>> 1024001 Bytes) > > > >>>>> > > > >>>>> Signed-off-by: Aman Gupta <aman1.gupta@xxxxxxxxxxx> > > > >>>>> Signed-off-by: Padmanabhan Rajanbabu > > > <p.rajanbabu@xxxxxxxxxxx> > > > >>>> > > > >>>> Adding Bjorn Helgaas to the thread. > > > >>>> > > > >>>> Adding pcit test under selftests is good. There is another > > > >>>> pcitest under tools/pci. I would like to see if the existing > > > >>>> code in tools/pci/pcitest.c can be leveraged. > > > >>>> > > > >>>> As part of this test work, also look into removing tools/pci so > > > >>>> we don't have to maintain duplicate code in two places. > > > >>>> > > > >>> > > > >>> It has been agreed in a thread with Greg [1] to {re}move the > > > >>> tests under tools/pci and utilize the kselftest. > > > >>> > > > >> > > > >> Inline with what I am suggesting. However, I don't see either > > > >> move or delete of tools/pci in the patch? > > > >> > > > >> The first patch could start with git mv of the existing files and > > > >> then make changes to preserver the history. > > > >> > > > > > > > > Right. This patch was posted independently of the series [1] that > > > > I submitted to fix the return values of IOCTL calls used in > > > > drivers/misc/pci_endpoint_test.c driver. > > > > > > > > Then in that series, it was decided to move the existing test to > > > > kselftest. So, I suggested Aman Gupta [2] to integrate my latest > > > > patches, add the kselftest patch on top, then remove the existing > > > > test > > > under tools/pci. > > > > > > > > The kselftest patch can also move the driver first and then make > > > > the change as you suggested. Either way it is fine by me. > > > > > > > > > > As I mentioned in my previous email, I prefer to see the move as the > > > first patch and then changes on top. This preserves the history and > cleaner. > > > > > > thanks, > > > -- Shuah > > > > > > > Hi Shuah, > > > > Thanks for review and suggestion. I understand that we would like to reuse > and preserve the history of tools/pci/pcietest.c. So we have two approaches: > > > > 1: Using git mv command move existing code from tools/pci/ to > tools/testing/selftest/drivers/pci/ and then update the file to convert to > kselftest framework. I thought about this but after movement, when we > move it to kselftest format it is going to be huge churn and we will be having > modification in almost all lines. > > > > 2: Develop kselftest based driver in tools/testing/selftest/drivers/pci/ and > eventually delete existing file from tools/pci/ folder providing justification in > commit message. > > > > From my viewpoint, going with the second approach makes more sense > because if almost complete file is getting modified, and it will make the > review process complex and anyways there is not much code reusability. > > Please let me know if you have any other thought process or if I am missing > anything to understand your approach. > > > > As Bjorn and Shuah said, I presume you are working on option 1. > > Thanks, > Mani > Hi Mani, Yes I am working on it, soon I will post the patches. Thanks , Aman Gupta > > Thanks, > > Aman Gupta > > > > > > > > > -- > மணிவண்ணன் சதாசிவம்