linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Tomasz Nowicki <tn@semihalf.com>,
	arnd@arndb.de, will.deacon@arm.com, catalin.marinas@arm.com,
	rafael@kernel.org, hanjun.guo@linaro.org, okaya@codeaurora.org,
	jchandra@broadcom.com, robert.richter@caviumnetworks.com,
	mw@semihalf.com, Liviu.Dudau@arm.com, ddaney@caviumnetworks.com,
	wangyijing@huawei.com, Suravee.Suthikulpanit@amd.com,
	msalter@redhat.com, linux-pci@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org, linaro-acpi@lists.linaro.org,
	jcm@redhat.com, andrea.gallo@linaro.org, dhdang@apm.com,
	jeremy.linton@arm.com, liudongdong3@huawei.com,
	cov@codeaurora.org
Subject: Re: [PATCH V8 5/9] pci, acpi: add acpi hook to assign domain number.
Date: Fri, 10 Jun 2016 16:14:58 +0100	[thread overview]
Message-ID: <20160610151458.GA23638@red-moon> (raw)
In-Reply-To: <20160608001559.GD4759@localhost>

Hi Bjorn, Tomasz,

On Tue, Jun 07, 2016 at 07:15:59PM -0500, Bjorn Helgaas wrote:

[...]

> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index eb431b5..2b52178 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -7,6 +7,7 @@
> >   *	Copyright 1997 -- 2000 Martin Mares <mj@ucw.cz>
> >   */
> >  
> > +#include <linux/acpi.h>
> >  #include <linux/kernel.h>
> >  #include <linux/delay.h>
> >  #include <linux/init.h>
> > @@ -4941,7 +4942,7 @@ int pci_get_new_domain_nr(void)
> >  }
> >  
> >  #ifdef CONFIG_PCI_DOMAINS_GENERIC
> > -void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
> > +static int of_pci_bus_domain_nr(struct device *parent)
> 
> Can we do a little cleanup before this patch?  
> 
>   - pci_bus_assign_domain_nr() is only used inside drivers/pci, so
>     maybe we move the prototype to drivers/pci/pci.h?
> 
>   - I don't really like the style of calling a function that
>     internally assigns bus->domain_nr.  Could we do something like
>     this instead?
> 
>     int pci_bus_domain_nr(...)
>     {
>       ...
>       return domain;
>     }
> 
>     ... pci_create_root_bus(...)
>     {
>       ...
>       b->domain_nr = pci_bus_domain_nr(...);

We noticed while preparing v9, that this would force us to
write an empty pci_bus_domain_nr() prototype for
!PCI_DOMAINS_GENERIC (ie every arch but ARM/ARM64) that should
return 0 to keep current behaviour unchanged.

That's why pci_bus_assign_domain_nr() was there, so that it
was compiled out on !PCI_DOMAINS_GENERIC.

I really would like v9 to be final so let's fix it before posting it
shortly please.

For the above we have three options:

1) Leave code as-is in v8

2) in pci_create_root_bus():
#ifdef CONFIG_PCI_DOMAINS_GENERIC
       b->domain_nr = pci_bus_domain_nr(...);
#endif

+ other changes requested above

3) in pci_create_root_bus()

b->domain_nr = pci_bus_domain_nr(...);

unguarded and a stub:

#ifndef CONFIG_PCI_DOMAINS_GENERIC
static inline int pci_bus_domain_nr() { return 0; }
#endif

+ other changes requested above

