2008-07-16 19:34:15

by User Linux

[permalink] [raw]
Subject: Problem with restricted I2C algorithms in kernel 2.6.26!

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3845de25c5f83cd52729570f7b501679d37ca8de

The patch at the preceeding url disables the users ability to select
I2C algorithms. Specifically the reason stated was:

"The algorithm drivers are
helper drivers that are selected automatically
as needed. There's no point in listing them in the config menu, it can
only confuse users and waste their time."

The algorithm drivers will not be 'selected automatically as needed'
if the user is compiling something outside of the kernel that requires
them! Just one example, there are drivers found in the V4L dvb driver
tree that require i2c bit-banging be enabled. The drivers are now
broken because the user is not allowed to enable bit-banging himself.
The only way around this is to revert the patch manually or enable
something else in the kernel, that he doesn't need, just to get
bit-banging.

It's a very bad idea to assume that nothing built outside of the
kernel may need i2c algorithms. Furthermore, the whole point of being
able to customize your kernel is so you can select only the things
which you need. It makes no good sense to intentionally
disable/restrict the users ability to do so. Additionally, assuming
the ability to select i2c algorithms will only confuse the user and
waste their time is ridiculous. The user should be allowed to decide
for himself what he needs regarding this!

One of the biggest reasons people choose to compile things from
cvs/svn/mercurial/etc. is because it gives them access to newer bug
fixes and support for things not yet present in the kernel source. A
perfect example, the multiproto dvb driver tree. Users wanting
support for dvb-s2 devices have to compile drivers outside of the
kernel because it's simply not available in the kernel and won't be
for some time.

I've contacted one of the i2c subsystem maintainers, Jean Delvare, but
unfortunately he doesn't seem to care about this problem and his
advice in dealing with it is to "Just get these drivers merged in the
kernel. Ah ah ah!"...

Clearly the more sane and reasonable solution is to not cripple the
menu options in the first place, especially when it creates no benefit
and only serves to limit/restrict the users ability to select what he
needs. I'm asking that the patch be reverted and anyone in agreement
to please voice their opinion here in public.

Best regards,
-Derek


2008-07-26 07:00:46

by Andrew Morton

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

