2014-11-11 19:17:51

by Suthikulpanit, Suravee

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

From: Suravee Suthikulpanit <[email protected]>

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


2014-11-11 19:18:06

by Suthikulpanit, Suravee

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

From: Suravee Suthikulpanit <[email protected]>

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 <[email protected]>
Cc: Liviu Dudau <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
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

2014-11-11 19:18:32

by Suthikulpanit, Suravee

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

From: Suravee Suthikulpanit <[email protected]>

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 <[email protected]>
Cc: Liviu Dudau <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
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

2014-11-12 06:56:52

by Srikanth Thokala

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

Hi,

On Wed, Nov 12, 2014 at 12:47 AM, <[email protected]> wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> 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 <[email protected]>
> Cc: Liviu Dudau <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-12 07:09:04

by Srikanth Thokala

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

Hi,

On Wed, Nov 12, 2014 at 12:26 PM, Srikanth Thokala
<[email protected]> wrote:
> Hi,
>
> On Wed, Nov 12, 2014 at 12:47 AM, <[email protected]> wrote:
>> From: Suravee Suthikulpanit <[email protected]>
>>
>> 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 <[email protected]>
>> Cc: Liviu Dudau <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>> ---
>> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-12 11:37:32

by Suthikulpanit, Suravee

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



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

2014-11-12 11:38:12

by Suthikulpanit, Suravee

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

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

2014-11-13 18:10:06

by Bjorn Helgaas

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

[+cc Yijing]

On Tue, Nov 11, 2014 at 01:17:32PM -0600, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> 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
>

2014-11-14 01:20:55

by Yijing Wang

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

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, [email protected] wrote:
>> From: Suravee Suthikulpanit <[email protected]>
>>
>> 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