[PATCH V2 0/2] PCI: generic: Assiging msi-controller to PCI hostbridge

[PATCH V2 0/2] PCI: generic: Assiging msi-controller to PCI hostbridge

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch set introduces a new callback function to allow PCI host drivers
to specify MSI controller to be used for the child buses / devices.

This is reabased from:
    git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/host-generic

Changes from V1:
    - Adding "msi-parent" device tree binding in documentation.
    - Rebase the patch to get rid of artifacts from other precursor patch
      accidently applied to the developement tree. This should now apply cleanly
      to the pci/host-generic branch.

Suravee Suthikulpanit (2):
  PCI: Add new pci_ops for setting MSI parent for PCI bus
  PCI: generic: Add set_msi_parent callback

 Documentation/devicetree/bindings/pci/host-generic-pci.txt |  3 +++
 drivers/pci/host/pci-host-generic.c                        | 13 +++++++++++++
 drivers/pci/probe.c                                        |  3 +++
 include/linux/pci.h                                        |  1 +
 4 files changed, 20 insertions(+)

-- 
1.9.3

[PATCH V2 1/2] PCI: Add new pci_ops for setting MSI parent for PCI bus

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

In the pci_scan_root_bus, pci_bus is created and passed down to:
    pci_scan_child_bus
        pci_scan_bridge
    	pci_add_new_bus
    	    pci_alloc_child_bus

In pci_alloc_child_bus, the parent's msi_chip is propagated to child,
and the referenced by PCI devices when calling arch_setup_msi_irq().

However, in the current implementation of pci_scan_root_bus, the msi_chip
of the root_bus is not set before handing off to pci_scan_child_bus.

This patch proposes a new callback function in the struct pci_ops
to allow host controller to provide a call back for specifying
msi_chip to be used.

Cc: Bjorn Helgass <bhelgaas@google.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/pci/probe.c | 3 +++
 include/linux/pci.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5ed9930..cf7114d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2081,6 +2081,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 	if (!b)
 		return NULL;
 
