Re: [PATCH] i2c: i2c-pca-platform: Change device name of request_irq
Hi Nobuhiro (long time no see :))! On Tue, Oct 19, 2010 at 11:45:31AM +0900, Nobuhiro Iwamatsu wrote: > i2c->adap.name shouldn't be used in request_irq. > Instead the driver name "i2c-pca-platform" should be used. > > Signed-off-by: Nobuhiro Iwamatsu Looks good to me, thanks for fixing it (and to Jean for the guidance) Acked-by: Wolfram Sang Back to holiday ;) Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
[PATCH] i2c: i2c-pca-platform: Change device name of request_irq
i2c->adap.name shouldn't be used in request_irq.
Instead the driver name "i2c-pca-platform" should be used.
Signed-off-by: Nobuhiro Iwamatsu
CC: Wolfram Sang
CC: Jean Delvare
---
drivers/i2c/busses/i2c-pca-platform.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-pca-platform.c
b/drivers/i2c/busses/i2c-pca-platform.c
index ef5c784..38a6654 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -221,7 +221,7 @@ static int __devinit i2c_pca_pf_probe(struct
platform_device *pdev)
if (irq) {
ret = request_irq(irq, i2c_pca_pf_handler,
- IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
+ IRQF_TRIGGER_FALLING, pdev->name, i2c);
if (ret)
goto e_reqirq;
}
--
1.7.1
--
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: [PATCH] i2c: i2c-pca-platform: Change i2c adapter name
Hi, Jean .
Thank for your comments.
Jean Delvare wrote:
Hi Nobuhiro,
On Mon, 18 Oct 2010 12:24:30 +0900, Nobuhiro Iwamatsu wrote:
In kernel which enableded this driver output The following warning,
because a slash (/) is in the i2c adapter name by the current driver.
This set another adapter name (i2c-pca-platform at 0x) what removed slash.
[2.872000] PCA9564/PCA9665 at 0x0600: Registering gpio failed!
[2.876000] name 'PCA9564/PCA9665 at 0x0600'
[2.88] [ cut here ]
[2.88] WARNING: at fs/proc/generic.c:323
[2.88] Modules linked in:
[2.88]
[2.88] Pid : 1, Comm: swapper
[2.88] CPU : 0 Not tainted (2.6.36-rc1-00238-g67e5b3a
#261)
[2.88]
[2.88] PC is at __xlate_proc_name+0x88/0xa4
[2.88] PR is at __xlate_proc_name+0x88/0xa4
[2.88] PC : 800b3bc4 SP : 9fc41d34 SR : 40008001 TEA : c0018000
[2.88] R0 : 0037 R1 : 9fc4 R2 : 9fc4 R3 : 8049db48
[2.88] R4 : 0001 R5 : 00f0 R6 : R7 : 8019e450
[2.88] R8 : R9 : 9fc41d9c R10 : 0007 R11 : 9fc41d9c
[2.88] R12 : 9fc41d80 R13 : 80167fce R14 : 9fc41d34
[2.88] MACH: 0461 MACL: GBR : PR : 800b3bc4
[2.88]
[2.88] Call trace:
[2.88] [<800b4056>] __proc_create+0x46/0x110
[2.88] [<80162b00>] strlen+0x0/0x58
[2.88] [<800b4824>] proc_mkdir_mode+0x24/0x60
[2.88] [<800b486e>] proc_mkdir+0xe/0x1c
[2.88] [<8004740a>] register_handler_proc+0x9e/0xd4
[2.88] [<80045ab2>] __setup_irq+0x226/0x2c0
[2.88] [<80075d50>] kmem_cache_alloc+0x94/0xf0
[2.88] [<80045bcc>] request_threaded_irq+0x80/0xdc
[2.88] [<802041c0>] i2c_pca_pf_handler+0x0/0x3c
[2.88] [<802ecb62>] i2c_pca_pf_probe+0x162/0x294
[2.88] [<801a3b38>] platform_drv_probe+0x14/0x1c
(...)
The i2c adapter names are strings which appear as the _contents_ of
sysfs attributes. They don't appear in procfs. What happens here is
that the adapter name is used also as the IRQ name:
if (irq) {
ret = request_irq(irq, i2c_pca_pf_handler,
IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
if (ret)
goto e_reqirq;
}
This is the problem. i2c->adap.name shouldn't be used here, instead the
driver name "i2c-pca-platform" should be used.
Oh, I didn't notice. I will fix this.
And hrere was a couse of the warning.
(...)
Signed-off-by: Nobuhiro Iwamatsu
CC: Wolfram Sang
---
drivers/i2c/busses/i2c-pca-platform.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-pca-platform.c
b/drivers/i2c/busses/i2c-pca-platform.c
index ef5c784..4cc3cdf 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -27,6 +27,8 @@
#include
+#define DRIVER_NAME "i2c-pca-platform"
+
struct i2c_pca_pf_data {
void __iomem*reg_base;
int irq;/* if 0, use polling */
@@ -171,7 +173,7 @@ static int __devinit i2c_pca_pf_probe(struct
platform_device *pdev)
i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
i2c->adap.owner = THIS_MODULE;
snprintf(i2c->adap.name, sizeof(i2c->adap.name),
-"PCA9564/PCA9665 at 0x%08lx",
+DRIVER_NAME " at 0x%08lx",
(unsigned long) res->start);
I would like to avoid changing the adapter name when we don't have to.
People may be using this string as an identifier in i2c tools. Please
leave the adapter name unchanged and only change the IRQ name.
I see.
But I noticed that it was not necessary to revice it here.
There was the cause of the warning for the name to request_irq.
I revert the revision of here.
i2c->adap.algo_data = &i2c->algo_data;
i2c->adap.dev.parent = &pdev->dev;
@@ -250,7 +252,7 @@ e_remap:
e_alloc:
release_mem_region(res->start, resource_size(res));
e_print:
- printk(KERN_ERR "Registering PCA9564/PCA9665 FAILED! (%d)\n", ret);
+ printk(KERN_ERR "Registering " DRIVER_NAME " FAILED! (%d)\n", ret);
This is semantically incorrect. We are registering a device here, not a
driver. The proposed change makes this error message confusing. Please
revert.
I agree.
Best regards,
Nobuhiro
--
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
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
RE: [PATCH] i2c: add irq_flags to board info
Jean Delvare wrote on 2010-10-18:
> Hi Michael, David,
>
> David, question for you below.
>
> On Mon, 18 Oct 2010 13:31:21 +0100, Hennerich, Michael wrote:
>> Jean Delvare wrote on 2010-10-18:
>>> Hi Michael,
>>>
>>> On Mon, 18 Oct 2010 10:55:57 +0100, Hennerich, Michael wrote:
Jean Delvare wrote on 2010-10-18:
> Hi Mike,
>
> On Sun, 17 Oct 2010 19:43:39 -0400, Mike Frysinger wrote:
>> From: Michael Hennerich
>>
>> 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);
>>>
>>> This example doesn't quite match the patch description. There's no
>>> "dedicated platform init code just to call set_irq_type()", as you
>>> already have platform code to call i2c_register_board_info(). It's
>>> really only calling set_irq_type() from platform code vs. from
>>> driver code.
>>
>> In the past I also saw initcalls just doing set_irq_type(), to make
>> drivers happy. That doesn't pass the right irq_flags along with
>> request_irq().
>
> But it's OK to call request_irq() without these flags, as long as
> set_irq_type() is called additionally, isn't it?
Yeah - it is. But fact is, set_irq_type() is typically called in advance.
So if the driver that requires the irq_type setting isn't loaded at all,
nothing calls request_irq().
> 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.
I would leave that up to the driver. The driver author knows which irq types the
device may support and which can be handled by the driver.
If someone passes an IRQF_TRIGGER_FALLING, but the hardware device only
supports rising edge
interrupts the driver can error verbose.
Drivers that doesn't set irq_type flags with request_irq() typically avoid it,
since
some architectures doesn't support edge or level sensitive interrupts.
So historically seen, it was a way to keep this decision out of the driver file.
>>
>> -- 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.
>>>
>>> I do not get why.
>>>
>>> You're not going to register several I2C devices using the same
>>> IRQ but requiring different IRQ flags anyway, as it wouldn't work.
>>> So you'll have to only register the I2C devices which are actually
>>> present on the system. Setting the IRQ type at the same time or
>>> deferring this action to the driver(s) doesn't make any difference
> then, does it?
>>
>> If I remember correctly i2c as well as spi doesn't check for irq
>> conflicts in irqs passed with struct i2c/spi_board_info. So you can
>> have multiple drivers build as modules all using the same irq number
> but with different flags.
>
> Indeed i2c doesn't check the IRQs at all, it passes them transparently
> from platform data to device driver.
>
> But you're mixing devices and drivers. Devices are bound to an IRQ,
> not drivers, and so do IRQ flags.
I know the difference. But I'm talking here about different drivers.
Think about a system where you have different hardware options, all handled by
the same
kernel image. Option a) modprobe driver_a, Option b) modprobe driver_b.
Both devices bound to driver_a and driver_b, both use IRQ_XY. However a)
requires IRQF_TRIGGER_FALLING,
while b) needs IRQF_LEVEL_LOW.
To me is sounds reasonable to set irq_type only if the request_irq() succeeds.
>>
>> The user decides which add-on module is plugged onto
Re: [PATCH] i2c: add irq_flags to board info
On Mon, 2010-10-18 at 16:33 +0200, Jean Delvare wrote: > > This is an interesting point. David, you're the one who added irq to > struct i2c_client, you didn't add irq_flags then, did you have a good > reason not to? There was no evident need to do so. The problem wasn't characterizing the IRQ, but knowing what IRQ was associated with a given I2C chip. The IRQs would, as a rule, only have one behavior, known in advance by the I2C chip's driver ... if it only had a way to know client->irq ... In one model of IRQ setup (like PC BIOS), it's handled before drivers do more than request the IRQ, and drivers just cope. irq_flags not needed ... it's often a hardware-to-driver convention, likely matching I2C chip defaults. (Or more awkwardly, coping with whatever trigger mode the platform uses). In another model, drivers configure their chips according to desired mode (level low, for the most common example, or edge triggered, etc) and again, irq_flags not needed, but in this case the platform code would have had *nothing* do do with such mode setup. > > > 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. Not a good example; that was changed earlier this year and, from a quick glance, may not have tried much to avoid that change. The driver had worked for years with that alleged "deficiency". - Dave -- 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: [PATCH] i2c: add irq_flags to board info
Hi Michael, David,
David, question for you below.
On Mon, 18 Oct 2010 13:31:21 +0100, Hennerich, Michael wrote:
> Jean Delvare wrote on 2010-10-18:
> > Hi Michael,
> >
> > On Mon, 18 Oct 2010 10:55:57 +0100, Hennerich, Michael wrote:
> >> Jean Delvare wrote on 2010-10-18:
> >>> Hi Mike,
> >>>
> >>> On Sun, 17 Oct 2010 19:43:39 -0400, Mike Frysinger wrote:
> From: Michael Hennerich
>
> 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);
> >
> > This example doesn't quite match the patch description. There's no
> > "dedicated platform init code just to call set_irq_type()", as you
> > already have platform code to call i2c_register_board_info(). It's
> > really only calling set_irq_type() from platform code vs. from driver
> > code.
>
> In the past I also saw initcalls just doing set_irq_type(), to make drivers
> happy.
> That doesn't pass the right irq_flags along with request_irq().
But it's OK to call request_irq() without these flags, as long as
set_irq_type() is called additionally, isn't it?
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.
>
> -- 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.
> >
> > I do not get why.
> >
> > You're not going to register several I2C devices using the same IRQ
> > but requiring different IRQ flags anyway, as it wouldn't work. So
> > you'll have to only register the I2C devices which are actually
> > present on the system. Setting the IRQ type at the same time or
> > deferring this action to the driver(s) doesn't make any difference then,
> > does it?
>
> If I remember correctly i2c as well as spi doesn't check for irq conflicts in
> irqs passed with
> struct i2c/spi_board_info. So you can have multiple drivers build as modules
> all using the same
> irq number but with different flags.
Indeed i2c doesn't check the IRQs at all, it passes them transparently
from platform data to device driver.
But you're mixing devices and drivers. Devices are bound to an IRQ, not
drivers, and so do IRQ flags.
>
> 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.
So the first problem you have to solve is how you instantiate the right
I2C devices depending on which extension is plugged in. As long as this
problem isn't solved, there's no point in discussing where the IRQ
flags should be set.
> >> There are some development boards featuring extension options, which
> >> all plug on the same socket but required different drivers/irq types.
> >
> > How do you figure out which extension option is plugged? You can't
> > instantiate an I2C device which isn't present, so you must have a way
> > to know what extension option is used, to instantiate the right I2C
> > device at the right address.
>
> The user decides.
> The platform code provides resources for all potential boards being used.
> And here is exactly the conflict.
"Prov
RE: [PATCH] i2c: add irq_flags to board info
Jean Delvare wrote on 2010-10-18:
> Hi Michael,
>
> On Mon, 18 Oct 2010 10:55:57 +0100, Hennerich, Michael wrote:
>> Jean Delvare wrote on 2010-10-18:
>>> Hi Mike,
>>>
>>> On Sun, 17 Oct 2010 19:43:39 -0400, Mike Frysinger wrote:
From: Michael Hennerich
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);
>
> This example doesn't quite match the patch description. There's no
> "dedicated platform init code just to call set_irq_type()", as you
> already have platform code to call i2c_register_board_info(). It's
> really only calling set_irq_type() from platform code vs. from driver
> code.
In the past I also saw initcalls just doing set_irq_type(), to make drivers
happy.
That doesn't pass the right irq_flags along with request_irq().
-- 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.
>
> I do not get why.
>
> You're not going to register several I2C devices using the same IRQ
> but requiring different IRQ flags anyway, as it wouldn't work. So
> you'll have to only register the I2C devices which are actually
> present on the system. Setting the IRQ type at the same time or
> deferring this action to the driver(s) doesn't make any difference then, does
> it?
If I remember correctly i2c as well as spi doesn't check for irq conflicts in
irqs passed with
struct i2c/spi_board_info. So you can have multiple drivers build as modules
all using the same
irq number but with different flags.
The user decides which add-on module is plugged onto the processor board, by
loading the appropriate
driver module.
>> There are some development boards featuring extension options, which
>> all plug on the same socket but required different drivers/irq types.
>
> How do you figure out which extension option is plugged? You can't
> instantiate an I2C device which isn't present, so you must have a way
> to know what extension option is used, to instantiate the right I2C
> device at the right address.
The user decides.
The platform code provides resources for all potential boards being used.
And here is exactly the conflict.
>> The ideal way is therefore to pass the irq_flags together with the
> SPI/I2C board info.
>
> All I see so far is two data structures made slightly larger, for no
> actual benefit.
I don't see a reason why i2c/spi bus drivers should be different from other bus
drivers.
The platform_device bus driver allows you to pass IORESOURCE_IRQ |
IORESOURCE_IRQ_LOWLEVEL with flags
in struct resource.
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.
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-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: add irq_flags to board info
Hi Michael,
On Mon, 18 Oct 2010 10:55:57 +0100, Hennerich, Michael wrote:
> Jean Delvare wrote on 2010-10-18:
> > Hi Mike,
> >
> > On Sun, 17 Oct 2010 19:43:39 -0400, Mike Frysinger wrote:
> >> From: Michael Hennerich
> >>
> >> 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);
This example doesn't quite match the patch description. There's no
"dedicated platform init code just to call set_irq_type()", as you
already have platform code to call i2c_register_board_info(). It's
really only calling set_irq_type() from platform code vs. from driver
code.
> >> -- 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.
I do not get why.
You're not going to register several I2C devices using the same IRQ but
requiring different IRQ flags anyway, as it wouldn't work. So you'll
have to only register the I2C devices which are actually present on the
system. Setting the IRQ type at the same time or deferring this action
to the driver(s) doesn't make any difference then, does it?
> There are some development boards featuring extension options, which all plug
> on the same
> socket but required different drivers/irq types.
How do you figure out which extension option is plugged? You can't
instantiate an I2C device which isn't present, so you must have a way
to know what extension option is used, to instantiate the right I2C
device at the right address.
> The ideal way is therefore to pass the irq_flags together with the SPI/I2C
> board info.
All I see so far is two data structures made slightly larger, for no
actual benefit.
--
Jean Delvare
--
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: [MeeGo-Dev][PATCH v2] Topcliff: Update PCH_I2C driver to 2.6.35
Hi I2C maintainer, I posted the i2c patch 1 month ago. But I haven't received any response from you. Could you review this patch ? Thanks, Ohtake(OKISemi) - Original Message - From: "Masayuki Ohtak" To: "Jean Delvare (PC drivers, core)" ; "Ben Dooks (embedded platforms)" ; "Crane Cai" ; "Samuel Ortiz" ; "Linus Walleij" ; "Ralf Baechle" ; "srinidhi kasagar" ; ; ; ; "Linus Walleij" ; "Wolfram Sang ; Alan Cox" Cc: ; ; ; ; "Tomoya MORINAGA" ; "Arnd Bergmann" ; "Masayuki Ohtake" Sent: Thursday, September 16, 2010 5:33 PM Subject: [MeeGo-Dev][PATCH v2] Topcliff: Update PCH_I2C driver to 2.6.35 > I2C driver of Topcliff PCH > > Topcliff PCH is the platform controller hub that is going to be used in > Intel's upcoming general embedded platform. All IO peripherals in > Topcliff PCH are actually devices sitting on AMBA bus. > Topcliff PCH has I2C I/F. Using this I/F, it is able to access system > devices connected to I2C. > > Signed-off-by: Masayuki Ohtake > Reviewed-by: Linus Walleij > --- > drivers/i2c/busses/Kconfig |8 + > drivers/i2c/busses/Makefile |1 + > drivers/i2c/busses/i2c-pch.c | 908 > ++ > 3 files changed, 917 insertions(+), 0 deletions(-) > create mode 100644 drivers/i2c/busses/i2c-pch.c > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index bceafbf..b7b132d 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -613,6 +613,14 @@ config I2C_XILINX > This driver can also be built as a module. If so, the module > will be called xilinx_i2c. > > +config PCH_I2C > + tristate "PCH I2C of Intel Topcliff" > + depends on PCI > + help > + This driver is for PCH(Platform controller Hub) I2C of Topcliff which > + is an IOH(Input/Output Hub) for x86 embedded processor. > + This driver can access PCH I2C bus device. > + > comment "External I2C/SMBus adapter drivers" > > config I2C_PARPORT > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 936880b..aa04135 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -59,6 +59,7 @@ obj-$(CONFIG_I2C_STU300) += i2c-stu300.o > obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o > obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o > obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o > +obj-$(CONFIG_PCH_I2C) += i2c-pch.o > > # External I2C/SMBus adapter drivers > obj-$(CONFIG_I2C_PARPORT) += i2c-parport.o > diff --git a/drivers/i2c/busses/i2c-pch.c b/drivers/i2c/busses/i2c-pch.c > new file mode 100644 > index 000..37491d7 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-pch.c > @@ -0,0 +1,908 @@ > +/* > + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define PCH_EVENT_SET 0 /* I2C Interrupt Event Set Status */ > +#define PCH_EVENT_NONE 1 /* I2C Interrupt Event Clear Status */ > +#define PCH_MAX_CLK 10 /* Maximum Clock speed in MHz */ > +#define PCH_BUFFER_MODE_ENABLE 0x0002 /* flag for Buffer mode enable */ > +#define PCH_EEPROM_SW_RST_MODE_ENABLE 0x0008 /* EEPROM SW RST enable flag */ > + > +#define PCH_I2CSADR 0x00 /* I2C slave address register */ > +#define PCH_I2CCTL 0x04 /* I2C control register */ > +#define PCH_I2CSR 0x08 /* I2C status register */ > +#define PCH_I2CDR 0x0C /* I2C data register */ > +#define PCH_I2CMON 0x10 /* I2C bus monitor register */ > +#define PCH_I2CBC 0x14 /* I2C bus transfer rate setup counter */ > +#define PCH_I2CMOD 0x18 /* I2C mode register */ > +#define PCH_I2CBUFSLV 0x1C /* I2C buffer mode slave address register */ > +#define PCH_I2CBUFSUB 0x20 /* I2C buffer mode subaddress register */ > +#define PCH_I2CBUFFOR 0x24 /* I2C buffer mode format register */ > +#define PCH_I2CBUFCTL 0x28 /* I2C buffer mode control register */ > +#define PCH_I2CBUFMSK 0x2C /* I2C buffer mode interrupt mask register */ > +#define PCH_I2CBUFSTA 0x30 /* I2C buffer mode status register */ > +#define PCH_I2CBUFLEV 0x34 /* I2C buffer mode level register */ > +#define PCH_I2CESRFOR 0x38 /* EEPROM software reset mode format register */ > +#define PCH_I2CESRCTL 0x3C /* EEPROM software reset mode
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
>>
>> 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
>> Signed-off-by: Mike Frysinger
>> ---
>> 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
>> longirq_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-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/2] Short timeout for I2C transfers
>-Original Message- >From: linux-i2c-ow...@vger.kernel.org [mailto:linux-i2c- >ow...@vger.kernel.org] On Behalf Of ext samu.p.onk...@nokia.com >>The timeout is a symptom as the controller level. What is the root >>cause at the LP5523 device level? What exactly happens on the wire? >> > >Chip seems to release ACK signal too soon causing rising edge while >SCL is high. This violates bus protocol and causes timeout at the >controller. It is no recognized as ACK nor NACK. > >>I am asking because maybe some of the already available protocol >>mangling flags would work for you, in particular I2C_M_IGNORE_NAK. See >>Documentation/i2c/i2c-protocol. >> >> >>You should also check why you hit the timeout condition. Ideally the >>hardware would report problems as they happen, quickly, rather than >>having to rely on the driver's timeout mechanism. The timeout should >>really only be a safety mechanism, for when the bus controller itself >>misbehaves at the hardware level. >> It seems that controller detects bus free condition when this happens. So, instead of the timeout, bus free condition should be detected. There is no such interrupt so this must be done by polling the status register. In general this is not a good idea. Would it make sense somehow tell that use polling and not the interrupt / timeout control? I.e. by setting a flag that this transfer doesn't end properly. I2C_M_IGNORE_TIMEOUT? In such case just either wait for bus free if supported by HW or wait for some short time after start of the transmission -Samu -- 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: [PATCH 0/2] Short timeout for I2C transfers
>-Original Message- >From: ext Jean Delvare [mailto:kh...@linux-fr.org] >Sent: 15 October, 2010 15:52 >To: Onkalo Samu.P (Nokia-MS/Tampere) >Cc: t...@atomide.com; linux-i2c@vger.kernel.org; linux- >ker...@vger.kernel.org >Subject: Re: [PATCH 0/2] Short timeout for I2C transfers > >Hi Samu, > >On Wed, 13 Oct 2010 16:45:08 +0300, Samu Onkalo wrote: >> There are chips which doesn't work perfectly in certain >> I2C operations. For example lp5523 led driver chip causes >> always I2C timeout when SW reset is commanded to the chip. > >The timeout is a symptom as the controller level. What is the root >cause at the LP5523 device level? What exactly happens on the wire? > Chip seems to release ACK signal too soon causing rising edge while SCL is high. This violates bus protocol and causes timeout at the controller. It is no recognized as ACK nor NACK. >I am asking because maybe some of the already available protocol >mangling flags would work for you, in particular I2C_M_IGNORE_NAK. See >Documentation/i2c/i2c-protocol. > >> Patches add a possibility to tell that this access is likely >> to cause time out and there is no need to wait normal time. >> For example in omap time out is 1 second and the bus is reserved >> all the time. > >You should ask yourself whether a 1 second timeout is a sane thing to >have in the first place. hmm.. 1 second is quite long time out for i2c. > >You should also check why you hit the timeout condition. Ideally the >hardware would report problems as they happen, quickly, rather than >having to rely on the driver's timeout mechanism. The timeout should >really only be a safety mechanism, for when the bus controller itself >misbehaves at the hardware level. > >> By setting I2C_M_SHORT_TIMEOUT flag to i2c-message, adapter is >> requested to use shorter timeout. >> >> Samu Onkalo (2): >> drivers: i2c-core: Add a flag to allow short timeout >> drivers: i2c-omap: Add support for shorten I2C timeout >> >> drivers/i2c/busses/i2c-omap.c |9 - >> drivers/i2c/i2c-core.c|2 +- >> include/linux/i2c.h |1 + >> 3 files changed, 10 insertions(+), 2 deletions(-) > >I'm skeptical. If you know that the transfer will "fail" and you don't >care, why do you want to wait a short time, rather than not waiting at >all? Well, at least some time should be spend to allow bits to go out. Maybe controller provides something for that. -Samu -- 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: [PATCH] i2c: add irq_flags to board info
Hi Mike,
On Sun, 17 Oct 2010 19:43:39 -0400, Mike Frysinger wrote:
> From: Michael Hennerich
>
> 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?
> -- which doesn't work very well when coupled with
> module drivers.
I don't quite get what you mean here.
> It also matches behavior of some other frameworks like
> IDE and UIO.
This is certainly a good point.
>
> Signed-off-by: Michael Hennerich
> Signed-off-by: Mike Frysinger
> ---
> 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;
> };
>
> /**
--
Jean Delvare
--
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: [PATCH] i2c: i2c-pca-platform: Change i2c adapter name
Hi Nobuhiro,
On Mon, 18 Oct 2010 12:24:30 +0900, Nobuhiro Iwamatsu wrote:
> In kernel which enableded this driver output The following warning,
> because a slash (/) is in the i2c adapter name by the current driver.
> This set another adapter name (i2c-pca-platform at 0x) what removed slash.
>
> [2.872000] PCA9564/PCA9665 at 0x0600: Registering gpio failed!
> [2.876000] name 'PCA9564/PCA9665 at 0x0600'
> [2.88] [ cut here ]
> [2.88] WARNING: at fs/proc/generic.c:323
> [2.88] Modules linked in:
> [2.88]
> [2.88] Pid : 1, Comm: swapper
> [2.88] CPU : 0 Not tainted
> (2.6.36-rc1-00238-g67e5b3a #261)
> [2.88]
> [2.88] PC is at __xlate_proc_name+0x88/0xa4
> [2.88] PR is at __xlate_proc_name+0x88/0xa4
> [2.88] PC : 800b3bc4 SP : 9fc41d34 SR : 40008001 TEA : c0018000
> [2.88] R0 : 0037 R1 : 9fc4 R2 : 9fc4 R3 : 8049db48
> [2.88] R4 : 0001 R5 : 00f0 R6 : R7 : 8019e450
> [2.88] R8 : R9 : 9fc41d9c R10 : 0007 R11 : 9fc41d9c
> [2.88] R12 : 9fc41d80 R13 : 80167fce R14 : 9fc41d34
> [2.88] MACH: 0461 MACL: GBR : PR : 800b3bc4
> [2.88]
> [2.88] Call trace:
> [2.88] [<800b4056>] __proc_create+0x46/0x110
> [2.88] [<80162b00>] strlen+0x0/0x58
> [2.88] [<800b4824>] proc_mkdir_mode+0x24/0x60
> [2.88] [<800b486e>] proc_mkdir+0xe/0x1c
> [2.88] [<8004740a>] register_handler_proc+0x9e/0xd4
> [2.88] [<80045ab2>] __setup_irq+0x226/0x2c0
> [2.88] [<80075d50>] kmem_cache_alloc+0x94/0xf0
> [2.88] [<80045bcc>] request_threaded_irq+0x80/0xdc
> [2.88] [<802041c0>] i2c_pca_pf_handler+0x0/0x3c
> [2.88] [<802ecb62>] i2c_pca_pf_probe+0x162/0x294
> [2.88] [<801a3b38>] platform_drv_probe+0x14/0x1c
> (...)
The i2c adapter names are strings which appear as the _contents_ of
sysfs attributes. They don't appear in procfs. What happens here is
that the adapter name is used also as the IRQ name:
if (irq) {
ret = request_irq(irq, i2c_pca_pf_handler,
IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
if (ret)
goto e_reqirq;
}
This is the problem. i2c->adap.name shouldn't be used here, instead the
driver name "i2c-pca-platform" should be used.
> (...)
> Signed-off-by: Nobuhiro Iwamatsu
> CC: Wolfram Sang
> ---
> drivers/i2c/busses/i2c-pca-platform.c |8 +---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pca-platform.c
> b/drivers/i2c/busses/i2c-pca-platform.c
> index ef5c784..4cc3cdf 100644
> --- a/drivers/i2c/busses/i2c-pca-platform.c
> +++ b/drivers/i2c/busses/i2c-pca-platform.c
> @@ -27,6 +27,8 @@
>
> #include
>
> +#define DRIVER_NAME "i2c-pca-platform"
> +
> struct i2c_pca_pf_data {
> void __iomem*reg_base;
> int irq;/* if 0, use polling */
> @@ -171,7 +173,7 @@ static int __devinit i2c_pca_pf_probe(struct
> platform_device *pdev)
> i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> i2c->adap.owner = THIS_MODULE;
> snprintf(i2c->adap.name, sizeof(i2c->adap.name),
> - "PCA9564/PCA9665 at 0x%08lx",
> + DRIVER_NAME " at 0x%08lx",
>(unsigned long) res->start);
I would like to avoid changing the adapter name when we don't have to.
People may be using this string as an identifier in i2c tools. Please
leave the adapter name unchanged and only change the IRQ name.
> i2c->adap.algo_data = &i2c->algo_data;
> i2c->adap.dev.parent = &pdev->dev;
> @@ -250,7 +252,7 @@ e_remap:
> e_alloc:
> release_mem_region(res->start, resource_size(res));
> e_print:
> - printk(KERN_ERR "Registering PCA9564/PCA9665 FAILED! (%d)\n", ret);
> + printk(KERN_ERR "Registering " DRIVER_NAME " FAILED! (%d)\n", ret);
This is semantically incorrect. We are registering a device here, not a
driver. The proposed change makes this error message confusing. Please
revert.
> return ret;
> }
>
> @@ -278,7 +280,7 @@ static struct platform_driver i2c_pca_pf_driver = {
> .probe = i2c_pca_pf_probe,
> .remove = __devexit_p(i2c_pca_pf_remove),
> .driver = {
> - .name = "i2c-pca-platform",
> + .name = DRIVER_NAME,
> .owner = THIS_MODULE,
> },
> };
--
Jean Delvare
--
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
