[PATCH 0/3] Add PCI domain support to R-Car drivers

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Phil Edworthy]

The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller drivers,
one for an external PCIe slot, the other for an internal PCI bridge to USB
controllers.

However, they currently do not work at the same time as they use the same PCI
domain and use the same root bus number. We can't use different root bus numbers
due to the way root bus numbers are assigned in pcibios_init_hw() in
arch/arm/kernel/bios32.c.

Since the two PCI controllers are completely independent, I think it makes sense
to use different PCI domains for them.

I've marked the third patch as RFC as I am not sure of the impact of enabling
PCI domains for all ARM devices. In the march to 'one kernel to rule them all',
I steered clear of mach specific changes.

These patches require the following patch from Liviu Dudau:
  [PATCH v11 07/10] OF: Introduce helper function for getting PCI domain_nr
Based on comments on this patch from Jason Gunthorpe, there is still the issue
that the domain numbers may change depending on the ordering at probe time.
However, this can be fixed later on by adding the entries in the DT files.


Phil Edworthy (3):
  PCI: rcar-pcie: Add call to get domain nr
  PCI: rcar-internal-pci: Add call to get domain nr
  ARM: Enable PCI domains

 arch/arm/Kconfig                 | 2 +-
 drivers/pci/host/pci-rcar-gen2.c | 6 ++++++
 drivers/pci/host/pcie-rcar.c     | 9 ++++++---
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.1.0

[PATCH 1/3] PCI: rcar-pcie: Add call to get domain nr

[Phil Edworthy]

