* [PATCH 1/1] irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum
@ 2015-10-02 22:16 Duc Dang
2015-10-03 13:26 ` Marc Zyngier
0 siblings, 1 reply; 6+ messages in thread
From: Duc Dang @ 2015-10-02 22:16 UTC (permalink / raw)
To: Marc Zyngier, Suravee Suthikulpanit, Jason Cooper, Thomas Gleixner
Cc: linux-kernel, linux-arm, Duc Dang
APM X-Gene GICv2m implementation has an erratum where the
MSI data needs to be the offset from the spi_start in order to
trigger the correct MSI interrupt. This is different from the
standard GICv2m implementation where the MSI data is the absolute
value within the range from spi_start to (spi_start + num_spis)
of each v2m frame.
This patch reads MSI_IIDR register (present in all GICv2m
implementations) to identify X-Gene GICv2m implementation and
apply workaround to change the data portion of MSI vector.
Signed-off-by: Duc Dang <dhdang@apm.com>
---
drivers/irqchip/irq-gic-v2m.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index db04fc1..470972c 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -43,6 +43,10 @@
#define V2M_MSI_TYPER_NUM_SPI(x) ((x) & V2M_MSI_TYPER_NUM_MASK)
+#define V2M_MSI_IIDR 0xFCC
+/* APM X-Gene with GICv2m MSI_IIDR register value */
+#define XGENE_GICV2M_MSI_IIDR 0x06000170
+
struct v2m_data {
spinlock_t msi_cnt_lock;
struct resource res; /* GICv2m resource */
@@ -98,6 +102,16 @@ static void gicv2m_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
msg->address_hi = (u32) (addr >> 32);
msg->address_lo = (u32) (addr);
msg->data = data->hwirq;
+ /*
+ * APM X-Gene GICv2m implementation has an erratum where
+ * the MSI data needs to be the offset from the spi_start
+ * in order to trigger the correct MSI interrupt. This is
+ * different from the standard GICv2m implementation where
+ * the MSI data is the absolute value within the range from
+ * spi_start to (spi_start + num_spis).
+ */
+ if (readl_relaxed(v2m->base + V2M_MSI_IIDR) == XGENE_GICV2M_MSI_IIDR)
+ msg->data = data->hwirq - v2m->spi_start;
}
static struct irq_chip gicv2m_irq_chip = {
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum
2015-10-02 22:16 [PATCH 1/1] irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum Duc Dang
@ 2015-10-03 13:26 ` Marc Zyngier
2015-10-06 22:30 ` Duc Dang
2015-10-06 22:32 ` [PATCH v2 " Duc Dang
0 siblings, 2 replies; 6+ messages in thread
From: Marc Zyngier @ 2015-10-03 13:26 UTC (permalink / raw)
To: Duc Dang
Cc: Suravee Suthikulpanit, Jason Cooper, Thomas Gleixner,
linux-kernel, linux-arm
On Fri, 2 Oct 2015 15:16:49 -0700
Duc Dang <dhdang@apm.com> wrote:
Hi Duc,
> APM X-Gene GICv2m implementation has an erratum where the
> MSI data needs to be the offset from the spi_start in order to
> trigger the correct MSI interrupt. This is different from the
> standard GICv2m implementation where the MSI data is the absolute
> value within the range from spi_start to (spi_start + num_spis)
> of each v2m frame.
>
> This patch reads MSI_IIDR register (present in all GICv2m
> implementations) to identify X-Gene GICv2m implementation and
> apply workaround to change the data portion of MSI vector.
>
> Signed-off-by: Duc Dang <dhdang@apm.com>
> ---
> drivers/irqchip/irq-gic-v2m.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> index db04fc1..470972c 100644
> --- a/drivers/irqchip/irq-gic-v2m.c
> +++ b/drivers/irqchip/irq-gic-v2m.c
> @@ -43,6 +43,10 @@
>
> #define V2M_MSI_TYPER_NUM_SPI(x) ((x) & V2M_MSI_TYPER_NUM_MASK)
>
> +#define V2M_MSI_IIDR 0xFCC
Please group this with the rest of the registers.
> +/* APM X-Gene with GICv2m MSI_IIDR register value */
> +#define XGENE_GICV2M_MSI_IIDR 0x06000170
> +
Is this value guaranteed to only identify the affected implementation?
Do we have strong guarantees that a fixed implementation will not have
this value?
If not, I'd strongly suggest using a different method (compatible
string).
> struct v2m_data {
> spinlock_t msi_cnt_lock;
> struct resource res; /* GICv2m resource */
> @@ -98,6 +102,16 @@ static void gicv2m_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> msg->address_hi = (u32) (addr >> 32);
> msg->address_lo = (u32) (addr);
> msg->data = data->hwirq;
> + /*
> + * APM X-Gene GICv2m implementation has an erratum where
> + * the MSI data needs to be the offset from the spi_start
> + * in order to trigger the correct MSI interrupt. This is
> + * different from the standard GICv2m implementation where
> + * the MSI data is the absolute value within the range from
> + * spi_start to (spi_start + num_spis).
> + */
> + if (readl_relaxed(v2m->base + V2M_MSI_IIDR) == XGENE_GICV2M_MSI_IIDR)
> + msg->data = data->hwirq - v2m->spi_start;
> }
>
> static struct irq_chip gicv2m_irq_chip = {
Please add a set of flags to struct v2m_data as well as a flag
(GICV2M_NEEDS_SPI_OFFSET?) that can be checked in the compose method
(we don't needs to read the IIDR register each time we program an MSI):
if (v2m->flags & GICV2M_NEEDS_SPI_OFFSET)
msg->data -= v2m->spi_start;
You can set that flag from the probe function.
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum
2015-10-03 13:26 ` Marc Zyngier
@ 2015-10-06 22:30 ` Duc Dang
2015-10-06 22:32 ` [PATCH v2 " Duc Dang
1 sibling, 0 replies; 6+ messages in thread
From: Duc Dang @ 2015-10-06 22:30 UTC (permalink / raw)
To: Marc Zyngier
Cc: Suravee Suthikulpanit, Jason Cooper, Thomas Gleixner,
Linux Kernel Mailing List, linux-arm
On Sat, Oct 3, 2015 at 6:26 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On Fri, 2 Oct 2015 15:16:49 -0700
> Duc Dang <dhdang@apm.com> wrote:
>
> Hi Duc,
>
>> APM X-Gene GICv2m implementation has an erratum where the
>> MSI data needs to be the offset from the spi_start in order to
>> trigger the correct MSI interrupt. This is different from the
>> standard GICv2m implementation where the MSI data is the absolute
>> value within the range from spi_start to (spi_start + num_spis)
>> of each v2m frame.
>>
>> This patch reads MSI_IIDR register (present in all GICv2m
>> implementations) to identify X-Gene GICv2m implementation and
>> apply workaround to change the data portion of MSI vector.
>>
>> Signed-off-by: Duc Dang <dhdang@apm.com>
>> ---
>> drivers/irqchip/irq-gic-v2m.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
>> index db04fc1..470972c 100644
>> --- a/drivers/irqchip/irq-gic-v2m.c
>> +++ b/drivers/irqchip/irq-gic-v2m.c
>> @@ -43,6 +43,10 @@
>>
>> #define V2M_MSI_TYPER_NUM_SPI(x) ((x) & V2M_MSI_TYPER_NUM_MASK)
>>
>> +#define V2M_MSI_IIDR 0xFCC
>
> Please group this with the rest of the registers.
>
>> +/* APM X-Gene with GICv2m MSI_IIDR register value */
>> +#define XGENE_GICV2M_MSI_IIDR 0x06000170
>> +
>
> Is this value guaranteed to only identify the affected implementation?
> Do we have strong guarantees that a fixed implementation will not have
> this value?
>
> If not, I'd strongly suggest using a different method (compatible
> string).
>
I got confirmation from my designer. This value (0x06000170) is unique
for affected implementation. The fixed implementation will not have
this value for MSI_IIDR register.
>> struct v2m_data {
>> spinlock_t msi_cnt_lock;
>> struct resource res; /* GICv2m resource */
>> @@ -98,6 +102,16 @@ static void gicv2m_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>> msg->address_hi = (u32) (addr >> 32);
>> msg->address_lo = (u32) (addr);
>> msg->data = data->hwirq;
>> + /*
>> + * APM X-Gene GICv2m implementation has an erratum where
>> + * the MSI data needs to be the offset from the spi_start
>> + * in order to trigger the correct MSI interrupt. This is
>> + * different from the standard GICv2m implementation where
>> + * the MSI data is the absolute value within the range from
>> + * spi_start to (spi_start + num_spis).
>> + */
>> + if (readl_relaxed(v2m->base + V2M_MSI_IIDR) == XGENE_GICV2M_MSI_IIDR)
>> + msg->data = data->hwirq - v2m->spi_start;
>> }
>>
>> static struct irq_chip gicv2m_irq_chip = {
>
> Please add a set of flags to struct v2m_data as well as a flag
> (GICV2M_NEEDS_SPI_OFFSET?) that can be checked in the compose method
> (we don't needs to read the IIDR register each time we program an MSI):
>
> if (v2m->flags & GICV2M_NEEDS_SPI_OFFSET)
> msg->data -= v2m->spi_start;
>
> You can set that flag from the probe function.
Yes, I will update the patch with your suggestion shortly.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny.
Regards,
Duc Dang.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/1] irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum
2015-10-03 13:26 ` Marc Zyngier
2015-10-06 22:30 ` Duc Dang
@ 2015-10-06 22:32 ` Duc Dang
2015-10-07 7:12 ` Marc Zyngier
1 sibling, 1 reply; 6+ messages in thread
From: Duc Dang @ 2015-10-06 22:32 UTC (permalink / raw)
To: Marc Zyngier, Suravee Suthikulpanit, Jason Cooper, Thomas Gleixner
Cc: linux-kernel, linux-arm, patches, Duc Dang
APM X-Gene GICv2m implementation has an erratum where the
MSI data needs to be the offset from the spi_start in order to
trigger the correct MSI interrupt. This is different from the
standard GICv2m implementation where the MSI data is the absolute
value within the range from spi_start to (spi_start + num_spis)
of each v2m frame.
This patch reads MSI_IIDR register (present in all GICv2m
implementations) to identify X-Gene GICv2m implementation and
apply workaround to change the data portion of MSI vector.
Signed-off-by: Duc Dang <dhdang@apm.com>
---
Changes since v1:
+ Group V2M_MSI_IIDR definition to V2M register group
+ Set v2m flag during init to indicate the erratum and
use that flag to manipulate the MSI data when composing MSI msg.
drivers/irqchip/irq-gic-v2m.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index db04fc1..4c17c18 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -37,12 +37,19 @@
#define V2M_MSI_SETSPI_NS 0x040
#define V2M_MIN_SPI 32
#define V2M_MAX_SPI 1019
+#define V2M_MSI_IIDR 0xFCC
#define V2M_MSI_TYPER_BASE_SPI(x) \
(((x) >> V2M_MSI_TYPER_BASE_SHIFT) & V2M_MSI_TYPER_BASE_MASK)
#define V2M_MSI_TYPER_NUM_SPI(x) ((x) & V2M_MSI_TYPER_NUM_MASK)
+/* APM X-Gene with GICv2m MSI_IIDR register value */
+#define XGENE_GICV2M_MSI_IIDR 0x06000170
+
+/* List of flags for specific v2m implementation */
+#define GICV2M_NEEDS_SPI_OFFSET 0x00000001
+
struct v2m_data {
spinlock_t msi_cnt_lock;
struct resource res; /* GICv2m resource */
@@ -50,6 +57,7 @@ struct v2m_data {
u32 spi_start; /* The SPI number that MSIs start */
u32 nr_spis; /* The number of SPIs for MSIs */
unsigned long *bm; /* MSI vector bitmap */
+ u32 flags; /* v2m flags for specific implementation */
};
static void gicv2m_mask_msi_irq(struct irq_data *d)
@@ -98,6 +106,9 @@ static void gicv2m_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
msg->address_hi = (u32) (addr >> 32);
msg->address_lo = (u32) (addr);
msg->data = data->hwirq;
+
+ if (v2m->flags & GICV2M_NEEDS_SPI_OFFSET)
+ msg->data -= v2m->spi_start;
}
static struct irq_chip gicv2m_irq_chip = {
@@ -266,6 +277,17 @@ static int __init gicv2m_init_one(struct device_node *node,
goto err_iounmap;
}
+ /*
+ * APM X-Gene GICv2m implementation has an erratum where
+ * the MSI data needs to be the offset from the spi_start
+ * in order to trigger the correct MSI interrupt. This is
+ * different from the standard GICv2m implementation where
+ * the MSI data is the absolute value within the range from
+ * spi_start to (spi_start + num_spis).
+ */
+ if (readl_relaxed(v2m->base + V2M_MSI_IIDR) == XGENE_GICV2M_MSI_IIDR)
+ v2m->flags |= GICV2M_NEEDS_SPI_OFFSET;
+
v2m->bm = kzalloc(sizeof(long) * BITS_TO_LONGS(v2m->nr_spis),
GFP_KERNEL);
if (!v2m->bm) {
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum
2015-10-06 22:32 ` [PATCH v2 " Duc Dang
@ 2015-10-07 7:12 ` Marc Zyngier
2015-10-07 18:08 ` Duc Dang
0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2015-10-07 7:12 UTC (permalink / raw)
To: Duc Dang, Suravee Suthikulpanit, Jason Cooper, Thomas Gleixner
Cc: linux-kernel, linux-arm, patches
On 06/10/15 23:32, Duc Dang wrote:
> APM X-Gene GICv2m implementation has an erratum where the
> MSI data needs to be the offset from the spi_start in order to
> trigger the correct MSI interrupt. This is different from the
> standard GICv2m implementation where the MSI data is the absolute
> value within the range from spi_start to (spi_start + num_spis)
> of each v2m frame.
>
> This patch reads MSI_IIDR register (present in all GICv2m
> implementations) to identify X-Gene GICv2m implementation and
> apply workaround to change the data portion of MSI vector.
>
> Signed-off-by: Duc Dang <dhdang@apm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum
2015-10-07 7:12 ` Marc Zyngier
@ 2015-10-07 18:08 ` Duc Dang
0 siblings, 0 replies; 6+ messages in thread
From: Duc Dang @ 2015-10-07 18:08 UTC (permalink / raw)
To: Marc Zyngier
Cc: Suravee Suthikulpanit, Jason Cooper, Thomas Gleixner,
Linux Kernel Mailing List, linux-arm, patches
On Wed, Oct 7, 2015 at 12:12 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 06/10/15 23:32, Duc Dang wrote:
>> APM X-Gene GICv2m implementation has an erratum where the
>> MSI data needs to be the offset from the spi_start in order to
>> trigger the correct MSI interrupt. This is different from the
>> standard GICv2m implementation where the MSI data is the absolute
>> value within the range from spi_start to (spi_start + num_spis)
>> of each v2m frame.
>>
>> This patch reads MSI_IIDR register (present in all GICv2m
>> implementations) to identify X-Gene GICv2m implementation and
>> apply workaround to change the data portion of MSI vector.
>>
>> Signed-off-by: Duc Dang <dhdang@apm.com>
>
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Thanks, Marc.
>
> M.
> --
> Jazz is not dead. It just smells funny...
Regards,
Duc Dang.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-07 18:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-02 22:16 [PATCH 1/1] irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum Duc Dang
2015-10-03 13:26 ` Marc Zyngier
2015-10-06 22:30 ` Duc Dang
2015-10-06 22:32 ` [PATCH v2 " Duc Dang
2015-10-07 7:12 ` Marc Zyngier
2015-10-07 18:08 ` Duc Dang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).