From: Michael Schmitz <schmitzmic@gmail.com>
To: Finn Thain <fthain@telegraphics.com.au>
Cc: Christoph Hellwig <hch@infradead.org>,
Michael Schmitz <schmitz@debian.org>,
Sam Creasey <sammy@sammy.net>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Linux/m68k <linux-m68k@vger.kernel.org>
Subject: Re: converting the NCR5380 drivers away from scsi_register
Date: Tue, 05 Aug 2014 21:06:54 +1200 [thread overview]
Message-ID: <53E09EAE.70308@gmail.com> (raw)
In-Reply-To: <alpine.LNX.2.00.1408041146400.21367@nippy.intranet>
Hi Finn,
> On Sun, 3 Aug 2014, Michael Schmitz wrote:
>
>
>>> NCR5380.c lacks support for tagged queueing, while sun3_NCR5380.c and
>>> atari_NCR5380.c have divergent implementations of this.
>>>
>>>
>> Might be equivalent for all I've seen - just array vs. bitmap to store
>> tags..
>>
>
> It is messier than that. setup_use_tagged_queuing is disabled by default
> on atari, and enabled by default on sun3. If we don't disable it by
>
It's configured but disabled by default on Atari because command_per_lun
is set to 1 by default. Can't remember why that was exactly - my best
guess would be that queueing up more commands in the lowlevel driver did
use to exacerbate timeout problems. But that was over a decade ago. On
Sun3, the code is not even configured from what I've seen (SUPPORT_TAGS
not defined).
> default everywhere then we need to resolve a change to the algorithm: the
> following code appears at the beginning of NCR5380_reselect() in
> atari_NCR5380.c but appears at the end of that routine in sun3_NCR5380.c.
>
That's what I meant to say before - sun3_NCR5380.c does peek at the
message byte without invoking NCR5380_transfer_pio() (needs to manually
set/clear ACK for that resason, finds the disconnected command at issue,
does some DMA setup stuff for that command, and then goes ahead to fetch
the tag message bytes if the phase is still at message in. No idea where
the variable 'phase' gets set in that routine so it might never get to
read tag messages.
atari_NCR5390.c does things a bit different - instead of peeking at the
message byte, NCR5380_transfer_pio() is used for the first message byte
(ACK set there, cleared manually later). NCR5380_transfer_pio() sets the
current phase, so the subsequent transfer of the tag message may work
there (again ACK raised in NCR5380_transfer_pio(), cleared manually later).
> I still have to try to figure out the implications of this change.
>
From what I've checked both should work, but the sun3 code never sets
'phase' so it should never actually have worked. It should not even
compile.
> #ifdef SUPPORT_TAGS
> /* If the phase is still MSGIN, the target wants to send some more
> * messages. In case it supports tagged queuing, this is probably a
> * SIMPLE_QUEUE_TAG for the I_T_L_Q nexus.
> */
> tag = TAG_NONE;
> if (phase == PHASE_MSGIN && setup_use_tagged_queuing) {
> /* Accept previous IDENTIFY message by clearing ACK */
> NCR5380_write( INITIATOR_COMMAND_REG, ICR_BASE );
> len = 2;
> data = msg+1;
> if (!NCR5380_transfer_pio(instance, &phase, &len, &data) &&
> msg[1] == SIMPLE_QUEUE_TAG)
> tag = msg[2];
> dprintk(NDEBUG_TAGS, "scsi%d: target mask %02x, lun %d sent tag %d at "
> "reselection\n", HOSTNO, target_mask, lun, tag);
> }
> #endif
>
>
>> Changes to NCR5380. are rather major though. I need to carefully go
>> through the diff again. ISTR trying to run the coroutine as delayed
>> workqueue instead of immediate but dropped that again for some reason.
>>
>
> I don't think it makes sense to unify NCR5380.c and atari_NCR5380.c at
> this stage. The ISA cards' requirements together are more peculiar than,
> say, ST-DMA, and are mostly irrelevant to the M68k, ARM and PCI devices.
>
OK, not for today then.
> As an aside, forking NCR5380.c into atari_NCR5380.c, sun3_NCR5380.c and
> mac_NCR5380.c (deleted in v2.6.3) was clearly misguided as it never
> actually produced a good alternative to NCR5380.c.
>
atari_NCR5380.c was forked at a time when the m68k port was developed
entirely independent from the main Linux development (which was i386
only at that stage). Linus did not want to deal with other
architectures, and merging m68k code into Linus' code was not something
the m68k authors envisaged at that stage. The fork was not meant to
produce a good alternative to the main driver, just something that
worked. At least that is as I recall it - I had little to do with Linux
kernel development at that stage.
Merging ongoing development on NCR5380.c into the Atari driver was
always something of an afterthought (Geert probably took care of most of
that). The sun3 and Mac drivers were then forked off the Atari one. I'm
guilty in part of the initial Mac one, but that was done mainly in order
to stop Alan Cox from whinging about the lack of a disk driver. (Don't
think he liked SCSI in PIO mode much.)
What I mean to say - in hindsight, it was misguided, but at the time we
did not have the benefit of a crystal ball (or git, or even fast cross
compilers).
> Forking just multiplied the maintenance demands, and diminishing man-power
> of course lead to neglect and breakage. So I can sympathise with
> Christoph's comment in NCR5380.c:
>
> * (Note from hch: unfortunately it was not enough for the different
> * m68k folks and instead of improving this driver they copied it
> * and hacked it up for their needs. As a consequence they lost
> * most updates to this driver. Maybe someone will fix all these
> * drivers to use a common core one day..)
>
Yep, I can now sympathise with that. When m68k began to merge with
Linus, we had already lost much of the initial man-power and expertise.
I'm sure we'd do things different now, but these lessons had to be
learned along the way.
Cheers,
Michael
next prev parent reply other threads:[~2014-08-05 9:07 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-16 14:18 converting the NCR5380 drivers away from scsi_register Christoph Hellwig
2014-06-17 8:20 ` Finn Thain
2014-06-17 8:38 ` Geert Uytterhoeven
2014-07-30 8:32 ` Finn Thain
2014-07-31 5:31 ` Michael Schmitz
2014-08-01 3:13 ` Finn Thain
2014-08-01 8:15 ` Michael Schmitz
2014-08-02 1:27 ` Finn Thain
2014-08-02 8:51 ` Michael Schmitz
2014-08-03 3:43 ` Finn Thain
2014-08-03 9:07 ` Michael Schmitz
2014-08-04 3:28 ` Finn Thain
2014-08-05 9:06 ` Michael Schmitz [this message]
2014-08-06 1:25 ` Sun3 SCSI DMA, was " Finn Thain
2014-08-06 14:42 ` Sam Creasey
2014-08-08 8:46 ` Michael Schmitz
2014-08-11 15:10 ` Sam Creasey
2014-08-13 5:29 ` Finn Thain
2014-08-13 9:14 ` Michael Schmitz
2014-08-14 1:43 ` Finn Thain
2014-08-14 8:57 ` Michael Schmitz
2014-08-15 1:46 ` Finn Thain
2014-08-15 1:51 ` Michael Schmitz
2014-08-15 2:09 ` Finn Thain
2014-08-15 3:03 ` Michael Schmitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53E09EAE.70308@gmail.com \
--to=schmitzmic@gmail.com \
--cc=fthain@telegraphics.com.au \
--cc=geert@linux-m68k.org \
--cc=hch@infradead.org \
--cc=linux-m68k@vger.kernel.org \
--cc=sammy@sammy.net \
--cc=schmitz@debian.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox