2019-03-31 12:35:57

by Hanna Hawa

[permalink] [raw]
Subject: [PATCH 0/7] al-msi: Rename driver and add support for ACPI

This series includes three major changes:
1. IOMMU DMA mapping MSI message fix.
2. Re-name the AL-MSIx driver to new name convention.
3. Add ACPI support for the driver.

Alpine is the name of the SoC family, while AL stands for Annapurna
Labs. Rename to the latter since the driver will appear in other SoC
families other than Alpine.

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).

GICv2m driver is called from context of parent interrupt controller,
which ensures that the parent interrupt domain exists and holds valid
information. As calling AL-MSIx driver from GICv3 driver would not make
sense, a new API was added, to get the GSI IRQ domain that was registered
by GICv3 driver in the ACPI framework.

Hanna Hawa (7):
irqchip/alpine-msi: Call IOMMU DMA mapping MSI message hook
irqchip/alpine-msi: Update driver license to use SPDX
irqchip/al-msi: Rename AL-MSIx driver
irqchip/al-msi: Update wrong parameter naming
ACPI / irq: Add GSI IRQ domain getter function
irqchip/al-msi: Refactor in preparation to add ACPI support
irqchip/al-msi: Add ACPI support

arch/arm/mach-alpine/Kconfig | 2 +-
arch/arm64/Kconfig.platforms | 2 +-
drivers/acpi/irq.c | 13 +
drivers/irqchip/Kconfig | 2 +-
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/{irq-alpine-msi.c => irq-al-msi.c} | 320 ++++++++++++++-------
include/linux/acpi.h | 1 +
7 files changed, 236 insertions(+), 106 deletions(-)
rename drivers/irqchip/{irq-alpine-msi.c => irq-al-msi.c} (40%)

--
2.7.4



2019-03-31 12:36:06

by Hanna Hawa

[permalink] [raw]
Subject: [PATCH 1/7] irqchip/alpine-msi: Call IOMMU DMA mapping MSI message hook

Add missing IOMMU DMA mapping MSI message hook in compose_msi_msg()
callback for Alpine MSIx driver, to map the MSI physical address to
IOMMU IOVA for any device attached to IOMMU DMA ops domains.

Signed-off-by: Hanna Hawa <[email protected]>
Co-developed-by: Vladimir Aerov <[email protected]>
Signed-off-by: Vladimir Aerov <[email protected]>
---
drivers/irqchip/irq-alpine-msi.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/irqchip/irq-alpine-msi.c b/drivers/irqchip/irq-alpine-msi.c
index 23a3b87..ae2fca7 100644
--- a/drivers/irqchip/irq-alpine-msi.c
+++ b/drivers/irqchip/irq-alpine-msi.c
@@ -12,6 +12,7 @@

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <linux/dma-iommu.h>
#include <linux/irqchip.h>
#include <linux/irqchip/arm-gic.h>
#include <linux/msi.h>
@@ -99,6 +100,12 @@ static void alpine_msix_compose_msi_msg(struct irq_data *data,
msg->address_hi = upper_32_bits(msg_addr);
msg->address_lo = lower_32_bits(msg_addr);
msg->data = 0;
+
+ /*
+ * Mapping MSI address to IOMMU IOVA, relevant for devices associated
+ * with IOMMU DMA ops domain.
+ */
+ iommu_dma_map_msi_msg(data->irq, msg);
}

