linux-kernel - RE: [PATCH] i2c: add irq

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]
Message-ID: <544AC56F16B56944AEC3BD4E3D5917713094520EDB@LIMKCMBX1.ad.analog.com>
Date:	Mon, 18 Oct 2010 10:55:57 +0100
From:	"Hennerich, Michael" <Michael.Hennerich@...log.com>
To:	Jean Delvare <khali@...ux-fr.org>,
	Mike Frysinger <vapier@...too.org>
CC:	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"uclinux-dist-devel@...ckfin.uclinux.org" 
	<uclinux-dist-devel@...ckfin.uclinux.org>,
	"device-drivers-devel@...ckfin.uclinux.org" 
	<device-drivers-devel@...ckfin.uclinux.org>
Subject: RE: [PATCH] i2c: add irq_flags to board info

Jean Delvare wrote on 2010-10-18:
> Hi Mike,
>
> On Sun, 17 Oct 2010 19:43:39 -0400, Mike Frysinger wrote:
>> From: Michael Hennerich <michael.hennerich@...log.com>
>>
>> These flags can be optionally defined - slave drivers may use them as
>> flags argument for request_irq().  In case they are left uninitialized
>> they will default to zero, and therefore shouldn't cause problems.
>>
>> This allows us to avoid having to add dedicated platform init code
>> just to call set_irq_type()
>
> Do you have examples of this? Can we see a preview of the amount of
> cleanups this patch would allow?

Examples can be found in various board setup files:

One example arch/sh/boards/mach-ecovec24/setup.c:

static struct i2c_board_info ts_i2c_clients = {
        I2C_BOARD_INFO("tsc2007", 0x48),
        .type           = "tsc2007",
        .platform_data  = &tsc2007_info,
        .irq            = IRQ0,
};

[--snip--]

                /* enable TouchScreen */
                i2c_register_board_info(0, &ts_i2c_clients, 1);
                set_irq_type(IRQ0, IRQ_TYPE_LEVEL_LOW);

>> -- which doesn't work very well when coupled with module drivers.
>
> I don't quite get what you mean here.

Since the driver doesn't setup the irq_type itself you need to do it
manually in advance using set_irq_type().
This happens most likely from your paltform setup/configuration file.

Assuming the driver is built as a module, this code gets executed unconditionally,
no matter if the driver gets finally loaded or not.

Assuming you have several drivers build as modules, using the same irq but with
different irq types, you run into problems here.

There are some development boards featuring extension options, which all plug on the same
socket but required different drivers/irq types.

The ideal way is therefore to pass the irq_flags together with the SPI/I2C board info.

>> It also matches behavior of some other frameworks like IDE and UIO.
>
> This is certainly a good point.
>
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich@...log.com>
>> Signed-off-by: Mike Frysinger <vapier@...too.org>
>> ---
>>  drivers/i2c/i2c-core.c |    1 +
>>  include/linux/i2c.h    |    4 ++++
>>  2 files changed, 5 insertions(+), 0 deletions(-)
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index
>> bea4c50..830528f 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -540,6 +540,7 @@ i2c_new_device(struct i2c_adapter *adap, struct
> i2c_board_info const *info)
>>      client->flags = info->flags;
>>      client->addr = info->addr;
>>      client->irq = info->irq;
>> +    client->irq_flags = info->irq_flags;
>>
>>      strlcpy(client->name, info->type, sizeof(client->name));
>> diff --git a/include/linux/i2c.h b/include/linux/i2c.h index
>> 4bae0b7..e6248c1 100644
>> --- a/include/linux/i2c.h
>> +++ b/include/linux/i2c.h
>> @@ -189,6 +189,7 @@ struct i2c_driver {
>>   * @driver: device's driver, hence pointer to access routines * @dev:
>>   Driver model device node for the slave. * @irq: indicates the IRQ
>>   generated by this device (if any) + * @irq_flags: The flags passed to
>>   request_irq() * @detected: member of an i2c_driver.clients list or
>>   i2c-core's *       userspace_devices list *
>> @@ -206,6 +207,7 @@ struct i2c_client {
>>      struct i2c_driver *driver;      /* and our access routines      */      struct
>>  device dev;         /* the device structure         */      int irq;                        /* irq issued by
>>  device              */ +    unsigned long irq_flags;        /* flags used by the irq        */
>>      struct list_head detected; }; #define to_i2c_client(d)
>>  container_of(d, struct i2c_client, dev) @@
>> -237,6 +239,7 @@ static inline void i2c_set_clientdata(struct
> i2c_client *dev, void *data)
>>   * @archdata: copied into i2c_client.dev.archdata * @of_node: pointer
>>   to OpenFirmware device node * @irq: stored in i2c_client.irq + *
>>   @irq_flags: The flags passed to request_irq() for i2c_client.irq * *
>>   I2C doesn't actually support hardware probing, although controllers
>>   and * devices may be able to use I2C_SMBUS_QUICK to tell whether or
>> not there's @@ -259,6 +262,7 @@ struct i2c_board_info {
>>      struct device_node *of_node; #endif     int             irq; +  unsigned
>>  long        irq_flags; };
>>
>>  /**
>
>

Greetings,
Michael

Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036 Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif


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