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 <[email protected]>
>>>>>
>>>>> ---
>>>>>  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 <[email protected]>

Cheers,

Hannes

-- 
Dr. Hannes Reinecke                   zSeries & Storage
[email protected]                          +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-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to