All platform MSI users and the PCI/MSI code handle per device MSI domains
when the irqdomain associated to the device provides MSI parent
functionality.
Remove the "global" platform domain related code and provide the MSI parent
functionality by filling in msi_parent_ops.
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
---
drivers/irqchip/Kconfig | 1
drivers/irqchip/irq-gic-msi-lib.c | 2 +
drivers/irqchip/irq-imx-mu-msi.c | 53 +++++++++++++++-----------------------
3 files changed, 25 insertions(+), 31 deletions(-)
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -494,6 +494,7 @@ config IMX_MU_MSI
select IRQ_DOMAIN
select IRQ_DOMAIN_HIERARCHY
select GENERIC_MSI_IRQ
+ select ARM_GIC_MSI_LIB
help
Provide a driver for the i.MX Messaging Unit block used as a
CPU-to-CPU MSI controller. This requires a specially crafted DT
--- a/drivers/irqchip/irq-gic-msi-lib.c
+++ b/drivers/irqchip/irq-gic-msi-lib.c
@@ -90,6 +90,8 @@ bool gic_msi_lib_init_dev_msi_info(struc
/* Chip updates for all child bus types */
if (!info->chip->irq_eoi)
info->chip->irq_eoi = irq_chip_eoi_parent;
+ if (!info->chip->irq_ack)
+ info->chip->irq_ack = irq_chip_ack_parent;
/*
* The device MSI domain can never have a set affinity callback it
--- a/drivers/irqchip/irq-imx-mu-msi.c
+++ b/drivers/irqchip/irq-imx-mu-msi.c
@@ -24,6 +24,8 @@
#include <linux/pm_domain.h>
#include <linux/spinlock.h>
+#include "irq-gic-msi-lib.h"
+
#define IMX_MU_CHANS 4
enum imx_mu_xcr {
@@ -114,20 +116,6 @@ static void imx_mu_msi_parent_ack_irq(st
imx_mu_read(msi_data, msi_data->cfg->xRR + data->hwirq * 4);
}
-static struct irq_chip imx_mu_msi_irq_chip = {
- .name = "MU-MSI",
- .irq_ack = irq_chip_ack_parent,
-};
-
-static struct msi_domain_ops imx_mu_msi_irq_ops = {
-};
-
-static struct msi_domain_info imx_mu_msi_domain_info = {
- .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
- .ops = &imx_mu_msi_irq_ops,
- .chip = &imx_mu_msi_irq_chip,
-};
-
static void imx_mu_msi_parent_compose_msg(struct irq_data *data,
struct msi_msg *msg)
{
@@ -195,6 +183,7 @@ static void imx_mu_msi_domain_irq_free(s
}
static const struct irq_domain_ops imx_mu_msi_domain_ops = {
+ .select = gic_msi_lib_irq_domain_select,
.alloc = imx_mu_msi_domain_irq_alloc,
.free = imx_mu_msi_domain_irq_free,
};
@@ -216,35 +205,37 @@ static void imx_mu_msi_irq_handler(struc
chained_irq_exit(chip, desc);
}
+#define IMX_MU_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
+ MSI_FLAG_USE_DEF_CHIP_OPS | \
+ MSI_FLAG_PARENT_PM_DEV)
+
+#define IMX_MU_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK)
+
+static const struct msi_parent_ops imx_mu_msi_parent_ops = {
+ .supported_flags = IMX_MU_MSI_FLAGS_SUPPORTED,
+ .required_flags = IMX_MU_MSI_FLAGS_REQUIRED,
+ .bus_select_token = DOMAIN_BUS_NEXUS,
+ .bus_select_mask = MATCH_PLATFORM_MSI,
+ .prefix = "MU-MSI-",
+ .init_dev_msi_info = gic_msi_lib_init_dev_msi_info,
+};
+
static int imx_mu_msi_domains_init(struct imx_mu_msi *msi_data, struct device *dev)
{
struct fwnode_handle *fwnodes = dev_fwnode(dev);
struct irq_domain *parent;
/* Initialize MSI domain parent */
- parent = irq_domain_create_linear(fwnodes,
- IMX_MU_CHANS,
- &imx_mu_msi_domain_ops,
- msi_data);
+ parent = irq_domain_create_linear(fwnodes, IMX_MU_CHANS, &imx_mu_msi_domain_ops, msi_data);
if (!parent) {
dev_err(dev, "failed to create IRQ domain\n");
return -ENOMEM;
}
irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
-
- msi_data->msi_domain = platform_msi_create_irq_domain(fwnodes,
- &imx_mu_msi_domain_info,
- parent);
-
- if (!msi_data->msi_domain) {
- dev_err(dev, "failed to create MSI domain\n");
- irq_domain_remove(parent);
- return -ENOMEM;
- }
-
- irq_domain_set_pm_device(msi_data->msi_domain, dev);
-
+ parent->dev = parent->pm_dev = dev;
+ parent->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
+ parent->msi_parent_ops = &imx_mu_msi_parent_ops;
return 0;
}
Frank!
On Mon, Nov 28 2022 at 15:47, Frank Li wrote:
> On Mon, Nov 21, 2022 at 03:40:09PM +0100, Thomas Gleixner wrote:
>> * The device MSI domain can never have a set affinity callback it
>> --- a/drivers/irqchip/irq-imx-mu-msi.c
>> +++ b/drivers/irqchip/irq-imx-mu-msi.c
>> @@ -24,6 +24,8 @@
>> #include <linux/pm_domain.h>
>> #include <linux/spinlock.h>
>>
>> +#include "irq-gic-msi-lib.h"
>> +
>
> I think irq-gic-msi-lib.h is not good name. Actually mu-msi is not arm gic.
> irq-gic-msi-lib do common work, which not related arm gic at all.
I realized that after a while too, but the main purpose of this series
was to establish that the core design holds up to handle the gazillions
of ARM variants out there and to solicit technical feedback from the
involved parties.
>> static int imx_mu_msi_domains_init(struct imx_mu_msi *msi_data, struct device *dev)
>> {
>> struct fwnode_handle *fwnodes = dev_fwnode(dev);
>> struct irq_domain *parent;
>>
>> /* Initialize MSI domain parent */
>> - parent = irq_domain_create_linear(fwnodes,
>> - IMX_MU_CHANS,
>> - &imx_mu_msi_domain_ops,
>> - msi_data);
>> + parent = irq_domain_create_linear(fwnodes, IMX_MU_CHANS, &imx_mu_msi_domain_ops, msi_data);
>
> coding style change should be in sperated patch.
Thanks for the thorough technical feedback!
tglx
From: Frank Li <[email protected]>
On Mon, Nov 21, 2022 at 03:40:09PM +0100, Thomas Gleixner wrote:
> All platform MSI users and the PCI/MSI code handle per device MSI domains
> when the irqdomain associated to the device provides MSI parent
> functionality.
>
> Remove the "global" platform domain related code and provide the MSI parent
> functionality by filling in msi_parent_ops.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Sascha Hauer <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> ---
> drivers/irqchip/Kconfig | 1
> drivers/irqchip/irq-gic-msi-lib.c | 2 +
> drivers/irqchip/irq-imx-mu-msi.c | 53 +++++++++++++++-----------------------
> 3 files changed, 25 insertions(+), 31 deletions(-)
>
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -494,6 +494,7 @@ config IMX_MU_MSI
> select IRQ_DOMAIN
> select IRQ_DOMAIN_HIERARCHY
> select GENERIC_MSI_IRQ
> + select ARM_GIC_MSI_LIB
> help
> Provide a driver for the i.MX Messaging Unit block used as a
> CPU-to-CPU MSI controller. This requires a specially crafted DT
> --- a/drivers/irqchip/irq-gic-msi-lib.c
> +++ b/drivers/irqchip/irq-gic-msi-lib.c
> @@ -90,6 +90,8 @@ bool gic_msi_lib_init_dev_msi_info(struc
> /* Chip updates for all child bus types */
> if (!info->chip->irq_eoi)
> info->chip->irq_eoi = irq_chip_eoi_parent;
> + if (!info->chip->irq_ack)
> + info->chip->irq_ack = irq_chip_ack_parent;
>
> /*
> * The device MSI domain can never have a set affinity callback it
> --- a/drivers/irqchip/irq-imx-mu-msi.c
> +++ b/drivers/irqchip/irq-imx-mu-msi.c
> @@ -24,6 +24,8 @@
> #include <linux/pm_domain.h>
> #include <linux/spinlock.h>
>
> +#include "irq-gic-msi-lib.h"
> +
I think irq-gic-msi-lib.h is not good name. Actually mu-msi is not arm gic.
irq-gic-msi-lib do common work, which not related arm gic at all.
> #define IMX_MU_CHANS 4
>
> enum imx_mu_xcr {
> @@ -114,20 +116,6 @@ static void imx_mu_msi_parent_ack_irq(st
> imx_mu_read(msi_data, msi_data->cfg->xRR + data->hwirq * 4);
> }
>
> -static struct irq_chip imx_mu_msi_irq_chip = {
> - .name = "MU-MSI",
> - .irq_ack = irq_chip_ack_parent,
> -};
> -
> -static struct msi_domain_ops imx_mu_msi_irq_ops = {
> -};
> -
> -static struct msi_domain_info imx_mu_msi_domain_info = {
> - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
> - .ops = &imx_mu_msi_irq_ops,
> - .chip = &imx_mu_msi_irq_chip,
> -};
> -
> static void imx_mu_msi_parent_compose_msg(struct irq_data *data,
> struct msi_msg *msg)
> {
> @@ -195,6 +183,7 @@ static void imx_mu_msi_domain_irq_free(s
> }
>
> static const struct irq_domain_ops imx_mu_msi_domain_ops = {
> + .select = gic_msi_lib_irq_domain_select,
> .alloc = imx_mu_msi_domain_irq_alloc,
> .free = imx_mu_msi_domain_irq_free,
> };
> @@ -216,35 +205,37 @@ static void imx_mu_msi_irq_handler(struc
> chained_irq_exit(chip, desc);
> }
>
> +#define IMX_MU_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
> + MSI_FLAG_USE_DEF_CHIP_OPS | \
> + MSI_FLAG_PARENT_PM_DEV)
> +
> +#define IMX_MU_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK)
> +
> +static const struct msi_parent_ops imx_mu_msi_parent_ops = {
> + .supported_flags = IMX_MU_MSI_FLAGS_SUPPORTED,
> + .required_flags = IMX_MU_MSI_FLAGS_REQUIRED,
> + .bus_select_token = DOMAIN_BUS_NEXUS,
> + .bus_select_mask = MATCH_PLATFORM_MSI,
> + .prefix = "MU-MSI-",
> + .init_dev_msi_info = gic_msi_lib_init_dev_msi_info,
> +};
> +
> static int imx_mu_msi_domains_init(struct imx_mu_msi *msi_data, struct device *dev)
> {
> struct fwnode_handle *fwnodes = dev_fwnode(dev);
> struct irq_domain *parent;
>
> /* Initialize MSI domain parent */
> - parent = irq_domain_create_linear(fwnodes,
> - IMX_MU_CHANS,
> - &imx_mu_msi_domain_ops,
> - msi_data);
> + parent = irq_domain_create_linear(fwnodes, IMX_MU_CHANS, &imx_mu_msi_domain_ops, msi_data);
coding style change should be in sperated patch.
> if (!parent) {
> dev_err(dev, "failed to create IRQ domain\n");
> return -ENOMEM;
> }
>
> irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
> -
> - msi_data->msi_domain = platform_msi_create_irq_domain(fwnodes,
> - &imx_mu_msi_domain_info,
> - parent);
> -
> - if (!msi_data->msi_domain) {
> - dev_err(dev, "failed to create MSI domain\n");
> - irq_domain_remove(parent);
> - return -ENOMEM;
> - }
> -
> - irq_domain_set_pm_device(msi_data->msi_domain, dev);
> -
> + parent->dev = parent->pm_dev = dev;
> + parent->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
> + parent->msi_parent_ops = &imx_mu_msi_parent_ops;
> return 0;
> }