R-Car devices (r8a7790 and r8a7791) need to place the internal PCI and external
PCIe controllers on separate domains so that they can work at the same time.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 4884ee5..db74371 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -128,6 +128,7 @@ struct rcar_pcie {
 	struct device		*dev;
 	void __iomem		*base;
 	struct resource		res[RCAR_PCI_MAX_RESOURCES];
+	int			domain;
 	struct resource		busn;
 	int			root_bus_nr;
 	struct clk		*clk;
@@ -393,13 +394,13 @@ static void rcar_pcie_enable(struct rcar_pcie *pcie)
 {
 	struct platform_device *pdev = to_platform_device(pcie->dev);
 
+#ifdef CONFIG_PCI_DOMAINS
+	rcar_pci.domain = pcie->domain;
+#endif
 	rcar_pci.nr_controllers = 1;
 	rcar_pci.private_data = (void **)&pcie;
 
 	pci_common_init_dev(&pdev->dev, &rcar_pci);
-#ifdef CONFIG_PCI_DOMAINS
-	rcar_pci.domain++;
-#endif
 }
 
 static int phy_wait_for_ack(struct rcar_pcie *pcie)
@@ -917,6 +918,8 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 	pcie->dev = &pdev->dev;
 	platform_set_drvdata(pdev, pcie);
 
+	pcie->domain = of_pci_get_domain_nr(pdev->dev.of_node, true);
+
 	/* Get the bus range */
 	if (of_pci_parse_bus_range(pdev->dev.of_node, &pcie->busn)) {
 		dev_err(&pdev->dev, "failed to parse bus-range property\n");
-- 
2.1.0

[PATCH 2/3] PCI: rcar-internal-pci: Add call to get domain nr

[Phil Edworthy]

R-Car devices (r8a7790 and r8a7791) need to place the internal PCI and external
PCIe controllers on separate domains so that they can work at the same time.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pci-rcar-gen2.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
index 3ef854f..17681ea 100644
--- a/drivers/pci/host/pci-rcar-gen2.c
+++ b/drivers/pci/host/pci-rcar-gen2.c
@@ -99,6 +99,7 @@ struct rcar_pci_priv {
 	struct resource io_res;
 	struct resource mem_res;
 	struct resource *cfg_res;
+	int domain;
 	unsigned busnr;
 	int irq;
 	unsigned long window_size;
@@ -373,6 +374,8 @@ static int rcar_pci_probe(struct platform_device *pdev)
 
 	priv->window_size = SZ_1G;
 
+	priv->domain = of_pci_get_domain_nr(pdev->dev.of_node, true);
+
 	if (pdev->dev.of_node) {
 		struct resource busnr;
 		int ret;
@@ -397,6 +400,9 @@ static int rcar_pci_probe(struct platform_device *pdev)
 	hw.map_irq = rcar_pci_map_irq;
 	hw.ops = &rcar_pci_ops;
 	hw.setup = rcar_pci_setup;
+#ifdef CONFIG_PCI_DOMAINS
+	hw.domain = priv->domain;
+#endif
 	pci_common_init_dev(&pdev->dev, &hw);
 	return 0;
 }
-- 
2.1.0

[RFC PATCH 3/3] ARM: Enable PCI domains

[Phil Edworthy]

Since there are ARM devices with multiple independant PCI controllers, enable
PCI domains for all ARM devices.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
Marked as RFC as I am not sure of the impact of enabling PCI domains for all
ARM devices. In the march to 'one kernel to rule them all', I steered clear of
mach specific changes.

 arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 32cbbd5..8741d3d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1274,6 +1274,7 @@ config ISA_DMA_API
 
 config PCI
 	bool "PCI support" if MIGHT_HAVE_PCI
+	select PCI_DOMAINS
 	help
 	  Find out whether you have a PCI motherboard. PCI is the name of a
 	  bus system, i.e. the way the CPU talks to the other stuff inside
@@ -1282,7 +1283,6 @@ config PCI
 
 config PCI_DOMAINS
 	bool
-	depends on PCI
 
 config PCI_NANOENGINE
 	bool "BSE nanoEngine PCI support"
-- 
2.1.0

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Liviu Dudau]

On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller drivers,
> one for an external PCIe slot, the other for an internal PCI bridge to USB
> controllers.
> 
> However, they currently do not work at the same time as they use the same PCI
> domain and use the same root bus number. We can't use different root bus numbers
> due to the way root bus numbers are assigned in pcibios_init_hw() in
> arch/arm/kernel/bios32.c.
> 
> Since the two PCI controllers are completely independent, I think it makes sense
> to use different PCI domains for them.
> 
> I've marked the third patch as RFC as I am not sure of the impact of enabling
> PCI domains for all ARM devices. In the march to 'one kernel to rule them all',
> I steered clear of mach specific changes.
> 
> These patches require the following patch from Liviu Dudau:
>   [PATCH v11 07/10] OF: Introduce helper function for getting PCI domain_nr
> Based on comments on this patch from Jason Gunthorpe, there is still the issue
> that the domain numbers may change depending on the ordering at probe time.
> However, this can be fixed later on by adding the entries in the DT files.

Hi Phil,

I'm happy that you found use for my patch, but I can't help wondering if it is
not a better idea to convert your drivers to the whole new API in my series.
Do you have any thoughts on that?

One other thing to note: Rob Herring is not very happy with the mix of OF
parsing and domain number allocation going in together, so I might remove the
OF parsing entirely for now. Would that cause you any problems (other than
the fact that if of_pci_get_domain_nr() doesn't do anything OF related I might
rename it to pci_get_domain_nr()). You don't seem to have any alias defined
for pci-domain in the DT, so I'll guess not.

Best regards,
Liviu

> 
> 
> Phil Edworthy (3):
>   PCI: rcar-pcie: Add call to get domain nr
>   PCI: rcar-internal-pci: Add call to get domain nr
>   ARM: Enable PCI domains
> 
>  arch/arm/Kconfig                 | 2 +-
>  drivers/pci/host/pci-rcar-gen2.c | 6 ++++++
>  drivers/pci/host/pcie-rcar.c     | 9 ++++++---
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> -- 
> 2.1.0
> 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Phil Edworthy]

Hi Liviu,

On 22 September 2014 12:29, Liviu wrote:
> On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> drivers,
> > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > controllers.
> >
> > However, they currently do not work at the same time as they use the
> same PCI
> > domain and use the same root bus number. We can't use different root
> bus numbers
> > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > arch/arm/kernel/bios32.c.
> >
> > Since the two PCI controllers are completely independent, I think it makes
> sense
> > to use different PCI domains for them.
> >
> > I've marked the third patch as RFC as I am not sure of the impact of
> enabling
> > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> all',
> > I steered clear of mach specific changes.
> >
> > These patches require the following patch from Liviu Dudau:
> >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> domain_nr
> > Based on comments on this patch from Jason Gunthorpe, there is still the
> issue
> > that the domain numbers may change depending on the ordering at probe
> time.
> > However, this can be fixed later on by adding the entries in the DT files.
> 
> Hi Phil,
> 
> I'm happy that you found use for my patch, but I can't help wondering if it is
> not a better idea to convert your drivers to the whole new API in my series.
> Do you have any thoughts on that?
The main reason being that Renesas also supports the LTSI trees, so this fix
will also need backporting to v3.10. 

> One other thing to note: Rob Herring is not very happy with the mix of OF
> parsing and domain number allocation going in together, so I might remove
> the
> OF parsing entirely for now. Would that cause you any problems (other than
> the fact that if of_pci_get_domain_nr() doesn't do anything OF related I
> might
> rename it to pci_get_domain_nr()). You don't seem to have any alias defined
> for pci-domain in the DT, so I'll guess not.
In the covering letter I mentioned that controllers should always use the same
domain number, so I was planning to add pci-domain into the DTs later on.

Thanks
Phil

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Liviu Dudau]

