linux-kernel - RE: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 29 Sep 2009 22:57:41 +0100
From:	"Hennerich, Michael" <Michael.Hennerich@...log.com>
To:	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Mike Frysinger" <vapier@...too.org>
CC:	<sameo@...ux.intel.com>, <linux-kernel@...r.kernel.org>,
	<uclinux-dist-devel@...ckfin.uclinux.org>, <cooloney@...nel.org>
Subject: RE: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

>From: Andrew Morton [mailto:akpm@...ux-foundation.org]
ject: Re: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad
Input Device Driver
>
>On Wed, 23 Sep 2009 01:11:04 -0400
>Mike Frysinger <vapier@...too.org> wrote:
>
>> +static void adp5520_irq_work(struct work_struct *work)
>> +{
>> +	struct adp5520_chip *chip =
>> +		container_of(work, struct adp5520_chip, irq_work);
>> +	unsigned int events;
>> +	uint8_t reg_val;
>> +	int ret;
>> +
>> +	ret = __adp5520_read(chip->client, MODE_STATUS, &reg_val);
>> +	if (ret)
>> +		goto out;
>> +
>> +	events =  reg_val & (OVP_INT | CMPR_INT | GPI_INT | KR_INT |
KP_INT);
>> +
>> +	blocking_notifier_call_chain(&chip->notifier_list, events,
NULL);
>> +	/* ACK, Sticky bits are W1C */
>> +	__adp5520_ack_bits(chip->client, MODE_STATUS, events);
>> +
>> +out:
>> +	enable_irq(chip->client->irq);
>> +}
>> +
>> +static irqreturn_t adp5520_irq_handler(int irq, void *data)
>> +{
>> +	struct adp5520_chip *chip = data;
>> +
>> +	disable_irq_nosync(irq);
>> +	schedule_work(&chip->irq_work);
>> +
>> +	return IRQ_HANDLED;
>> +}
>
>Disabling interrutps for an arbitrarily long period of time is pretty
>nasty.  Especially if some poor innocent device is trying to share that
>irq (can this happen?).
>
>Is there no other way?

Well - incredible slow peripherals such as I2C can't be directly
serviced from non-sleepy context such as IRQ - in case someone wants to
use level sensitive IRQs - disabling the interrupt source until serviced
used to be the common way. Typically work queues are serviced in the
shadow of the IRQ caused the Interrupt. Shared IRQs are in addition
somehow OR'ed with each other. So it really doesn't matter, if another
device asserts a SHARED IRQ; 1 OR 1 = 1  - the kernel just needs to make
sure    
that IRQs are serviced until absence... (While in return the device
caused the IRQ de-asserts its signal to the host)

This driver targets mostly embedded devices, they typically feature
dedicated interrupt channels for each Interrupt source.   

>(can this happen?)

I don't think so 

-Michael

 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/