2006-10-19 15:26:56

by Adrian Bunk

[permalink] [raw]
Subject: [2.6.19 patch] drivers/ide/pci/generic.c: re-add the __setup("all-generic-ide",...)

On Wed, Oct 18, 2006 at 07:18:44PM -0400, Alan Cox wrote:
> On Thu, Oct 19, 2006 at 12:15:20AM +0200, Adrian Bunk wrote:
> > IOW, your patch does break existing setups since the change to
> > module_param() requires prefixing with the module name (the ata_generic
> > option with the same name is irrelevant)?
> >
> > Considering that drivers/ide/ is slowly approaching a RIP status,
> > is such an incompatible change really required?
> >
> > I'd be more inclined to revert your patch.
>
> We shouldn't revert it - there is a real problem for some users whose
> distro has it modular this fixed. We might want to honour both

Agreed, patch below.

cu
Adrian


<-- snip -->


The change from __setup() to module_param_named() requires users to
prefix the option with "generic.".

This patch re-adds the __setup() additionally to the
module_param_named().

Usually it would make sense getting rid of such an obsolete __setup() at
some time, but considering that drivers/ide/ is slowly approaching a RIP
status it's already implicitely scheduled for removal.

This patch fixes kernel Bugzilla #7353.

Signed-off-by: Adrian Bunk <[email protected]>

--- linux-2.6/drivers/ide/pci/generic.c.old 2006-10-19 16:35:15.000000000 +0200
+++ linux-2.6/drivers/ide/pci/generic.c 2006-10-19 16:46:21.000000000 +0200
@@ -40,6 +40,19 @@

static int ide_generic_all; /* Set to claim all devices */

+/*
+ * the module_param_named() was added for the modular case
+ * the __setup() is left as compatibility for existing setups
+ */
+#ifndef MODULE
+static int __init ide_generic_all_on(char *unused)
+{
+ ide_generic_all = 1;
+ printk(KERN_INFO "IDE generic will claim all unknown PCI IDE storage controllers.");
+ return 1;
+}
+__setup("all-generic-ide", ide_generic_all_on);
+#endif
module_param_named(all_generic_ide, ide_generic_all, bool, 0444);
MODULE_PARM_DESC(all_generic_ide, "IDE generic will claim all unknown PCI IDE storage controllers.");



2006-10-19 16:06:44

by Randy Dunlap

[permalink] [raw]
Subject: Re: [2.6.19 patch] drivers/ide/pci/generic.c: re-add the __setup("all-generic-ide",...)

On Thu, 19 Oct 2006 17:26:51 +0200 Adrian Bunk wrote:

> On Wed, Oct 18, 2006 at 07:18:44PM -0400, Alan Cox wrote:
> > On Thu, Oct 19, 2006 at 12:15:20AM +0200, Adrian Bunk wrote:
> > > IOW, your patch does break existing setups since the change to
> > > module_param() requires prefixing with the module name (the ata_generic
> > > option with the same name is irrelevant)?
> > >
> > > Considering that drivers/ide/ is slowly approaching a RIP status,
> > > is such an incompatible change really required?
> > >
> > > I'd be more inclined to revert your patch.
> >
> > We shouldn't revert it - there is a real problem for some users whose
> > distro has it modular this fixed. We might want to honour both
>
> Agreed, patch below.
>
> cu
> Adrian
>
>
> <-- snip -->
>
>
> The change from __setup() to module_param_named() requires users to
> prefix the option with "generic.".
>
> This patch re-adds the __setup() additionally to the
> module_param_named().
>
> Usually it would make sense getting rid of such an obsolete __setup() at
> some time, but considering that drivers/ide/ is slowly approaching a RIP
> status it's already implicitely scheduled for removal.
>
> This patch fixes kernel Bugzilla #7353.
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> --- linux-2.6/drivers/ide/pci/generic.c.old 2006-10-19 16:35:15.000000000 +0200
> +++ linux-2.6/drivers/ide/pci/generic.c 2006-10-19 16:46:21.000000000 +0200
> @@ -40,6 +40,19 @@
>
> static int ide_generic_all; /* Set to claim all devices */
>
> +/*
> + * the module_param_named() was added for the modular case
> + * the __setup() is left as compatibility for existing setups
> + */
> +#ifndef MODULE
> +static int __init ide_generic_all_on(char *unused)
> +{
> + ide_generic_all = 1;
> + printk(KERN_INFO "IDE generic will claim all unknown PCI IDE storage controllers.");
> + return 1;
> +}
> +__setup("all-generic-ide", ide_generic_all_on);
> +#endif
> module_param_named(all_generic_ide, ide_generic_all, bool, 0444);
> MODULE_PARM_DESC(all_generic_ide, "IDE generic will claim all unknown PCI IDE storage controllers.");

Missing update to Documentation/kernel-parameters.txt ?
(maybe it's been missing forever?)

---
~Randy

2006-10-19 16:13:42

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6.19 patch] drivers/ide/pci/generic.c: re-add the __setup("all-generic-ide",...)