On Mon, Sep 22, 2014 at 12:40:42PM +0100, Phil Edworthy wrote:
> Hi Liviu,
> 
> On 22 September 2014 12:29, Liviu wrote:
> > On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> > drivers,
> > > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > > controllers.
> > >
> > > However, they currently do not work at the same time as they use the
> > same PCI
> > > domain and use the same root bus number. We can't use different root
> > bus numbers
> > > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > > arch/arm/kernel/bios32.c.
> > >
> > > Since the two PCI controllers are completely independent, I think it makes
> > sense
> > > to use different PCI domains for them.
> > >
> > > I've marked the third patch as RFC as I am not sure of the impact of
> > enabling
> > > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> > all',
> > > I steered clear of mach specific changes.
> > >
> > > These patches require the following patch from Liviu Dudau:
> > >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> > domain_nr
> > > Based on comments on this patch from Jason Gunthorpe, there is still the
> > issue
> > > that the domain numbers may change depending on the ordering at probe
> > time.
> > > However, this can be fixed later on by adding the entries in the DT files.
> > 
> > Hi Phil,
> > 
> > I'm happy that you found use for my patch, but I can't help wondering if it is
> > not a better idea to convert your drivers to the whole new API in my series.
> > Do you have any thoughts on that?
> The main reason being that Renesas also supports the LTSI trees, so this fix
> will also need backporting to v3.10. 
> 
> > One other thing to note: Rob Herring is not very happy with the mix of OF
> > parsing and domain number allocation going in together, so I might remove
> > the
> > OF parsing entirely for now. Would that cause you any problems (other than
> > the fact that if of_pci_get_domain_nr() doesn't do anything OF related I
> > might
> > rename it to pci_get_domain_nr()). You don't seem to have any alias defined
> > for pci-domain in the DT, so I'll guess not.
> In the covering letter I mentioned that controllers should always use the same
> domain number, so I was planning to add pci-domain into the DTs later on.

Rob Herring seems to dislike the pci-domain alias solution, so it might have
to be a node in the host controller parent node, with linux,pci-domain.

Best regards,
Liviu

> 
> Thanks
> Phil


-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Bjorn Helgaas]

On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller drivers,
> one for an external PCIe slot, the other for an internal PCI bridge to USB
> controllers.
> 
> However, they currently do not work at the same time as they use the same PCI
> domain and use the same root bus number. We can't use different root bus numbers
> due to the way root bus numbers are assigned in pcibios_init_hw() in
> arch/arm/kernel/bios32.c.
> 
> Since the two PCI controllers are completely independent, I think it makes sense
> to use different PCI domains for them.
> 
> I've marked the third patch as RFC as I am not sure of the impact of enabling
> PCI domains for all ARM devices. In the march to 'one kernel to rule them all',
> I steered clear of mach specific changes.
> 
> These patches require the following patch from Liviu Dudau:
>   [PATCH v11 07/10] OF: Introduce helper function for getting PCI domain_nr
> Based on comments on this patch from Jason Gunthorpe, there is still the issue
> that the domain numbers may change depending on the ordering at probe time.
> However, this can be fixed later on by adding the entries in the DT files.
> 
> 
> Phil Edworthy (3):
>   PCI: rcar-pcie: Add call to get domain nr
>   PCI: rcar-internal-pci: Add call to get domain nr
>   ARM: Enable PCI domains

I'm deferring these for now because they depend on Liviu's work, which I
haven't merged yet, and I suspect some minor adaptation will be required
here.

For what it's worth, I agree with Rob's hesitation about mixing lookup with
domain number allocation in of_pci_get_domain_nr().  That seems
unnecessarily complicated.

>  arch/arm/Kconfig                 | 2 +-
>  drivers/pci/host/pci-rcar-gen2.c | 6 ++++++
>  drivers/pci/host/pcie-rcar.c     | 9 ++++++---
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> -- 
> 2.1.0
> 

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Phil Edworthy]

Hi Bjorn,

On 22 September 2014 22:01, Bjorn wrote:
> On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> drivers,
> > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > controllers.
> >
> > However, they currently do not work at the same time as they use the
> same PCI
> > domain and use the same root bus number. We can't use different root
> bus numbers
> > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > arch/arm/kernel/bios32.c.
> >
> > Since the two PCI controllers are completely independent, I think it makes
> sense
> > to use different PCI domains for them.
> >
> > I've marked the third patch as RFC as I am not sure of the impact of
> enabling
> > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> all',
> > I steered clear of mach specific changes.
> >
> > These patches require the following patch from Liviu Dudau:
> >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> domain_nr
> > Based on comments on this patch from Jason Gunthorpe, there is still the
> issue
> > that the domain numbers may change depending on the ordering at probe
> time.
> > However, this can be fixed later on by adding the entries in the DT files.
> >
> >
> > Phil Edworthy (3):
> >   PCI: rcar-pcie: Add call to get domain nr
> >   PCI: rcar-internal-pci: Add call to get domain nr
> >   ARM: Enable PCI domains
> 
> I'm deferring these for now because they depend on Liviu's work, which I
> haven't merged yet, and I suspect some minor adaptation will be required
> here.
> 
> For what it's worth, I agree with Rob's hesitation about mixing lookup with
> domain number allocation in of_pci_get_domain_nr().  That seems
> unnecessarily complicated.
I could create patches to add an optional "pci-domain" property for the R-Car
PCI drivers, and just attempt to get the property in the drivers. If not found,
the drivers will assume the domain is 0.

We would then have fixed PCI domain numbering and I don't have to worry about
Liviu's work.

Would that be ok?
 
Thanks
Phil

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Liviu Dudau]

