Re: [PATCH v2 6/6] misc: pci_endpoint_test: Add Device ID for R-Car V4H PCIe controller

From: Frank Li <Frank.li@nxp.com>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: "lpieralisi@kernel.org" <lpieralisi@kernel.org>,
	"kw@linux.com" <kw@linux.com>,
	"robh@kernel.org" <robh@kernel.org>,
	"bhelgaas@google.com" <bhelgaas@google.com>,
	"krzysztof.kozlowski+dt@linaro.org"
	<krzysztof.kozlowski+dt@linaro.org>,
	"conor+dt@kernel.org" <conor+dt@kernel.org>,
	"jingoohan1@gmail.com" <jingoohan1@gmail.com>,
	"gustavo.pimentel@synopsys.com" <gustavo.pimentel@synopsys.com>,
	"mani@kernel.org" <mani@kernel.org>,
	"marek.vasut+renesas@gmail.com" <marek.vasut+renesas@gmail.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org"
	<linux-renesas-soc@vger.kernel.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>
Subject: Re: [PATCH v2 6/6] misc: pci_endpoint_test: Add Device ID for R-Car V4H PCIe controller
Date: Tue, 26 Mar 2024 10:08:40 -0400	[thread overview]
Message-ID: <ZgLW6CGYfUW6Uskz@lizhi-Precision-Tower-5810> (raw)
In-Reply-To: <TYCPR01MB1104072DA4D6B3DF120A10D0AD8352@TYCPR01MB11040.jpnprd01.prod.outlook.com>

On Tue, Mar 26, 2024 at 05:47:23AM +0000, Yoshihiro Shimoda wrote:
> Hi Frank,
> 
> > From: Frank Li, Sent: Tuesday, March 26, 2024 12:21 PM
>  
> > On Tue, Mar 26, 2024 at 11:45:40AM +0900, Yoshihiro Shimoda wrote:
> > > Add Renesas R8A779G0 in pci_device_id table so that pci-epf-test
> > > can be used for testing PCIe EP on R-Car V4H.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > >  drivers/misc/pci_endpoint_test.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> > > index c38a6083f0a7..2fa3c6473c7d 100644
> > > --- a/drivers/misc/pci_endpoint_test.c
> > > +++ b/drivers/misc/pci_endpoint_test.c
> > > @@ -83,6 +83,7 @@
> > >  #define PCI_DEVICE_ID_RENESAS_R8A774C0		0x002d
> > >  #define PCI_DEVICE_ID_RENESAS_R8A774E1		0x0025
> > >  #define PCI_DEVICE_ID_RENESAS_R8A779F0		0x0031
> > > +#define PCI_DEVICE_ID_RENESAS_R8A779G0		0x0030
> > >
> > >  static DEFINE_IDA(pci_endpoint_test_ida);
> > >
> > > @@ -1005,6 +1006,9 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = {
> > >  	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, PCI_DEVICE_ID_RENESAS_R8A779F0),
> > >  	  .driver_data = (kernel_ulong_t)&default_data,
> > >  	},
> > > +	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, PCI_DEVICE_ID_RENESAS_R8A779G0),
> > > +	  .driver_data = (kernel_ulong_t)&default_data,
> > > +	},
> > 
> > You use default_data, why need new device_id? I think you can use 0x0031
> > to do test.
> 
> I thought we can add a new device_id freely like other devices.
> Since the PCIe controller's endpoint mode can configure the device id,
> I can use 0x0031 to do test though.
> 
> If such a reusable entry exists, is adding a new device id into the driver prohibited?

I just think it is not necessary. This list will become longer and longer.
And difference device id can't help us at all. 

We should use difference production as difference functions, or difference
configuration.  Such as usb gadget product id, we use 0x4545 for all mass
storage. 

Using difference devices id for difference function, such as 0x31 for
ep_test 0x30 for virtual net, 0x29 for virtual console ...

Or using difference devices id indicate some features. For example, use
0x30 means support write to EP MSI ITS to trigger irq.

Donate a device_id to more valuable things.

Frank

> 
> Best regards,
> Yoshihiro Shimoda
> 
> > Frank
> > 
> > >  	{ PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_J721E),
> > >  	  .driver_data = (kernel_ulong_t)&j721e_data,
> > >  	},
> > > --
> > > 2.25.1
> > >