Prev: X86_PAT & mprotect
Next: [PATCH] kbuild: escape meta characters in regular expression in make TAGS
From: Jean Delvare on 7 May 2008 03:10
Hi Maciej,

On Wed, 7 May 2008 01:40:27 +0100 (BST), Maciej W. Rozycki wrote:
> (...)
> 2. The i2c-sibyte.c BCM1250A SMBus controller driver now registers its
> buses as numbered so that board setup is correctly applied. Plus minor
> corrections.

Minor corrections which would ideally belong to a separate patch
(there's a whole lot more cleanups that could be done in that driver,
BTW...)

> (...)
> --- linux-2.6.26-rc1-20080505.macro/drivers/i2c/busses/i2c-sibyte.c 2008-05-05 02:55:25.000000000 +0000
> +++ linux-2.6.26-rc1-20080505/drivers/i2c/busses/i2c-sibyte.c 2008-05-06 23:45:32.000000000 +0000
> @@ -2,6 +2,7 @@
> * Copyright (C) 2004 Steven J. Hill
> * Copyright (C) 2001,2002,2003 Broadcom Corporation
> * Copyright (C) 1995-2000 Simon G. Vogl
> + * Copyright (C) 2008 Maciej W. Rozycki

I don't think that the minor changes below are enough for you to claim
copyright on that driver.

> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License
> @@ -132,18 +133,18 @@ static const struct i2c_algorithm i2c_si
> /*
> * registering functions to load algorithms at runtime
> */
> -int __init i2c_sibyte_add_bus(struct i2c_adapter *i2c_adap, int speed)
> +static int __init i2c_sibyte_add_bus(struct i2c_adapter *i2c_adap, int speed)
> {
> struct i2c_algo_sibyte_data *adap = i2c_adap->algo_data;
>
> - /* register new adapter to i2c module... */
> + /* Register new adapter to i2c module... */
> i2c_adap->algo = &i2c_sibyte_algo;
>
> - /* Set the frequency to 100 kHz */
> + /* Set the requested frequency. */

Why do you double the space and the end of comments? Never seen that
before, and I can't see the idea.

> csr_out32(speed, SMB_CSR(adap,R_SMB_FREQ));
> csr_out32(0, SMB_CSR(adap,R_SMB_CONTROL));
>
> - return i2c_add_adapter(i2c_adap);
> + return i2c_add_numbered_adapter(i2c_adap);
> }
>
>
> @@ -159,6 +160,7 @@ static struct i2c_adapter sibyte_board_a
> .class = I2C_CLASS_HWMON,
> .algo = NULL,
> .algo_data = &sibyte_board_data[0],
> + .nr = 0,
> .name = "SiByte SMBus 0",
> },
> {
> @@ -167,6 +169,7 @@ static struct i2c_adapter sibyte_board_a
> .class = I2C_CLASS_HWMON,
> .algo = NULL,
> .algo_data = &sibyte_board_data[1],
> + .nr = 1,
> .name = "SiByte SMBus 1",
> },
> };

I'm not sure how you intend to push these changes upstream. I would
take a patch only touching drivers/i2c/busses/i2c-sibyte.c in my i2c
tree, however a patch also touching arch code, must be handled be the
maintainer for that architecture or platform.

--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jean Delvare on 7 May 2008 03:10
Oh, BTW...

On Wed, 7 May 2008 01:40:27 +0100 (BST), Maciej W. Rozycki wrote:
> (...)
> 1. i2c-swarm.c -- SWARM I2C board setup, currently for the M41T80 chip on
> the bus #1 only.
> (...)
> --- linux-2.6.26-rc1-20080505.macro/arch/mips/sibyte/swarm/Makefile 2004-01-29 04:57:05.000000000 +0000
> +++ linux-2.6.26-rc1-20080505/arch/mips/sibyte/swarm/Makefile 2008-05-06 01:18:21.000000000 +0000
> @@ -1,3 +1,4 @@
> -lib-y = setup.o rtc_xicor1241.o rtc_m41t81.o
> +obj-y := setup.o rtc_xicor1241.o rtc_m41t81.o
>
> -lib-$(CONFIG_KGDB) += dbg_io.o
> +obj-$(CONFIG_I2C_BOARDINFO) += i2c-swarm.o
> +obj-$(CONFIG_KGDB) += dbg_io.o
> (...)
> --- linux-2.6.26-rc1-20080505.macro/arch/mips/sibyte/swarm/i2c-swarm.c 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.26-rc1-20080505/arch/mips/sibyte/swarm/i2c-swarm.c 2008-05-06 23:51:34.000000000 +0000

i2c-foo.c is consistently used for i2c bus driver themselves so far.
It's somewhat confusing to see you name platform code that way. It's
also redundant, given that the file lives in the swarm platform
directory. May I suggest naming this file just
arch/mips/sibyte/swarm/i2c.c? Other architectures (cris, arm) are doing
this already.