On Tue, Sep 23, 2014 at 11:10:29AM +0100, Phil Edworthy wrote:
> Hi Bjorn,
> 
> On 22 September 2014 22:01, Bjorn wrote:
> > On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> > drivers,
> > > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > > controllers.
> > >
> > > However, they currently do not work at the same time as they use the
> > same PCI
> > > domain and use the same root bus number. We can't use different root
> > bus numbers
> > > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > > arch/arm/kernel/bios32.c.
> > >
> > > Since the two PCI controllers are completely independent, I think it makes
> > sense
> > > to use different PCI domains for them.
> > >
> > > I've marked the third patch as RFC as I am not sure of the impact of
> > enabling
> > > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> > all',
> > > I steered clear of mach specific changes.
> > >
> > > These patches require the following patch from Liviu Dudau:
> > >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> > domain_nr
> > > Based on comments on this patch from Jason Gunthorpe, there is still the
> > issue
> > > that the domain numbers may change depending on the ordering at probe
> > time.
> > > However, this can be fixed later on by adding the entries in the DT files.
> > >
> > >
> > > Phil Edworthy (3):
> > >   PCI: rcar-pcie: Add call to get domain nr
> > >   PCI: rcar-internal-pci: Add call to get domain nr
> > >   ARM: Enable PCI domains
> > 
> > I'm deferring these for now because they depend on Liviu's work, which I
> > haven't merged yet, and I suspect some minor adaptation will be required
> > here.
> > 
> > For what it's worth, I agree with Rob's hesitation about mixing lookup with
> > domain number allocation in of_pci_get_domain_nr().  That seems
> > unnecessarily complicated.
> I could create patches to add an optional "pci-domain" property for the R-Car
> PCI drivers, and just attempt to get the property in the drivers. If not found,
> the drivers will assume the domain is 0.
> 
> We would then have fixed PCI domain numbering and I don't have to worry about
> Liviu's work.

I will split the current of_pci_get_domain_nr() even further and replace it with
two functions: pci_get_domain_nr() which will do just the allocation (still based
on the boolean flag passed as parameter) and of_get_pci_domain_nr() that will
retrieve a "linux,pci-domain" value from a property belonging to a given device
node.

I plan to leave for the moment the check for mandatory presence of "linux,pci-domain"
to the host bridge driver so that everyone can implement their own policies.

How does that sound?

Best regards,
Liviu

> 
> Would that be ok?
>  
> Thanks
> Phil
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Phil Edworthy]

Hi Liviu,

On 23 September 2014 11:32, Liviu wrote:
> On Tue, Sep 23, 2014 at 11:10:29AM +0100, Phil Edworthy wrote:
> > Hi Bjorn,
> >
> > On 22 September 2014 22:01, Bjorn wrote:
> > > On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > > > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> > > drivers,
> > > > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > > > controllers.
> > > >
> > > > However, they currently do not work at the same time as they use the
> > > same PCI
> > > > domain and use the same root bus number. We can't use different root
> > > bus numbers
> > > > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > > > arch/arm/kernel/bios32.c.
> > > >
> > > > Since the two PCI controllers are completely independent, I think it makes
> > > sense
> > > > to use different PCI domains for them.
> > > >
> > > > I've marked the third patch as RFC as I am not sure of the impact of
> > > enabling
> > > > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> > > all',
> > > > I steered clear of mach specific changes.
> > > >
> > > > These patches require the following patch from Liviu Dudau:
> > > >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> > > domain_nr
> > > > Based on comments on this patch from Jason Gunthorpe, there is still the
> > > issue
> > > > that the domain numbers may change depending on the ordering at probe
> > > time.
> > > > However, this can be fixed later on by adding the entries in the DT files.
> > > >
> > > >
> > > > Phil Edworthy (3):
> > > >   PCI: rcar-pcie: Add call to get domain nr
> > > >   PCI: rcar-internal-pci: Add call to get domain nr
> > > >   ARM: Enable PCI domains
> > >
> > > I'm deferring these for now because they depend on Liviu's work, which I
> > > haven't merged yet, and I suspect some minor adaptation will be required
> > > here.
> > >
> > > For what it's worth, I agree with Rob's hesitation about mixing lookup with
> > > domain number allocation in of_pci_get_domain_nr().  That seems
> > > unnecessarily complicated.
> > I could create patches to add an optional "pci-domain" property for the R-Car
> > PCI drivers, and just attempt to get the property in the drivers. If not found,
> > the drivers will assume the domain is 0.
> >
> > We would then have fixed PCI domain numbering and I don't have to worry about
> > Liviu's work.
> 
> I will split the current of_pci_get_domain_nr() even further and replace it with
> two functions: pci_get_domain_nr() which will do just the allocation (still based
> on the boolean flag passed as parameter) and of_get_pci_domain_nr() that will
> retrieve a "linux,pci-domain" value from a property belonging to a given device
> node.
This doesn't solve the problem of different domain numbers based on different
probe ordering. If you have multiple domains then I think you must have the
"linux,pci-domain" property for each controller.

If we assume the above, of_get_pci_domain_nr() can just return a bus number of
0 if the "linux,pci-domain" property doesn't exist.

I suppose you could have a temporary solution that allocates the domain numbers
until the necessary drivers have been updated.

> I plan to leave for the moment the check for mandatory presence of
> "linux,pci-domain"
> to the host bridge driver so that everyone can implement their own policies.
> 
> How does that sound?

