i2c module aliases

i2c module aliases

[Geert Uytterhoeven]

>From browsing through file2alias.c, I have the impression there's a
missing TO_NATIVE() conversion in the handling of 16-bit (hence,
endian-affected) i2c IDs.
This will cause the module aliases to be incorrect when cross-compiling
for a system with a different endianness.  Is that correct?
If yes, the patch below adds the missing conversion.

Of course, no driver seems to use `MODULE_DEVICE_TABLE(i2c, ...)' yet,
hence nobody noticed.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -390,6 +390,7 @@ static int do_vio_entry(const char *file
 
 static int do_i2c_entry(const char *filename, struct i2c_device_id *i2c, char *alias)
 {
+	i2c->id = TO_NATIVE(i2c->id);
 	strcpy(alias, "i2c:");
 	ADD(alias, "id", 1, i2c->id);
 	return 1;

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [i2c] i2c module aliases

[Jean Delvare]

Hi Geert,

On Wed, 6 Jun 2007 23:32:38 +0200 (CEST), Geert Uytterhoeven wrote:
> >From browsing through file2alias.c, I have the impression there's a
> missing TO_NATIVE() conversion in the handling of 16-bit (hence,
> endian-affected) i2c IDs.
> This will cause the module aliases to be incorrect when cross-compiling
> for a system with a different endianness.  Is that correct?
> If yes, the patch below adds the missing conversion.
> 
> Of course, no driver seems to use `MODULE_DEVICE_TABLE(i2c, ...)' yet,
> hence nobody noticed.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -390,6 +390,7 @@ static int do_vio_entry(const char *file
>  
>  static int do_i2c_entry(const char *filename, struct i2c_device_id *i2c, char *alias)
>  {
> +	i2c->id = TO_NATIVE(i2c->id);
>  	strcpy(alias, "i2c:");
>  	ADD(alias, "id", 1, i2c->id);
>  	return 1;
> 

Looks correct. However I am curious what this "i2c->id" is supposed to
represent, given that I2C devices do not have any form of ID as PCI or
USB devices have. No driver uses "MODULE_DEVICE_TABLE(i2c, ...)"
probably just because it doesn't make sense. So, rather than fixing
this, shouldn't we simply wipe out struct i2c_device_id? Greg, David?

-- 
Jean Delvare

Re: [i2c] i2c module aliases

[Greg KH]

On Thu, Jun 07, 2007 at 05:53:42PM +0200, Jean Delvare wrote:
> Hi Geert,
> 
> On Wed, 6 Jun 2007 23:32:38 +0200 (CEST), Geert Uytterhoeven wrote:
> > >From browsing through file2alias.c, I have the impression there's a
> > missing TO_NATIVE() conversion in the handling of 16-bit (hence,
> > endian-affected) i2c IDs.
> > This will cause the module aliases to be incorrect when cross-compiling
> > for a system with a different endianness.  Is that correct?
> > If yes, the patch below adds the missing conversion.
> > 
> > Of course, no driver seems to use `MODULE_DEVICE_TABLE(i2c, ...)' yet,
> > hence nobody noticed.
> > 
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > 
> > --- a/scripts/mod/file2alias.c
> > +++ b/scripts/mod/file2alias.c
> > @@ -390,6 +390,7 @@ static int do_vio_entry(const char *file
> >  
> >  static int do_i2c_entry(const char *filename, struct i2c_device_id *i2c, char *alias)
> >  {
> > +	i2c->id = TO_NATIVE(i2c->id);
> >  	strcpy(alias, "i2c:");
> >  	ADD(alias, "id", 1, i2c->id);
> >  	return 1;
> > 
> 
> Looks correct. However I am curious what this "i2c->id" is supposed to
> represent, given that I2C devices do not have any form of ID as PCI or
> USB devices have. No driver uses "MODULE_DEVICE_TABLE(i2c, ...)"
> probably just because it doesn't make sense. So, rather than fixing
> this, shouldn't we simply wipe out struct i2c_device_id? Greg, David?

If no one is using it, then yes, it should be removed.

Sorry, I added it a while ago thinking it was going to be used (I had
test patches using it in my tree, but they never worked out in the end.)

thanks,

greg k-h

Re: [i2c] i2c module aliases

[Jean Delvare]

On Thu, 7 Jun 2007 09:02:14 -0700, Greg KH wrote:
> On Thu, Jun 07, 2007 at 05:53:42PM +0200, Jean Delvare wrote:
> > Looks correct. However I am curious what this "i2c->id" is supposed to
> > represent, given that I2C devices do not have any form of ID as PCI or
> > USB devices have. No driver uses "MODULE_DEVICE_TABLE(i2c, ...)"
> > probably just because it doesn't make sense. So, rather than fixing
> > this, shouldn't we simply wipe out struct i2c_device_id? Greg, David?
> 
> If no one is using it, then yes, it should be removed.
> 
> Sorry, I added it a while ago thinking it was going to be used (I had
> test patches using it in my tree, but they never worked out in the end.)

OK. Geert, care to submit a new patch removing struct i2c_device_id
altogether?

Thanks,
-- 
Jean Delvare

Re: [i2c] i2c module aliases

[Geert Uytterhoeven]

On Fri, 8 Jun 2007, Jean Delvare wrote:
> On Thu, 7 Jun 2007 09:02:14 -0700, Greg KH wrote:
> > On Thu, Jun 07, 2007 at 05:53:42PM +0200, Jean Delvare wrote:
> > > Looks correct. However I am curious what this "i2c->id" is supposed to
> > > represent, given that I2C devices do not have any form of ID as PCI or
> > > USB devices have. No driver uses "MODULE_DEVICE_TABLE(i2c, ...)"
> > > probably just because it doesn't make sense. So, rather than fixing
> > > this, shouldn't we simply wipe out struct i2c_device_id? Greg, David?
> > 
> > If no one is using it, then yes, it should be removed.
> > 
> > Sorry, I added it a while ago thinking it was going to be used (I had
> > test patches using it in my tree, but they never worked out in the end.)
> 
> OK. Geert, care to submit a new patch removing struct i2c_device_id
> altogether?

Sorry, probably not...

I just thought I found a grave bug while looking into adding zorro
support to scripts/mod/file2alias.c, so that's why I reported it.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [i2c] i2c module aliases

[Jean Delvare]

On Fri, 8 Jun 2007 16:48:45 +0200 (CEST), Geert Uytterhoeven wrote:
> On Fri, 8 Jun 2007, Jean Delvare wrote:
> > OK. Geert, care to submit a new patch removing struct i2c_device_id
> > altogether?
> 
> Sorry, probably not...
> 
> I just thought I found a grave bug while looking into adding zorro
> support to scripts/mod/file2alias.c, so that's why I reported it.

OK, no problem. I gave it a try myself. Greg, is it OK or did I miss
other parts which can be removed?

* * * * *

Subject: i2c: Kill struct i2c_device_id

I2C devices do not have any form of ID as PCI or USB devices have.
No driver uses "MODULE_DEVICE_TABLE(i2c, ...)" because it doesn't
make sense. So we can get rid of struct i2c_device_id and the
associated support code.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Greg KH <greg@kroah.com>
---
 include/linux/mod_devicetable.h |    5 -----
 scripts/mod/file2alias.c        |   11 -----------
 2 files changed, 16 deletions(-)

--- linux-2.6.22-rc4.orig/include/linux/mod_devicetable.h	2007-05-13 10:01:23.000000000 +0200
+++ linux-2.6.22-rc4/include/linux/mod_devicetable.h	2007-06-08 19:08:29.000000000 +0200
@@ -255,11 +255,6 @@ struct pcmcia_device_id {
 #define PCMCIA_DEV_ID_MATCH_FAKE_CIS	0x0200
 #define PCMCIA_DEV_ID_MATCH_ANONYMOUS	0x0400
 
-/* I2C */
-struct i2c_device_id {
-	__u16 id;
-};
-
 /* Input */
 #define INPUT_DEVICE_ID_EV_MAX		0x1f
 #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING	0x71
--- linux-2.6.22-rc4.orig/scripts/mod/file2alias.c	2007-06-08 18:56:36.000000000 +0200
+++ linux-2.6.22-rc4/scripts/mod/file2alias.c	2007-06-08 19:14:18.000000000 +0200
@@ -388,13 +388,6 @@ static int do_vio_entry(const char *file
 	return 1;
 }
 
-static int do_i2c_entry(const char *filename, struct i2c_device_id *i2c, char *alias)
-{
-	strcpy(alias, "i2c:");
-	ADD(alias, "id", 1, i2c->id);
-	return 1;
-}
-
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 static void do_input(char *alias,
@@ -571,10 +564,6 @@ void handle_moddevtable(struct module *m
 		do_table(symval, sym->st_size,
 			 sizeof(struct vio_device_id), "vio",
 			 do_vio_entry, mod);
-	else if (sym_is(symname, "__mod_i2c_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct i2c_device_id), "i2c",
-			 do_i2c_entry, mod);
 	else if (sym_is(symname, "__mod_input_device_table"))
 		do_table(symval, sym->st_size,
 			 sizeof(struct input_device_id), "input",



-- 
Jean Delvare

Re: [i2c] i2c module aliases

[Greg KH]

On Sat, Jun 09, 2007 at 12:52:21PM +0200, Jean Delvare wrote:
> On Fri, 8 Jun 2007 16:48:45 +0200 (CEST), Geert Uytterhoeven wrote:
> > On Fri, 8 Jun 2007, Jean Delvare wrote:
> > > OK. Geert, care to submit a new patch removing struct i2c_device_id
> > > altogether?
> > 
> > Sorry, probably not...
> > 
> > I just thought I found a grave bug while looking into adding zorro
> > support to scripts/mod/file2alias.c, so that's why I reported it.
> 
> OK, no problem. I gave it a try myself. Greg, is it OK or did I miss
> other parts which can be removed?

Looks good to me.  Want me to take this through my tree or through
yours?

thanks,

greg k-h

Re: [i2c] i2c module aliases

[Jean Delvare]

On Thu, 14 Jun 2007 12:14:24 -0700, Greg KH wrote:
> On Sat, Jun 09, 2007 at 12:52:21PM +0200, Jean Delvare wrote:
> > OK, no problem. I gave it a try myself. Greg, is it OK or did I miss
> > other parts which can be removed?
> 
> Looks good to me.  Want me to take this through my tree or through
> yours?

It's already in my i2c tree.

Thanks,
-- 
Jean Delvare