linux-kernel - Re: [PATCH V10 2/5] PCI: Create device tree node for bridge

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: <20230630144235.GA483557@bhelgaas>
Date:   Fri, 30 Jun 2023 09:42:35 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Rob Herring <robh@...nel.org>
Cc:     Lizhi Hou <lizhi.hou@....com>, linux-pci@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        max.zhen@....com, sonal.santan@....com,
        stefano.stabellini@...inx.com
Subject: Re: [PATCH V10 2/5] PCI: Create device tree node for bridge

On Thu, Jun 29, 2023 at 05:55:51PM -0600, Rob Herring wrote:
> On Thu, Jun 29, 2023 at 05:56:31PM -0500, Bjorn Helgaas wrote:
> > On Thu, Jun 29, 2023 at 10:19:47AM -0700, Lizhi Hou wrote:
> > > The PCI endpoint device such as Xilinx Alveo PCI card maps the register
> > > spaces from multiple hardware peripherals to its PCI BAR. Normally,
> > > the PCI core discovers devices and BARs using the PCI enumeration process.
> > > There is no infrastructure to discover the hardware peripherals that are
> > > present in a PCI device, and which can be accessed through the PCI BARs.

> > > --- a/drivers/pci/of.c
> > > +++ b/drivers/pci/of.c
> > > @@ -487,6 +487,15 @@ static int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *
> > >  		} else {
> > >  			/* We found a P2P bridge, check if it has a node */
> > >  			ppnode = pci_device_to_OF_node(ppdev);
> > > +#if IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES)
> > 
> > I would use plain #ifdef here instead of IS_ENABLED(), as you did in
> > pci.h below.  IS_ENABLED() is true if the Kconfig symbol is set to
> > either "y" or "m".
> 
> Actually, IS_ENABLED() with a C 'if' rather than a preprocessor #if 
> would work here and is preferred.

Makes sense; I see the justification at [1].  I do wish it didn't have
to be different between this usage and the "#ifdef
CONFIG_PCI_DYNAMIC_OF_NODES" in pci.h for the stubs.  But this is OK
by me.

Bjorn

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=v6.4#n1162