On Thu, Oct 19, 2006 at 09:07:41AM -0700, Randy Dunlap wrote:
> On Thu, 19 Oct 2006 17:26:51 +0200 Adrian Bunk wrote:
>...
> > --- linux-2.6/drivers/ide/pci/generic.c.old 2006-10-19 16:35:15.000000000 +0200
> > +++ linux-2.6/drivers/ide/pci/generic.c 2006-10-19 16:46:21.000000000 +0200
> > @@ -40,6 +40,19 @@
> >
> > static int ide_generic_all; /* Set to claim all devices */
> >
> > +/*
> > + * the module_param_named() was added for the modular case
> > + * the __setup() is left as compatibility for existing setups
> > + */
> > +#ifndef MODULE
> > +static int __init ide_generic_all_on(char *unused)
> > +{
> > + ide_generic_all = 1;
> > + printk(KERN_INFO "IDE generic will claim all unknown PCI IDE storage controllers.");
> > + return 1;
> > +}
> > +__setup("all-generic-ide", ide_generic_all_on);
> > +#endif
> > module_param_named(all_generic_ide, ide_generic_all, bool, 0444);
> > MODULE_PARM_DESC(all_generic_ide, "IDE generic will claim all unknown PCI IDE storage controllers.");
>
> Missing update to Documentation/kernel-parameters.txt ?
> (maybe it's been missing forever?)

It's been missing forever.

I'm not sure whether documenting it now where it's deprecated and nearly
dead makes sense..

> ~Randy

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

2006-10-19 16:31:10

by Alan

[permalink] [raw]
Subject: Re: [2.6.19 patch] drivers/ide/pci/generic.c: re-add the __setup("all-generic-ide",...)

Ar Iau, 2006-10-19 am 18:13 +0200, ysgrifennodd Adrian Bunk:
> > Missing update to Documentation/kernel-parameters.txt ?
> > (maybe it's been missing forever?)
>
> It's been missing forever.
>
> I'm not sure whether documenting it now where it's deprecated and nearly
> dead makes sense..

Its not dead, its so useful that drivers/ata also supports it

2006-10-20 21:05:37

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6.19 patch] drivers/ide/pci/generic.c: re-add the __setup("all-generic-ide",...)

On Thu, Oct 19, 2006 at 05:29:58PM +0100, Alan Cox wrote:
> Ar Iau, 2006-10-19 am 18:13 +0200, ysgrifennodd Adrian Bunk:
> > > Missing update to Documentation/kernel-parameters.txt ?
> > > (maybe it's been missing forever?)
> >
> > It's been missing forever.
> >
> > I'm not sure whether documenting it now where it's deprecated and nearly
> > dead makes sense..
>
> Its not dead, its so useful that drivers/ata also supports it

But in the drivers/ata case it's a module parameter, not a __setup
kernel parameter.

And I don't think it makes sense to manually add module parameters to
kernel-parameters.txt

If a documentation of all module parameters is considered useful,
someone should write a script to automatically generate such a list.

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

2006-10-21 17:53:01

by Randy Dunlap

[permalink] [raw]
Subject: Re: [2.6.19 patch] drivers/ide/pci/generic.c: re-add the __setup("all-generic-ide",...)

On Fri, 20 Oct 2006 23:05:33 +0200 Adrian Bunk wrote:

> On Thu, Oct 19, 2006 at 05:29:58PM +0100, Alan Cox wrote:
> > Ar Iau, 2006-10-19 am 18:13 +0200, ysgrifennodd Adrian Bunk:
> > > > Missing update to Documentation/kernel-parameters.txt ?
> > > > (maybe it's been missing forever?)
> > >
> > > It's been missing forever.
> > >
> > > I'm not sure whether documenting it now where it's deprecated and nearly
> > > dead makes sense..
> >
> > Its not dead, its so useful that drivers/ata also supports it
>
> But in the drivers/ata case it's a module parameter, not a __setup
> kernel parameter.

That's just an implementation nit/detail. Users don't care which
way it's implemented, they just need to see some reasonable
documentation.

> And I don't think it makes sense to manually add module parameters to
> kernel-parameters.txt

There are module parameters there already...
so we are being inconsistent.

> If a documentation of all module parameters is considered useful,
> someone should write a script to automatically generate such a list.

I think that sounds great -- in theory. Really, I do.
I even wrote a (simple) script for it last night.[1]
(but someone else is free to redo it, and probably not in
shell script :)

And maybe one "development community" answer is that this is
a distro problem, let them handle it. (I don't like that answer,
but possibly the distros are OK with it. I don't know.)

Ideally, users would be able to see/read documentation (like kernel
or module parameters) (a) without reading the source code and
(b) without building the module binary files. Maybe that's too
much to ask of the development community, so the users can just
build all 1500 or so (and growing) loadable kernel modules
and run 'modinfo' on them to see what the possible module
parameters are. Of course, if they need this information to be
able to install their (only) Linux system, then they are out of
luck, or they can use their other (or working) OS to search the
internet for such documenation.

Anyway, regarding your suggestion: Yes, I think that it would be
good to generate such documentation instead of maintaining it
(and sometimes not doing that). Maybe someone can & will make
that happen.

---
~Randy
[1] http://www.xenotime.net/linux/scripts/module-params