static struct msi_domain_info alpine_msix_domain_info = {
--
2.7.4


2019-03-31 12:36:23

by Hanna Hawa

[permalink] [raw]
Subject: [PATCH 2/7] irqchip/alpine-msi: Update driver license to use SPDX

Update driver license to be in-line with Linux conventions.

Signed-off-by: Hanna Hawa <[email protected]>
---
drivers/irqchip/irq-alpine-msi.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-alpine-msi.c b/drivers/irqchip/irq-alpine-msi.c
index ae2fca7..ec6a606 100644
--- a/drivers/irqchip/irq-alpine-msi.c
+++ b/drivers/irqchip/irq-alpine-msi.c
@@ -1,13 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Annapurna Labs MSIX support services
*
* Copyright (C) 2016, Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Antoine Tenart <[email protected]>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
*/

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
--
2.7.4


2019-03-31 12:36:47

by Hanna Hawa

[permalink] [raw]
Subject: [PATCH 3/7] irqchip/al-msi: Rename AL-MSIx driver

Alpine is the name of the SoC family, while AL stands for Annapurna
Labs. Rename to the latter since the driver will appear in other SoC
families other than Alpine.

Add device tree match for "amazon/al-msix".

Signed-off-by: Hanna Hawa <[email protected]>
---
arch/arm/mach-alpine/Kconfig | 2 +-
arch/arm64/Kconfig.platforms | 2 +-
drivers/irqchip/Kconfig | 2 +-
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/{irq-alpine-msi.c => irq-al-msi.c} | 93 +++++++++++-----------
5 files changed, 49 insertions(+), 52 deletions(-)
rename drivers/irqchip/{irq-alpine-msi.c => irq-al-msi.c} (68%)

diff --git a/arch/arm/mach-alpine/Kconfig b/arch/arm/mach-alpine/Kconfig
index bc04c91..2d9879a 100644
--- a/arch/arm/mach-alpine/Kconfig
+++ b/arch/arm/mach-alpine/Kconfig
@@ -2,7 +2,7 @@
config ARCH_ALPINE
bool "Annapurna Labs Alpine platform"
depends on ARCH_MULTI_V7
- select ALPINE_MSI
+ select AL_MSI
select ARM_AMBA
select ARM_GIC
select GENERIC_IRQ_CHIP
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 251ecf3..e3e4c3c 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -18,7 +18,7 @@ config ARCH_SUNXI

config ARCH_ALPINE
bool "Annapurna Labs Alpine platform"
- select ALPINE_MSI if PCI
+ select AL_MSI if PCI
help
This enables support for the Annapurna Labs Alpine
Soc family.
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 5dcb545..79683cd 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -83,7 +83,7 @@ config ARMADA_370_XP_IRQ
select PCI_MSI if PCI
select GENERIC_IRQ_EFFECTIVE_AFF_MASK

-config ALPINE_MSI
+config AL_MSI
bool
depends on PCI
select PCI_MSI
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 7acd0e3..c718ffc 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_IRQCHIP) += irqchip.o

-obj-$(CONFIG_ALPINE_MSI) += irq-alpine-msi.o
+obj-$(CONFIG_AL_MSI) += irq-al-msi.o
obj-$(CONFIG_ATH79) += irq-ath79-cpu.o
obj-$(CONFIG_ATH79) += irq-ath79-misc.o
obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
diff --git a/drivers/irqchip/irq-alpine-msi.c b/drivers/irqchip/irq-al-msi.c
similarity index 68%
rename from drivers/irqchip/irq-alpine-msi.c
rename to drivers/irqchip/irq-al-msi.c
index ec6a606..a1bbefc 100644
--- a/drivers/irqchip/irq-alpine-msi.c
+++ b/drivers/irqchip/irq-al-msi.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Annapurna Labs MSIX support services
+ * Amazon's Annapurna Labs MSIX support services.
*
* Copyright (C) 2016, Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
@@ -21,12 +21,11 @@
#include <linux/slab.h>

#include <asm/irq.h>
-#include <asm/msi.h>

/* MSIX message address format: local GIC target */
-#define ALPINE_MSIX_SPI_TARGET_CLUSTER0 BIT(16)
+#define AL_MSIX_SPI_TARGET_CLUSTER0 BIT(16)

