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]

Message-ID: 
 <BY3PR18MB4707DBB80B5949EA26F7ABECA0582@BY3PR18MB4707.namprd18.prod.outlook.com>
Date: Wed, 28 Feb 2024 12:37:02 +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: Monday, February 26, 2024 10:31 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 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.
> 
> > > > +#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