Message ID | c882fe329932409131be76ce47b81a6155595ce4.1733726057.git.unicorn_wang@outlook.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | irqchip: Add Sophgo SG2042 MSI controller | expand |
On Mon, Dec 09, 2024 at 03:12:00PM +0800, Chen Wang wrote: > +static void sg2042_msi_irq_ack(struct irq_data *d) > +{ > + struct sg2042_msi_data *data = irq_data_get_irq_chip_data(d); > + int bit_off = d->hwirq - data->irq_first; > + > + writel(1 << bit_off, (unsigned int *)data->reg_clr); > + > + irq_chip_ack_parent(d); > +} > + > +static void sg2042_msi_irq_compose_msi_msg(struct irq_data *data, > + struct msi_msg *msg) > +{ > + struct sg2042_msi_data *priv = irq_data_get_irq_chip_data(data); > + > + msg->address_hi = upper_32_bits(priv->doorbell_addr); > + msg->address_lo = lower_32_bits(priv->doorbell_addr); > + msg->data = 1 << (data->hwirq - priv->irq_first); > + > + pr_debug("%s hwirq[%ld]: address_hi[%#x], address_lo[%#x], data[%#x]\n", > + __func__, data->hwirq, msg->address_hi, msg->address_lo, msg->data); Don't print addresses, it is useless - it will be a constant. > +} > + > +static struct irq_chip sg2042_msi_middle_irq_chip = { > + .name = "SG2042 MSI", > + .irq_ack = sg2042_msi_irq_ack, > + .irq_mask = irq_chip_mask_parent, > + .irq_unmask = irq_chip_unmask_parent, > +#ifdef CONFIG_SMP > + .irq_set_affinity = irq_chip_set_affinity_parent, > +#endif > + .irq_compose_msi_msg = sg2042_msi_irq_compose_msi_msg, > +}; ... > +static int sg2042_msi_probe(struct platform_device *pdev) > +{ > + struct of_phandle_args args = {}; > + struct sg2042_msi_data *data; > + int ret; > + > + data = devm_kzalloc(&pdev->dev, sizeof(struct sg2042_msi_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->reg_clr = devm_platform_ioremap_resource_byname(pdev, "clr"); > + if (IS_ERR(data->reg_clr)) { > + dev_err(&pdev->dev, "Failed to map clear register\n"); > + return PTR_ERR(data->reg_clr); > + } > + > + if (of_property_read_u64(pdev->dev.of_node, "sophgo,msi-doorbell-addr", > + &data->doorbell_addr)) { > + dev_err(&pdev->dev, "Unable to parse MSI doorbell addr\n"); > + return -EINVAL; > + } > + > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "msi-ranges", > + "#interrupt-cells", 0, &args); > + if (ret) { > + dev_err(&pdev->dev, "Unable to parse MSI vec base\n"); > + return ret; > + } You leak the phandle. You leak much more, btw... > + data->irq_first = (u32)args.args[0]; > + > + ret = of_property_read_u32_index(pdev->dev.of_node, "msi-ranges", > + args.args_count + 1, &data->num_irqs); > + if (ret) { > + dev_err(&pdev->dev, "Unable to parse MSI vec number\n"); > + return ret; > + } > + > + if (data->irq_first < SG2042_VECTOR_MIN || > + (data->irq_first + data->num_irqs - 1) > SG2042_VECTOR_MAX) { > + dev_err(&pdev->dev, "msi-ranges is incorrect!\n"); > + return -EINVAL; > + } > + > + mutex_init(&data->msi_map_lock); > + > + data->msi_map = bitmap_zalloc(data->num_irqs, GFP_KERNEL); This also leaks during removal. > + if (!data->msi_map) > + return -ENOMEM; > + > + ret = sg2042_msi_init_domains(data, pdev->dev.of_node); > + if (ret) > + bitmap_free(data->msi_map); > + > + return ret; > +} > + > +static const struct of_device_id sg2042_msi_of_match[] = { > + { .compatible = "sophgo,sg2042-msi" }, > + {} > +}; > + > +static struct platform_driver sg2042_msi_driver = { > + .driver = { > + .name = "sg2042-msi", > + .of_match_table = of_match_ptr(sg2042_msi_of_match), Drop of_match_ptr(), unnecessary and might lead to warnings even if this is not a module. Best regards, Krzysztof
Hi Chen, kernel test robot noticed the following build warnings: [auto build test WARNING on fac04efc5c793dccbd07e2d59af9f90b7fc0dca4] url: https://github.com/intel-lab-lkp/linux/commits/Chen-Wang/dt-bindings-interrupt-controller-Add-Sophgo-SG2042-MSI/20241209-151429 base: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4 patch link: https://lore.kernel.org/r/c882fe329932409131be76ce47b81a6155595ce4.1733726057.git.unicorn_wang%40outlook.com patch subject: [PATCH v2 2/3] irqchip: Add the Sophgo SG2042 MSI interrupt controller config: arm-randconfig-r131-20241210 (https://download.01.org/0day-ci/archive/20241210/202412101545.Psk65SvD-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0 reproduce: (https://download.01.org/0day-ci/archive/20241210/202412101545.Psk65SvD-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202412101545.Psk65SvD-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/irqchip/irq-sg2042-msi.c:64:9: sparse: sparse: cast removes address space '__iomem' of expression >> drivers/irqchip/irq-sg2042-msi.c:64:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got unsigned int * @@ drivers/irqchip/irq-sg2042-msi.c:64:9: sparse: expected void volatile [noderef] __iomem *addr drivers/irqchip/irq-sg2042-msi.c:64:9: sparse: got unsigned int * vim +/__iomem +64 drivers/irqchip/irq-sg2042-msi.c 58 59 static void sg2042_msi_irq_ack(struct irq_data *d) 60 { 61 struct sg2042_msi_data *data = irq_data_get_irq_chip_data(d); 62 int bit_off = d->hwirq - data->irq_first; 63 > 64 writel(1 << bit_off, (unsigned int *)data->reg_clr); 65 66 irq_chip_ack_parent(d); 67 } 68
Hi Chen, kernel test robot noticed the following build warnings: [auto build test WARNING on fac04efc5c793dccbd07e2d59af9f90b7fc0dca4] url: https://github.com/intel-lab-lkp/linux/commits/Chen-Wang/dt-bindings-interrupt-controller-Add-Sophgo-SG2042-MSI/20241209-151429 base: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4 patch link: https://lore.kernel.org/r/c882fe329932409131be76ce47b81a6155595ce4.1733726057.git.unicorn_wang%40outlook.com patch subject: [PATCH v2 2/3] irqchip: Add the Sophgo SG2042 MSI interrupt controller config: arm-randconfig-r131-20241210 (https://download.01.org/0day-ci/archive/20241211/202412110248.fdcNDwnt-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0 reproduce: (https://download.01.org/0day-ci/archive/20241211/202412110248.fdcNDwnt-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202412110248.fdcNDwnt-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/irqchip/irq-sg2042-msi.c:64:9: sparse: sparse: cast removes address space '__iomem' of expression >> drivers/irqchip/irq-sg2042-msi.c:64:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got unsigned int * @@ drivers/irqchip/irq-sg2042-msi.c:64:9: sparse: expected void volatile [noderef] __iomem *addr drivers/irqchip/irq-sg2042-msi.c:64:9: sparse: got unsigned int * vim +/__iomem +64 drivers/irqchip/irq-sg2042-msi.c 58 59 static void sg2042_msi_irq_ack(struct irq_data *d) 60 { 61 struct sg2042_msi_data *data = irq_data_get_irq_chip_data(d); 62 int bit_off = d->hwirq - data->irq_first; 63 > 64 writel(1 << bit_off, (unsigned int *)data->reg_clr); 65 66 irq_chip_ack_parent(d); 67 } 68
Hi Chen, kernel test robot noticed the following build warnings: [auto build test WARNING on fac04efc5c793dccbd07e2d59af9f90b7fc0dca4] url: https://github.com/intel-lab-lkp/linux/commits/Chen-Wang/dt-bindings-interrupt-controller-Add-Sophgo-SG2042-MSI/20241209-151429 base: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4 patch link: https://lore.kernel.org/r/c882fe329932409131be76ce47b81a6155595ce4.1733726057.git.unicorn_wang%40outlook.com patch subject: [PATCH v2 2/3] irqchip: Add the Sophgo SG2042 MSI interrupt controller config: alpha-randconfig-r063-20241211 (https://download.01.org/0day-ci/archive/20241211/202412110221.35ohPaia-lkp@intel.com/config) compiler: alpha-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241211/202412110221.35ohPaia-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202412110221.35ohPaia-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/irqchip/irq-sg2042-msi.c:273:34: warning: 'sg2042_msi_of_match' defined but not used [-Wunused-const-variable=] 273 | static const struct of_device_id sg2042_msi_of_match[] = { | ^~~~~~~~~~~~~~~~~~~ vim +/sg2042_msi_of_match +273 drivers/irqchip/irq-sg2042-msi.c 272 > 273 static const struct of_device_id sg2042_msi_of_match[] = { 274 { .compatible = "sophgo,sg2042-msi" }, 275 {} 276 }; 277
On 2024-12-09 1:12 AM, Chen Wang wrote: > From: Chen Wang <unicorn_wang@outlook.com> > > Add driver for Sophgo SG2042 MSI interrupt controller. > > Signed-off-by: Chen Wang <unicorn_wang@outlook.com> > --- > drivers/irqchip/Kconfig | 12 ++ > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-sg2042-msi.c | 285 +++++++++++++++++++++++++++++++ > 3 files changed, 298 insertions(+) > create mode 100644 drivers/irqchip/irq-sg2042-msi.c > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index 9bee02db1643..161fb5df857f 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -749,6 +749,18 @@ config MCHP_EIC > help > Support for Microchip External Interrupt Controller. > > +config SOPHGO_SG2042_MSI > + bool "Sophgo SG2042 MSI Controller" > + depends on ARCH_SOPHGO || COMPILE_TEST > + depends on PCI > + select IRQ_DOMAIN_HIERARCHY > + select IRQ_MSI_LIB > + select PCI_MSI > + help > + Support for the Sophgo SG2042 MSI Controller. > + This on-chip interrupt controller enables MSI sources to be > + routed to the primary PLIC controller on SoC. > + > config SUNPLUS_SP7021_INTC > bool "Sunplus SP7021 interrupt controller" if COMPILE_TEST > default SOC_SP7021 > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index 25e9ad29b8c4..dd60e597491d 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -128,4 +128,5 @@ obj-$(CONFIG_WPCM450_AIC) += irq-wpcm450-aic.o > obj-$(CONFIG_IRQ_IDT3243X) += irq-idt3243x.o > obj-$(CONFIG_APPLE_AIC) += irq-apple-aic.o > obj-$(CONFIG_MCHP_EIC) += irq-mchp-eic.o > +obj-$(CONFIG_SOPHGO_SG2042_MSI) += irq-sg2042-msi.o > obj-$(CONFIG_SUNPLUS_SP7021_INTC) += irq-sp7021-intc.o > diff --git a/drivers/irqchip/irq-sg2042-msi.c b/drivers/irqchip/irq-sg2042-msi.c > new file mode 100644 > index 000000000000..495ee2b45eb2 > --- /dev/null > +++ b/drivers/irqchip/irq-sg2042-msi.c > @@ -0,0 +1,285 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SG2042 MSI Controller > + * > + * Copyright (C) 2024 Sophgo Technology Inc. > + * Copyright (C) 2024 Chen Wang <unicorn_wang@outlook.com> > + */ > + > +#include <linux/cleanup.h> > +#include <linux/io.h> > +#include <linux/irq.h> > +#include <linux/irqdomain.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/msi.h> > +#include <linux/of.h> > +#include <linux/of_irq.h> > +#include <linux/of_pci.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > + > +#include "irq-msi-lib.h" > + > +#define SG2042_VECTOR_MIN 64 > +#define SG2042_VECTOR_MAX 95 > + > +struct sg2042_msi_data { > + void __iomem *reg_clr; // clear reg, see TRM, 10.1.33, GP_INTR0_CLR > + > + u64 doorbell_addr; // see TRM, 10.1.32, GP_INTR0_SET > + > + u32 irq_first; // The vector number that MSIs starts > + u32 num_irqs; // The number of vectors for MSIs > + > + unsigned long *msi_map; > + struct mutex msi_map_lock; // lock for msi_map > +}; > + > +static int sg2042_msi_allocate_hwirq(struct sg2042_msi_data *priv, int num_req) > +{ > + int first; > + > + guard(mutex)(&priv->msi_map_lock); > + first = bitmap_find_free_region(priv->msi_map, priv->num_irqs, > + get_count_order(num_req)); > + return first >= 0 ? priv->irq_first + first : -ENOSPC; How does this work? The irqdomain is instantiated with size == priv->num_irqs, so hwirqs must be less than priv->num_irqs. It also quite simplifies the code for hwirq be a number between 0 and priv->num_irqs. You only need to apply the offset when allocating the parent IRQ: fwspec.param[0] = priv->irq_first + hwirq; > +} > + > +static void sg2042_msi_free_hwirq(struct sg2042_msi_data *priv, > + int hwirq, int num_req) > +{ > + int first = hwirq - priv->irq_first; > + > + guard(mutex)(&priv->msi_map_lock); > + bitmap_release_region(priv->msi_map, first, get_count_order(num_req)); > +} > + > +static void sg2042_msi_irq_ack(struct irq_data *d) > +{ > + struct sg2042_msi_data *data = irq_data_get_irq_chip_data(d); > + int bit_off = d->hwirq - data->irq_first; > + > + writel(1 << bit_off, (unsigned int *)data->reg_clr); > + > + irq_chip_ack_parent(d); > +} > + > +static void sg2042_msi_irq_compose_msi_msg(struct irq_data *data, > + struct msi_msg *msg) > +{ > + struct sg2042_msi_data *priv = irq_data_get_irq_chip_data(data); > + > + msg->address_hi = upper_32_bits(priv->doorbell_addr); > + msg->address_lo = lower_32_bits(priv->doorbell_addr); > + msg->data = 1 << (data->hwirq - priv->irq_first); > + > + pr_debug("%s hwirq[%ld]: address_hi[%#x], address_lo[%#x], data[%#x]\n", > + __func__, data->hwirq, msg->address_hi, msg->address_lo, msg->data); > +} > + > +static struct irq_chip sg2042_msi_middle_irq_chip = { > + .name = "SG2042 MSI", > + .irq_ack = sg2042_msi_irq_ack, > + .irq_mask = irq_chip_mask_parent, > + .irq_unmask = irq_chip_unmask_parent, > +#ifdef CONFIG_SMP > + .irq_set_affinity = irq_chip_set_affinity_parent, > +#endif > + .irq_compose_msi_msg = sg2042_msi_irq_compose_msi_msg, > +}; > + > +static int sg2042_msi_parent_domain_alloc(struct irq_domain *domain, > + unsigned int virq, int hwirq) > +{ > + struct irq_fwspec fwspec; > + struct irq_data *d; > + int ret; > + > + fwspec.fwnode = domain->parent->fwnode; > + fwspec.param_count = 2; > + fwspec.param[0] = hwirq; > + fwspec.param[1] = IRQ_TYPE_EDGE_RISING; > + > + ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec); > + if (ret) > + return ret; > + > + d = irq_domain_get_irq_data(domain->parent, virq); > + return d->chip->irq_set_type(d, IRQ_TYPE_EDGE_RISING); > +} > + > +static int sg2042_msi_middle_domain_alloc(struct irq_domain *domain, > + unsigned int virq, > + unsigned int nr_irqs, void *args) > +{ > + struct sg2042_msi_data *priv = domain->host_data; > + int hwirq, err, i; > + > + hwirq = sg2042_msi_allocate_hwirq(priv, nr_irqs); > + if (hwirq < 0) > + return hwirq; > + > + for (i = 0; i < nr_irqs; i++) { > + err = sg2042_msi_parent_domain_alloc(domain, virq + i, hwirq + i); > + if (err) > + goto err_hwirq; > + > + pr_debug("%s: virq[%d], hwirq[%d]\n", __func__, virq + i, hwirq + i); > + > + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, > + &sg2042_msi_middle_irq_chip, priv); > + } > + > + return 0; > + > +err_hwirq: > + sg2042_msi_free_hwirq(priv, hwirq, nr_irqs); > + irq_domain_free_irqs_parent(domain, virq, i); > + > + return err; > +} > + > +static void sg2042_msi_middle_domain_free(struct irq_domain *domain, > + unsigned int virq, > + unsigned int nr_irqs) > +{ > + struct irq_data *d = irq_domain_get_irq_data(domain, virq); > + struct sg2042_msi_data *priv = irq_data_get_irq_chip_data(d); > + > + irq_domain_free_irqs_parent(domain, virq, nr_irqs); > + sg2042_msi_free_hwirq(priv, d->hwirq, nr_irqs); > +} > + > +static const struct irq_domain_ops sg2042_msi_middle_domain_ops = { > + .alloc = sg2042_msi_middle_domain_alloc, > + .free = sg2042_msi_middle_domain_free, > + .select = msi_lib_irq_domain_select, > +}; > + > +#define SG2042_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \ > + MSI_FLAG_USE_DEF_CHIP_OPS) > + > +#define SG2042_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK > + > +static struct msi_parent_ops sg2042_msi_parent_ops = { > + .required_flags = SG2042_MSI_FLAGS_REQUIRED, > + .supported_flags = SG2042_MSI_FLAGS_SUPPORTED, > + .bus_select_mask = MATCH_PCI_MSI, > + .bus_select_token = DOMAIN_BUS_NEXUS, > + .prefix = "SG2042-", > + .init_dev_msi_info = msi_lib_init_dev_msi_info, > +}; > + > +static int sg2042_msi_init_domains(struct sg2042_msi_data *priv, > + struct device_node *node) > +{ > + struct fwnode_handle *fwnode = of_node_to_fwnode(node); > + struct irq_domain *plic_domain, *middle_domain; > + struct device_node *plic_node; > + > + if (!of_find_property(node, "interrupt-parent", NULL)) { > + pr_err("Can't find interrupt-parent!\n"); > + return -EINVAL; > + } > + > + plic_node = of_irq_find_parent(node); > + if (!plic_node) { > + pr_err("Failed to find the PLIC node!\n"); > + return -ENXIO; > + } > + > + plic_domain = irq_find_host(plic_node); > + of_node_put(plic_node); > + if (!plic_domain) { > + pr_err("Failed to find the PLIC domain\n"); > + return -ENXIO; > + } > + > + middle_domain = irq_domain_create_hierarchy(plic_domain, 0, priv->num_irqs, > + fwnode, > + &sg2042_msi_middle_domain_ops, > + priv); > + if (!middle_domain) { > + pr_err("Failed to create the MSI middle domain\n"); > + return -ENOMEM; > + } > + > + irq_domain_update_bus_token(middle_domain, DOMAIN_BUS_NEXUS); > + > + middle_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT; > + middle_domain->msi_parent_ops = &sg2042_msi_parent_ops; > + > + return 0; > +} > + > +static int sg2042_msi_probe(struct platform_device *pdev) > +{ > + struct of_phandle_args args = {}; > + struct sg2042_msi_data *data; > + int ret; > + > + data = devm_kzalloc(&pdev->dev, sizeof(struct sg2042_msi_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->reg_clr = devm_platform_ioremap_resource_byname(pdev, "clr"); > + if (IS_ERR(data->reg_clr)) { > + dev_err(&pdev->dev, "Failed to map clear register\n"); > + return PTR_ERR(data->reg_clr); > + } > + > + if (of_property_read_u64(pdev->dev.of_node, "sophgo,msi-doorbell-addr", > + &data->doorbell_addr)) { > + dev_err(&pdev->dev, "Unable to parse MSI doorbell addr\n"); > + return -EINVAL; > + } > + > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "msi-ranges", > + "#interrupt-cells", 0, &args); > + if (ret) { > + dev_err(&pdev->dev, "Unable to parse MSI vec base\n"); > + return ret; > + } > + data->irq_first = (u32)args.args[0]; > + > + ret = of_property_read_u32_index(pdev->dev.of_node, "msi-ranges", > + args.args_count + 1, &data->num_irqs); > + if (ret) { > + dev_err(&pdev->dev, "Unable to parse MSI vec number\n"); > + return ret; > + } > + > + if (data->irq_first < SG2042_VECTOR_MIN || > + (data->irq_first + data->num_irqs - 1) > SG2042_VECTOR_MAX) { > + dev_err(&pdev->dev, "msi-ranges is incorrect!\n"); > + return -EINVAL; > + } > + > + mutex_init(&data->msi_map_lock); > + > + data->msi_map = bitmap_zalloc(data->num_irqs, GFP_KERNEL); > + if (!data->msi_map) > + return -ENOMEM; > + > + ret = sg2042_msi_init_domains(data, pdev->dev.of_node); > + if (ret) > + bitmap_free(data->msi_map); > + > + return ret; > +} > + > +static const struct of_device_id sg2042_msi_of_match[] = { > + { .compatible = "sophgo,sg2042-msi" }, > + {} > +}; > + > +static struct platform_driver sg2042_msi_driver = { > + .driver = { > + .name = "sg2042-msi", > + .of_match_table = of_match_ptr(sg2042_msi_of_match), > + }, > + .probe = sg2042_msi_probe, > +}; > +builtin_platform_driver(sg2042_msi_driver);
On 2024/12/9 17:34, Krzysztof Kozlowski wrote: > On Mon, Dec 09, 2024 at 03:12:00PM +0800, Chen Wang wrote: >> +static void sg2042_msi_irq_ack(struct irq_data *d) >> +{ >> + struct sg2042_msi_data *data = irq_data_get_irq_chip_data(d); >> + int bit_off = d->hwirq - data->irq_first; >> + >> + writel(1 << bit_off, (unsigned int *)data->reg_clr); >> + >> + irq_chip_ack_parent(d); >> +} >> + >> +static void sg2042_msi_irq_compose_msi_msg(struct irq_data *data, >> + struct msi_msg *msg) >> +{ >> + struct sg2042_msi_data *priv = irq_data_get_irq_chip_data(data); >> + >> + msg->address_hi = upper_32_bits(priv->doorbell_addr); >> + msg->address_lo = lower_32_bits(priv->doorbell_addr); >> + msg->data = 1 << (data->hwirq - priv->irq_first); >> + >> + pr_debug("%s hwirq[%ld]: address_hi[%#x], address_lo[%#x], data[%#x]\n", >> + __func__, data->hwirq, msg->address_hi, msg->address_lo, msg->data); > Don't print addresses, it is useless - it will be a constant. Ok. > >> +} >> + >> +static struct irq_chip sg2042_msi_middle_irq_chip = { >> + .name = "SG2042 MSI", >> + .irq_ack = sg2042_msi_irq_ack, >> + .irq_mask = irq_chip_mask_parent, >> + .irq_unmask = irq_chip_unmask_parent, >> +#ifdef CONFIG_SMP >> + .irq_set_affinity = irq_chip_set_affinity_parent, >> +#endif >> + .irq_compose_msi_msg = sg2042_msi_irq_compose_msi_msg, >> +}; > ... > >> +static int sg2042_msi_probe(struct platform_device *pdev) >> +{ >> + struct of_phandle_args args = {}; >> + struct sg2042_msi_data *data; >> + int ret; >> + >> + data = devm_kzalloc(&pdev->dev, sizeof(struct sg2042_msi_data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->reg_clr = devm_platform_ioremap_resource_byname(pdev, "clr"); >> + if (IS_ERR(data->reg_clr)) { >> + dev_err(&pdev->dev, "Failed to map clear register\n"); >> + return PTR_ERR(data->reg_clr); >> + } >> + >> + if (of_property_read_u64(pdev->dev.of_node, "sophgo,msi-doorbell-addr", >> + &data->doorbell_addr)) { >> + dev_err(&pdev->dev, "Unable to parse MSI doorbell addr\n"); >> + return -EINVAL; >> + } >> + >> + ret = of_parse_phandle_with_args(pdev->dev.of_node, "msi-ranges", >> + "#interrupt-cells", 0, &args); >> + if (ret) { >> + dev_err(&pdev->dev, "Unable to parse MSI vec base\n"); >> + return ret; >> + } > You leak the phandle. You leak much more, btw... I will double-check, thanks. > >> + data->irq_first = (u32)args.args[0]; >> + >> + ret = of_property_read_u32_index(pdev->dev.of_node, "msi-ranges", >> + args.args_count + 1, &data->num_irqs); >> + if (ret) { >> + dev_err(&pdev->dev, "Unable to parse MSI vec number\n"); >> + return ret; >> + } >> + >> + if (data->irq_first < SG2042_VECTOR_MIN || >> + (data->irq_first + data->num_irqs - 1) > SG2042_VECTOR_MAX) { >> + dev_err(&pdev->dev, "msi-ranges is incorrect!\n"); >> + return -EINVAL; >> + } >> + >> + mutex_init(&data->msi_map_lock); >> + >> + data->msi_map = bitmap_zalloc(data->num_irqs, GFP_KERNEL); > This also leaks during removal. Got, thanks. > >> + if (!data->msi_map) >> + return -ENOMEM; >> + >> + ret = sg2042_msi_init_domains(data, pdev->dev.of_node); >> + if (ret) >> + bitmap_free(data->msi_map); >> + >> + return ret; >> +} >> + >> +static const struct of_device_id sg2042_msi_of_match[] = { >> + { .compatible = "sophgo,sg2042-msi" }, >> + {} >> +}; >> + >> +static struct platform_driver sg2042_msi_driver = { >> + .driver = { >> + .name = "sg2042-msi", >> + .of_match_table = of_match_ptr(sg2042_msi_of_match), > Drop of_match_ptr(), unnecessary and might lead to warnings even if this > is not a module. Got, I will remove it, thanks. > > Best regards, > Krzysztof >
On 2024/12/11 7:13, Samuel Holland wrote: > On 2024-12-09 1:12 AM, Chen Wang wrote: [......] >> + >> +static int sg2042_msi_allocate_hwirq(struct sg2042_msi_data *priv, int num_req) >> +{ >> + int first; >> + >> + guard(mutex)(&priv->msi_map_lock); >> + first = bitmap_find_free_region(priv->msi_map, priv->num_irqs, >> + get_count_order(num_req)); >> + return first >= 0 ? priv->irq_first + first : -ENOSPC; > How does this work? The irqdomain is instantiated with size == priv->num_irqs, > so hwirqs must be less than priv->num_irqs. It also quite simplifies the code > for hwirq be a number between 0 and priv->num_irqs. You only need to apply the > offset when allocating the parent IRQ: > > fwspec.param[0] = priv->irq_first + hwirq; I will double check, seems this need to be improved. Thanks, Chen [......]
Le 09/12/2024 à 08:12, Chen Wang a écrit : > From: Chen Wang <unicorn_wang@outlook.com> > > Add driver for Sophgo SG2042 MSI interrupt controller. > > Signed-off-by: Chen Wang <unicorn_wang@outlook.com> ... > +#define SG2042_VECTOR_MIN 64 > +#define SG2042_VECTOR_MAX 95 ... > +static struct irq_chip sg2042_msi_middle_irq_chip = { const? > + .name = "SG2042 MSI", > + .irq_ack = sg2042_msi_irq_ack, > + .irq_mask = irq_chip_mask_parent, > + .irq_unmask = irq_chip_unmask_parent, > +#ifdef CONFIG_SMP > + .irq_set_affinity = irq_chip_set_affinity_parent, > +#endif > + .irq_compose_msi_msg = sg2042_msi_irq_compose_msi_msg, > +}; ... > +static int sg2042_msi_probe(struct platform_device *pdev) > +{ > + struct of_phandle_args args = {}; > + struct sg2042_msi_data *data; > + int ret; > + > + data = devm_kzalloc(&pdev->dev, sizeof(struct sg2042_msi_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->reg_clr = devm_platform_ioremap_resource_byname(pdev, "clr"); > + if (IS_ERR(data->reg_clr)) { > + dev_err(&pdev->dev, "Failed to map clear register\n"); > + return PTR_ERR(data->reg_clr); > + } > + > + if (of_property_read_u64(pdev->dev.of_node, "sophgo,msi-doorbell-addr", > + &data->doorbell_addr)) { > + dev_err(&pdev->dev, "Unable to parse MSI doorbell addr\n"); > + return -EINVAL; > + } > + > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "msi-ranges", > + "#interrupt-cells", 0, &args); > + if (ret) { > + dev_err(&pdev->dev, "Unable to parse MSI vec base\n"); > + return ret; > + } > + data->irq_first = (u32)args.args[0]; > + > + ret = of_property_read_u32_index(pdev->dev.of_node, "msi-ranges", > + args.args_count + 1, &data->num_irqs); > + if (ret) { > + dev_err(&pdev->dev, "Unable to parse MSI vec number\n"); > + return ret; > + } > + > + if (data->irq_first < SG2042_VECTOR_MIN || > + (data->irq_first + data->num_irqs - 1) > SG2042_VECTOR_MAX) { > + dev_err(&pdev->dev, "msi-ranges is incorrect!\n"); > + return -EINVAL; > + } > + > + mutex_init(&data->msi_map_lock); > + > + data->msi_map = bitmap_zalloc(data->num_irqs, GFP_KERNEL); IIUC, num_irqs is between 0 and (SG2042_VECTOR_MAX - SG2042_VECTOR_MIN) (maybe + or -1). So around 32. Would it make sence to use DECLARE_BITMAP(msi_map, <correct_size>) in sg2042_msi_data to avoid this allocation and an indirection at runtime? > + if (!data->msi_map) > + return -ENOMEM; > + > + ret = sg2042_msi_init_domains(data, pdev->dev.of_node); > + if (ret) > + bitmap_free(data->msi_map); > + > + return ret; > +} ... CJ
On 2024/12/12 0:32, Christophe JAILLET wrote: > Le 09/12/2024 à 08:12, Chen Wang a écrit : >> From: Chen Wang <unicorn_wang@outlook.com> >> >> Add driver for Sophgo SG2042 MSI interrupt controller. >> >> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> > > ... > >> +#define SG2042_VECTOR_MIN 64 >> +#define SG2042_VECTOR_MAX 95 > > ... > >> +static struct irq_chip sg2042_msi_middle_irq_chip = { > > const? Yes, I will add this in next version, thanks. > >> + .name = "SG2042 MSI", >> + .irq_ack = sg2042_msi_irq_ack, >> + .irq_mask = irq_chip_mask_parent, >> + .irq_unmask = irq_chip_unmask_parent, >> +#ifdef CONFIG_SMP >> + .irq_set_affinity = irq_chip_set_affinity_parent, >> +#endif >> + .irq_compose_msi_msg = sg2042_msi_irq_compose_msi_msg, >> +}; > > ... > >> +static int sg2042_msi_probe(struct platform_device *pdev) >> +{ >> + struct of_phandle_args args = {}; >> + struct sg2042_msi_data *data; >> + int ret; >> + >> + data = devm_kzalloc(&pdev->dev, sizeof(struct sg2042_msi_data), >> GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->reg_clr = devm_platform_ioremap_resource_byname(pdev, "clr"); >> + if (IS_ERR(data->reg_clr)) { >> + dev_err(&pdev->dev, "Failed to map clear register\n"); >> + return PTR_ERR(data->reg_clr); >> + } >> + >> + if (of_property_read_u64(pdev->dev.of_node, >> "sophgo,msi-doorbell-addr", >> + &data->doorbell_addr)) { >> + dev_err(&pdev->dev, "Unable to parse MSI doorbell addr\n"); >> + return -EINVAL; >> + } >> + >> + ret = of_parse_phandle_with_args(pdev->dev.of_node, "msi-ranges", >> + "#interrupt-cells", 0, &args); >> + if (ret) { >> + dev_err(&pdev->dev, "Unable to parse MSI vec base\n"); >> + return ret; >> + } >> + data->irq_first = (u32)args.args[0]; >> + >> + ret = of_property_read_u32_index(pdev->dev.of_node, "msi-ranges", >> + args.args_count + 1, &data->num_irqs); >> + if (ret) { >> + dev_err(&pdev->dev, "Unable to parse MSI vec number\n"); >> + return ret; >> + } >> + >> + if (data->irq_first < SG2042_VECTOR_MIN || >> + (data->irq_first + data->num_irqs - 1) > SG2042_VECTOR_MAX) { >> + dev_err(&pdev->dev, "msi-ranges is incorrect!\n"); >> + return -EINVAL; >> + } >> + >> + mutex_init(&data->msi_map_lock); >> + >> + data->msi_map = bitmap_zalloc(data->num_irqs, GFP_KERNEL); > > IIUC, num_irqs is between 0 and (SG2042_VECTOR_MAX - > SG2042_VECTOR_MIN) (maybe + or -1). > So around 32. > > Would it make sence to use DECLARE_BITMAP(msi_map, <correct_size>) in > sg2042_msi_data to avoid this allocation and an indirection at runtime? This is also a good choice. I will double check this. Thanks, Chen > >> + if (!data->msi_map) >> + return -ENOMEM; >> + >> + ret = sg2042_msi_init_domains(data, pdev->dev.of_node); >> + if (ret) >> + bitmap_free(data->msi_map); >> + >> + return ret; >> +} > > ... > > CJ
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 9bee02db1643..161fb5df857f 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -749,6 +749,18 @@ config MCHP_EIC help Support for Microchip External Interrupt Controller. +config SOPHGO_SG2042_MSI + bool "Sophgo SG2042 MSI Controller" + depends on ARCH_SOPHGO || COMPILE_TEST + depends on PCI + select IRQ_DOMAIN_HIERARCHY + select IRQ_MSI_LIB + select PCI_MSI + help + Support for the Sophgo SG2042 MSI Controller. + This on-chip interrupt controller enables MSI sources to be + routed to the primary PLIC controller on SoC. + config SUNPLUS_SP7021_INTC bool "Sunplus SP7021 interrupt controller" if COMPILE_TEST default SOC_SP7021 diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 25e9ad29b8c4..dd60e597491d 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -128,4 +128,5 @@ obj-$(CONFIG_WPCM450_AIC) += irq-wpcm450-aic.o obj-$(CONFIG_IRQ_IDT3243X) += irq-idt3243x.o obj-$(CONFIG_APPLE_AIC) += irq-apple-aic.o obj-$(CONFIG_MCHP_EIC) += irq-mchp-eic.o +obj-$(CONFIG_SOPHGO_SG2042_MSI) += irq-sg2042-msi.o obj-$(CONFIG_SUNPLUS_SP7021_INTC) += irq-sp7021-intc.o diff --git a/drivers/irqchip/irq-sg2042-msi.c b/drivers/irqchip/irq-sg2042-msi.c new file mode 100644 index 000000000000..495ee2b45eb2 --- /dev/null +++ b/drivers/irqchip/irq-sg2042-msi.c @@ -0,0 +1,285 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SG2042 MSI Controller + * + * Copyright (C) 2024 Sophgo Technology Inc. + * Copyright (C) 2024 Chen Wang <unicorn_wang@outlook.com> + */ + +#include <linux/cleanup.h> +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/msi.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_pci.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +#include "irq-msi-lib.h" + +#define SG2042_VECTOR_MIN 64 +#define SG2042_VECTOR_MAX 95 + +struct sg2042_msi_data { + void __iomem *reg_clr; // clear reg, see TRM, 10.1.33, GP_INTR0_CLR + + u64 doorbell_addr; // see TRM, 10.1.32, GP_INTR0_SET + + u32 irq_first; // The vector number that MSIs starts + u32 num_irqs; // The number of vectors for MSIs + + unsigned long *msi_map; + struct mutex msi_map_lock; // lock for msi_map +}; + +static int sg2042_msi_allocate_hwirq(struct sg2042_msi_data *priv, int num_req) +{ + int first; + + guard(mutex)(&priv->msi_map_lock); + first = bitmap_find_free_region(priv->msi_map, priv->num_irqs, + get_count_order(num_req)); + return first >= 0 ? priv->irq_first + first : -ENOSPC; +} + +static void sg2042_msi_free_hwirq(struct sg2042_msi_data *priv, + int hwirq, int num_req) +{ + int first = hwirq - priv->irq_first; + + guard(mutex)(&priv->msi_map_lock); + bitmap_release_region(priv->msi_map, first, get_count_order(num_req)); +} + +static void sg2042_msi_irq_ack(struct irq_data *d) +{ + struct sg2042_msi_data *data = irq_data_get_irq_chip_data(d); + int bit_off = d->hwirq - data->irq_first; + + writel(1 << bit_off, (unsigned int *)data->reg_clr); + + irq_chip_ack_parent(d); +} + +static void sg2042_msi_irq_compose_msi_msg(struct irq_data *data, + struct msi_msg *msg) +{ + struct sg2042_msi_data *priv = irq_data_get_irq_chip_data(data); + + msg->address_hi = upper_32_bits(priv->doorbell_addr); + msg->address_lo = lower_32_bits(priv->doorbell_addr); + msg->data = 1 << (data->hwirq - priv->irq_first); + + pr_debug("%s hwirq[%ld]: address_hi[%#x], address_lo[%#x], data[%#x]\n", + __func__, data->hwirq, msg->address_hi, msg->address_lo, msg->data); +} + +static struct irq_chip sg2042_msi_middle_irq_chip = { + .name = "SG2042 MSI", + .irq_ack = sg2042_msi_irq_ack, + .irq_mask = irq_chip_mask_parent, + .irq_unmask = irq_chip_unmask_parent, +#ifdef CONFIG_SMP + .irq_set_affinity = irq_chip_set_affinity_parent, +#endif + .irq_compose_msi_msg = sg2042_msi_irq_compose_msi_msg, +}; + +static int sg2042_msi_parent_domain_alloc(struct irq_domain *domain, + unsigned int virq, int hwirq) +{ + struct irq_fwspec fwspec; + struct irq_data *d; + int ret; + + fwspec.fwnode = domain->parent->fwnode; + fwspec.param_count = 2; + fwspec.param[0] = hwirq; + fwspec.param[1] = IRQ_TYPE_EDGE_RISING; + + ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec); + if (ret) + return ret; + + d = irq_domain_get_irq_data(domain->parent, virq); + return d->chip->irq_set_type(d, IRQ_TYPE_EDGE_RISING); +} + +static int sg2042_msi_middle_domain_alloc(struct irq_domain *domain, + unsigned int virq, + unsigned int nr_irqs, void *args) +{ + struct sg2042_msi_data *priv = domain->host_data; + int hwirq, err, i; + + hwirq = sg2042_msi_allocate_hwirq(priv, nr_irqs); + if (hwirq < 0) + return hwirq; + + for (i = 0; i < nr_irqs; i++) { + err = sg2042_msi_parent_domain_alloc(domain, virq + i, hwirq + i); + if (err) + goto err_hwirq; + + pr_debug("%s: virq[%d], hwirq[%d]\n", __func__, virq + i, hwirq + i); + + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, + &sg2042_msi_middle_irq_chip, priv); + } + + return 0; + +err_hwirq: + sg2042_msi_free_hwirq(priv, hwirq, nr_irqs); + irq_domain_free_irqs_parent(domain, virq, i); + + return err; +} + +static void sg2042_msi_middle_domain_free(struct irq_domain *domain, + unsigned int virq, + unsigned int nr_irqs) +{ + struct irq_data *d = irq_domain_get_irq_data(domain, virq); + struct sg2042_msi_data *priv = irq_data_get_irq_chip_data(d); + + irq_domain_free_irqs_parent(domain, virq, nr_irqs); + sg2042_msi_free_hwirq(priv, d->hwirq, nr_irqs); +} + +static const struct irq_domain_ops sg2042_msi_middle_domain_ops = { + .alloc = sg2042_msi_middle_domain_alloc, + .free = sg2042_msi_middle_domain_free, + .select = msi_lib_irq_domain_select, +}; + +#define SG2042_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \ + MSI_FLAG_USE_DEF_CHIP_OPS) + +#define SG2042_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK + +static struct msi_parent_ops sg2042_msi_parent_ops = { + .required_flags = SG2042_MSI_FLAGS_REQUIRED, + .supported_flags = SG2042_MSI_FLAGS_SUPPORTED, + .bus_select_mask = MATCH_PCI_MSI, + .bus_select_token = DOMAIN_BUS_NEXUS, + .prefix = "SG2042-", + .init_dev_msi_info = msi_lib_init_dev_msi_info, +}; + +static int sg2042_msi_init_domains(struct sg2042_msi_data *priv, + struct device_node *node) +{ + struct fwnode_handle *fwnode = of_node_to_fwnode(node); + struct irq_domain *plic_domain, *middle_domain; + struct device_node *plic_node; + + if (!of_find_property(node, "interrupt-parent", NULL)) { + pr_err("Can't find interrupt-parent!\n"); + return -EINVAL; + } + + plic_node = of_irq_find_parent(node); + if (!plic_node) { + pr_err("Failed to find the PLIC node!\n"); + return -ENXIO; + } + + plic_domain = irq_find_host(plic_node); + of_node_put(plic_node); + if (!plic_domain) { + pr_err("Failed to find the PLIC domain\n"); + return -ENXIO; + } + + middle_domain = irq_domain_create_hierarchy(plic_domain, 0, priv->num_irqs, + fwnode, + &sg2042_msi_middle_domain_ops, + priv); + if (!middle_domain) { + pr_err("Failed to create the MSI middle domain\n"); + return -ENOMEM; + } + + irq_domain_update_bus_token(middle_domain, DOMAIN_BUS_NEXUS); + + middle_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT; + middle_domain->msi_parent_ops = &sg2042_msi_parent_ops; + + return 0; +} + +static int sg2042_msi_probe(struct platform_device *pdev) +{ + struct of_phandle_args args = {}; + struct sg2042_msi_data *data; + int ret; + + data = devm_kzalloc(&pdev->dev, sizeof(struct sg2042_msi_data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->reg_clr = devm_platform_ioremap_resource_byname(pdev, "clr"); + if (IS_ERR(data->reg_clr)) { + dev_err(&pdev->dev, "Failed to map clear register\n"); + return PTR_ERR(data->reg_clr); + } + + if (of_property_read_u64(pdev->dev.of_node, "sophgo,msi-doorbell-addr", + &data->doorbell_addr)) { + dev_err(&pdev->dev, "Unable to parse MSI doorbell addr\n"); + return -EINVAL; + } + + ret = of_parse_phandle_with_args(pdev->dev.of_node, "msi-ranges", + "#interrupt-cells", 0, &args); + if (ret) { + dev_err(&pdev->dev, "Unable to parse MSI vec base\n"); + return ret; + } + data->irq_first = (u32)args.args[0]; + + ret = of_property_read_u32_index(pdev->dev.of_node, "msi-ranges", + args.args_count + 1, &data->num_irqs); + if (ret) { + dev_err(&pdev->dev, "Unable to parse MSI vec number\n"); + return ret; + } + + if (data->irq_first < SG2042_VECTOR_MIN || + (data->irq_first + data->num_irqs - 1) > SG2042_VECTOR_MAX) { + dev_err(&pdev->dev, "msi-ranges is incorrect!\n"); + return -EINVAL; + } + + mutex_init(&data->msi_map_lock); + + data->msi_map = bitmap_zalloc(data->num_irqs, GFP_KERNEL); + if (!data->msi_map) + return -ENOMEM; + + ret = sg2042_msi_init_domains(data, pdev->dev.of_node); + if (ret) + bitmap_free(data->msi_map); + + return ret; +} + +static const struct of_device_id sg2042_msi_of_match[] = { + { .compatible = "sophgo,sg2042-msi" }, + {} +}; + +static struct platform_driver sg2042_msi_driver = { + .driver = { + .name = "sg2042-msi", + .of_match_table = of_match_ptr(sg2042_msi_of_match), + }, + .probe = sg2042_msi_probe, +}; +builtin_platform_driver(sg2042_msi_driver);