Re: [PATCH v2 19/36] mac_scsi: Add module option to Kconfig

Hannes Reinecke <hare@xxxxxxx> · Mon, 10 Nov 2014 08:02:36 +0100

On 11/09/2014 01:17 PM, Finn Thain wrote:

On Fri, 31 Oct 2014, Hannes Reinecke wrote:

On 10/31/2014 08:17 AM, Finn Thain wrote:

On Thu, 30 Oct 2014, Hannes Reinecke wrote:

On 10/27/2014 06:26 AM, Finn Thain wrote:
Allow mac_scsi to be built as a module. Replace the old validation of 
__setup options with code that validates both module and __setup 
options.

Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>

---
 drivers/scsi/Kconfig    |    2 
 drivers/scsi/mac_scsi.c |  112 +++++++++++++++---------------------------------
 2 files changed, 38 insertions(+), 76 deletions(-)

Index: linux/drivers/scsi/Kconfig
===================================================================

--- linux.orig/drivers/scsi/Kconfig	2014-10-27 16:17:59.000000000 +1100
+++ linux/drivers/scsi/Kconfig	2014-10-27 16:25:42.000000000 +1100
@@ -1595,7 +1595,7 @@ config ATARI_SCSI_RESET_BOOT
 	  that leave the devices with SCSI operations partway completed.
 
 config MAC_SCSI
-	bool "Macintosh NCR5380 SCSI"
+	tristate "Macintosh NCR5380 SCSI"
 	depends on MAC && SCSI=y
 	select SCSI_SPI_ATTRS
 	help
Index: linux/drivers/scsi/mac_scsi.c
===================================================================
--- linux.orig/drivers/scsi/mac_scsi.c	2014-10-27 16:25:40.000000000 +1100
+++ linux/drivers/scsi/mac_scsi.c	2014-10-27 16:25:42.000000000 +1100
@@ -62,15 +62,18 @@
 static void mac_scsi_reset_boot(struct Scsi_Host *instance);
 #endif
 
-static int setup_called = 0;
 static int setup_can_queue = -1;
+module_param(setup_can_queue, int, 0);
 static int setup_cmd_per_lun = -1;
+module_param(setup_cmd_per_lun, int, 0);
 static int setup_sg_tablesize = -1;
+module_param(setup_sg_tablesize, int, 0);
 static int setup_use_pdma = -1;
-#ifdef SUPPORT_TAGS
+module_param(setup_use_pdma, int, 0);
 static int setup_use_tagged_queuing = -1;
-#endif
+module_param(setup_use_tagged_queuing, int, 0);
 static int setup_hostid = -1;