(cc's added)

On Wed, 16 Jul 2008 12:33:57 -0700 "D. Kelly" <[email protected]> wrote:

> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3845de25c5f83cd52729570f7b501679d37ca8de
>
> The patch at the preceeding url disables the users ability to select
> I2C algorithms. Specifically the reason stated was:
>
> "The algorithm drivers are
> helper drivers that are selected automatically
> as needed. There's no point in listing them in the config menu, it can
> only confuse users and waste their time."
>
> The algorithm drivers will not be 'selected automatically as needed'
> if the user is compiling something outside of the kernel that requires
> them! Just one example, there are drivers found in the V4L dvb driver
> tree that require i2c bit-banging be enabled. The drivers are now
> broken because the user is not allowed to enable bit-banging himself.
> The only way around this is to revert the patch manually or enable
> something else in the kernel, that he doesn't need, just to get
> bit-banging.
>
> It's a very bad idea to assume that nothing built outside of the
> kernel may need i2c algorithms. Furthermore, the whole point of being
> able to customize your kernel is so you can select only the things
> which you need. It makes no good sense to intentionally
> disable/restrict the users ability to do so. Additionally, assuming
> the ability to select i2c algorithms will only confuse the user and
> waste their time is ridiculous. The user should be allowed to decide
> for himself what he needs regarding this!
>
> One of the biggest reasons people choose to compile things from
> cvs/svn/mercurial/etc. is because it gives them access to newer bug
> fixes and support for things not yet present in the kernel source. A
> perfect example, the multiproto dvb driver tree. Users wanting
> support for dvb-s2 devices have to compile drivers outside of the
> kernel because it's simply not available in the kernel and won't be
> for some time.
>
> I've contacted one of the i2c subsystem maintainers, Jean Delvare, but
> unfortunately he doesn't seem to care about this problem and his
> advice in dealing with it is to "Just get these drivers merged in the
> kernel. Ah ah ah!"...
>
> Clearly the more sane and reasonable solution is to not cripple the
> menu options in the first place, especially when it creates no benefit
> and only serves to limit/restrict the users ability to select what he
> needs. I'm asking that the patch be reverted and anyone in agreement
> to please voice their opinion here in public.
>
> Best regards,
> -Derek

2008-07-26 14:34:32

by [email protected]

[permalink] [raw]
Subject: Re: [i2c] Problem with restricted I2C algorithms in kernel 2.6.26!

On 7/26/08, Andrew Morton <[email protected]> wrote:
> (cc's added)
>
>
> On Wed, 16 Jul 2008 12:33:57 -0700 "D. Kelly" <[email protected]> wrote:
>
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3845de25c5f83cd52729570f7b501679d37ca8de
> >
> > The patch at the preceeding url disables the users ability to select
> > I2C algorithms. Specifically the reason stated was:
> >
> > "The algorithm drivers are
> > helper drivers that are selected automatically
> > as needed. There's no point in listing them in the config menu, it can
> > only confuse users and waste their time."

I support Jean's decision on this. Very few people know how to
correctly enable those algorithms and most people get them wrong. Now
they are set automatically.

What about merging a placeholder driver for dvb-s2 that selects the
needed i2c algorithm? Then merge a real driver as soon as possible.

My out of tree drivers are written as a patch against the kernel. The
patch contains a select for the algorithm in my Kconfig additions.
When the drivers work, I'll submit them.



> >
> > The algorithm drivers will not be 'selected automatically as needed'
> > if the user is compiling something outside of the kernel that requires
> > them! Just one example, there are drivers found in the V4L dvb driver
> > tree that require i2c bit-banging be enabled. The drivers are now
> > broken because the user is not allowed to enable bit-banging himself.
> > The only way around this is to revert the patch manually or enable
> > something else in the kernel, that he doesn't need, just to get
> > bit-banging.
> >
> > It's a very bad idea to assume that nothing built outside of the
> > kernel may need i2c algorithms. Furthermore, the whole point of being
> > able to customize your kernel is so you can select only the things
> > which you need. It makes no good sense to intentionally
> > disable/restrict the users ability to do so. Additionally, assuming
> > the ability to select i2c algorithms will only confuse the user and
> > waste their time is ridiculous. The user should be allowed to decide
> > for himself what he needs regarding this!
> >
> > One of the biggest reasons people choose to compile things from
> > cvs/svn/mercurial/etc. is because it gives them access to newer bug
> > fixes and support for things not yet present in the kernel source. A
> > perfect example, the multiproto dvb driver tree. Users wanting
> > support for dvb-s2 devices have to compile drivers outside of the
> > kernel because it's simply not available in the kernel and won't be
> > for some time.
> >
> > I've contacted one of the i2c subsystem maintainers, Jean Delvare, but
> > unfortunately he doesn't seem to care about this problem and his
> > advice in dealing with it is to "Just get these drivers merged in the
> > kernel. Ah ah ah!"...
> >
> > Clearly the more sane and reasonable solution is to not cripple the
> > menu options in the first place, especially when it creates no benefit
> > and only serves to limit/restrict the users ability to select what he
> > needs. I'm asking that the patch be reverted and anyone in agreement
> > to please voice their opinion here in public.
> >
> > Best regards,
> > -Derek
>
>
>
> _______________________________________________
> i2c mailing list
> [email protected]
> http://lists.lm-sensors.org/mailman/listinfo/i2c
>


--
Jon Smirl
[email protected]

2008-08-07 12:34:22

by Jean Delvare

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

Hi all,

Sorry for the late answer, I am just back from vacation.

On Wed, 16 Jul 2008 12:33:57 -0700, D. Kelly wrote:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3845de25c5f83cd52729570f7b501679d37ca8de
>
> The patch at the preceeding url disables the users ability to select
> I2C algorithms. Specifically the reason stated was:
>
> "The algorithm drivers are
> helper drivers that are selected automatically
> as needed. There's no point in listing them in the config menu, it can
> only confuse users and waste their time."
>
> The algorithm drivers will not be 'selected automatically as needed'
> if the user is compiling something outside of the kernel that requires
> them! Just one example, there are drivers found in the V4L dvb driver
> tree that require i2c bit-banging be enabled. The drivers are now
> broken because the user is not allowed to enable bit-banging himself.
> The only way around this is to revert the patch manually or enable
> something else in the kernel, that he doesn't need, just to get
> bit-banging.

Note how bad a user experience this is in the first place: not only the
users don't get their driver directly in the kernel tree, but they also
need to follow specific instructions about how to configure the kernel
for said driver to build externally. This can be automated for drivers
which live in the kernel tree.

A 3rd way around this is to simply merge the driver in question in the
upstream kernel. This makes things much easier for kernel maintainers,
drivers authors and end users.

Alternatively, I am curious if our build system couldn't allow 3rd
party drivers to select in-tree modules. Obviously this would require
the complete kernel source tree to be available, instead of just the
header and config files as is usually the case. Sam, is this possible
to do that at this time? If not, is this something that could be
implemented, or is this too much work for the thin benefit?

> It's a very bad idea to assume that nothing built outside of the
> kernel may need i2c algorithms. Furthermore, the whole point of being
> able to customize your kernel is so you can select only the things
> which you need. It makes no good sense to intentionally
> disable/restrict the users ability to do so. Additionally, assuming
> the ability to select i2c algorithms will only confuse the user and
> waste their time is ridiculous. The user should be allowed to decide
> for himself what he needs regarding this!

You obviously haven't been the i2c subsystem maintainer for years. I
have been, and clearly remember users repeatedly asking about these
options. More generally, the kernel configuration menu has grown over
the past few years to a point where newcomers can hardly make their way
through all the options. Greg KH's "Linux kernel in a nutshell" helps a
bit, but still, making the kernel configuration as simple as possible
still stands as a valid goal.

> One of the biggest reasons people choose to compile things from
> cvs/svn/mercurial/etc. is because it gives them access to newer bug
> fixes and support for things not yet present in the kernel source. A
> perfect example, the multiproto dvb driver tree. Users wanting
> support for dvb-s2 devices have to compile drivers outside of the
> kernel because it's simply not available in the kernel and won't be
> for some time.

So basically you are telling that "thanks" to drivers being maintainers
in external repositories, bugs are not fixed in the upstream kernel in
a timely manner, and new features take more time to go there too? That
must be the reason why kernel developers and users alike don't like
external repositories in the first place.

And please don't tell me how some drivers "must" live in external
repositories. They just don't. In almost all cases that's a poor
decision by the driver author, period.

> I've contacted one of the i2c subsystem maintainers, Jean Delvare, but
> unfortunately he doesn't seem to care about this problem and his
> advice in dealing with it is to "Just get these drivers merged in the
> kernel. Ah ah ah!"...

Quoting people out of context and without their permission isn't fair.
You forget to mention that you contacted me anonymously, boldly claimed
that the 2.6.26 kernel was broken, omitted to explain exactly how the
change in question was a problem to you, and insulted me later in the
course of the discussion. So you received the treatment you deserved
(i.e. I ignored you.)

> Clearly the more sane and reasonable solution is to not cripple the
> menu options in the first place, especially when it creates no benefit
> and only serves to limit/restrict the users ability to select what he
> needs. I'm asking that the patch be reverted and anyone in agreement
> to please voice their opinion here in public.

That was 3 weeks ago and apparently nobody spoke in your favor. So
either nobody cares, or everyone is on vacation. Or both.

Note though that bug #11140 has been created for the same issue:
http://bugzilla.kernel.org/show_bug.cgi?id=11140

--
Jean Delvare

2008-08-07 16:08:23

by Trent Piepho

[permalink] [raw]
Subject: Re: [i2c] Problem with restricted I2C algorithms in kernel 2.6.26!

On Thu, 7 Aug 2008, Jean Delvare wrote:
> > One of the biggest reasons people choose to compile things from
> > cvs/svn/mercurial/etc. is because it gives them access to newer bug
> > fixes and support for things not yet present in the kernel source. A
> > perfect example, the multiproto dvb driver tree. Users wanting
> > support for dvb-s2 devices have to compile drivers outside of the
> > kernel because it's simply not available in the kernel and won't be
> > for some time.
>
> So basically you are telling that "thanks" to drivers being maintainers
> in external repositories, bugs are not fixed in the upstream kernel in
> a timely manner, and new features take more time to go there too? That
> must be the reason why kernel developers and users alike don't like
> external repositories in the first place.

Code needs to get testing before it's put in the kernel. How's that
supposed to happen if it's not made available outside the kernel tree
first?

Why does the kernel build system support building out of tree modules if no
one should do it?

Maybe an option to turn i2c algorithms on could do into the Library
Routines menu. There are already options for things like the crc routines
here so they can be turned on if an out of tree driver needs them but
nothing in the kernel does.

2008-08-07 16:14:40

by Jean Delvare

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

Hi Trent,

On Thu, 7 Aug 2008 09:01:35 -0700 (PDT), Trent Piepho wrote:
> On Thu, 7 Aug 2008, Jean Delvare wrote:
> > > One of the biggest reasons people choose to compile things from
> > > cvs/svn/mercurial/etc. is because it gives them access to newer bug
> > > fixes and support for things not yet present in the kernel source. A
> > > perfect example, the multiproto dvb driver tree. Users wanting
> > > support for dvb-s2 devices have to compile drivers outside of the
> > > kernel because it's simply not available in the kernel and won't be
> > > for some time.
> >
> > So basically you are telling that "thanks" to drivers being maintainers
> > in external repositories, bugs are not fixed in the upstream kernel in
> > a timely manner, and new features take more time to go there too? That
> > must be the reason why kernel developers and users alike don't like
> > external repositories in the first place.
>
> Code needs to get testing before it's put in the kernel. How's that
> supposed to happen if it's not made available outside the kernel tree
> first?

linux-next.

> Why does the kernel build system support building out of tree modules if no
> one should do it?

I guess it can be convenient at times. But people doing that shouldn't
dare to complain that everything isn't perfect for them.

> Maybe an option to turn i2c algorithms on could do into the Library
> Routines menu. There are already options for things like the crc routines
> here so they can be turned on if an out of tree driver needs them but
> nothing in the kernel does.

Having I2C-specific options selectable under the Library menu would
probably be even more confusing. However, it would be possible to do
something similar under the I2C menu. Much like
CONFIG_VIDEO_HELPER_CHIPS_AUTO does for the V4L subsystem:
CONFIG_I2C_ALGOS_AUTO would default to Y and would hide I2C algo driver
selection (as is the case in 2.6.26), changing it to N would present
the old menu for users to select the aldo drivers manually (as was the
case in 2.6.25.)

Which doesn't change my point that most people complaining about the
change would rather merge their drivers in the upstream kernel.

--
Jean Delvare

2008-08-07 17:20:15

by Jean Delvare

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

On Thu, 7 Aug 2008 18:14:16 +0200, Jean Delvare wrote:
> Hi Trent,
>
> On Thu, 7 Aug 2008 09:01:35 -0700 (PDT), Trent Piepho wrote:
> > Maybe an option to turn i2c algorithms on could do into the Library
> > Routines menu. There are already options for things like the crc routines
> > here so they can be turned on if an out of tree driver needs them but
> > nothing in the kernel does.
>
> Having I2C-specific options selectable under the Library menu would
> probably be even more confusing. However, it would be possible to do
> something similar under the I2C menu. Much like
> CONFIG_VIDEO_HELPER_CHIPS_AUTO does for the V4L subsystem:
> CONFIG_I2C_ALGOS_AUTO would default to Y and would hide I2C algo driver
> selection (as is the case in 2.6.26), changing it to N would present
> the old menu for users to select the aldo drivers manually (as was the
> case in 2.6.25.)

Something like this...

Subject: i2c: Let users select algorithm drivers manually again

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/i2c/Kconfig | 14 ++++++++++++++
drivers/i2c/algos/Kconfig | 11 ++++++++---
2 files changed, 22 insertions(+), 3 deletions(-)

--- linux-2.6.27-rc2.orig/drivers/i2c/Kconfig 2008-04-17 04:49:44.000000000 +0200
+++ linux-2.6.27-rc2/drivers/i2c/Kconfig 2008-08-07 19:14:37.000000000 +0200
@@ -38,6 +38,20 @@ config I2C_CHARDEV
This support is also available as a module. If so, the module
will be called i2c-dev.

+config I2C_HELPER_AUTO
+ bool "Autoselect pertinent helper modules"
+ default y
+ help
+ Some I2C bus drivers require so-called "I2C algorithm" modules
+ to work. These are basically software-only abstractions of generic
+ I2C interfaces. This option will autoselect them so that you don't
+ have to care.
+
+ Unselect this only if you need to enable additional helper
+ modules, for example for use with external I2C bus drivers.
+
+ In doubt, say Y.
+
source drivers/i2c/algos/Kconfig
source drivers/i2c/busses/Kconfig
source drivers/i2c/chips/Kconfig
--- linux-2.6.27-rc2.orig/drivers/i2c/algos/Kconfig 2008-07-14 11:14:59.000000000 +0200
+++ linux-2.6.27-rc2/drivers/i2c/algos/Kconfig 2008-08-07 18:50:43.000000000 +0200
@@ -2,15 +2,20 @@
# I2C algorithm drivers configuration
#

+menu "I2C Algorithms"
+ depends on !I2C_HELPER_AUTO
+
config I2C_ALGOBIT
- tristate
+ tristate "I2C bit-banging interfaces"

config I2C_ALGOPCF
- tristate
+ tristate "I2C PCF 8584 interfaces"

config I2C_ALGOPCA
- tristate
+ tristate "I2C PCA 9564 interfaces"

config I2C_ALGO_SGI
tristate
depends on SGI_IP22 || SGI_IP32 || X86_VISWS
+
+endmenu

Comment anyone?

--
Jean Delvare

2008-08-07 17:40:37

by Randy Dunlap

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

On Thu, 7 Aug 2008 19:19:43 +0200 Jean Delvare wrote:

> On Thu, 7 Aug 2008 18:14:16 +0200, Jean Delvare wrote:
> > Hi Trent,
> >
> > On Thu, 7 Aug 2008 09:01:35 -0700 (PDT), Trent Piepho wrote:
> > > Maybe an option to turn i2c algorithms on could do into the Library
> > > Routines menu. There are already options for things like the crc routines
> > > here so they can be turned on if an out of tree driver needs them but
> > > nothing in the kernel does.
> >
> > Having I2C-specific options selectable under the Library menu would
> > probably be even more confusing. However, it would be possible to do
> > something similar under the I2C menu. Much like
> > CONFIG_VIDEO_HELPER_CHIPS_AUTO does for the V4L subsystem:
> > CONFIG_I2C_ALGOS_AUTO would default to Y and would hide I2C algo driver
> > selection (as is the case in 2.6.26), changing it to N would present
> > the old menu for users to select the aldo drivers manually (as was the
> > case in 2.6.25.)
>
> Something like this...
>
> Subject: i2c: Let users select algorithm drivers manually again
>
> Signed-off-by: Jean Delvare <[email protected]>
> ---
> drivers/i2c/Kconfig | 14 ++++++++++++++
> drivers/i2c/algos/Kconfig | 11 ++++++++---
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> --- linux-2.6.27-rc2.orig/drivers/i2c/Kconfig 2008-04-17 04:49:44.000000000 +0200
> +++ linux-2.6.27-rc2/drivers/i2c/Kconfig 2008-08-07 19:14:37.000000000 +0200
> @@ -38,6 +38,20 @@ config I2C_CHARDEV
> This support is also available as a module. If so, the module
> will be called i2c-dev.
>
> +config I2C_HELPER_AUTO
> + bool "Autoselect pertinent helper modules"
> + default y
> + help
> + Some I2C bus drivers require so-called "I2C algorithm" modules
> + to work. These are basically software-only abstractions of generic
> + I2C interfaces. This option will autoselect them so that you don't
> + have to care.
> +
> + Unselect this only if you need to enable additional helper
> + modules, for example for use with external I2C bus drivers.
> +
> + In doubt, say Y.
> +
> source drivers/i2c/algos/Kconfig
> source drivers/i2c/busses/Kconfig
> source drivers/i2c/chips/Kconfig
> --- linux-2.6.27-rc2.orig/drivers/i2c/algos/Kconfig 2008-07-14 11:14:59.000000000 +0200
> +++ linux-2.6.27-rc2/drivers/i2c/algos/Kconfig 2008-08-07 18:50:43.000000000 +0200
> @@ -2,15 +2,20 @@
> # I2C algorithm drivers configuration
> #
>
> +menu "I2C Algorithms"
> + depends on !I2C_HELPER_AUTO
> +
> config I2C_ALGOBIT
> - tristate
> + tristate "I2C bit-banging interfaces"
>
> config I2C_ALGOPCF
> - tristate
> + tristate "I2C PCF 8584 interfaces"
>
> config I2C_ALGOPCA
> - tristate
> + tristate "I2C PCA 9564 interfaces"
>
> config I2C_ALGO_SGI
> tristate
> depends on SGI_IP22 || SGI_IP32 || X86_VISWS
> +
> +endmenu
>
> Comment anyone?


Seems reasonable to me.

---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

2008-08-07 18:40:51

by Sam Ravnborg

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

>
> Alternatively, I am curious if our build system couldn't allow 3rd
> party drivers to select in-tree modules. Obviously this would require
> the complete kernel source tree to be available, instead of just the
> header and config files as is usually the case. Sam, is this possible
> to do that at this time? If not, is this something that could be
> implemented, or is this too much work for the thin benefit?

In general I recommnd to have the full kernel source available
simply because we have no in-kernel solution to create the required
set of files to build external modules.

And today there is no way to hook into the kernel configuration
for an external module. First of we cannot allow changes in
the build kernel module as this would destroy module versioning
for instance.

And in this case you ask because you would change the kernel
configuration.

And I fail to see why this stuff cannot be done inside the
kernel source tree. Merging new kernel updates should be absolutely
trivial and then the drivers are better prepared for upstream anyway.

Sam

2008-08-07 18:49:23

by Jean Delvare

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

On Thu, 7 Aug 2008 20:39:44 +0200, Sam Ravnborg wrote:
> >
> > Alternatively, I am curious if our build system couldn't allow 3rd
> > party drivers to select in-tree modules. Obviously this would require
> > the complete kernel source tree to be available, instead of just the
> > header and config files as is usually the case. Sam, is this possible
> > to do that at this time? If not, is this something that could be
> > implemented, or is this too much work for the thin benefit?
>
> In general I recommnd to have the full kernel source available
> simply because we have no in-kernel solution to create the required
> set of files to build external modules.
>
> And today there is no way to hook into the kernel configuration
> for an external module. First of we cannot allow changes in
> the build kernel module as this would destroy module versioning
> for instance.
>
> And in this case you ask because you would change the kernel
> configuration.
>
> And I fail to see why this stuff cannot be done inside the
> kernel source tree. Merging new kernel updates should be absolutely
> trivial and then the drivers are better prepared for upstream anyway.

OK, thanks for the clarification.

Maybe one way would be for the external module build system to copy (or
link to) the missing driver source from the kernel tree and build it
locally with the rest of the external drivers, if it wasn't enabled in
the original kernel configuration and they need it. That's probably
just a couple lines of shell script, should be pretty easy. Obviously
this only works for kernel options which only select a given driver and
have no side effect on the rest of the kernel. And this is more
user-friendly than having the user check its kernel configuration and
rebuild his/her kernel.

Just a thought anyway...

--
Jean Delvare

2008-08-07 19:03:46

by Michael Ira Krufky

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

On Thu, Aug 7, 2008 at 2:49 PM, Jean Delvare <[email protected]> wrote:
> On Thu, 7 Aug 2008 20:39:44 +0200, Sam Ravnborg wrote:
>> >
>> > Alternatively, I am curious if our build system couldn't allow 3rd
>> > party drivers to select in-tree modules. Obviously this would require
>> > the complete kernel source tree to be available, instead of just the
>> > header and config files as is usually the case. Sam, is this possible
>> > to do that at this time? If not, is this something that could be
>> > implemented, or is this too much work for the thin benefit?
>>
>> In general I recommnd to have the full kernel source available
>> simply because we have no in-kernel solution to create the required
>> set of files to build external modules.
>>
>> And today there is no way to hook into the kernel configuration
>> for an external module. First of we cannot allow changes in
>> the build kernel module as this would destroy module versioning
>> for instance.
>>
>> And in this case you ask because you would change the kernel
>> configuration.
>>
>> And I fail to see why this stuff cannot be done inside the
>> kernel source tree. Merging new kernel updates should be absolutely
>> trivial and then the drivers are better prepared for upstream anyway.
>
> OK, thanks for the clarification.
>
> Maybe one way would be for the external module build system to copy (or
> link to) the missing driver source from the kernel tree and build it
> locally with the rest of the external drivers, if it wasn't enabled in
> the original kernel configuration and they need it. That's probably
> just a couple lines of shell script, should be pretty easy. Obviously
> this only works for kernel options which only select a given driver and
> have no side effect on the rest of the kernel. And this is more
> user-friendly than having the user check its kernel configuration and
> rebuild his/her kernel.
>
> Just a thought anyway...
>
> --
> Jean Delvare

I agree with Trent and D.Kelly

These options should be made available to the user -- We should go
with the patch that Jean posted, "Subject: i2c: Let users select
algorithm drivers manually again" -- this is a fair compromise for
both sides -- users that dont know should leave the automatic
selection enabled. Users that know better can disable the automatic
selection and enable what they need.

The statement, "just have the external driver merged into the kernel"
is not a solution.

Removing the option to build those additional algos is a regression, IMHO

Regards,

Mike Krufky

2008-08-07 21:07:16

by Jean Delvare

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

Hi Michael,

On Thu, 7 Aug 2008 15:03:36 -0400, Michael Krufky wrote:
> I agree with Trent and D.Kelly
>
> These options should be made available to the user -- We should go
> with the patch that Jean posted, "Subject: i2c: Let users select
> algorithm drivers manually again" -- this is a fair compromise for
> both sides -- users that dont know should leave the automatic
> selection enabled. Users that know better can disable the automatic
> selection and enable what they need.
>
> The statement, "just have the external driver merged into the kernel"
> is not a solution.

Why not, please? A vast majority of drivers work fine that way today. I
am still waiting for someone to give me a good reason why some other
drivers supposedly can't be merged upstream (something better than
"believe me, it's impossible".)

> Removing the option to build those additional algos is a regression, IMHO

Will be addressed soon, do not worry.

--
Jean Delvare

2008-08-07 21:35:46

by Michael Ira Krufky

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

Jean Delvare wrote:
> Hi Michael,
>
> On Thu, 7 Aug 2008 15:03:36 -0400, Michael Krufky wrote:
>
>> I agree with Trent and D.Kelly
>>
>> These options should be made available to the user -- We should go
>> with the patch that Jean posted, "Subject: i2c: Let users select
>> algorithm drivers manually again" -- this is a fair compromise for
>> both sides -- users that dont know should leave the automatic
>> selection enabled. Users that know better can disable the automatic
>> selection and enable what they need.
>>
>> The statement, "just have the external driver merged into the kernel"
>> is not a solution.
>>
>
> Why not, please? A vast majority of drivers work fine that way today. I
> am still waiting for someone to give me a good reason why some other
> drivers supposedly can't be merged upstream (something better than
> "believe me, it's impossible".)
>
>

