From: Gavin Shan <shangw@linux•vnet.ibm.com>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: linuxppc-dev@lists•ozlabs.org, Gavin Shan <shangw@linux•vnet.ibm.com>
Subject: Re: [PATCH 23/27] powernv/opal: Notifier for OPAL events
Date: Wed, 12 Jun 2013 11:15:29 +0800 [thread overview]
Message-ID: <20130612031529.GA9492@shangw.(null)> (raw)
In-Reply-To: <1370997149.8250.158.camel@pasglop>
On Wed, Jun 12, 2013 at 10:32:29AM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2013-06-05 at 15:34 +0800, Gavin Shan wrote:
>> The patch intends to implement the notifier for variable OPAL events.
>> It's notable that the notifier can be disabled dynamically. Also, the
>> notifier could be fired upon incoming OPAL interrupts, or enabling
>> the OPAL notifier.
>
>"This patch implements a notifier to receive a notification on OPAL
>event mask changes." is probably better. No need to blurb about
>enable/disable, however add something along the lines of
>
>"The notifier is only called as a result of an OPAL interrupt, which
>will happen upon reception of FSP messages or PCI errors. Any event
>mask change detected as a result of opal_poll_events() will not result
>in a notifier call.
>
>With OPALv3, opal_poll_event() will not clear interrupt conditions from
>the FSP however, even if it consumes the messages (and thus updates the
>event mask). Thus the interrupt notifier is a reliable way to get
>the completion for FSP based OPAL operations. The specific list will
>be added to the header file.
>
>
Thanks, Ben. Will update the changelog accordingly.
>> Signed-off-by: Gavin Shan <shangw@linux•vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/opal.h | 3 +
>> arch/powerpc/platforms/powernv/opal.c | 79 ++++++++++++++++++++++++++++++++-
>> 2 files changed, 81 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
>> index 2880797..64e7c84 100644
>> --- a/arch/powerpc/include/asm/opal.h
>> +++ b/arch/powerpc/include/asm/opal.h
>> @@ -644,6 +644,9 @@ extern void hvc_opal_init_early(void);
>> extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
>> int depth, void *data);
>>
>> +extern int opal_notifier_register(uint64_t mask, void (*cb)(uint64_t));
>> +extern void opal_notifier_enable(bool enable);
>
>Make it two functions
>
>opal_enable_notifier() vs. opal_disable_notifier()
>
Ok. Will do.
>> extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
>> extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
>> index 628c564..9bbbf93 100644
>> --- a/arch/powerpc/platforms/powernv/opal.c
>> +++ b/arch/powerpc/platforms/powernv/opal.c
>> @@ -26,11 +26,20 @@ struct opal {
>> u64 entry;
>> } opal;
>>
>> +struct opal_cb {
>> + struct list_head list;
>> + uint64_t mask;
>> + void (*cb)(uint64_t);
>> +};
>> +
>> static struct device_node *opal_node;
>> static DEFINE_SPINLOCK(opal_write_lock);
>> extern u64 opal_mc_secondary_handler[];
>> static unsigned int *opal_irqs;
>> static unsigned int opal_irq_count;
>> +static LIST_HEAD(opal_notifier);
>> +static DEFINE_SPINLOCK(opal_notifier_lock);
>> +static atomic_t opal_notifier_hold = ATOMIC_INIT(0);
>>
>> int __init early_init_dt_scan_opal(unsigned long node,
>> const char *uname, int depth, void *data)
>> @@ -95,6 +104,74 @@ static int __init opal_register_exception_handlers(void)
>>
>> early_initcall(opal_register_exception_handlers);
>>
>> +int opal_notifier_register(uint64_t mask, void (*cb)(uint64_t))
>> +{
>> + unsigned long flags;
>> + struct opal_cb *p, *tmp;
>> +
>> + if (!mask || !cb) {
>> + pr_warning("%s: Invalid argument (%llx, %p)!\n",
>> + __func__, mask, cb);
>> + return -EINVAL;
>> + }
>> +
>> + p = kzalloc(sizeof(*p), GFP_KERNEL);
>> + if (!p) {
>> + pr_warning("%s: Out of memory (%llx, %p)!\n",
>> + __func__, mask, cb);
>> + return -ENOMEM;
>> + }
>> + p->mask = mask;
>> + p->cb = cb;
>> +
>> + spin_lock_irqsave(&opal_notifier_lock, flags);
>> + list_for_each_entry(tmp, &opal_notifier, list) {
>> + if (tmp->cb == cb || tmp->mask & mask) {
>> + pr_warning("%s: Duplicate evnet handler (%llx, %p)\n",
>> + __func__, tmp->mask, tmp->cb);
>> + spin_unlock_irqrestore(&opal_notifier_lock, flags);
>> + kfree(p);
>> + return -EEXIST;
>> + }
>> + }
>
>Don't bother with checking the list already. This is not useful. Also
>it's fine for two things to listen on the same event.
>
Ok. Will update in next revision.
>> +
>> + list_add_tail(&p->list, &opal_notifier);
>> + spin_unlock_irqrestore(&opal_notifier_lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static void opal_do_notifier(uint64_t events)
>> +{
>> + struct opal_cb *tmp;
>> +
>> + if (atomic_read(&opal_notifier_hold))
>> + return;
>> + if (!events)
>> + return;
>> +
>> + list_for_each_entry(tmp, &opal_notifier, list) {
>> + if (events & tmp->mask)
>> + tmp->cb(events & tmp->mask);
>> + }
>> +}
>
>My idea was to call this if the event bit has changed since the last
>time we called opal_do_notifier. IE. Use a static last_notified_mask
>and do something like
>
> changed_mask = last_notified_mask ^ events;
>
> list_for_each_entry(tmp, &opal_notifier, list) {
> if (changed_mask & tmp->mask)
> tmp->cb(events);
>
>Also, always pass the whole events to the callback, no point in
>filtering.
>
>BTW, "tmp" isn't a nice name here.
>
Ok. Will update in next revision:
- Allow multiple "clients" for same event.
- Make the variable "tmp" to have better name.
>> +void opal_notifier_enable(bool enable)
>> +{
>> + int64_t rc;
>> + uint64_t evt = 0;
>> +
>> + if (enable) {
>> + atomic_set(&opal_notifier_hold, 0);
>> +
>> + /* Process pending events */
>> + rc = opal_poll_events(&evt);
>> + if (rc == OPAL_SUCCESS && evt)
>> + opal_do_notifier(evt);
>> + } else
>> + atomic_set(&opal_notifier_hold, 1);
>> +}
>
>As I said, two functions.
>
Ok.
>> int opal_get_chars(uint32_t vtermno, char *buf, int count)
>> {
>> s64 len, rc;
>> @@ -297,7 +374,7 @@ static irqreturn_t opal_interrupt(int irq, void *data)
>>
>> opal_handle_interrupt(virq_to_hw(irq), &events);
>>
>> - /* XXX TODO: Do something with the events */
>> + opal_do_notifier(events);
>>
>> return IRQ_HANDLED;
>> }
>
Thanks,
Gavin
next prev parent reply other threads:[~2013-06-12 3:15 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-05 7:34 [PATCH v3 00/27] EEH Support for PowerNV platform Gavin Shan
2013-06-05 7:34 ` [PATCH 01/27] powerpc/eeh: Fix fetching bus for single-dev-PE Gavin Shan
2013-06-05 7:34 ` [PATCH 02/27] powerpc/eeh: Enhance converting EEH dev Gavin Shan
2013-06-05 7:34 ` [PATCH 03/27] powerpc/eeh: Make eeh_phb_pe_get() public Gavin Shan
2013-06-05 7:34 ` [PATCH 04/27] powerpc/eeh: Make eeh_pe_get() public Gavin Shan
2013-06-05 7:34 ` [PATCH 05/27] powerpc/eeh: Trace PCI bus from PE Gavin Shan
2013-06-05 7:34 ` [PATCH 06/27] powerpc/eeh: Make eeh_init() public Gavin Shan
2013-06-05 7:34 ` [PATCH 07/27] powerpc/eeh: EEH post initialization operation Gavin Shan
2013-06-05 7:34 ` [PATCH 08/27] powerpc/eeh: Refactor eeh_reset_pe_once() Gavin Shan
2013-06-05 7:34 ` [PATCH 09/27] powerpc/eeh: Delay EEH probe during hotplug Gavin Shan
2013-06-05 7:34 ` [PATCH 10/27] powerpc/eeh: Differentiate EEH events Gavin Shan
2013-06-05 7:34 ` [PATCH 11/27] powerpc/eeh: Sync OPAL API with firmware Gavin Shan
2013-06-05 7:34 ` [PATCH 12/27] powerpc/eeh: EEH backend for P7IOC Gavin Shan
2013-06-05 7:34 ` [PATCH 13/27] powerpc/eeh: I/O chip post initialization Gavin Shan
2013-06-05 7:34 ` [PATCH 14/27] powerpc/eeh: I/O chip EEH enable option Gavin Shan
2013-06-05 7:34 ` [PATCH 15/27] powerpc/eeh: I/O chip EEH state retrieval Gavin Shan
2013-06-11 7:37 ` Benjamin Herrenschmidt
2013-06-12 3:32 ` Gavin Shan
2013-06-12 4:19 ` Benjamin Herrenschmidt
2013-06-13 4:26 ` Gavin Shan
2013-06-13 4:42 ` Benjamin Herrenschmidt
2013-06-13 5:50 ` Gavin Shan
2013-06-05 7:34 ` [PATCH 16/27] powerpc/eeh: I/O chip PE reset Gavin Shan
2013-06-05 7:34 ` [PATCH 17/27] powerpc/eeh: I/O chip PE log and bridge setup Gavin Shan
2013-06-11 7:37 ` Benjamin Herrenschmidt
2013-06-12 3:33 ` Gavin Shan
2013-06-05 7:34 ` [PATCH 18/27] powerpc/eeh: PowerNV EEH backends Gavin Shan
2013-06-05 7:34 ` [PATCH 19/27] powerpc/eeh: Initialization for PowerNV Gavin Shan
2013-06-05 7:34 ` [PATCH 20/27] powerpc/eeh: Enable EEH check for config access Gavin Shan
2013-06-05 7:34 ` [PATCH 21/27] powerpc/eeh: Process interrupts caused by EEH Gavin Shan
2013-06-11 8:13 ` Benjamin Herrenschmidt
2013-06-13 4:14 ` Gavin Shan
2013-06-05 7:34 ` [PATCH 22/27] powerpc/eeh: Allow to check fenced PHB proactively Gavin Shan
2013-06-05 7:34 ` [PATCH 23/27] powernv/opal: Notifier for OPAL events Gavin Shan
2013-06-12 0:32 ` Benjamin Herrenschmidt
2013-06-12 3:15 ` Gavin Shan [this message]
2013-06-05 7:34 ` [PATCH 24/27] powernv/opal: Disable OPAL notifier upon poweroff Gavin Shan
2013-06-05 7:34 ` [PATCH 25/27] powerpc/eeh: Register OPAL notifier for PCI error Gavin Shan
2013-06-05 7:34 ` [PATCH 26/27] powerpc/powernv: Debugfs directory for PHB Gavin Shan
2013-06-05 7:34 ` [PATCH 27/27] powerpc/eeh: Debugfs for error injection Gavin Shan
2013-06-11 7:46 ` [PATCH v3 00/27] EEH Support for PowerNV platform Benjamin Herrenschmidt
2013-06-12 3:18 ` Gavin Shan
-- strict thread matches above, loose matches on Subject: below --
2013-06-15 9:02 [PATCH v4 " Gavin Shan
2013-06-15 9:03 ` [PATCH 23/27] powernv/opal: Notifier for OPAL events Gavin Shan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='20130612031529.GA9492@shangw.(null)' \
--to=shangw@linux$(echo .)vnet.ibm.com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox