[PATCH] sym53c8xx module parameter passing not working (2.5.6)

[PATCH] sym53c8xx module parameter passing not working (2.5.6)

[Heinzmann, Robert]

[-- Attachment #1: Type: text/plain, Size: 648 bytes --]

Hello, 

attached there is a patch that fixes the module parameter passing with
sym53c8xx_2. This is for SLES9 SuSE kernel, but it is so small that it
should apply for other kernels too.

Regards, 

Robert Heinzmann

------------------------------------------------------------------------
COMPUTER CONCEPT 
CC Computersysteme und Kommunikationstechnik GmbH 
Robert Heinzmann
Wiener Str. 114 - 116		Email:	heinzmann@cc-dresden.de
01219 Dresden			Telefon:	+49 (0)351/8 76 92-0
					Telefax:	+49 (0)351/8 76
92-99
					Internet:
http://www.cc-dresden.de
------------------------------------------------------------------------


[-- Attachment #2: sym53c8xx_2-module_param_handling_fix.2.6.5-7.97-default.diff --]
[-- Type: application/octet-stream, Size: 1427 bytes --]

--- linux/drivers/scsi/sym53c8xx_2/sym_glue.c.orig	2004-09-29 16:54:16.000000000 +0200
+++ linux/drivers/scsi/sym53c8xx_2/sym_glue.c	2004-10-01 17:11:29.093154424 +0200
@@ -1899,10 +1899,11 @@
 
 static struct sym_driver_setup
 	sym_driver_safe_setup __initdata = SYM_LINUX_DRIVER_SAFE_SETUP;
-#ifdef	MODULE
-char *sym53c8xx = 0;	/* command line passed by insmod */
-MODULE_PARM(sym53c8xx, "s");
-#endif
+/*
+ *  Proper module parameter handling in 2.6
+ */
+char *sym53c8xx = "sym53c8xx                                                              ";	/* command line passed by insmod */
+module_param(sym53c8xx, charp, 0);
 
 #define OPT_MAX_TAG		1
 #define OPT_BURST_ORDER		2
@@ -2404,6 +2405,10 @@
 
 static int __init sym2_init(void)
 {
+	#ifdef MODULE 
+	if (sym53c8xx)
+	        sym53c8xx_setup(sym53c8xx);
+	#endif
 	pci_register_driver(&sym2_driver);
 	return 0;
 }
--- linux/drivers/scsi/sym53c8xx_2/sym_glue.h.orig	2004-09-29 16:55:03.000000000 +0200
+++ linux/drivers/scsi/sym53c8xx_2/sym_glue.h	2004-10-01 17:08:18.941061936 +0200
@@ -61,6 +61,15 @@
 #include <linux/timer.h>
 #include <linux/types.h>
 
+/* Proper module parameter passing in 2.6 */
+#ifndef LINUX_VERSION_CODE
+#include <linux/version.h>
+#endif /* LINUX_VERSION_CODE */
+
+#if defined(MODULE) && LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+#include <linux/moduleparam.h>
+#endif 
+
 #include <asm/io.h>
 #ifdef __sparc__
 #  include <asm/irq.h>

[-- Attachment #3: sym53c8xx_2-module_param_handling_fix.2.6.5-7.97-default.Readme.txt --]
[-- Type: text/plain, Size: 353 bytes --]

The 2.6 version of sym53c8xx_2 did not read the module command line parameter properly. 
This was due to a missing call to sym53c8xx_setup().

This fix includes: 
- added missing call to sym53c8xx_setup()
- introduces proper 2.6 module_param() handling
  (changed MODULE_PARM to module_param());

Robert Heinzmann (Heinzmann@cc-dresden.de)

Re: [PATCH] sym53c8xx module parameter passing not working (2.5.6)

[Christoph Hellwig]

On Fri, Oct 01, 2004 at 05:26:21PM +0200, Heinzmann, Robert wrote:
> Hello, 
> 
> attached there is a patch that fixes the module parameter passing with
> sym53c8xx_2. This is for SLES9 SuSE kernel, but it is so small that it
> should apply for other kernels too.

Given that no one except you noticed the lack of options so far what
about moving it to normal parameters instead, aka one option per setting
like all non-scsi and resent scsi drivers do?

RE: [PATCH] sym53c8xx module parameter passing not working (2.5.6)

[Heinzmann, Robert]

Hello Christoph, 

> Given that no one except you noticed the lack of options so 
> far what about moving it to normal parameters instead, aka 
> one option per setting like all non-scsi and resent scsi drivers do?

The options are usually required for multi initiator configurations with
sym53c8xx only, which is not so common. Thus I just wrote this litte
(dirty ?) fix. Moving to normal parameters is something that should be
done, but I'm currently not able to do this :) If everything is beeing
moved to normal parameters, the documentation needs to be fixed too.

I think the current fix is sufficient, considering the installation base
of multi initiator sym53c8xx clusters.

Robert Heinzmann

Re: [PATCH] sym53c8xx module parameter passing not working (2.5.6)

[Matthew Wilcox]

On Fri, Oct 01, 2004 at 04:29:39PM +0100, Christoph Hellwig wrote:
> Given that no one except you noticed the lack of options so far what
> about moving it to normal parameters instead, aka one option per setting
> like all non-scsi and resent scsi drivers do?

I think it's definitely time to make this change.  I'll look at it later
today.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] sym53c8xx module parameter passing not working (2.5.6)

[Matthew Wilcox]

On Fri, Oct 01, 2004 at 05:26:21PM +0200, Heinzmann, Robert wrote:
> The 2.6 version of sym53c8xx_2 did not read the module command line parameter properly. 
> This was due to a missing call to sym53c8xx_setup().

I don't understand, actually.

#ifndef MODULE
__setup("sym53c8xx=", sym53c8xx_setup);
#endif

should cause sym53c8xx_setup to be called.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] sym53c8xx module parameter passing not working (2.5.6)

[Randy.Dunlap]

On Fri, 1 Oct 2004 22:11:36 +0100 Matthew Wilcox wrote:

| On Fri, Oct 01, 2004 at 05:26:21PM +0200, Heinzmann, Robert wrote:
| > The 2.6 version of sym53c8xx_2 did not read the module command line parameter properly. 
| > This was due to a missing call to sym53c8xx_setup().
| 
| I don't understand, actually.
| 
| #ifndef MODULE
| __setup("sym53c8xx=", sym53c8xx_setup);
| #endif
| 
| should cause sym53c8xx_setup to be called.

It should for the non-loadable-module case.  I think that Robert
is saying that it doesn't work for the loadable-module case.
I.e., these lines aren't doing what is expected:

#ifdef	MODULE
char *sym53c8xx;	/* command line passed by insmod */
MODULE_PARM(sym53c8xx, "s");
#endif

-- 
~Randy

Re: [PATCH] sym53c8xx module parameter passing not working (2.5.6)

[Matthew Wilcox]

On Fri, Oct 01, 2004 at 05:55:55PM +0100, Matthew Wilcox wrote:
> On Fri, Oct 01, 2004 at 04:29:39PM +0100, Christoph Hellwig wrote:
> > Given that no one except you noticed the lack of options so far what
> > about moving it to normal parameters instead, aka one option per setting
> > like all non-scsi and resent scsi drivers do?
> 
> I think it's definitely time to make this change.  I'll look at it later
> today.

I have it working ... but there's no way to do a graceful transition.
I had in mind something like:

+       printk(NAME53C8XX "Using old-style setup code.  Please switch to new "
+                       "module param code.\n"
+                       "See Documentation/scsi/sym53c8xx_2.txt for "
+                       "details.\n");
+

to allow people to switch over.  But when I try this, I get:

sym53c8xx: Unknown parameter `hostid'
sym53c8xx: Ignoring new-style parameters in presence of obsolete ones

It'd be nice to be able to use either for a while.  Rusty, is there any
chance of allowing this?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] sym53c8xx module parameter passing not working (2.5.6)

[Rusty Russell]

On Fri, 2004-10-08 at 11:30, Matthew Wilcox wrote:
> On Fri, Oct 01, 2004 at 05:55:55PM +0100, Matthew Wilcox wrote:
> to allow people to switch over.  But when I try this, I get:
> 
> sym53c8xx: Unknown parameter `hostid'
> sym53c8xx: Ignoring new-style parameters in presence of obsolete ones
> 
> It'd be nice to be able to use either for a while.  Rusty, is there any
> chance of allowing this?

Well, it'd never be neat.  I'd rather encourage migration: in 2.6.10 I
will start my jihad on MODULE_PARM, ideally finished before 2.6.11. 

It's usually better that someone who knows the code does the transition
(you can often do better than a naive substitution), so my first step
might be to clean up Linus' compile, then make MODULE_PARM warn, to
encourage people to help.

Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

New-style module parameters in SCSI drivers

[Matthew Wilcox]

On Fri, Oct 08, 2004 at 12:29:16PM +1000, Rusty Russell wrote:
> Well, it'd never be neat.  I'd rather encourage migration: in 2.6.10 I
> will start my jihad on MODULE_PARM, ideally finished before 2.6.11. 

OK.  As long as the barrage of complaints is omnidirectional and it's
not just me who sucks.

> It's usually better that someone who knows the code does the transition
> (you can often do better than a naive substitution), so my first step
> might be to clean up Linus' compile, then make MODULE_PARM warn, to
> encourage people to help.

While we're breaking the world, we should probably take this opportunity
to Do Things Right.  Here are all the ways I've noticed with a quick
grep of doing two common tasks in SCSI drivers.

First, the maximum number of commands that may be in flight to any
given device simultaneously:

drivers/scsi/3w-xxxx.c:                 cmds_per_lun
drivers/scsi/atari_scsi.c:              setup_cmd_per_lun
drivers/scsi/sun3_scsi.c:               setup_cmd_per_lun
drivers/scsi/sun3_scsi_vme.c:           setup_cmd_per_lun
drivers/scsi/dc395x.c:                  tags
drivers/scsi/megaraid.c:                max_cmd_per_lun
drivers/scsi/megaraid/megaraid_mbox.c:  cmd_per_lun
drivers/scsi/qla2xxx/qla_os.c:          ql2xmaxqdepth
drivers/scsi/sym53c8xx_2/sym_glue.c:    max_tag

Second, what host ID to use:

drivers/scsi/aha152x.c:                 scsiid
drivers/scsi/dc395x.c:                  adapter_id
drivers/scsi/ibmmca.c:                  scsi_id
drivers/scsi/sun3_scsi.c:               setup_hostid
drivers/scsi/sun3_scsi_vme.c:           setup_hostid
drivers/scsi/pcmcia/aha152x_stub.c:     host_id
drivers/scsi/sym53c8xx_2/sym_glue.c:    hostid

I honestly don't think any more variants _could_ exist, for either
of these oh-so-common things to do.  Not to mention that using these
parameters is a small-system thing to do -- what if you have 25 scsi
adapters and some of them should be different host IDs from each other?
What if you need to send only two tagged commands at once to your crappy
cd-rom, but your raid array needs 256 tags to be in use in order to get
any kind of performance?

Admittedly, there are hacks for the second case in many drivers (see the
sym2 driver -- puke!)  But before we break the world, let's discuss how
to do this right so we can at least have a model to work towards.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain