Re: [Device-drivers-devel] [PATCH] i2c: add irq_flags to board info

2010-10-25 Thread Jonathan Cameron
On 10/25/10 01:45, Ben Dooks wrote:
 On Mon, Oct 18, 2010 at 03:51:49PM -0400, Mike Frysinger wrote:
 On Mon, Oct 18, 2010 at 10:33, Jean Delvare wrote:
 Why do we have set_irq_type() if we're not supposed to call it? I am
 not claiming to be an expert in the area, but it seems totally
 reasonable to me that the same piece of code instantiating an I2C
 device is also responsible for setting its IRQ type.

 but we're back to the same issue mentioned earlier -- you cant have a
 single kernel build with modules supporting multiple drivers
 simultaneously.  we like to ship development boards with a single
 kernel build on it with many modules.  then people can pick the addon
 boards they wish to prototype with at runtime by plugging in the card
 and loading the module.
 
 I also dislike set_irq_type() as it doesn't check whether there is anyone
 registered with the interrupt, which means that you could set the irq
 type of someone else's irq.
 
 I wonder if we should pass a struct resource instead, in case there
 are multiple interrupt sources, as well as having it registered with
 the right resource systems.
 
Either works as far as I am concerned. Having seen a large set of drivers
using the flags option (posted to linux-iio yesterday) I'm definitely convinced
some means of allowing devices to match what the board config asks for is 
useful.

I personally prefer the struct resource option as I have multiple drivers in IIO
which have two interrupts and this is the only reason some of them use platform
data.

Thanks,

Jonathan
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Device-drivers-devel] [PATCH] i2c: add irq_flags to board info

2010-10-18 Thread Mike Frysinger
On Mon, Oct 18, 2010 at 10:33, Jean Delvare wrote:
 Why do we have set_irq_type() if we're not supposed to call it? I am
 not claiming to be an expert in the area, but it seems totally
 reasonable to me that the same piece of code instantiating an I2C
 device is also responsible for setting its IRQ type.

but we're back to the same issue mentioned earlier -- you cant have a
single kernel build with modules supporting multiple drivers
simultaneously.  we like to ship development boards with a single
kernel build on it with many modules.  then people can pick the addon
boards they wish to prototype with at runtime by plugging in the card
and loading the module.

if the only way to change flags on a pin is via set_irq_type(), that
kernel build is instantly tied to whatever device you've decided to
compile statically into the board's init code.  so now to prototype
multiple boards, we have to tell people to boot different kernels !?
and we have to store multiple kernels and sets of kernel modules in
the rootfs !?

 The user decides which add-on module is plugged onto the processor board,
 by loading the appropriate driver module.

 This can't work, sorry. Unless you are using I2C device auto-detection,
 which I seem to understand is NOT used in the embedded world at all,
 loading a driver module doesn't do anything if a device it supports
 hasn't been instantiated first. So the user can't just load a driver
 and see the required I2C devices magically appear for the driver to
 bind to them.

why cant user selection work ?  the user picks `modprobe foo` because
they have a 'foo' device plugged in.

i'm not sure what I2C device auto-detection means.  are you talking
about a userspace app/daemon that scans the I2C bus, looks up the
slave ids in the module db, and then automatically loads the modules
that it finds bindings for ?

 There are drivers that work around this deficiency, by adding irq_flags to 
 the bus
 clients dev.platform_data.  See include/linux/spi/ads7846.h for one example.

 Interesting example, better matching the initial comment that came
 along with the patch. But it also seems to be an isolated case, as I
 can't find any other irq_flags in include/linux/{i2c,spi}/.

we've been holding off posting things because we need the core extended first
-mike
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html