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).
--
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