Nobody said that a driver "...can't be merged upstream" ... but
REQUIRING a driver to be merged upstream to allow development and / or
testing is a problem, IMHO.

If you required that all of my development happens within a git
development repository, preventing me from working against distro-kernel
xyz, then I would simply spend more time on Windows driver development
and my Linux contributions would cease.

External subsystem development repositories allow us to work against
stable kernels at our own pace. When driver X is ready to be merged, it
gets merged.

With the model that you propose, "use linux-next for development" ...
well then what about testing? Who is going to test my driver if it
requires a full kernel compile?

Khali, you know me, and you know that I am always in favor of merging
drivers into the kernel. The ability to choose a kernel's features is
an option that should not be removed.

>> Removing the option to build those additional algos is a regression, IMHO
>>
>
> Will be addressed soon, do not worry.
Regards,

Mike

2008-08-07 23:41:20

by Trent Piepho

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

On Thu, 7 Aug 2008, Jean Delvare wrote:
> On Thu, 7 Aug 2008 09:01:35 -0700 (PDT), Trent Piepho wrote:
> > On Thu, 7 Aug 2008, Jean Delvare wrote:
> > > > One of the biggest reasons people choose to compile things from
> > > > cvs/svn/mercurial/etc. is because it gives them access to newer bug
> > > > fixes and support for things not yet present in the kernel source. A
> > > > perfect example, the multiproto dvb driver tree. Users wanting
> > > > support for dvb-s2 devices have to compile drivers outside of the
> > > > kernel because it's simply not available in the kernel and won't be
> > > > for some time.
> > >
> > > So basically you are telling that "thanks" to drivers being maintainers
> > > in external repositories, bugs are not fixed in the upstream kernel in
> > > a timely manner, and new features take more time to go there too? That
> > > must be the reason why kernel developers and users alike don't like
> > > external repositories in the first place.
> >
> > Code needs to get testing before it's put in the kernel. How's that
> > supposed to happen if it's not made available outside the kernel tree
> > first?
>
> linux-next.

