linux-kernel - RE: [PATCH v2 1/2] PCI: altera: refactor driver for supporting new platform

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:   Fri, 8 Sep 2023 09:09:34 +0000
From:   "D M, Sharath Kumar" <sharath.kumar.d.m@...el.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
        "kw@...ux.com" <kw@...ux.com>, "robh@...nel.org" <robh@...nel.org>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "dinguyen@...nel.org" <dinguyen@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 1/2] PCI: altera: refactor driver for supporting new
 platform



> -----Original Message-----
> From: Bjorn Helgaas <helgaas@...nel.org>
> Sent: Wednesday, September 6, 2023 10:38 PM
> To: D M, Sharath Kumar <sharath.kumar.d.m@...el.com>
> Cc: lpieralisi@...nel.org; kw@...ux.com; robh@...nel.org;
> bhelgaas@...gle.com; linux-pci@...r.kernel.org; dinguyen@...nel.org;
> linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v2 1/2] PCI: altera: refactor driver for supporting new
> platform
> 
> Capitalize subject line to match history:
> 
>   PCI: altera: Refactor driver for supporting new platform
> 
> On Wed, Sep 06, 2023 at 04:39:17PM +0530, sharath.kumar.d.m@...el.com
> wrote:
> > From: D M Sharath Kumar <sharath.kumar.d.m@...el.com>
> 
> Needs a commit log.  It's OK to repeat the subject line.  Also helpful if you can
> hint about why it needs to be refactored.  In this case, it looks like you'll need
> to have a different ISR for Agilex, and also something different about root
> bus vs downstream config accesses.
> 
ok
> > @@ -100,10 +101,15 @@ struct altera_pcie_ops {
> >  	void (*tlp_write_pkt)(struct altera_pcie *pcie, u32 *headers,
> >  			      u32 data, bool align);
> >  	bool (*get_link_status)(struct altera_pcie *pcie);
> > -	int (*rp_read_cfg)(struct altera_pcie *pcie, int where,
> > -			   int size, u32 *value);
> > +	int (*rp_read_cfg)(struct altera_pcie *pcie, u8 busno,
> > +			unsigned int devfn, int where, int size, u32 *value);
> >  	int (*rp_write_cfg)(struct altera_pcie *pcie, u8 busno,
> > -			    int where, int size, u32 value);
> > +			unsigned int devfn, int where, int size, u32 value);
> > +	int (*ep_read_cfg)(struct altera_pcie *pcie, u8 busno,
> > +			unsigned int devfn, int where, int size, u32 *value);
> > +	int (*ep_write_cfg)(struct altera_pcie *pcie, u8 busno,
> > +			unsigned int devfn, int where, int size, u32 value);
> 
> "ep_read_cfg" isn't the ideal name because it suggests "endpoint", but it may
> be either an endpoint or a switch upstream port.  The rockchip driver uses
> "other", which isn't super descriptive either but might be better.
> 
Ok will change to "nonrp_read_cfg" ?
> > +	void (*rp_isr)(struct irq_desc *desc);
> >  };
> 
> > +static int _altera_pcie_cfg_read(struct altera_pcie *pcie, u8 busno,
> > +				 unsigned int devfn, int where, int size,
> > +				 u32 *value)
> > +{
> > +	if (busno == pcie->root_bus_nr && pcie->pcie_data->ops-
> >rp_read_cfg)
> > +		return pcie->pcie_data->ops->rp_read_cfg(pcie, busno,
> devfn,
> > +							where, size, value);
> 
> Several other drivers use pci_is_root_bus() instead of keeping track of
> root_bus_nr and watching for changes to it.  Maybe simpler and more
> reliable?  That would be best as a separate patch all by itself if you go that
> direction.
> 
Will not take up as part of this commit
> > +
> > +	if (pcie->pcie_data->ops->ep_read_cfg)
> > +		return pcie->pcie_data->ops->ep_read_cfg(pcie, busno,
> devfn,
> > +							where, size, value);
> > +	return PCIBIOS_FUNC_NOT_SUPPORTED;
> > +}