diff mbox

[04/17] irqchip/irq-mvebu-icu: fix wrong user data retrieval

Message ID 20180421135537.24716-5-miquel.raynal@bootlin.com
State New, archived
Headers show

Commit Message

Miquel Raynal April 21, 2018, 1:55 p.m. UTC
The irq_domain structure has an host_data pointer that just stores user
data. It is meant to not be touched by the IRQ core. However, when it
comes to MSI, the MSI layer adds its own private data there with a
structure that also has a host_data pointer.

Because this IRQ domain is an MSI domain, to access user data we should
do a d->host_data->host_data, also wrapped as
'platform_msi_get_host_data()'.

This bug was lying there silently because the 'icu' structure retrieved
this way was just called by dev_err(), only producing a
'(NULL device *):' output on the console.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/irqchip/irq-mvebu-icu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Petazzoni April 30, 2018, 1:49 p.m. UTC | #1
Hello,

In the title, I think "user data" is not really appropriate. "private
date" or "device private data" maybe ?

On Sat, 21 Apr 2018 15:55:24 +0200, Miquel Raynal wrote:
> The irq_domain structure has an host_data pointer that just stores user
> data. It is meant to not be touched by the IRQ core. However, when it
> comes to MSI, the MSI layer adds its own private data there with a
> structure that also has a host_data pointer.
> 
> Because this IRQ domain is an MSI domain, to access user data we should
> do a d->host_data->host_data, also wrapped as
> 'platform_msi_get_host_data()'.
> 
> This bug was lying there silently because the 'icu' structure retrieved
> this way was just called by dev_err(), only producing a
> '(NULL device *):' output on the console.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Otherwise:

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks!

Thomas
Miquel Raynal May 3, 2018, 2:57 p.m. UTC | #2
Hi Thomas,

On Mon, 30 Apr 2018 15:49:13 +0200, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:

> Hello,
> 
> In the title, I think "user data" is not really appropriate. "private
> date" or "device private data" maybe ?

Sure, I will change to "fix wrong private data retrieval".

> 
> On Sat, 21 Apr 2018 15:55:24 +0200, Miquel Raynal wrote:
> > The irq_domain structure has an host_data pointer that just stores user
> > data. It is meant to not be touched by the IRQ core. However, when it
> > comes to MSI, the MSI layer adds its own private data there with a
> > structure that also has a host_data pointer.
> > 
> > Because this IRQ domain is an MSI domain, to access user data we should
> > do a d->host_data->host_data, also wrapped as
> > 'platform_msi_get_host_data()'.
> > 
> > This bug was lying there silently because the 'icu' structure retrieved
> > this way was just called by dev_err(), only producing a
> > '(NULL device *):' output on the console.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> Otherwise:
> 
> Reviewed-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks,
Miquèl
diff mbox

Patch

diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c
index e18c48d3a92e..6a77b9ea8e41 100644
--- a/drivers/irqchip/irq-mvebu-icu.c
+++ b/drivers/irqchip/irq-mvebu-icu.c
@@ -92,7 +92,7 @@  static int
 mvebu_icu_irq_domain_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
 			       unsigned long *hwirq, unsigned int *type)
 {
-	struct mvebu_icu *icu = d->host_data;
+	struct mvebu_icu *icu = platform_msi_get_host_data(d);
 	unsigned int icu_group;
 
 	/* Check the count of the parameters in dt */