--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 7 May 2008 03:20
On Wed, 7 May 2008 08:59:53 +0200 Jean Delvare <khali(a)linux-fr.org> wrote:

> I'm not sure how you intend to push these changes upstream. I would
> take a patch only touching drivers/i2c/busses/i2c-sibyte.c in my i2c
> tree, however a patch also touching arch code, must be handled be the
> maintainer for that architecture or platform.

Not "must". The arch maintainer could ask you to merge it or you could ask
the arch maintainer to merge it.

It's some little one-line change like this one appeared to be, it's
fair to assume the arch maintainer won't care much about it. View it as an
i2c patch?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Geert Uytterhoeven on 7 May 2008 03:40
On Wed, 7 May 2008, Jean Delvare wrote:
> Oh, BTW...
>
> On Wed, 7 May 2008 01:40:27 +0100 (BST), Maciej W. Rozycki wrote:
> > (...)
> > 1. i2c-swarm.c -- SWARM I2C board setup, currently for the M41T80 chip on
> > the bus #1 only.
> > (...)
> > --- linux-2.6.26-rc1-20080505.macro/arch/mips/sibyte/swarm/Makefile 2004-01-29 04:57:05.000000000 +0000
> > +++ linux-2.6.26-rc1-20080505/arch/mips/sibyte/swarm/Makefile 2008-05-06 01:18:21.000000000 +0000
> > @@ -1,3 +1,4 @@
> > -lib-y = setup.o rtc_xicor1241.o rtc_m41t81.o
> > +obj-y := setup.o rtc_xicor1241.o rtc_m41t81.o
> >
> > -lib-$(CONFIG_KGDB) += dbg_io.o
> > +obj-$(CONFIG_I2C_BOARDINFO) += i2c-swarm.o
> > +obj-$(CONFIG_KGDB) += dbg_io.o
> > (...)
> > --- linux-2.6.26-rc1-20080505.macro/arch/mips/sibyte/swarm/i2c-swarm.c 1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6.26-rc1-20080505/arch/mips/sibyte/swarm/i2c-swarm.c 2008-05-06 23:51:34.000000000 +0000
>
> i2c-foo.c is consistently used for i2c bus driver themselves so far.
> It's somewhat confusing to see you name platform code that way. It's
> also redundant, given that the file lives in the swarm platform
> directory. May I suggest naming this file just
> arch/mips/sibyte/swarm/i2c.c? Other architectures (cris, arm) are doing
> this already.

Is there any chance CONFIG_I2C_BOARDINFO could become tristate?
If yes, it's problematic if you have multiple modules called i2c.ko.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert(a)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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jean Delvare on 7 May 2008 03:50
Hi Geert,

On Wed, 7 May 2008 09:37:01 +0200 (CEST), Geert Uytterhoeven wrote:
> On Wed, 7 May 2008, Jean Delvare wrote:
> > Oh, BTW...
> >
> > On Wed, 7 May 2008 01:40:27 +0100 (BST), Maciej W. Rozycki wrote:
> > > (...)
> > > 1. i2c-swarm.c -- SWARM I2C board setup, currently for the M41T80 chip on
> > > the bus #1 only.
> > > (...)
> > > --- linux-2.6.26-rc1-20080505.macro/arch/mips/sibyte/swarm/Makefile 2004-01-29 04:57:05.000000000 +0000
> > > +++ linux-2.6.26-rc1-20080505/arch/mips/sibyte/swarm/Makefile 2008-05-06 01:18:21.000000000 +0000
> > > @@ -1,3 +1,4 @@
> > > -lib-y = setup.o rtc_xicor1241.o rtc_m41t81.o
> > > +obj-y := setup.o rtc_xicor1241.o rtc_m41t81.o
> > >
> > > -lib-$(CONFIG_KGDB) += dbg_io.o
> > > +obj-$(CONFIG_I2C_BOARDINFO) += i2c-swarm.o
> > > +obj-$(CONFIG_KGDB) += dbg_io.o
> > > (...)
> > > --- linux-2.6.26-rc1-20080505.macro/arch/mips/sibyte/swarm/i2c-swarm.c 1970-01-01 00:00:00.000000000 +0000
> > > +++ linux-2.6.26-rc1-20080505/arch/mips/sibyte/swarm/i2c-swarm.c 2008-05-06 23:51:34.000000000 +0000
> >
> > i2c-foo.c is consistently used for i2c bus driver themselves so far.
> > It's somewhat confusing to see you name platform code that way. It's
> > also redundant, given that the file lives in the swarm platform
> > directory. May I suggest naming this file just
> > arch/mips/sibyte/swarm/i2c.c? Other architectures (cris, arm) are doing
> > this already.
>
> Is there any chance CONFIG_I2C_BOARDINFO could become tristate?
> If yes, it's problematic if you have multiple modules called i2c.ko.

No, CONFIG_I2C_BOARDINFO is boolean by nature, it will never become
tristate.

--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: X86_PAT & mprotect
Next: [PATCH] kbuild: escape meta characters in regular expression in make TAGS