-struct alpine_msix_data {
+struct al_msix_data {
spinlock_t msi_map_lock;
phys_addr_t addr;
u32 spi_first; /* The SGI number that MSIs start */
@@ -34,27 +33,27 @@ struct alpine_msix_data {
unsigned long *msi_map;
};

-static void alpine_msix_mask_msi_irq(struct irq_data *d)
+static void al_msix_mask_msi_irq(struct irq_data *d)
{
pci_msi_mask_irq(d);
irq_chip_mask_parent(d);
}

-static void alpine_msix_unmask_msi_irq(struct irq_data *d)
+static void al_msix_unmask_msi_irq(struct irq_data *d)
{
pci_msi_unmask_irq(d);
irq_chip_unmask_parent(d);
}

-static struct irq_chip alpine_msix_irq_chip = {
+static struct irq_chip al_msix_irq_chip = {
.name = "MSIx",
- .irq_mask = alpine_msix_mask_msi_irq,
- .irq_unmask = alpine_msix_unmask_msi_irq,
+ .irq_mask = al_msix_mask_msi_irq,
+ .irq_unmask = al_msix_unmask_msi_irq,
.irq_eoi = irq_chip_eoi_parent,
.irq_set_affinity = irq_chip_set_affinity_parent,
};

-static int alpine_msix_allocate_sgi(struct alpine_msix_data *priv, int num_req)
+static int al_msix_allocate_sgi(struct al_msix_data *priv, int num_req)
{
int first;

@@ -74,8 +73,8 @@ static int alpine_msix_allocate_sgi(struct alpine_msix_data *priv, int num_req)
return priv->spi_first + first;
}

-static void alpine_msix_free_sgi(struct alpine_msix_data *priv, unsigned sgi,
- int num_req)
+static void al_msix_free_sgi(struct al_msix_data *priv, unsigned int sgi,
+ int num_req)
{
int first = sgi - priv->spi_first;

@@ -86,10 +85,9 @@ static void alpine_msix_free_sgi(struct alpine_msix_data *priv, unsigned sgi,
spin_unlock(&priv->msi_map_lock);
}

-static void alpine_msix_compose_msi_msg(struct irq_data *data,
- struct msi_msg *msg)
+static void al_msix_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
{
- struct alpine_msix_data *priv = irq_data_get_irq_chip_data(data);
+ struct al_msix_data *priv = irq_data_get_irq_chip_data(data);
phys_addr_t msg_addr = priv->addr;

msg_addr |= (data->hwirq << 3);
@@ -105,23 +103,23 @@ static void alpine_msix_compose_msi_msg(struct irq_data *data,
iommu_dma_map_msi_msg(data->irq, msg);
}

-static struct msi_domain_info alpine_msix_domain_info = {
+static struct msi_domain_info al_msix_domain_info = {
.flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
MSI_FLAG_PCI_MSIX,
- .chip = &alpine_msix_irq_chip,
+ .chip = &al_msix_irq_chip,
};

static struct irq_chip middle_irq_chip = {
- .name = "alpine_msix_middle",
+ .name = "al_msix_middle",
.irq_mask = irq_chip_mask_parent,
.irq_unmask = irq_chip_unmask_parent,
.irq_eoi = irq_chip_eoi_parent,
.irq_set_affinity = irq_chip_set_affinity_parent,
- .irq_compose_msi_msg = alpine_msix_compose_msi_msg,
+ .irq_compose_msi_msg = al_msix_compose_msi_msg,
};

-static int alpine_msix_gic_domain_alloc(struct irq_domain *domain,
- unsigned int virq, int sgi)
+static int al_msix_gic_domain_alloc(struct irq_domain *domain,
+ unsigned int virq, int sgi)
{
struct irq_fwspec fwspec;
struct irq_data *d;
@@ -146,19 +144,19 @@ static int alpine_msix_gic_domain_alloc(struct irq_domain *domain,
return 0;
}

-static int alpine_msix_middle_domain_alloc(struct irq_domain *domain,
- unsigned int virq,
- unsigned int nr_irqs, void *args)
+static int al_msix_middle_domain_alloc(struct irq_domain *domain,
+ unsigned int virq,
+ unsigned int nr_irqs, void *args)
{
- struct alpine_msix_data *priv = domain->host_data;
+ struct al_msix_data *priv = domain->host_data;
int sgi, err, i;

- sgi = alpine_msix_allocate_sgi(priv, nr_irqs);
+ sgi = al_msix_allocate_sgi(priv, nr_irqs);
if (sgi < 0)
return sgi;

for (i = 0; i < nr_irqs; i++) {
- err = alpine_msix_gic_domain_alloc(domain, virq + i, sgi + i);
+ err = al_msix_gic_domain_alloc(domain, virq + i, sgi + i);
if (err)
goto err_sgi;

@@ -171,28 +169,28 @@ static int alpine_msix_middle_domain_alloc(struct irq_domain *domain,
err_sgi:
while (--i >= 0)
irq_domain_free_irqs_parent(domain, virq, i);
- alpine_msix_free_sgi(priv, sgi, nr_irqs);
+ al_msix_free_sgi(priv, sgi, nr_irqs);
return err;
}

-static void alpine_msix_middle_domain_free(struct irq_domain *domain,
- unsigned int virq,
- unsigned int nr_irqs)
+static void al_msix_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 alpine_msix_data *priv = irq_data_get_irq_chip_data(d);
+ struct al_msix_data *priv = irq_data_get_irq_chip_data(d);

irq_domain_free_irqs_parent(domain, virq, nr_irqs);
- alpine_msix_free_sgi(priv, d->hwirq, nr_irqs);
+ al_msix_free_sgi(priv, d->hwirq, nr_irqs);
}

-static const struct irq_domain_ops alpine_msix_middle_domain_ops = {
- .alloc = alpine_msix_middle_domain_alloc,
- .free = alpine_msix_middle_domain_free,
+static const struct irq_domain_ops al_msix_middle_domain_ops = {
+ .alloc = al_msix_middle_domain_alloc,
+ .free = al_msix_middle_domain_free,
};

-static int alpine_msix_init_domains(struct alpine_msix_data *priv,
- struct device_node *node)
+static int al_msix_init_domains(struct al_msix_data *priv,
+ struct device_node *node)
{
struct irq_domain *middle_domain, *msi_domain, *gic_domain;
struct device_node *gic_node;
@@ -209,8 +207,7 @@ static int alpine_msix_init_domains(struct alpine_msix_data *priv,
return -ENXIO;
}

- middle_domain = irq_domain_add_tree(NULL,
- &alpine_msix_middle_domain_ops,
+ middle_domain = irq_domain_add_tree(NULL, &al_msix_middle_domain_ops,
priv);
if (!middle_domain) {
pr_err("Failed to create the MSIX middle domain\n");
@@ -220,7 +217,7 @@ static int alpine_msix_init_domains(struct alpine_msix_data *priv,
middle_domain->parent = gic_domain;

msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(node),
- &alpine_msix_domain_info,
+ &al_msix_domain_info,
middle_domain);
if (!msi_domain) {
pr_err("Failed to create MSI domain\n");
@@ -231,10 +228,9 @@ static int alpine_msix_init_domains(struct alpine_msix_data *priv,
return 0;
}

-static int alpine_msix_init(struct device_node *node,
- struct device_node *parent)
+static int al_msix_init(struct device_node *node, struct device_node *parent)
{
- struct alpine_msix_data *priv;
+ struct al_msix_data *priv;
struct resource res;
int ret;

@@ -257,8 +253,8 @@ static int alpine_msix_init(struct device_node *node,
* To select the primary GIC as the target GIC, bits [18:17] must be set
* to 0x0. In this case, bit 16 (SPI_TARGET_CLUSTER0) must be set.
*/
- priv->addr = res.start & GENMASK_ULL(63,20);
- priv->addr |= ALPINE_MSIX_SPI_TARGET_CLUSTER0;
+ priv->addr = res.start & GENMASK_ULL(63, 20);
+ priv->addr |= AL_MSIX_SPI_TARGET_CLUSTER0;

if (of_property_read_u32(node, "al,msi-base-spi", &priv->spi_first)) {
pr_err("Unable to parse MSI base\n");
@@ -283,7 +279,7 @@ static int alpine_msix_init(struct device_node *node,
pr_debug("Registering %d msixs, starting at %d\n",
priv->num_spis, priv->spi_first);

- ret = alpine_msix_init_domains(priv, node);
+ ret = al_msix_init_domains(priv, node);
if (ret)
goto err_map;

@@ -295,4 +291,5 @@ static int alpine_msix_init(struct device_node *node,
kfree(priv);
return ret;
}
-IRQCHIP_DECLARE(alpine_msix, "al,alpine-msix", alpine_msix_init);
+IRQCHIP_DECLARE(alpine_msix, "al,alpine-msix", al_msix_init);
+IRQCHIP_DECLARE(al_msix, "amazon,al-msix", al_msix_init);
--
2.7.4


2019-03-31 12:37:02

by Hanna Hawa

[permalink] [raw]
Subject: [PATCH 4/7] irqchip/al-msi: Update wrong parameter naming

Update the naming of sgi parameter to be spi, and fix comments.

The message interrupts target the SPIs in the target GIC, and not the
SGIs.

No functional change in this patch.

Signed-off-by: Hanna Hawa <[email protected]>
---
drivers/irqchip/irq-al-msi.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/irqchip/irq-al-msi.c b/drivers/irqchip/irq-al-msi.c
index a1bbefc..f04a311c 100644
--- a/drivers/irqchip/irq-al-msi.c
+++ b/drivers/irqchip/irq-al-msi.c
@@ -28,8 +28,8 @@
struct al_msix_data {
spinlock_t msi_map_lock;
phys_addr_t addr;
- u32 spi_first; /* The SGI number that MSIs start */
- u32 num_spis; /* The number of SGIs for MSIs */
+ u32 spi_first; /* The SPI number that MSIs start */
+ u32 num_spis; /* The number of SPIs for MSIs */
unsigned long *msi_map;
};

@@ -53,7 +53,7 @@ static struct irq_chip al_msix_irq_chip = {
.irq_set_affinity = irq_chip_set_affinity_parent,
};

-static int al_msix_allocate_sgi(struct al_msix_data *priv, int num_req)
+static int al_msix_allocate_spi(struct al_msix_data *priv, int num_req)
{
int first;

@@ -73,10 +73,10 @@ static int al_msix_allocate_sgi(struct al_msix_data *priv, int num_req)
return priv->spi_first + first;
}

-static void al_msix_free_sgi(struct al_msix_data *priv, unsigned int sgi,
+static void al_msix_free_spi(struct al_msix_data *priv, unsigned int spi,
int num_req)
{
- int first = sgi - priv->spi_first;
+ int first = spi - priv->spi_first;

spin_lock(&priv->msi_map_lock);

@@ -119,7 +119,7 @@ static struct irq_chip middle_irq_chip = {
};

static int al_msix_gic_domain_alloc(struct irq_domain *domain,
- unsigned int virq, int sgi)
+ unsigned int virq, int spi)
{
struct irq_fwspec fwspec;
struct irq_data *d;
@@ -131,7 +131,7 @@ static int al_msix_gic_domain_alloc(struct irq_domain *domain,
fwspec.fwnode = domain->parent->fwnode;
fwspec.param_count = 3;
fwspec.param[0] = 0;
- fwspec.param[1] = sgi;
+ fwspec.param[1] = spi;
fwspec.param[2] = IRQ_TYPE_EDGE_RISING;

ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
@@ -149,27 +149,27 @@ static int al_msix_middle_domain_alloc(struct irq_domain *domain,
unsigned int nr_irqs, void *args)
{
struct al_msix_data *priv = domain->host_data;
- int sgi, err, i;
+ int spi, err, i;

- sgi = al_msix_allocate_sgi(priv, nr_irqs);
- if (sgi < 0)
- return sgi;
+ spi = al_msix_allocate_spi(priv, nr_irqs);
+ if (spi < 0)
+ return spi;

for (i = 0; i < nr_irqs; i++) {
- err = al_msix_gic_domain_alloc(domain, virq + i, sgi + i);
+ err = al_msix_gic_domain_alloc(domain, virq + i, spi + i);
if (err)
- goto err_sgi;
+ goto err_spi;

- irq_domain_set_hwirq_and_chip(domain, virq + i, sgi + i,
+ irq_domain_set_hwirq_and_chip(domain, virq + i, spi + i,
&middle_irq_chip, priv);
}

return 0;

-err_sgi:
+err_spi:
while (--i >= 0)
irq_domain_free_irqs_parent(domain, virq, i);
- al_msix_free_sgi(priv, sgi, nr_irqs);
+ al_msix_free_spi(priv, spi, nr_irqs);
return err;
}

@@ -181,7 +181,7 @@ static void al_msix_middle_domain_free(struct irq_domain *domain,
struct al_msix_data *priv = irq_data_get_irq_chip_data(d);

irq_domain_free_irqs_parent(domain, virq, nr_irqs);
- al_msix_free_sgi(priv, d->hwirq, nr_irqs);
+ al_msix_free_spi(priv, d->hwirq, nr_irqs);
}

static const struct irq_domain_ops al_msix_middle_domain_ops = {
--
2.7.4


2019-03-31 12:48:17

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH 2/7] irqchip/alpine-msi: Update driver license to use SPDX


On 3/31/2019 6:04 PM, Hanna Hawa wrote:
> Update driver license to be in-line with Linux conventions.
>
> Signed-off-by: Hanna Hawa <[email protected]>

> ---
> drivers/irqchip/irq-alpine-msi.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-alpine-msi.c b/drivers/irqchip/irq-alpine-msi.c
> index ae2fca7..ec6a606 100644
> --- a/drivers/irqchip/irq-alpine-msi.c
> +++ b/drivers/irqchip/irq-alpine-msi.c
> @@ -1,13 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0


Please fix this as per.
https://lkml.org/lkml/2019/2/13/570


cheers,
Mukesh

> /*
> * Annapurna Labs MSIX support services
> *
> * Copyright (C) 2016, Amazon.com, Inc. or its affiliates. All Rights Reserved.
> *
> * Antoine Tenart <[email protected]>
> - *
> - * This file is licensed under the terms of the GNU General Public
> - * License version 2. This program is licensed "as is" without any
> - * warranty of any kind, whether express or implied.
> */
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

2019-03-31 13:02:26

by Hanna Hawa

[permalink] [raw]
Subject: Re: [PATCH 2/7] irqchip/alpine-msi: Update driver license to use SPDX



On 3/31/2019 3:46 PM, Mukesh Ojha wrote:
>
> On 3/31/2019 6:04 PM, Hanna Hawa wrote:
>> Update driver license to be in-line with Linux conventions.
>>
>> Signed-off-by: Hanna Hawa <[email protected]>
>
>> ---
>>   drivers/irqchip/irq-alpine-msi.c | 5 +----
>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-alpine-msi.c
>> b/drivers/irqchip/irq-alpine-msi.c
>> index ae2fca7..ec6a606 100644
>> --- a/drivers/irqchip/irq-alpine-msi.c
>> +++ b/drivers/irqchip/irq-alpine-msi.c
>> @@ -1,13 +1,10 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
>
> Please fix this as per.
> https://lkml.org/lkml/2019/2/13/570
>
I'll fix in next patch-set, I'll wait for more inputs on other patches
in the series.

Thanks,
Hanna
>
> cheers,
> Mukesh
>
>>   /*
>>    * Annapurna Labs MSIX support services
>>    *
>>    * Copyright (C) 2016, Amazon.com, Inc. or its affiliates. All
>> Rights Reserved.
>>    *
>>    * Antoine Tenart <[email protected]>
>> - *
>> - * This file is licensed under the terms of the GNU General Public
>> - * License version 2. This program is licensed "as is" without any
>> - * warranty of any kind, whether express or implied.
>>    */
>>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

2019-04-01 01:50:23

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH 3/7] irqchip/al-msi: Rename AL-MSIx driver

On Sun, 31 Mar 2019 13:34:11 +0100,
Hanna Hawa <[email protected]> wrote:
>
> Alpine is the name of the SoC family, while AL stands for Annapurna
> Labs. Rename to the latter since the driver will appear in other SoC
> families other than Alpine.
>
> Add device tree match for "amazon/al-msix".
>
> Signed-off-by: Hanna Hawa <[email protected]>
> ---
> arch/arm/mach-alpine/Kconfig | 2 +-
> arch/arm64/Kconfig.platforms | 2 +-
> drivers/irqchip/Kconfig | 2 +-
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/{irq-alpine-msi.c => irq-al-msi.c} | 93 +++++++++++-----------
> 5 files changed, 49 insertions(+), 52 deletions(-)
> rename drivers/irqchip/{irq-alpine-msi.c => irq-al-msi.c} (68%)

Why do we need this churn? It feels absolutely pointless. Adding your
new compatibility string is absolutely fine; repainting the whole
driver because you've found a "better" name for it isn't.

Thanks,

M.

--
Jazz is not dead, it just smell funny.

2019-04-01 02:03:56

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH 0/7] al-msi: Rename driver and add support for ACPI

On Sun, 31 Mar 2019 13:34:08 +0100,
Hanna Hawa <[email protected]> wrote:
>
> This series includes three major changes:
> 1. IOMMU DMA mapping MSI message fix.
> 2. Re-name the AL-MSIx driver to new name convention.
> 3. Add ACPI support for the driver.
>
> Alpine is the name of the SoC family, while AL stands for Annapurna
> Labs. Rename to the latter since the driver will appear in other SoC
> families other than Alpine.
>
> 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).

[+Lorenzo, as the arm64 ACPI maintainer]

So you're happy to explicitly violate the letter of the specification?
That's not really going to fly. We've pushed back on such things in
the past (MBIGEN, XGene MSI controller), and I don't see any
compelling reason to change our tune.

> GICv2m driver is called from context of parent interrupt controller,
> which ensures that the parent interrupt domain exists and holds valid
> information. As calling AL-MSIx driver from GICv3 driver would not make
> sense, a new API was added, to get the GSI IRQ domain that was registered
> by GICv3 driver in the ACPI framework.

What does this mean? Either your system has a GICv2m or it has a
GICv3. Please explain what this is all about.

Thanks,

M.

--
Jazz is not dead, it just smell funny.

2019-04-01 11:54:52

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH 2/7] irqchip/alpine-msi: Update driver license to use SPDX

On Sun, 2019-03-31 at 18:16 +0530, Mukesh Ojha wrote:
> On 3/31/2019 6:04 PM, Hanna Hawa wrote:
> > Update driver license to be in-line with Linux conventions.
> >
> > Signed-off-by: Hanna Hawa <[email protected]>
> > ---
> > drivers/irqchip/irq-alpine-msi.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-alpine-msi.c
> > b/drivers/irqchip/irq-alpine-msi.c
> > index ae2fca7..ec6a606 100644
> > --- a/drivers/irqchip/irq-alpine-msi.c
> > +++ b/drivers/irqchip/irq-alpine-msi.c
> > @@ -1,13 +1,10 @@
> > +// SPDX-License-Identifier: GPL-2.0
>
>
> Please fix this as per.
> https://lkml.org/lkml/2019/2/13/570

I find myself struggling to care. And that was even before I spotted:

$ grep -A6 "Style" Documentation/process/license-rules.rst
2. Style:

The SPDX license identifier is added in form of a comment. The comment
style depends on the file type::

C source: // SPDX-License-Identifier: <SPDX License Expression>
C header: /* SPDX-License-Identifier: <SPDX License Expression> */



Attachments:
smime.p7s (5.05 kB)