+	if (ops->set_msi_parent)
+		ops->set_msi_parent(b);
+
 	if (!found) {
 		dev_info(&b->dev,
 		 "No busn resource found for root bus, will use [bus %02x-ff]\n",
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5be8db4..6093544 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -560,6 +560,7 @@ static inline int pcibios_err_to_errno(int err)
 struct pci_ops {
 	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
 	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
+	int (*set_msi_parent)(struct pci_bus *bus);
 };
 
 /*
-- 
1.9.3

[PATCH V2 2/2] PCI: generic: Add msi_parent DT binding

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch introduces a new DT binding, msi-parent, which can
be used to specify MSI-parent phandle for a particular PCI
generic host controller.

Also, it implements and registers set_msi_parent callback.

Cc: Bjorn Helgass <bhelgaas@google.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 Documentation/devicetree/bindings/pci/host-generic-pci.txt |  3 +++
 drivers/pci/host/pci-host-generic.c                        | 13 +++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
index cf3e205..6996af7 100644
--- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt
+++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
@@ -36,6 +36,8 @@ Properties of the host controller node:
 - reg            : The Configuration Space base address and size, as accessed
                    from the parent bus.
 
+- msi-parent     : Specify the phandle of the corresponded MSI controller
+                   for this PCI host controller.
 
 Properties of the /chosen node:
 
@@ -77,6 +79,7 @@ pci {
     device_type = "pci";
     #address-cells = <3>;
     #size-cells = <2>;
+    msi-parent = <&msictrl0>;
     bus-range = <0x0 0x1>;
 
     // CPU_PHYSICAL(2)  SIZE(2)
diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 1895907..c4fbcda 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -42,6 +42,7 @@ struct gen_pci {
 	struct pci_host_bridge			host;
 	struct gen_pci_cfg_windows		cfg;
 	struct list_head			resources;
+	struct msi_chip				*mchip;
 };
 
 static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
@@ -122,9 +123,19 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
+static int gen_pci_set_msi_parent(struct pci_bus *bus)
+{
+	struct gen_pci *pci = bus_to_gen_pci(bus);
+
+	bus->msi = pci->mchip;
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
 static struct pci_ops gen_pci_ops = {
 	.read	= gen_pci_config_read,
 	.write	= gen_pci_config_write,
+	.set_msi_parent = gen_pci_set_msi_parent,
 };
 
 static const struct of_device_id gen_pci_of_match[] = {
@@ -303,6 +314,8 @@ static int gen_pci_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	pci->mchip = of_pci_find_msi_chip_by_node(of_parse_phandle(np,
+						  "msi-parent", 0));
 	pci_common_init_dev(dev, &hw);
 	return 0;
 }
-- 
1.9.3

Re: [PATCH V2 2/2] PCI: generic: Add msi_parent DT binding

[Srikanth Thokala]

Hi,

On Wed, Nov 12, 2014 at 12:47 AM,  <suravee.suthikulpanit@amd.com> wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>
> This patch introduces a new DT binding, msi-parent, which can
> be used to specify MSI-parent phandle for a particular PCI
> generic host controller.
>
> Also, it implements and registers set_msi_parent callback.
>
> Cc: Bjorn Helgass <bhelgaas@google.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
>  Documentation/devicetree/bindings/pci/host-generic-pci.txt |  3 +++
>  drivers/pci/host/pci-host-generic.c                        | 13 +++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
> index cf3e205..6996af7 100644
> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
> @@ -36,6 +36,8 @@ Properties of the host controller node:
>  - reg            : The Configuration Space base address and size, as accessed
>                     from the parent bus.
>
> +- msi-parent     : Specify the phandle of the corresponded MSI controller
> +                   for this PCI host controller.
>
>  Properties of the /chosen node:
>
> @@ -77,6 +79,7 @@ pci {
>      device_type = "pci";
>      #address-cells = <3>;
>      #size-cells = <2>;
> +    msi-parent = <&msictrl0>;
>      bus-range = <0x0 0x1>;
>
>      // CPU_PHYSICAL(2)  SIZE(2)
> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 1895907..c4fbcda 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -42,6 +42,7 @@ struct gen_pci {
>         struct pci_host_bridge                  host;
>         struct gen_pci_cfg_windows              cfg;
>         struct list_head                        resources;
> +       struct msi_chip                         *mchip;
>  };
>
>  static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
> @@ -122,9 +123,19 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
>         return PCIBIOS_SUCCESSFUL;
>  }
>
> +static int gen_pci_set_msi_parent(struct pci_bus *bus)
> +{
> +       struct gen_pci *pci = bus_to_gen_pci(bus);
> +
> +       bus->msi = pci->mchip;
> +
> +       return PCIBIOS_SUCCESSFUL;
> +}
> +
>  static struct pci_ops gen_pci_ops = {
>         .read   = gen_pci_config_read,
>         .write  = gen_pci_config_write,
> +       .set_msi_parent = gen_pci_set_msi_parent,

We assign the msi chip through add_bus() call, do we still need this?

Srikanth

>  };
>
>  static const struct of_device_id gen_pci_of_match[] = {
> @@ -303,6 +314,8 @@ static int gen_pci_probe(struct platform_device *pdev)
>                 return err;
>         }
>
> +       pci->mchip = of_pci_find_msi_chip_by_node(of_parse_phandle(np,
> +                                                 "msi-parent", 0));
>         pci_common_init_dev(dev, &hw);
>         return 0;
>  }
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] PCI: generic: Add msi_parent DT binding

[Srikanth Thokala]

Hi,

On Wed, Nov 12, 2014 at 12:26 PM, Srikanth Thokala
<sriku.linux@gmail.com> wrote:
> Hi,
>
> On Wed, Nov 12, 2014 at 12:47 AM,  <suravee.suthikulpanit@amd.com> wrote:
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>
>> This patch introduces a new DT binding, msi-parent, which can
>> be used to specify MSI-parent phandle for a particular PCI
>> generic host controller.
>>
>> Also, it implements and registers set_msi_parent callback.
>>
>> Cc: Bjorn Helgass <bhelgaas@google.com>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> ---
>>  Documentation/devicetree/bindings/pci/host-generic-pci.txt |  3 +++
>>  drivers/pci/host/pci-host-generic.c                        | 13 +++++++++++++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
>> index cf3e205..6996af7 100644
>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt
>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt
>> @@ -36,6 +36,8 @@ Properties of the host controller node:
>>  - reg            : The Configuration Space base address and size, as accessed
>>                     from the parent bus.
>>
>> +- msi-parent     : Specify the phandle of the corresponded MSI controller
>> +                   for this PCI host controller.
>>
>>  Properties of the /chosen node:
>>
>> @@ -77,6 +79,7 @@ pci {
>>      device_type = "pci";
>>      #address-cells = <3>;
>>      #size-cells = <2>;
>> +    msi-parent = <&msictrl0>;
>>      bus-range = <0x0 0x1>;
>>
>>      // CPU_PHYSICAL(2)  SIZE(2)
>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
>> index 1895907..c4fbcda 100644
>> --- a/drivers/pci/host/pci-host-generic.c
>> +++ b/drivers/pci/host/pci-host-generic.c
>> @@ -42,6 +42,7 @@ struct gen_pci {
>>         struct pci_host_bridge                  host;
>>         struct gen_pci_cfg_windows              cfg;
>>         struct list_head                        resources;
>> +       struct msi_chip                         *mchip;
>>  };
>>
>>  static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
>> @@ -122,9 +123,19 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
>>         return PCIBIOS_SUCCESSFUL;
>>  }
>>
>> +static int gen_pci_set_msi_parent(struct pci_bus *bus)
>> +{
>> +       struct gen_pci *pci = bus_to_gen_pci(bus);
>> +
>> +       bus->msi = pci->mchip;
>> +
>> +       return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>>  static struct pci_ops gen_pci_ops = {
>>         .read   = gen_pci_config_read,
>>         .write  = gen_pci_config_write,
>> +       .set_msi_parent = gen_pci_set_msi_parent,
>
> We assign the msi chip through add_bus() call, do we still need this?
>

There are series of patches from Yijing Wang around this.  You may have
to go through them.

Srikanth

> Srikanth
>
>>  };
>>
>>  static const struct of_device_id gen_pci_of_match[] = {
>> @@ -303,6 +314,8 @@ static int gen_pci_probe(struct platform_device *pdev)
>>                 return err;
>>         }
>>
>> +       pci->mchip = of_pci_find_msi_chip_by_node(of_parse_phandle(np,
>> +                                                 "msi-parent", 0));
>>         pci_common_init_dev(dev, &hw);
>>         return 0;
>>  }
>> --
>> 1.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] PCI: generic: Add msi_parent DT binding

[Suravee Suthikulpanit]

On 11/12/14 14:09, Srikanth Thokala wrote:
>>> @@ -122,9 +123,19 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
>>> >>         return PCIBIOS_SUCCESSFUL;
>>> >>  }
>>> >>
>>> >>+static int gen_pci_set_msi_parent(struct pci_bus *bus)
>>> >>+{
>>> >>+       struct gen_pci *pci = bus_to_gen_pci(bus);
>>> >>+
>>> >>+       bus->msi = pci->mchip;
>>> >>+
>>> >>+       return PCIBIOS_SUCCESSFUL;
>>> >>+}
>>> >>+
>>> >>  static struct pci_ops gen_pci_ops = {
>>> >>         .read   = gen_pci_config_read,
>>> >>         .write  = gen_pci_config_write,
>>> >>+       .set_msi_parent = gen_pci_set_msi_parent,
>> >
>> >We assign the msi chip through add_bus() call, do we still need this?
>> >
> There are series of patches from Yijing Wang around this.  You may have
> to go through them.
>
> Srikanth
>

Hm.. I was not aware of this patch. Thanks for the pointer. I'll check 
them out.

Suravee

Re: [PATCH V2 2/2] PCI: generic: Add msi_parent DT binding

[Suravee Suthikulpanit]

On 11/12/14 13:56, Srikanth Thokala wrote:
>> @@ -122,9 +123,19 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
>> >         return PCIBIOS_SUCCESSFUL;
>> >  }
>> >
>> >+static int gen_pci_set_msi_parent(struct pci_bus *bus)
>> >+{
>> >+       struct gen_pci *pci = bus_to_gen_pci(bus);
>> >+
>> >+       bus->msi = pci->mchip;
>> >+
>> >+       return PCIBIOS_SUCCESSFUL;
>> >+}
>> >+
>> >  static struct pci_ops gen_pci_ops = {
>> >         .read   = gen_pci_config_read,
>> >         .write  = gen_pci_config_write,
>> >+       .set_msi_parent = gen_pci_set_msi_parent,
> We assign the msi chip through add_bus() call, do we still need this?

Certain architecture (i.e arm64) doesn't implement struct hw_pci. 
Therefore, there is no add_bus().  However, this new API should allow 
non-arch dependent implementation.

Suravee

Re: [PATCH V2 0/2] PCI: generic: Assiging msi-controller to PCI hostbridge

[Bjorn Helgaas]

[+cc Yijing]

On Tue, Nov 11, 2014 at 01:17:32PM -0600, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch set introduces a new callback function to allow PCI host drivers
> to specify MSI controller to be used for the child buses / devices.

Hi Suravee, 

As Srikanth mentioned, there's a series of MSI-related stuff from Yijing.
I recently put them on my pci/msi branch, and they overlap a bit with
what you're doing here.

So I'll drop these for now.  Maybe there's a way you can accomplish what
you need by implementing pcibios_msi_controller(), which Yijing added?

Also note that "msi-controller" is already being used in device tree (see
Documentation/devicetree/bindings/interrupt-controller/marvell), so maybe
you can do the name.

Bjorn

> This is reabased from:
>     git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/host-generic
> 
> Changes from V1:
>     - Adding "msi-parent" device tree binding in documentation.
>     - Rebase the patch to get rid of artifacts from other precursor patch
>       accidently applied to the developement tree. This should now apply cleanly
>       to the pci/host-generic branch.
> 
> Suravee Suthikulpanit (2):
>   PCI: Add new pci_ops for setting MSI parent for PCI bus
>   PCI: generic: Add set_msi_parent callback
> 
>  Documentation/devicetree/bindings/pci/host-generic-pci.txt |  3 +++
>  drivers/pci/host/pci-host-generic.c                        | 13 +++++++++++++
>  drivers/pci/probe.c                                        |  3 +++
>  include/linux/pci.h                                        |  1 +
>  4 files changed, 20 insertions(+)
> 
> -- 
> 1.9.3
> 

Re: [PATCH V2 0/2] PCI: generic: Assiging msi-controller to PCI hostbridge

[Yijing Wang]

On 2014/11/14 2:09, Bjorn Helgaas wrote:
> [+cc Yijing]

Hi, I'm doing the same thing to associate MSI_controller and PCI host bridge(not PCI bus),
In my working series, I go one step further, rip out pci host bridge of pci root bus creation.
And we could put the common things in the generic pci host bridge. I will post it out these
days. :)

Thanks!
Yijing.

> 
> On Tue, Nov 11, 2014 at 01:17:32PM -0600, suravee.suthikulpanit@amd.com wrote:
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>
>> This patch set introduces a new callback function to allow PCI host drivers
>> to specify MSI controller to be used for the child buses / devices.
> 
> Hi Suravee, 
> 
> As Srikanth mentioned, there's a series of MSI-related stuff from Yijing.
> I recently put them on my pci/msi branch, and they overlap a bit with
> what you're doing here.
> 
> So I'll drop these for now.  Maybe there's a way you can accomplish what
> you need by implementing pcibios_msi_controller(), which Yijing added?
> 
> Also note that "msi-controller" is already being used in device tree (see
> Documentation/devicetree/bindings/interrupt-controller/marvell), so maybe
> you can do the name.
> 
> Bjorn
> 
>> This is reabased from:
>>     git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/host-generic
>>
>> Changes from V1:
>>     - Adding "msi-parent" device tree binding in documentation.
>>     - Rebase the patch to get rid of artifacts from other precursor patch
>>       accidently applied to the developement tree. This should now apply cleanly
>>       to the pci/host-generic branch.
>>
>> Suravee Suthikulpanit (2):
>>   PCI: Add new pci_ops for setting MSI parent for PCI bus
>>   PCI: generic: Add set_msi_parent callback
>>
>>  Documentation/devicetree/bindings/pci/host-generic-pci.txt |  3 +++
>>  drivers/pci/host/pci-host-generic.c                        | 13 +++++++++++++
>>  drivers/pci/probe.c                                        |  3 +++
>>  include/linux/pci.h                                        |  1 +
>>  4 files changed, 20 insertions(+)
>>
>> -- 
>> 1.9.3
>>
> 
> .
> 


-- 
Thanks!
Yijing