Thanks
Phil

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Liviu Dudau]

On Tue, Sep 23, 2014 at 12:00:41PM +0100, Phil Edworthy wrote:
> Hi Liviu,
> 
> On 23 September 2014 11:32, Liviu wrote:
> > On Tue, Sep 23, 2014 at 11:10:29AM +0100, Phil Edworthy wrote:
> > > Hi Bjorn,
> > >
> > > On 22 September 2014 22:01, Bjorn wrote:
> > > > On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > > > > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> > > > drivers,
> > > > > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > > > > controllers.
> > > > >
> > > > > However, they currently do not work at the same time as they use the
> > > > same PCI
> > > > > domain and use the same root bus number. We can't use different root
> > > > bus numbers
> > > > > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > > > > arch/arm/kernel/bios32.c.
> > > > >
> > > > > Since the two PCI controllers are completely independent, I think it makes
> > > > sense
> > > > > to use different PCI domains for them.
> > > > >
> > > > > I've marked the third patch as RFC as I am not sure of the impact of
> > > > enabling
> > > > > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> > > > all',
> > > > > I steered clear of mach specific changes.
> > > > >
> > > > > These patches require the following patch from Liviu Dudau:
> > > > >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> > > > domain_nr
> > > > > Based on comments on this patch from Jason Gunthorpe, there is still the
> > > > issue
> > > > > that the domain numbers may change depending on the ordering at probe
> > > > time.
> > > > > However, this can be fixed later on by adding the entries in the DT files.
> > > > >
> > > > >
> > > > > Phil Edworthy (3):
> > > > >   PCI: rcar-pcie: Add call to get domain nr
> > > > >   PCI: rcar-internal-pci: Add call to get domain nr
> > > > >   ARM: Enable PCI domains
> > > >
> > > > I'm deferring these for now because they depend on Liviu's work, which I
> > > > haven't merged yet, and I suspect some minor adaptation will be required
> > > > here.
> > > >
> > > > For what it's worth, I agree with Rob's hesitation about mixing lookup with
> > > > domain number allocation in of_pci_get_domain_nr().  That seems
> > > > unnecessarily complicated.
> > > I could create patches to add an optional "pci-domain" property for the R-Car
> > > PCI drivers, and just attempt to get the property in the drivers. If not found,
> > > the drivers will assume the domain is 0.
> > >
> > > We would then have fixed PCI domain numbering and I don't have to worry about
> > > Liviu's work.
> > 
> > I will split the current of_pci_get_domain_nr() even further and replace it with
> > two functions: pci_get_domain_nr() which will do just the allocation (still based
> > on the boolean flag passed as parameter) and of_get_pci_domain_nr() that will
> > retrieve a "linux,pci-domain" value from a property belonging to a given device
> > node.
> This doesn't solve the problem of different domain numbers based on different
> probe ordering. If you have multiple domains then I think you must have the
> "linux,pci-domain" property for each controller.

Correct, but I've said I'm going to leave for the moment the check to the host controller(s).
So if you care about ordering or mandating the presence of "linux,pci-domain" in the
DT, please add the check in your driver.

> 
> If we assume the above, of_get_pci_domain_nr() can just return a bus number of
> 0 if the "linux,pci-domain" property doesn't exist.

You mean *domain* number of 0. But that is still a valid value and you won't be able
to distinguish between DT providing a "linux,pci-domain" value of zero and the
property not being present.

> 
> I suppose you could have a temporary solution that allocates the domain numbers
> until the necessary drivers have been updated.

And that is what pci_get_domain_nr() will do. If you don't care about ordering, one
will use that one. If you care about ordering then "linux,pci-domain" is required
and one can use of_get_pci_domain_nr() to retrieve it and fail if the information
is missing.

Does that look like a workable solution?

Best regards,
Liviu

> 
> > I plan to leave for the moment the check for mandatory presence of
> > "linux,pci-domain"
> > to the host bridge driver so that everyone can implement their own policies.
> > 
> > How does that sound?
> 
> Thanks
> Phil

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Phil Edworthy]

Hi Liviu,