Expecting every developer to keep abreast of linux-next and the tens of
thousands of patches it gets just isn't realisitic.

The embedded platforms I develop on won't run linux-next. Continuously
porting them to linux-next is simply impossible. The man hours required to
do that would be staggering.

The pool of testers available to a driver that requires running linux-next
is going to be orders of magnitude less that a driver that can be compiled
out of tree against 2.6.19 to 2.6.27.

> Having I2C-specific options selectable under the Library menu would
> probably be even more confusing. However, it would be possible to do
> something similar under the I2C menu. Much like
> CONFIG_VIDEO_HELPER_CHIPS_AUTO does for the V4L subsystem:
> CONFIG_I2C_ALGOS_AUTO would default to Y and would hide I2C algo driver
> selection (as is the case in 2.6.26), changing it to N would present
> the old menu for users to select the aldo drivers manually (as was the
> case in 2.6.25.)

This seems perfectly reasonable to me.

> Which doesn't change my point that most people complaining about the
> change would rather merge their drivers in the upstream kernel.

Somtimes maintainers aren't interested in the drivers.....

2008-08-08 00:18:41

by Stefan Richter

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

[email protected] wrote:
> With the model that you propose, "use linux-next for development" ...
> well then what about testing? Who is going to test my driver if it
> requires a full kernel compile?

