linux-kernel - Re: [PATCH v2 4/5] PCI: mediatek: Add new generation controller support

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: <1501908763.8298.11.camel@mtkswgap22>
Date:   Sat, 5 Aug 2017 12:52:43 +0800
From:   Ryder Lee <ryder.lee@...iatek.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     Honghui Zhang <honghui.zhang@...iatek.com>, <bhelgaas@...gle.com>,
        <robh@...enl.org>, <robh+dt@...nel.org>, <matthias.bgg@...il.com>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>, <linux-pci@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <yingjoe.chen@...iatek.com>, <eddie.huang@...iatek.com>,
        <hongkun.cao@...iatek.com>, <youlin.pei@...iatek.com>,
        <yong.wu@...iatek.com>, <yt.shen@...iatek.com>,
        <sean.wang@...iatek.com>, <xinping.qian@...iatek.com>
Subject: Re: [PATCH v2 4/5] PCI: mediatek: Add new generation controller
 support

Hi Honghui, Bjorn,

On Fri, 2017-08-04 at 08:18 -0500, Bjorn Helgaas wrote:
> On Fri, Aug 04, 2017 at 04:39:36PM +0800, Honghui Zhang wrote:
> > On Thu, 2017-08-03 at 17:42 -0500, Bjorn Helgaas wrote:
> > > > +
> > > > +static struct mtk_pcie_port *mtk_pcie_find_port(struct mtk_pcie *pcie,
> > > > +						struct pci_bus *bus, int devfn)
> > > > +{
> > > > +	struct pci_dev *dev;
> > > > +	struct pci_bus *pbus;
> > > > +	struct mtk_pcie_port *port, *tmp;
> > > > +
> > > > +	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> > > > +		if (bus->number == 0 && port->index == PCI_SLOT(devfn)) {
> > > > +			return port;
> > > > +		} else if (bus->number != 0) {
> > > > +			pbus = bus;
> > > > +			do {
> > > > +				dev = pbus->self;
> > > > +				if (port->index == PCI_SLOT(dev->devfn))
> > > > +					return port;
> > > > +				pbus = dev->bus;
> > > > +			} while (dev->bus->number != 0);
> > > > +		}
> > > > +	}
> > > > +
> > > > +	return NULL;
> > > 
> > > You should be able to use sysdata to avoid searching the list.
> > > See drivers/pci/host/pci-aardvark.c, for example.
> > > 
> > 
> > I could put the mtk_pcie * in sysdata, but still need to searching the
> > list to get the mtk_pcie_port *, how about:
> > 
> > 	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> > 		if (port->index == PCI_SLOT(devfn))
> > 			return port;
> > 	}
> 
> No.  Other drivers don't need to search the list.  Please take a look
> at them and see how they solve this problem.  I don't think your
> hardware is fundamentally different in a way that means you need to
> search when the others don't.
> 

I'm not directly involved in this generation, but I guess the main reason why Honghui need to do that is just because this hardware access configuration space via per-port registers, not just for the guard.  
Currently, We had a host bridge with two ports (two subnodes in binding text), thus he tried to tells them apart so that he can get the correct registers.

Some platforms don't need to do that since they just have a single port (no more subnodes), the others might have specific/shared registers to access configuration space. (e.g. Tegra, MTK legacy IP block).
Or, he can split them into two independent nodes, but it will break common probing flow by doing so. (I'd prefer to use subnodes.)

Ryder