On 23 September 2014 12:10, Liviu wrote:
> On Tue, Sep 23, 2014 at 12:00:41PM +0100, Phil Edworthy wrote:
> > Hi Liviu,
> >
> > On 23 September 2014 11:32, Liviu wrote:
> > > On Tue, Sep 23, 2014 at 11:10:29AM +0100, Phil Edworthy wrote:
> > > > Hi Bjorn,
> > > >
> > > > On 22 September 2014 22:01, Bjorn wrote:
> > > > > On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > > > > > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> > > > > drivers,
> > > > > > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > > > > > controllers.
> > > > > >
> > > > > > However, they currently do not work at the same time as they use the
> > > > > same PCI
> > > > > > domain and use the same root bus number. We can't use different root
> > > > > bus numbers
> > > > > > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > > > > > arch/arm/kernel/bios32.c.
> > > > > >
> > > > > > Since the two PCI controllers are completely independent, I think it makes
> > > > > sense
> > > > > > to use different PCI domains for them.
> > > > > >
> > > > > > I've marked the third patch as RFC as I am not sure of the impact of
> > > > > enabling
> > > > > > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> > > > > all',
> > > > > > I steered clear of mach specific changes.
> > > > > >
> > > > > > These patches require the following patch from Liviu Dudau:
> > > > > >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> > > > > domain_nr
> > > > > > Based on comments on this patch from Jason Gunthorpe, there is still the
> > > > > issue
> > > > > > that the domain numbers may change depending on the ordering at probe
> > > > > time.
> > > > > > However, this can be fixed later on by adding the entries in the DT files.
> > > > > >
> > > > > >
> > > > > > Phil Edworthy (3):
> > > > > >   PCI: rcar-pcie: Add call to get domain nr
> > > > > >   PCI: rcar-internal-pci: Add call to get domain nr
> > > > > >   ARM: Enable PCI domains
> > > > >
> > > > > I'm deferring these for now because they depend on Liviu's work, which I
> > > > > haven't merged yet, and I suspect some minor adaptation will be required
> > > > > here.
> > > > >
> > > > > For what it's worth, I agree with Rob's hesitation about mixing lookup with
> > > > > domain number allocation in of_pci_get_domain_nr().  That seems
> > > > > unnecessarily complicated.
> > > > I could create patches to add an optional "pci-domain" property for the R-Car
> > > > PCI drivers, and just attempt to get the property in the drivers. If not found,
> > > > the drivers will assume the domain is 0.
> > > >
> > > > We would then have fixed PCI domain numbering and I don't have to worry about
> > > > Liviu's work.
> > >
> > > I will split the current of_pci_get_domain_nr() even further and replace it with
> > > two functions: pci_get_domain_nr() which will do just the allocation (still based
> > > on the boolean flag passed as parameter) and of_get_pci_domain_nr() that will
> > > retrieve a "linux,pci-domain" value from a property belonging to a given device
> > > node.
> > This doesn't solve the problem of different domain numbers based on different
> > probe ordering. If you have multiple domains then I think you must have the
> > "linux,pci-domain" property for each controller.
> 
> Correct, but I've said I'm going to leave for the moment the check to the host
> controller(s).
> So if you care about ordering or mandating the presence of "linux,pci-
> domain" in the DT, please add the check in your driver.
>
> >
> > If we assume the above, of_get_pci_domain_nr() can just return a bus number of
> > 0 if the "linux,pci-domain" property doesn't exist.
> 
> You mean *domain* number of 0. But that is still a valid value and you won't
> be able to distinguish between DT providing a "linux,pci-domain" value of zero and
> the property not being present.
Yes, I meant domain.
If a device only has a single PCI domain, no "linux,pci-domain" prop is needed
and a default domain number of 0 is ok.
If there is more than one domain, userspace needs to have consistent domain
numbering over kernel versions, so we have to specify the domain in the DT.

I am probably missing something, it wouldn?t be the first time, but I don?t see
why of_get_pci_domain_nr() needs to allocate domain numbers.
 
> >
> > I suppose you could have a temporary solution that allocates the domain numbers
> > until the necessary drivers have been updated.
> 
> And that is what pci_get_domain_nr() will do. If you don't care about ordering, one
> will use that one. If you care about ordering then "linux,pci-domain" is required
> and one can use of_get_pci_domain_nr() to retrieve it and fail if the information
> is missing.
> 
> Does that look like a workable solution?
> 
> Best regards,
> Liviu
> 
> >
> > > I plan to leave for the moment the check for mandatory presence of
> > > "linux,pci-domain"
> > > to the host bridge driver so that everyone can implement their own
> policies.
> > >
> > > How does that sound?
> >

Thanks
Phil

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Liviu Dudau]

