Subject: Re: [PATCH v3 2/8] ipmi/powernv: Convert to irq event interface

From: Alistair Popple <[email-masked]>
Date: Mon, 11 May 2015 12:07:54 +1000

Hi,

On Thu, 7 May 2015 12:43:11 Corey Minyard wrote:
> 
> The only thing I would suggest is passing the irq level
> (IRQ_TYPE_LEVEL_HIGH) as
> part of the openfirmware data instead of hard-coding it.

I intend to do that in future once our firmware is updated to pass this information (via 
the device-tree). However systems with older firmware won't pass this information, 
hence we hard code it for the existing events.

- Alistair

> 
> Acked-by: Corey Minyard <[email-masked]>
> 
> >  drivers/char/ipmi/ipmi_powernv.c | 39
> >  ++++++++++++++++++++++----------------- 1 file changed, 22
> >  insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/char/ipmi/ipmi_powernv.c
> > b/drivers/char/ipmi/ipmi_powernv.c index 8753b0f..9b409c0 100644
> > --- a/drivers/char/ipmi/ipmi_powernv.c
> > +++ b/drivers/char/ipmi/ipmi_powernv.c
> > @@ -15,6 +15,8 @@
> > 
> >  #include <linux/list.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> > 
> > +#include <linux/of_irq.h>
> > +#include <linux/interrupt.h>
> > 
> >  #include <asm/opal.h>
> > 
> > @@ -23,8 +25,7 @@ struct ipmi_smi_powernv {
> > 
> >  	u64			interface_id;
> >  	struct ipmi_device_id	ipmi_id;
> >  	ipmi_smi_t		intf;
> > 
> > -	u64			event;
> > -	struct notifier_block	event_nb;
> > +	unsigned int		irq;
> > 
> >  	/**
> >  	
> >  	 * We assume that there can only be one outstanding request, so
> > 
> > @@ -197,15 +198,12 @@ static struct ipmi_smi_handlers
> > ipmi_powernv_smi_handlers = {> 
> >  	.poll			= ipmi_powernv_poll,
> >  
> >  };
> > 
> > -static int ipmi_opal_event(struct notifier_block *nb,
> > -			  unsigned long events, void *change)
> > +static irqreturn_t ipmi_opal_event(int irq, void *data)
> > 
> >  {
> > 
> > -	struct ipmi_smi_powernv *smi = container_of(nb,
> > -					struct 
ipmi_smi_powernv, event_nb);
> > +	struct ipmi_smi_powernv *smi = data;
> > 
> > -	if (events & smi->event)
> > -		ipmi_powernv_recv(smi);
> > -	return 0;
> > +	ipmi_powernv_recv(smi);
> > +	return IRQ_HANDLED;
> > 
> >  }
> >  
> >  static int ipmi_powernv_probe(struct platform_device *pdev)
> > 
> > @@ -240,13 +238,16 @@ static int ipmi_powernv_probe(struct platform_device
> > *pdev)> 
> >  		goto err_free;
> >  	
> >  	}
> > 
> > -	ipmi->event = 1ull << prop;
> > -	ipmi->event_nb.notifier_call = ipmi_opal_event;
> > +	ipmi->irq = irq_of_parse_and_map(dev->of_node, 0);
> > +	if (!ipmi->irq) {
> > +		dev_info(dev, "Unable to map irq from device tree\n");
> > +		ipmi->irq = opal_event_request(prop);
> > +	}
> > 
> > -	rc = opal_notifier_register(&ipmi->event_nb);
> > -	if (rc) {
> > -		dev_warn(dev, "OPAL notifier registration failed 
(%d)\n", rc);
> > -		goto err_free;
> > +	if (request_irq(ipmi->irq, ipmi_opal_event, IRQ_TYPE_LEVEL_HIGH,
> > +				"opal-ipmi", ipmi)) {
> > +		dev_warn(dev, "Unable to request irq\n");
> > +		goto err_dispose;
> > 
> >  	}
> >  	
> >  	ipmi->opal_msg = devm_kmalloc(dev,
> > 
> > @@ -271,7 +272,9 @@ static int ipmi_powernv_probe(struct platform_device
> > *pdev)> 
> >  err_free_msg:
> >  	devm_kfree(dev, ipmi->opal_msg);
> >  
> >  err_unregister:
> > -	opal_notifier_unregister(&ipmi->event_nb);
> > +	free_irq(ipmi->irq, ipmi);
> > +err_dispose:
> > +	irq_dispose_mapping(ipmi->irq);
> > 
> >  err_free:
> >  	devm_kfree(dev, ipmi);
> >  	return rc;
> > 
> > @@ -282,7 +285,9 @@ static int ipmi_powernv_remove(struct platform_device
> > *pdev)> 
> >  	struct ipmi_smi_powernv *smi = dev_get_drvdata(&pdev->dev);
> >  	
> >  	ipmi_unregister_smi(smi->intf);
> > 
> > -	opal_notifier_unregister(&smi->event_nb);
> > +	free_irq(smi->irq, smi);
> > +	irq_dispose_mapping(smi->irq);
> > +
> > 
> >  	return 0;
> >  
> >  }