[PATCH 23/27] powernv/opal: Notifier for OPAL events

[Gavin Shan]

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