We almost _never_ use linux-next in the way that we base our development
on it. We use it to prepare integration.


BTW, looking back at this thread in the list archive, I see some people
thoroughly confusing
- unmerged code,
- external developer repositories/ external patchkits,
- module source files outside a complete kernel source directory.
These are all independent matters.

All code starts as unmerged code.
All developers use external repositories.

However, module source files which are outside of the kernel source
directory are not quite the norm. (Because the kernel build system
doesn't support this.)
--
Stefan Richter
-=====-==--- =--- -=---
http://arcgraph.de/sr/

2008-08-08 09:29:17

by Jean Delvare

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

Hi Michael,

On Thu, 7 Aug 2008 17:34:30 -0400 , [email protected] wrote:
> Jean Delvare wrote:
> > Why not, please? A vast majority of drivers work fine that way today. I
> > am still waiting for someone to give me a good reason why some other
> > drivers supposedly can't be merged upstream (something better than
> > "believe me, it's impossible".)
>
> Nobody said that a driver "...can't be merged upstream" ... but
> REQUIRING a driver to be merged upstream to allow development and / or
> testing is a problem, IMHO.
>
> If you required that all of my development happens within a git
> development repository, preventing me from working against distro-kernel
> xyz, then I would simply spend more time on Windows driver development
> and my Linux contributions would cease.