+module_param(setup_hostid, int, 0);
 
 /* Time (in jiffies) to wait after a reset; the SCSI standard calls for 250ms,
  * we usually do 0.5s to be on the safe side. But Toshiba CD-ROMs once more
@@ -102,72 +105,34 @@ static __inline__ void macscsi_write(str
   out_8(instance->io_port + (reg<<4), value);
 }
 
-/*
- * Function : mac_scsi_setup(char *str)
- *
- * Purpose : booter command line initialization of the overrides array,
- *
- * Inputs : str - comma delimited list of options
- *
- */
-
-static int __init mac_scsi_setup(char *str) {
+#ifndef MODULE
+static int __init mac_scsi_setup(char *str)
+{
 	int ints[7];
-	
-	(void)get_options( str, ARRAY_SIZE(ints), ints);
-	
-	if (setup_called++ || ints[0] < 1 || ints[0] > 6) {
-	    printk(KERN_WARNING "scsi: <mac5380>"
-		" Usage: mac5380=<can_queue>[,<cmd_per_lun>,<sg_tablesize>,<hostid>,<use_tags>,<use_pdma>]\n");
-	    printk(KERN_ALERT "scsi: <mac5380> Bad Penguin parameters?\n");
-	    return 0;
-	}
-	    
-	if (ints[0] >= 1) {
-		if (ints[1] > 0)
-			/* no limits on this, just > 0 */
-			setup_can_queue = ints[1];
-	}
-	if (ints[0] >= 2) {
-		if (ints[2] > 0)
-			setup_cmd_per_lun = ints[2];
-	}
-	if (ints[0] >= 3) {
-		if (ints[3] >= 0) {
-			setup_sg_tablesize = ints[3];
-			/* Must be <= SG_ALL (255) */
-			if (setup_sg_tablesize > SG_ALL)
-				setup_sg_tablesize = SG_ALL;
-		}
-	}
-	if (ints[0] >= 4) {
-		/* Must be between 0 and 7 */
-		if (ints[4] >= 0 && ints[4] <= 7)
-			setup_hostid = ints[4];
-		else if (ints[4] > 7)
-			printk(KERN_WARNING "mac_scsi_setup: invalid host ID %d !\n", ints[4] );
-	}
-#ifdef SUPPORT_TAGS	
-	if (ints[0] >= 5) {
-		if (ints[5] >= 0)
-			setup_use_tagged_queuing = !!ints[5];
+
+	(void)get_options(str, ARRAY_SIZE(ints), ints);
+
+	if (ints[0] < 1 || ints[0] > 6) {
+		pr_err("Usage: mac5380=<can_queue>[,<cmd_per_lun>[,<sg_tablesize>[,<hostid>[,<use_tags>[,<use_pdma>]]]]]\n");
+		return 0;
 	}
-	
-	if (ints[0] == 6) {
-	    if (ints[6] >= 0)
+	if (ints[0] >= 1)
+		setup_can_queue = ints[1];
+	if (ints[0] >= 2)
+		setup_cmd_per_lun = ints[2];
+	if (ints[0] >= 3)
+		setup_sg_tablesize = ints[3];
+	if (ints[0] >= 4)
+		setup_hostid = ints[4];
+	if (ints[0] >= 5)
+		setup_use_tagged_queuing = ints[5];
+	if (ints[0] >= 6)
 		setup_use_pdma = ints[6];
-	}
-#else
-	if (ints[0] == 5) {
-	    if (ints[5] >= 0)
-		setup_use_pdma = ints[5];
-	}
-#endif /* SUPPORT_TAGS */
-	
 	return 1;
 }
 
 __setup("mac5380=", mac_scsi_setup);
+#endif /* !MODULE */
 
 /*
  * Function : int macscsi_detect(struct scsi_host_template * tpnt)
@@ -199,13 +164,8 @@ int __init macscsi_detect(struct scsi_ho
 		tpnt->cmd_per_lun = setup_cmd_per_lun;
 	if (setup_sg_tablesize >= 0)
 		tpnt->sg_tablesize = setup_sg_tablesize;
-
-    if (setup_hostid >= 0)
-	tpnt->this_id = setup_hostid;
-    else {
-	/* use 7 as default */
-	tpnt->this_id = 7;
-    }
+	if (setup_hostid >= 0)
+		tpnt->this_id = setup_hostid & 7;
 
 #ifdef SUPPORT_TAGS
     if (setup_use_tagged_queuing < 0)
@@ -219,15 +179,15 @@ int __init macscsi_detect(struct scsi_ho
 	return 0;
 
     if (macintosh_config->ident == MAC_MODEL_IIFX) {
-	mac_scsi_regp  = via1+0x8000;
-	mac_scsi_drq   = via1+0xE000;
-	mac_scsi_nodrq = via1+0xC000;
+	mac_scsi_regp  = (unsigned char *) VIA1_BASE + 0x8000;
+	mac_scsi_drq   = (unsigned char *) VIA1_BASE + 0xE000;
+	mac_scsi_nodrq = (unsigned char *) VIA1_BASE + 0xC000;
 	/* The IIFX should be able to do true DMA, but pseudo-dma doesn't work */
 	flags = FLAG_NO_PSEUDO_DMA;
     } else {
-	mac_scsi_regp  = via1+0x10000;
-	mac_scsi_drq   = via1+0x6000;
-	mac_scsi_nodrq = via1+0x12000;
+	mac_scsi_regp  = (unsigned char *) VIA1_BASE + 0x10000;
+	mac_scsi_drq   = (unsigned char *) VIA1_BASE + 0x6000;
+	mac_scsi_nodrq = (unsigned char *) VIA1_BASE + 0x12000;
     }
 
     if (! setup_use_pdma)
Hmm. mac_via.h has this:

/*
 * Base addresses for the VIAs. There are two in every machine,
 * although on some machines the second is an RBV or an OSS.
 * The OSS is different enough that it's handled separately.
 *
 * Do not use these values directly; use the via1 and via2 variables
 * instead (and don't forget to check rbv_present when using via2!)
 */

#define VIA1_BASE	(0x50F00000)
#define VIA2_BASE	(0x50F02000)
#define  RBV_BASE	(0x50F26000)

So either that comment is obsolete or you should revert the above
bit ...

Use of via1 had to be changed because that symbol is not exported.

The comment you quoted is probably not wrong; it relates to programming 
the VIA chip itself (see ADB drivers for example). But in this driver we 
aren't interested in the VIA1 chip so I'd say niether via1 or VIA1_BASE is 
good style. Hence my use of bare addresses in the platform resource 
initializers in patch 21.

This example of VIA1_BASE + offset disappears in patch 21 (you can find an 
unrelated example in arch/m68k/mac/config.c but that was not my doing).

This patch is not meant to change the actual addresses used by mac_scsi; 
it just makes the driver modular. Use of VIA1_BASE seemed like a good way 
to express the fact that the addresses are not changed yet (some models 
use new addresses after patch 21).

Basically, I just wanted a modular driver for debugging and bisection and 
I wanted it early in the patch series. A modular mac_scsi.c aligns better 
with atari_scsi.c and sun3_scsi.c as they are already modular. That means 
that the three subsequent patches which convert {atari,mac,sun3}_scsi.c to 
platform drivers also align nicely (so I could cross check them).

Fair enough.

Just needed some clarification here. My Mac experience ceased with a
IIcx :-)

Hannes, can I get a reviewed-by tag for this patch please?

Sure.
Reviewed-by: Hannes Reinecke <hare@xxxxxxx>

Cheers,

Hannes

-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@xxxxxxx			      +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html