I think it is a matter of details and frankly I am not sure (2) or (3)
make things much cleaner than they are (given that as you know with
Arnd's changes we will rewrite this code anyway), so please let's
choose an option so that we can post v9 and hopefully this series is
ready to go.

Thanks !
Lorenzo

  parent reply	other threads:[~2016-06-10 15:14 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-30 15:14 [PATCH V8 0/9] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
2016-05-30 15:14 ` [PATCH V8 1/9] PCI: ecam: move ecam.h to linux/include/pci-ecam.h Tomasz Nowicki
2016-06-02  9:48   ` Lorenzo Pieralisi
2016-05-30 15:14 ` [PATCH V8 2/9] PCI: ecam: Add parent device field to pci_config_window Tomasz Nowicki
2016-06-02 10:13   ` Lorenzo Pieralisi
2016-05-30 15:14 ` [PATCH V8 3/9] pci: Add new function to unmap IO resources Tomasz Nowicki
2016-06-02 16:50   ` Lorenzo Pieralisi
2016-05-30 15:14 ` [PATCH V8 4/9] acpi, pci: Support IO resources when parsing PCI host bridge resources Tomasz Nowicki
2016-06-02 17:30   ` Lorenzo Pieralisi
2016-06-07 23:56   ` Bjorn Helgaas
2016-05-30 15:14 ` [PATCH V8 5/9] pci, acpi: add acpi hook to assign domain number Tomasz Nowicki
2016-06-08  0:15   ` Bjorn Helgaas
2016-06-08 10:21     ` Tomasz Nowicki
2016-06-08 13:22       ` Bjorn Helgaas
2016-06-10 15:14     ` Lorenzo Pieralisi [this message]
2016-06-10 15:49       ` Lorenzo Pieralisi
2016-06-10 16:49         ` Tomasz Nowicki
2016-06-10 18:18           ` Bjorn Helgaas
2016-06-10 18:54             ` Jon Masters
2016-05-30 15:14 ` [PATCH V8 6/9] arm64, pci, acpi: ACPI support for legacy IRQs parsing and consolidation with DT code Tomasz Nowicki
2016-05-30 15:14 ` [PATCH V8 7/9] acpi: Add generic MCFG table handling Tomasz Nowicki
2016-06-03 11:38   ` Lorenzo Pieralisi
2016-06-06 12:55     ` Tomasz Nowicki
2016-06-08  1:56   ` Bjorn Helgaas
2016-06-08 12:21     ` Tomasz Nowicki
2016-06-08 13:17       ` Bjorn Helgaas
2016-06-08 13:44         ` Tomasz Nowicki
2016-05-30 15:14 ` [PATCH V8 8/9] arm64, pci, acpi: Provide ACPI-specific prerequisites for PCI bus enumeration Tomasz Nowicki
2016-06-02  9:45   ` Lorenzo Pieralisi
2016-06-02  9:51     ` Tomasz Nowicki
2016-05-30 15:14 ` [PATCH V8 9/9] pci, acpi: ARM64 support for ACPI based generic PCI host controller Tomasz Nowicki
2016-05-30 15:38   ` Arnd Bergmann
2016-05-30 16:13     ` Jayachandran C
2016-06-02  9:35   ` Lorenzo Pieralisi
2016-06-02  9:44     ` Tomasz Nowicki
2016-06-08  2:14   ` Bjorn Helgaas
2016-06-01  7:36 ` [PATCH V8 0/9] Support for ARM64 ACPI based " Gabriele Paoloni
2016-06-02  7:31   ` Jon Masters
2016-06-02 10:06     ` Gabriele Paoloni
2016-06-02  8:52   ` Tomasz Nowicki
2016-06-02  9:58     ` Gabriele Paoloni
2016-06-02  8:48 ` Jon Masters
2016-06-07 23:13 ` Bjorn Helgaas
2016-06-08  9:20 ` Dongdong Liu
2016-06-09 16:45 ` Suravee Suthikulanit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160610151458.GA23638@red-moon \
    --to=lorenzo.pieralisi@arm.com \
    --cc=Liviu.Dudau@arm.com \
    --cc=Suravee.Suthikulpanit@amd.com \
    --cc=andrea.gallo@linaro.org \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=cov@codeaurora.org \
    --cc=ddaney@caviumnetworks.com \
    --cc=dhdang@apm.com \
    --cc=hanjun.guo@linaro.org \
    --cc=helgaas@kernel.org \
    --cc=jchandra@broadcom.com \
    --cc=jcm@redhat.com \
    --cc=jeremy.linton@arm.com \
    --cc=linaro-acpi@lists.linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=liudongdong3@huawei.com \
    --cc=msalter@redhat.com \
    --cc=mw@semihalf.com \
    --cc=okaya@codeaurora.org \
    --cc=rafael@kernel.org \
    --cc=robert.richter@caviumnetworks.com \
    --cc=tn@semihalf.com \
    --cc=wangyijing@huawei.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).