netdev - Re: [net-next PATCH v2] octeontx2: Add PTP clock driver for Octeon PTM clock.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 29 Feb 2024 04:57:26 +0000
From: Sai Krishna Gajula <saikrishnag@...vell.com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>,
        "richardcochran@...il.com"
	<richardcochran@...il.com>,
        "horms@...nel.org" <horms@...nel.org>,
        "vinicius.gomes@...el.com" <vinicius.gomes@...el.com>,
        "vadim.fedorenko@...ux.dev" <vadim.fedorenko@...ux.dev>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org"
	<kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Sunil Kovvuri
 Goutham <sgoutham@...vell.com>,
        Geethasowjanya Akula <gakula@...vell.com>,
        Linu Cherian <lcherian@...vell.com>,
        Hariprasad Kelam <hkelam@...vell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
        Naveen Mamindlapalli
	<naveenm@...vell.com>
Subject: Re: [net-next PATCH v2] octeontx2: Add PTP clock driver for Octeon
 PTM clock.


> -----Original Message-----
> From: Bjorn Helgaas <helgaas@...nel.org>
> Sent: Wednesday, February 28, 2024 9:39 PM
> To: Sai Krishna Gajula <saikrishnag@...vell.com>
> Cc: bhelgaas@...gle.com; linux-pci@...r.kernel.org;
> richardcochran@...il.com; horms@...nel.org; vinicius.gomes@...el.com;
> vadim.fedorenko@...ux.dev; davem@...emloft.net; kuba@...nel.org;
> netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Sunil Kovvuri
> Goutham <sgoutham@...vell.com>; Geethasowjanya Akula
> <gakula@...vell.com>; Linu Cherian <lcherian@...vell.com>; Hariprasad
> Kelam <hkelam@...vell.com>; Subbaraya Sundeep Bhatta
> <sbhatta@...vell.com>; Naveen Mamindlapalli <naveenm@...vell.com>
> Subject: Re: [net-next PATCH v2] octeontx2: Add PTP clock driver for
> Octeon PTM clock.
> 
> On Wed, Feb 28, 2024 at 12:37:02PM +0000, Sai Krishna Gajula wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@...nel.org>
> > > 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@...nel.org>
> > > > > 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.

Though this device does not appear as a PCI device on EP firmware, but there are some other internal PCI devices that will be enumerated. 
We will remove the dependency of reading the PTP device to check the SoC versions, instead we will read the config space of this PCI device itself.
I hope this clears your doubt whether this driver is running on Host or EP device.

> 
> Bjorn