Not my goal, obviously.

> External subsystem development repositories allow us to work against
> stable kernels at our own pace. When driver X is ready to be merged, it
> gets merged.
>
> With the model that you propose, "use linux-next for development" ...
> well then what about testing? Who is going to test my driver if it
> requires a full kernel compile?

Some distributions do package linux-next. And this seems to be a very
easy way to get end users to test bleeding edge code. You just tell the
user to install the linux-next package and he/she's done. No need to
build anything.

--
Jean Delvare

2008-08-08 09:38:18

by Jean Delvare

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

Hi Trent,

On Thu, 7 Aug 2008 16:41:10 -0700 (PDT), Trent Piepho wrote:
> Expecting every developer to keep abreast of linux-next and the tens of
> thousands of patches it gets just isn't realisitic.
>
> The embedded platforms I develop on won't run linux-next. Continuously
> porting them to linux-next is simply impossible. The man hours required to
> do that would be staggering.

Once again a "believe me it's impossible" without any good reason
given. I fail to see why embedded platforms would be any different from
other platforms or subsystem trees. Please enlighten me.

> The pool of testers available to a driver that requires running linux-next
> is going to be orders of magnitude less that a driver that can be compiled
> out of tree against 2.6.19 to 2.6.27.

Except that distributions start packaging linux-next, while in general
they don't package out-of-tree versions of packages that are also
available in the kernel tree. If the v4l-dvb tip was in linux-next (it
isn't, is it?), I suspect that you would get many more testers than you
have at the time being. Which doesn't mean that you can't additionally
provide out-of-tree drivers for older kernels if you think it's worth
the additional effort (I don't think it is, but it's up to whoever does
the work.)

--
Jean Delvare

2008-08-08 17:52:19

by Trent Piepho

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

On Fri, 8 Aug 2008, Jean Delvare wrote:
> On Thu, 7 Aug 2008 16:41:10 -0700 (PDT), Trent Piepho wrote:
> > Expecting every developer to keep abreast of linux-next and the tens of
> > thousands of patches it gets just isn't realisitic.
> >
> > The embedded platforms I develop on won't run linux-next. Continuously
> > porting them to linux-next is simply impossible. The man hours required to
> > do that would be staggering.
>
> Once again a "believe me it's impossible" without any good reason
> given. I fail to see why embedded platforms would be any different from
> other platforms or subsystem trees. Please enlighten me.

Because they usually require lots of additional patches that aren't in the
kernel. Sometimes embedded developers try to get these patches into the
mainline kernel, but some maintainers aren't interested in accepting
patches that don't appear to be useful to desktop users.

> > The pool of testers available to a driver that requires running linux-next
> > is going to be orders of magnitude less that a driver that can be compiled
> > out of tree against 2.6.19 to 2.6.27.
>
> Except that distributions start packaging linux-next, while in general
> they don't package out-of-tree versions of packages that are also
> available in the kernel tree. If the v4l-dvb tip was in linux-next (it

Sure they do. There are packages of the latest v4l-dvb, nvidia and ATI
drivers, and more. There is even a system, DKMS, used to allow kernel
module source packages to be installed and automatically rebuilt for new
kernels.

You can install and run a new module without even rebooting. Installing
linux next is quite a bit more complex. Users won't test your driver if
they have to install a new kernel for every revision.

It's a nightmare for development too. I can't develop the entire kernel,
it's too big and there are too many changes. If my driver has become
unstable, is it something I did or one of the 20,000 patches that have
appeared in linux-next? Like Mike said, one needs a stable platform to
develop on.

2008-08-10 11:08:53

by Adrian Bunk

[permalink] [raw]
Subject: Re: Problem with restricted I2C algorithms in kernel 2.6.26!

On Fri, Aug 08, 2008 at 10:52:05AM -0700, Trent Piepho wrote:
> On Fri, 8 Aug 2008, Jean Delvare wrote:
> > On Thu, 7 Aug 2008 16:41:10 -0700 (PDT), Trent Piepho wrote:
> > > Expecting every developer to keep abreast of linux-next and the tens of
> > > thousands of patches it gets just isn't realisitic.
> > >
> > > The embedded platforms I develop on won't run linux-next. Continuously
> > > porting them to linux-next is simply impossible. The man hours required to
> > > do that would be staggering.
> >
> > Once again a "believe me it's impossible" without any good reason
> > given. I fail to see why embedded platforms would be any different from
> > other platforms or subsystem trees. Please enlighten me.
>
> Because they usually require lots of additional patches that aren't in the
> kernel. Sometimes embedded developers try to get these patches into the
> mainline kernel, but some maintainers aren't interested in accepting
> patches that don't appear to be useful to desktop users.
>...

The last thing mustn't happen.

Can you give examples?

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed