Re: [RESEND][PATCH 1/1] fpga: altera-cvp: Limit driver registration to specific device ID

[Xu Yilun <yilun.xu@linux.intel.com>]

On Wed, Nov 05, 2025 at 06:29:06AM +0000, Murugasen Krishnan, Kuhanh wrote:
> On 3/11/2025 4:44 pm, Xu Yilun wrote:
> > On Thu, Oct 30, 2025 at 03:05:44AM +0800, Kuhanh Murugasen Krishnan wrote:
> >> From: "Murugasen Krishnan, Kuhanh" <kuhanh.murugasen.krishnan@altera.com>
> > 
> > Is this your first post?
> > 
> > https://lore.kernel.org/all/20250212223553.2717304-1-kuhanh.murugasen.krishnan@intel.com/
> > 
> > Please mark the patch v2 if this patch is for the same issue. And please
> > firstly response the talk and make clear all previous concerns, rather than
> > just sent the patch and left.
> > 
> Thanks Yilun for your review. Yes that was the first post, however I do 
> not have access to my @intel email address anymore and had to resend 
> this with a better commit title and description for clearer explanation 
> of this patch. Apologies for the inconvenience.
> 
> >>
> >> The Altera CvP driver previously used PCI_ANY_ID, which caused it to
> >> bind to all PCIe devices with the Altera vendor ID. This led to
> >> incorrect driver association when multiple PCIe devices with different
> >> device IDs were present on the same platform.
> >>
> >> Update the device ID table to use 0x00 instead of PCI_ANY_ID so that
> >> the driver only attaches to the intended device.
> > 
> > So could you please answer the previous concern here?
> > 
> > Does dev_id 0x00 covers all supported devices? Do you have any DOC for
> > this?
> > 
> Yes it will connect to all supported Altera FPGA devices correctly, 

Because your change is trying to reduce the scope of devices the driver
could support. I want to be cautious and ask for Public Documentation
for reference. I don't want to see someone later yells "Oh, my device is
broken!".

Please also add the Link of the Documentation in changelog.

> there was a bug previously which caused incorrect driver association 

So this is a bug, please tag with "fixes:" and Cc stable kernel.

> with the use of PCI_ANY_ID. Limiting the driver registration to 0x00 
> allows the driver to attach to the intended Altera FPGA device correctly 
> since the FPGA default address is 0x00. Using PCI_ANY_ID could 
> potentially allow the CVP driver to associate to other PCI devices.
> 
> >>
> >> Reviewed-by: Dinh Nguyen <dinguyen@kernel.org>
> > 
> > I didn't see where the tag is from. Generally we don't prefer a
> > Reviewed-by tag firstly appear from other than the person named.
> > 
> > Thanks,
> > Yilun
> > 
> This patch was reviewed internally by Dinh and the tag was added. Should 
> I send a v2 patch with this "Reviewed-by" removed?

Yes, fix previous comments in your v2, and removes the Reviewed-by.
Reviewed-by is welcomed but should be publicly originated from the
named person.

Thanks,
Yilun

> 
> >> Signed-off-by: Ang Tien Sung <tien.sung.ang@altera.com>
> >> Signed-off-by: Murugasen Krishnan, Kuhanh <kuhanh.murugasen.krishnan@altera.com>
> >> ---
> >>   drivers/fpga/altera-cvp.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
> >> index 5af0bd33890c..97e9d4d981ad 100644
> >> --- a/drivers/fpga/altera-cvp.c
> >> +++ b/drivers/fpga/altera-cvp.c
> >> @@ -560,7 +560,7 @@ static int altera_cvp_probe(struct pci_dev *pdev,
> >>   static void altera_cvp_remove(struct pci_dev *pdev);
> >>   
> >>   static struct pci_device_id altera_cvp_id_tbl[] = {
> >> -	{ PCI_VDEVICE(ALTERA, PCI_ANY_ID) },
> >> +	{ PCI_VDEVICE(ALTERA, 0x00) },
> >>   	{ }
> >>   };
> >>   MODULE_DEVICE_TABLE(pci, altera_cvp_id_tbl);
> >> -- 
> >> 2.25.1
> >>
> >>
>