On Sun, 31 Mar 2019 13:35:33 +0100,
Hanna Hawa <hhhawa@xxxxxxxxxx> wrote:
>
> This patch adds ACPI support for AL-MSIx driver.
>
> The AL-MSIx controller is not standard, is not included in the UEFI
> specification, and will not be added. The driver ACPI binding is
> performed when the following conditions are true:
> - OEM ID is AMAZON
> - MADT table type is 0x80 (part of the OEM reserved range).
>
> Signed-off-by: Hanna Hawa <hhhawa@xxxxxxxxxx>
> Co-developed-by: Vladimir Aerov <vaerov@xxxxxxxxxx>
> Signed-off-by: Vladimir Aerov <vaerov@xxxxxxxxxx>
> ---
> drivers/irqchip/irq-al-msi.c | 118 ++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 111 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-al-msi.c b/drivers/irqchip/irq-al-msi.c
> index ec27455..cb80c1e 100644
> --- a/drivers/irqchip/irq-al-msi.c
> +++ b/drivers/irqchip/irq-al-msi.c
> @@ -9,6 +9,7 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/acpi.h>
> #include <linux/dma-iommu.h>
> #include <linux/irqchip.h>
> #include <linux/irqchip/arm-gic.h>
> @@ -126,14 +127,20 @@ static int al_msix_gic_domain_alloc(struct irq_domain *domain,
> struct irq_data *d;
> int ret;
>
> - if (!is_of_node(domain->parent->fwnode))
> + if (is_of_node(domain->parent->fwnode)) {
> + fwspec.fwnode = domain->parent->fwnode;
> + fwspec.param_count = 3;
> + fwspec.param[0] = 0;
> + fwspec.param[1] = spi;
> + fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> + } else if (is_fwnode_irqchip(domain->parent->fwnode)) {
> + fwspec.fwnode = domain->parent->fwnode;
> + fwspec.param_count = 2;
> + fwspec.param[0] = spi + 32;
> + fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> + } else {
> return -EINVAL;
> -
> - fwspec.fwnode = domain->parent->fwnode;
> - fwspec.param_count = 3;
> - fwspec.param[0] = 0;
> - fwspec.param[1] = spi;
> - fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> + }
>
> ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
> if (ret)
> @@ -304,3 +311,100 @@ static int al_msix_init(struct device_node *node, struct device_node *parent)
> }
> IRQCHIP_DECLARE(alpine_msix, "al,alpine-msix", al_msix_init);
> IRQCHIP_DECLARE(al_msix, "amazon,al-msix", al_msix_init);
> +
> +#ifdef CONFIG_ACPI
> +static struct al_msix_data *priv;
> +
> +#define ACPI_AMZN_MADT_OEM_TYPE 0x80
> +#define ACPI_AMZN_OEM_ID "AMAZON"
> +
> +struct acpi_madt_msix_oem_frame {
> + struct acpi_subtable_header header;
> + u64 base_address;
> + u32 base_address_len;
> + u16 spi_count;
> + u16 spi_base;
> +};
> +
> +static struct fwnode_handle *al_msi_get_fwnode(struct device *dev)
> +{
> + return priv->msi_domain_handle;
> +}
> +
> +static int __init al_msix_acpi_probe(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + struct irq_domain *gic_domain;
> + struct fwnode_handle *gic_domain_handle;
> + struct acpi_madt_msix_oem_frame *m;
> + int ret;
> +
> + m = container_of(header, struct acpi_madt_msix_oem_frame, header);
> + if (BAD_MADT_ENTRY(m, end))
> + return -EINVAL;
> +
> + gic_domain_handle = acpi_get_gsi_domain_id();
> + if (!gic_domain_handle) {
> + pr_err("Failed to find the GIC domain handle\n");
> + return -ENXIO;
> + }
> +
> + gic_domain = irq_find_matching_fwnode(gic_domain_handle,
> + DOMAIN_BUS_ANY);
> + if (!gic_domain) {
> + pr_err("Failed to find the GIC domain\n");
> + return -ENXIO;
> + }
Why do you do this in the iterator? It won't change over time, right?
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->spi_first = m->spi_base;
> + priv->num_spis = m->spi_count;
> +
> + priv->msi_domain_handle = irq_domain_alloc_fwnode((void *)
> + m->base_address);
> + if (!priv->msi_domain_handle) {
> + pr_err("Unable to allocate msi domain token\n");
> + ret = -EINVAL;
> + goto err_acpi_priv;
> + }
> +
> + ret = al_msix_init_common(priv, m->base_address);
> + if (ret)
> + goto err_acpi_priv;
> +
> + ret = al_msix_init_domains(priv, gic_domain);
> + if (ret)
> + goto err_acpi_map;
> +
> + pci_msi_register_fwnode_provider(&al_msi_get_fwnode);
> +
> + return 0;
> +
> +err_acpi_map:
> + kfree(priv->msi_map);
> +err_acpi_priv:
> + kfree(priv);
> + return ret;
> +}
> +
> +static int __init al_msix_acpi_init(void)
> +{
> + static struct acpi_table_madt *madt;
> + acpi_status status;
> +
> + /* if ACPI MADT table is not Amazon defined return */
> + status = acpi_get_table(ACPI_SIG_MADT, 0,
> + (struct acpi_table_header **)&madt);
> + if (ACPI_FAILURE(status) || (madt && memcmp(madt->header.oem_id,
> + ACPI_AMZN_OEM_ID,
> + ACPI_OEM_ID_SIZE)))
> + return -ENODEV;
> +
> + return acpi_table_parse_madt(ACPI_AMZN_MADT_OEM_TYPE,
> + al_msix_acpi_probe, 0);
> +}
> +early_initcall(al_msix_acpi_init);
These initcalls are not maintainable in the long run. Either this is
part of the core ACPI discovery (IRQCHIP_ACPI_DECLARE), or it should
use another probing method. If you need to express dependencies, make
it an actual device driver (I suspect you're in complete control of
the FW anyway), which would make a lot more sense.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
