Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1273079 > unrolled thread

Re: [PATCH V3 2/6] acpi: pci: Setup MSI domain for ACPI based pci devices

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V3 2/6] acpi: pci: Setup MSI domain for ACPI based pci  devices Marc Zyngier <marc.zyngier@arm.com> - 2015-11-19 13:10 +0100
    Re: [PATCH V3 2/6] acpi: pci: Setup MSI domain for ACPI based pci  devices Marc Zyngier <marc.zyngier@arm.com> - 2015-11-22 12:40 +0100

#1273079 — Re: [PATCH V3 2/6] acpi: pci: Setup MSI domain for ACPI based pci devices

On Wed, 21 Oct 2015 11:47:25 -0700
Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> wrote:

Hi Suravee,

Sorry it took so long to get to this series. Comments below.

> This patch introduces pci_host_bridge_acpi_msi_domain(), which returns
> the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI
> bus token. Then, it is assigned to pci device.
> 
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
>  drivers/pci/pci-acpi.c | 13 +++++++++++++
>  drivers/pci/probe.c    |  2 ++
>  include/linux/pci.h    |  7 +++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index a32ba75..0e21ef4 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -9,7 +9,9 @@
>  
>  #include <linux/delay.h>
>  #include <linux/init.h>
> +#include <linux/irqdomain.h>
>  #include <linux/pci.h>
> +#include <linux/msi.h>
>  #include <linux/pci_hotplug.h>
>  #include <linux/module.h>
>  #include <linux/pci-aspm.h>
> @@ -689,6 +691,17 @@ static struct acpi_bus_type acpi_pci_bus = {
>  	.cleanup = pci_acpi_cleanup,
>  };
>  
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus)
> +{
> +	struct irq_domain *dom = NULL;
> +	struct fwnode_handle *fwnode = pci_msi_get_fwnode(&bus->dev);
> +
> +	if (fwnode)
> +		dom = irq_find_matching_fwnode(fwnode,
> +					       DOMAIN_BUS_PCI_MSI);
> +	return dom;
> +}
> +

Given this, I really question the need for what you define in patch #1
to be standalone. It is only used by ACPI (DT has its own private
helpers), and it is so far unlikely that it will be of any use for
other firmware interfaces.

My suggestion is to get rid of pci_msi_get_fwnode() and move the
registration helper into this file. That'd be much simpler.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1274851

On Sat, 21 Nov 2015 15:18:45 -0600
Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> wrote:

> Hi Marc,
> 
> On 11/19/15 06:08, Marc Zyngier wrote:
> > On Wed, 21 Oct 2015 11:47:25 -0700
> > Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> wrote:
> >
> > Hi Suravee,
> >
> > Sorry it took so long to get to this series. Comments below.
> 
> No worry.
> 
> >
> >> This patch introduces pci_host_bridge_acpi_msi_domain(), which returns
> >> the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI
> >> bus token. Then, it is assigned to pci device.
> >>
> >> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >> ---
> >>   drivers/pci/pci-acpi.c | 13 +++++++++++++
> >>   drivers/pci/probe.c    |  2 ++
> >>   include/linux/pci.h    |  7 +++++++
> >>   3 files changed, 22 insertions(+)
> >>
> >> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> >> index a32ba75..0e21ef4 100644
> >> --- a/drivers/pci/pci-acpi.c
> >> +++ b/drivers/pci/pci-acpi.c
> >> @@ -9,7 +9,9 @@
> >>
> >>   #include <linux/delay.h>
> >>   #include <linux/init.h>
> >> +#include <linux/irqdomain.h>
> >>   #include <linux/pci.h>
> >> +#include <linux/msi.h>
> >>   #include <linux/pci_hotplug.h>
> >>   #include <linux/module.h>
> >>   #include <linux/pci-aspm.h>
> >> @@ -689,6 +691,17 @@ static struct acpi_bus_type acpi_pci_bus = {
> >>   	.cleanup = pci_acpi_cleanup,
> >>   };
> >>
> >> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus)
> >> +{
> >> +	struct irq_domain *dom = NULL;
> >> +	struct fwnode_handle *fwnode = pci_msi_get_fwnode(&bus->dev);
> >> +
> >> +	if (fwnode)
> >> +		dom = irq_find_matching_fwnode(fwnode,
> >> +					       DOMAIN_BUS_PCI_MSI);
> >> +	return dom;
> >> +}
> >> +
> >
> > Given this, I really question the need for what you define in patch #1
> > to be standalone. It is only used by ACPI (DT has its own private
> > helpers), and it is so far unlikely that it will be of any use for
> > other firmware interfaces.
> >
> > My suggestion is to get rid of pci_msi_get_fwnode() and move the
> > registration helper into this file. That'd be much simpler.
> >
> > Thanks,
> >
> > 	M.
> >
> 
> Ok, I'll take care of this. I assume the rest of the patches looks ok.

I still need to finish going through it (I have minor comments so far
on some of the other patches), but I want to get the core ACPI stuff in
shape before doing anything else.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web