On Wed, Feb 28, 2024 at 12:37:02PM +0000, Sai Krishna Gajula wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> > Sent: Monday, February 26, 2024 10:31 PM
> ...
> > On Mon, Feb 26, 2024 at 03:40:25PM +0000, Sai Krishna Gajula wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> > > > Sent: Wednesday, February 14, 2024 10:59 PM ...
> > > > On Wed, Feb 14, 2024 at 06:38:53PM +0530, Sai Krishna wrote:
> > > > > The PCIe PTM(Precision time measurement) protocol provides precise
> > > > > coordination of events across multiple components like PCIe host
> > > > > clock, PCIe EP PHC local clocks of PCIe devices. This patch adds
> > > > > support for ptp clock based PTM clock. We can use this PTP device
> > > > > to sync the PTM time with CLOCK_REALTIME or other PTP PHC devices
> > > > > using phc2sys.
> > > > > +static int __init ptp_oct_ptm_init(void) {
> > > > > + struct pci_dev *pdev = NULL;
> > > > > +
> > > > > + pdev = pci_get_device(PCI_VENDOR_ID_CAVIUM,
> > > > > + PCI_DEVID_OCTEONTX2_PTP, pdev);
> > > >
> > > > pci_get_device() is a sub-optimal method for a driver to claim a device.
> > > > pci_register_driver() is the preferred method. If you can't use
> > > > that, a comment here explaining why not would be helpful.
> > >
> > > We just want to check the PTP device availability in the system as one
> > > of the use case is to sync PTM time to PTP.
> >
> > This doesn't explain why you can't use pci_register_driver(). Can
> > you clarify that?
>
> This is not a PCI endpoint driver. This piece of code is used to
> identify the silicon version. We will update the code by reading
> the silicon version from Endpoint internal BAR register offsets.
> > I assume the PCI_DEVID_OCTEONTX2_PTP device is a PCIe Endpoint,
> > and this driver runs on the host? I.e., this driver does not run
> > as firmware on the Endpoint itself? So if you run lspci on the
> > host, you would see this device as one of the PCI devices?
> >
> > If that's the case, a driver would normally operate the device via
> > MMIO accesses to regions described by PCI BARs. "lspci -v" would
> > show those addresses.
>
> This driver don't run on Host but runs on the EP firmware itself.
The "endpoint driver" terminology is a bit confusing here. See
Documentation/PCI/endpoint/pci-endpoint.rst for details.
If this driver actually runs as part of the Endpoint firmware, it
would not normally see a hierarchy of pci_devs, and I don't think
pci_get_device() would work.
So I suspect this driver actually runs on the host, and it looks like
it wants to use the same device (0x177d:0xa00c) as these two drivers:
drivers/net/ethernet/cavium/common/cavium_ptp.c:#define PCI_DEVICE_ID_CAVIUM_PTP 0xA00C
drivers/net/ethernet/marvell/octeontx2/af/ptp.c:#define PCI_DEVID_OCTEONTX2_PTP 0xA00C
It seems like maybe it should be integrated into them? Otherwise you
have multiple drivers thinking they are controlling a single device.
Bjorn
