On Mon, Feb 26, 2024 at 03:40:25PM +0000, Sai Krishna Gajula wrote:

quoted

-----Original Message-----
From: Bjorn Helgaas <helgaas@kernel.org>
Sent: Wednesday, February 14, 2024 10:59 PM
...
On Wed, Feb 14, 2024 at 06:38:53PM +0530, Sai Krishna wrote:

quoted

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.

quoted

quoted

+#define PCI_VENDOR_ID_CAVIUM			0x177d

Already defined in pci_ids.h.

quoted

quoted

+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?

quoted

quoted

+	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.

Bjorn