> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: Monday, February 26, 2024 10:31 PM
> To: Sai Krishna Gajula <saikrishnag@xxxxxxxxxxx>
> Cc: bhelgaas@xxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx;
> richardcochran@xxxxxxxxx; horms@xxxxxxxxxx; vinicius.gomes@xxxxxxxxx;
> vadim.fedorenko@xxxxxxxxx; davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sunil Kovvuri Goutham
> <sgoutham@xxxxxxxxxxx>; Geethasowjanya Akula <gakula@xxxxxxxxxxx>;
> Linu Cherian <lcherian@xxxxxxxxxxx>; Hariprasad Kelam
> <hkelam@xxxxxxxxxxx>; Subbaraya Sundeep Bhatta <sbhatta@xxxxxxxxxxx>;
> Naveen Mamindlapalli <naveenm@xxxxxxxxxxx>
> Subject: Re: [net-next PATCH v2] octeontx2: Add PTP clock driver for
> Octeon PTM clock.
>
> 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.
>
> > > > +#define PCI_VENDOR_ID_CAVIUM 0x177d
>
> Already defined in pci_ids.h.
Ack, will use this V3 patch
>
> > > > +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.
>
> > > > + ptm_ctl_addr = ioremap(PEMX_PTM_CTL, 8);
> > >
> > > Hard-coded register addresses? That can't be right. Shouldn't this
> > > be discoverable either as a PCI BAR or via DT or similar firmware
> > > interface?
> >
> > Ack, will explore the DT implementation for register addresses access
> > and submit patch V3. Thanks for the review.
>
> 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.
>
> Bjorn
Thanks,
Sai
