From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ipmail06.adl6.internode.on.net (ipmail06.adl6.internode.on.net [150.101.137.145]) by lists.ozlabs.org (Postfix) with ESMTP id C5AE61A0010 for ; Mon, 11 May 2015 12:07:56 +1000 (AEST) From: Alistair Popple To: minyard@acm.org Subject: Re: [PATCH v3 2/8] ipmi/powernv: Convert to irq event interface Date: Mon, 11 May 2015 12:07:54 +1000 Message-ID: <1756932.94IyepIzdi@mexican> In-Reply-To: <554BA42F.6070001@acm.org> References: <1430968578-23527-1-git-send-email-alistair@popple.id.au> <1430968578-23527-2-git-send-email-alistair@popple.id.au> <554BA42F.6070001@acm.org> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="nextPart1605686.lyWlFJj8Fz" Cc: openipmi-developer@lists.sourceforge.net, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , This is a multi-part message in MIME format. --nextPart1605686.lyWlFJj8Fz Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 > > > 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 > > #include > > #include > > > > +#include > > +#include > > > > #include > > > > @@ -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; > > > > } --nextPart1605686.lyWlFJj8Fz Content-Transfer-Encoding: 7Bit Content-Type: text/html; charset="us-ascii"

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 <cminyard@mvista.com>

>

> > 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;

> >

> > }

> >

> > --

> > 1.8.3.2

 

--nextPart1605686.lyWlFJj8Fz--