Re: [PATCH v3] PCI: hotplug: Add OCTEON PCI hotplug controller driver

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Nov 08, 2024 at 12:17:22PM +0000, Shijith Thotton wrote:
> >> This patch introduces a PCI hotplug controller driver for the OCTEON
> >> PCIe device, a multi-function PCIe device where the first function acts
> >> as a hotplug controller. It is equipped with MSI-x interrupts to notify
> >> the host of hotplug events from the OCTEON firmware.

> >> +config HOTPLUG_PCI_OCTEONEP
> >> +	bool "OCTEON PCI device Hotplug controller driver"
> >
> >s/Marvell PCI device/Cavium OCTEON PCI/ to match other entries.
> 
> Cavium was acquired by Marvell, but we are still using the PCI
> vendor ID  `PCI_VENDOR_ID_CAVIUM`. I hope using Marvell is
> acceptable; please let me  know if this poses any issues.

Oops, sorry, this was my mistake.  I meant to suggest "Marvell OCTEON
PCI Hotplug driver", but I messed up the editing.

> >> +#define OCTEP_DEVID_HP_CONTROLLER 0xa0e3
> >
> >Even though this is a private #define, I think something like
> >PCI_DEVICE_ID_CAVIUM_OCTEP_HP_CTLR would be nice because that's the
> >typical pattern of include/linux/pci_ids.h.
> 
> The same reason mentioned above for not using the name CAVIUM.
> Is it okay to use `PCI_DEVICE_ID_MARVELL_OCTEP_HP_CTLR`?

In this case, I think you should use PCI_DEVICE_ID_CAVIUM... because
the PCI_VENDOR_ID_CAVIUM identifies the Device ID namespace, so the
#defines for the Vendor ID and the Device IDs in that namespace should
match.

> >> +static struct pci_device_id octep_hp_pci_map[] = {
> >> +	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM,
> >OCTEP_DEVID_HP_CONTROLLER) },
> >> +	{ },
> >> +};