On Tue, Sep 23, 2014 at 12:38:28PM +0100, Phil Edworthy wrote:
> Hi Liviu,
> 
> On 23 September 2014 12:10, Liviu wrote:
> > On Tue, Sep 23, 2014 at 12:00:41PM +0100, Phil Edworthy wrote:
> > > Hi Liviu,
> > >
> > > On 23 September 2014 11:32, Liviu wrote:
> > > > On Tue, Sep 23, 2014 at 11:10:29AM +0100, Phil Edworthy wrote:
> > > > > Hi Bjorn,
> > > > >
> > > > > On 22 September 2014 22:01, Bjorn wrote:
> > > > > > On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > > > > > > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> > > > > > drivers,
> > > > > > > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > > > > > > controllers.
> > > > > > >
> > > > > > > However, they currently do not work at the same time as they use the
> > > > > > same PCI
> > > > > > > domain and use the same root bus number. We can't use different root
> > > > > > bus numbers
> > > > > > > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > > > > > > arch/arm/kernel/bios32.c.
> > > > > > >
> > > > > > > Since the two PCI controllers are completely independent, I think it makes
> > > > > > sense
> > > > > > > to use different PCI domains for them.
> > > > > > >
> > > > > > > I've marked the third patch as RFC as I am not sure of the impact of
> > > > > > enabling
> > > > > > > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> > > > > > all',
> > > > > > > I steered clear of mach specific changes.
> > > > > > >
> > > > > > > These patches require the following patch from Liviu Dudau:
> > > > > > >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> > > > > > domain_nr
> > > > > > > Based on comments on this patch from Jason Gunthorpe, there is still the
> > > > > > issue
> > > > > > > that the domain numbers may change depending on the ordering at probe
> > > > > > time.
> > > > > > > However, this can be fixed later on by adding the entries in the DT files.
> > > > > > >
> > > > > > >
> > > > > > > Phil Edworthy (3):
> > > > > > >   PCI: rcar-pcie: Add call to get domain nr
> > > > > > >   PCI: rcar-internal-pci: Add call to get domain nr
> > > > > > >   ARM: Enable PCI domains
> > > > > >
> > > > > > I'm deferring these for now because they depend on Liviu's work, which I
> > > > > > haven't merged yet, and I suspect some minor adaptation will be required
> > > > > > here.
> > > > > >
> > > > > > For what it's worth, I agree with Rob's hesitation about mixing lookup with
> > > > > > domain number allocation in of_pci_get_domain_nr().  That seems
> > > > > > unnecessarily complicated.
> > > > > I could create patches to add an optional "pci-domain" property for the R-Car
> > > > > PCI drivers, and just attempt to get the property in the drivers. If not found,
> > > > > the drivers will assume the domain is 0.
> > > > >
> > > > > We would then have fixed PCI domain numbering and I don't have to worry about
> > > > > Liviu's work.
> > > >
> > > > I will split the current of_pci_get_domain_nr() even further and replace it with
> > > > two functions: pci_get_domain_nr() which will do just the allocation (still based
> > > > on the boolean flag passed as parameter) and of_get_pci_domain_nr() that will
> > > > retrieve a "linux,pci-domain" value from a property belonging to a given device
> > > > node.
> > > This doesn't solve the problem of different domain numbers based on different
> > > probe ordering. If you have multiple domains then I think you must have the
> > > "linux,pci-domain" property for each controller.
> > 
> > Correct, but I've said I'm going to leave for the moment the check to the host
> > controller(s).
> > So if you care about ordering or mandating the presence of "linux,pci-
> > domain" in the DT, please add the check in your driver.
> >
> > >
> > > If we assume the above, of_get_pci_domain_nr() can just return a bus number of
> > > 0 if the "linux,pci-domain" property doesn't exist.
> > 
> > You mean *domain* number of 0. But that is still a valid value and you won't
> > be able to distinguish between DT providing a "linux,pci-domain" value of zero and
> > the property not being present.
> Yes, I meant domain.
> If a device only has a single PCI domain, no "linux,pci-domain" prop is needed
> and a default domain number of 0 is ok.

Agree, except it is not a "device" choice but a platform choice. In other words,
having a host bridge capable of running on multi-host-bridge setups, then
your HB driver will have to mandate "linux,pci-domain" presence. But the decision
to have more than one HB will be present in the device tree, not in the driver.

> If there is more than one domain, userspace needs to have consistent domain
> numbering over kernel versions, so we have to specify the domain in the DT.

Agree, and the DT will pin down the domain numbers.

> 
> I am probably missing something, it wouldn't be the first time, but I don't see
> why of_get_pci_domain_nr() needs to allocate domain numbers.

Maybe because I have confused you. Let's try again:
  - of_pci_get_domain_nr(): function present in v11, parses "pci-domain" alias,
    allocates a new domain number if "pci-domain" info is missing. Will be
    removed in v12.
  - of_get_pci_domain_nr(): subtle change of name to flag the change of behaviour
    from the previous function. Only parses "linux,pci-domain" property of the
    host bridge node, returns the property value as an integer between 0 and 255,
    or a negative value if the property is missing.
  - pci_get_new_domain_nr(): this function is only present in v12 and allocates
    a new domain number each time it is called.

Hope this helps,

Liviu

>  
> > >
> > > I suppose you could have a temporary solution that allocates the domain numbers
> > > until the necessary drivers have been updated.
> > 
> > And that is what pci_get_domain_nr() will do. If you don't care about ordering, one
> > will use that one. If you care about ordering then "linux,pci-domain" is required
> > and one can use of_get_pci_domain_nr() to retrieve it and fail if the information
> > is missing.
> > 
> > Does that look like a workable solution?
> > 
> > Best regards,
> > Liviu
> > 
> > >
> > > > I plan to leave for the moment the check for mandatory presence of
> > > > "linux,pci-domain"
> > > > to the host bridge driver so that everyone can implement their own
> > policies.
> > > >
> > > > How does that sound?
> > >
> 
> Thanks
> Phil

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

[PATCH 0/3] Add PCI domain support to R-Car drivers

[Phil Edworthy]

Hi Liviu,

On 23 September 2014 13:11, Liviu wrote:
> On Tue, Sep 23, 2014 at 12:38:28PM +0100, Phil Edworthy wrote:
> > On 23 September 2014 12:10, Liviu wrote:
> > > On Tue, Sep 23, 2014 at 12:00:41PM +0100, Phil Edworthy wrote:
> > > > On 23 September 2014 11:32, Liviu wrote:
> > > > > On Tue, Sep 23, 2014 at 11:10:29AM +0100, Phil Edworthy wrote:
> > > > > > Hi Bjorn,
> > > > > >
> > > > > > On 22 September 2014 22:01, Bjorn wrote:
> > > > > > > On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > > > > > > > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> > > > > > > drivers,
> > > > > > > > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > > > > > > > controllers.
> > > > > > > >
> > > > > > > > However, they currently do not work at the same time as they use the
> > > > > > > same PCI
> > > > > > > > domain and use the same root bus number. We can't use different root
> > > > > > > bus numbers
> > > > > > > > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > > > > > > > arch/arm/kernel/bios32.c.
> > > > > > > >
> > > > > > > > Since the two PCI controllers are completely independent, I think it makes
> > > > > > > sense
> > > > > > > > to use different PCI domains for them.
> > > > > > > >
> > > > > > > > I've marked the third patch as RFC as I am not sure of the impact of
> > > > > > > enabling
> > > > > > > > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> > > > > > > all',
> > > > > > > > I steered clear of mach specific changes.
> > > > > > > >
> > > > > > > > These patches require the following patch from Liviu Dudau:
> > > > > > > >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> > > > > > > domain_nr
> > > > > > > > Based on comments on this patch from Jason Gunthorpe, there is still the
> > > > > > > issue
> > > > > > > > that the domain numbers may change depending on the ordering at probe
> > > > > > > time.
> > > > > > > > However, this can be fixed later on by adding the entries in the DT files.
> > > > > > > >
> > > > > > > >
> > > > > > > > Phil Edworthy (3):
> > > > > > > >   PCI: rcar-pcie: Add call to get domain nr
> > > > > > > >   PCI: rcar-internal-pci: Add call to get domain nr
> > > > > > > >   ARM: Enable PCI domains
> > > > > > >
> > > > > > > I'm deferring these for now because they depend on Liviu's work, which I
> > > > > > > haven't merged yet, and I suspect some minor adaptation will be required
> > > > > > > here.
> > > > > > >
> > > > > > > For what it's worth, I agree with Rob's hesitation about mixing lookup with
> > > > > > > domain number allocation in of_pci_get_domain_nr().  That seems
> > > > > > > unnecessarily complicated.
> > > > > > I could create patches to add an optional "pci-domain" property for the R-Car
> > > > > > PCI drivers, and just attempt to get the property in the drivers. If not found,
> > > > > > the drivers will assume the domain is 0.
> > > > > >
> > > > > > We would then have fixed PCI domain numbering and I don't have to worry about
> > > > > > Liviu's work.
> > > > >
> > > > > I will split the current of_pci_get_domain_nr() even further and replace it with
> > > > > two functions: pci_get_domain_nr() which will do just the allocation (still based
> > > > > on the boolean flag passed as parameter) and of_get_pci_domain_nr() that will
> > > > > retrieve a "linux,pci-domain" value from a property belonging to a given device
> > > > > node.
> > > > This doesn't solve the problem of different domain numbers based on different
> > > > probe ordering. If you have multiple domains then I think you must have the
> > > > "linux,pci-domain" property for each controller.
> > >
> > > Correct, but I've said I'm going to leave for the moment the check to the host
> > > controller(s).
> > > So if you care about ordering or mandating the presence of "linux,pci-
> > > domain" in the DT, please add the check in your driver.
> > >
> > > >
> > > > If we assume the above, of_get_pci_domain_nr() can just return a bus number of
> > > > 0 if the "linux,pci-domain" property doesn't exist.
> > >
> > > You mean *domain* number of 0. But that is still a valid value and you won't
> > > be able to distinguish between DT providing a "linux,pci-domain" value of zero and
> > > the property not being present.
> > Yes, I meant domain.
> > If a device only has a single PCI domain, no "linux,pci-domain" prop is needed
> > and a default domain number of 0 is ok.
> 
> Agree, except it is not a "device" choice but a platform choice. In other words,
> having a host bridge capable of running on multi-host-bridge setups, then
> your HB driver will have to mandate "linux,pci-domain" presence. But the
> decision
> to have more than one HB will be present in the device tree, not in the
> driver.
Ok, I actually meant the device dtsi, but yes the decision lies with the DT.


> > If there is more than one domain, userspace needs to have consistent domain
> > numbering over kernel versions, so we have to specify the domain in the DT.
> 
> Agree, and the DT will pin down the domain numbers.
> 
> >
> > I am probably missing something, it wouldn't be the first time, but I don't see
> > why of_get_pci_domain_nr() needs to allocate domain numbers.
> 
> Maybe because I have confused you. Let's try again:
>   - of_pci_get_domain_nr(): function present in v11, parses "pci-domain" alias,
>     allocates a new domain number if "pci-domain" info is missing. Will be
>     removed in v12.
>   - of_get_pci_domain_nr(): subtle change of name to flag the change of behaviour
>     from the previous function. Only parses "linux,pci-domain" property of the
>     host bridge node, returns the property value as an integer between 0 and 255,
>     or a negative value if the property is missing.
>   - pci_get_new_domain_nr(): this function is only present in v12 and allocates
>     a new domain number each time it is called.
Ok, I missed the subtle name change and extra function for v12.

Sounds good to me!

Thanks
Phil