converting the NCR5380 drivers away from scsi_register

converting the NCR5380 drivers away from scsi_register

[Christoph Hellwig]

Hi Finn,

now that you've officially taken over NCR5380 is there a chance to look
into converting the drivers away from scsi_register to the modern
scsi_host_alloc/scsi_add_host method?  The NCR5380 drivers are a large
armound of the remaining users of this interface, so converting them
over would be a great help in killing that 10 year old legacy off.

Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Mon, 16 Jun 2014, Christoph Hellwig wrote:

> now that you've officially taken over NCR5380 is there a chance to look 
> into converting the drivers away from scsi_register to the modern 
> scsi_host_alloc/scsi_add_host method?

Have any other conversions taken place? I'm curious to see what was 
involved. Hopefully not a re-write, like esp_scsi (?)

I have not yet managed to get mac_scsi working as well as it should: PDMA 
is not working, either because I'm running it on the wrong (poorly 
documented) hardware or because the driver has bugs.

I still plan to set up a machine with a DMX3191D pci card for testing. It 
would be nice to have the driver working on that hardware (at least) 
before making (or just testing) any substantial changes.

This will take time...

-- 

Re: converting the NCR5380 drivers away from scsi_register

[Geert Uytterhoeven]

Hi Finn,

On Tue, Jun 17, 2014 at 10:20 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
>> now that you've officially taken over NCR5380 is there a chance to look
>> into converting the drivers away from scsi_register to the modern
>> scsi_host_alloc/scsi_add_host method?
>
> Have any other conversions taken place? I'm curious to see what was
> involved. Hopefully not a re-write, like esp_scsi (?)

I did a few of them in 2009:

commit c2a24a4ca1137473971842461612e56a654e7edb
("m68k: amiga - A3000 SCSI platform device conversion")
commit c1d288a58936cd0654844d807e53a203f4838fb4
("m68k: amiga - GVP Series II SCSI zorro_driver conversion")
commit c737e22cde37e4e2ad126316e4aab7349a491ab3
("m68k: amiga - A2091/A590 SCSI zorro_driver conversion")

All of them without access to the hardware. So far I haven't heard any
complaints ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Tue, 17 Jun 2014, Geert Uytterhoeven wrote:

> Hi Finn,
> 
> On Tue, Jun 17, 2014 at 10:20 AM, Finn Thain wrote:
> >> now that you've officially taken over NCR5380 is there a chance to 
> >> look into converting the drivers away from scsi_register to the 
> >> modern scsi_host_alloc/scsi_add_host method?
> >
> > Have any other conversions taken place? I'm curious to see what was 
> > involved. Hopefully not a re-write, like esp_scsi (?)
> 
> I did a few of them in 2009:
> 
> commit c2a24a4ca1137473971842461612e56a654e7edb
> ("m68k: amiga - A3000 SCSI platform device conversion")
> commit c1d288a58936cd0654844d807e53a203f4838fb4
> ("m68k: amiga - GVP Series II SCSI zorro_driver conversion")
> commit c737e22cde37e4e2ad126316e4aab7349a491ab3
> ("m68k: amiga - A2091/A590 SCSI zorro_driver conversion")
> 
> All of them without access to the hardware. So far I haven't heard any
> complaints ;-)

Thanks Geert. You do make it look easy to avoid regressions!

sun3x_esp and mac_esp are platform devices so I guess I should convert 
sun3_scsi and mac_scsi to platform devices also. I'm not sure about 
atari_scsi, perhaps there's a different bus that can probe it. Michael?

I see that the PCI driver (dmx3191d) and the ARM drivers (oak and 
cumana_1) have already been converted. So the remaining NCR5380 drivers 
that still use scsi_register() are the ISA cards: g_NCR5380, dtc, 
pas16 and t128.

I don't have any ISA hardware. I certainly don't mind if those drivers 
ultimately get removed along with scsi_register() itself.

Those ISA drivers depend on functionality in the NCR5380.c core driver 
that was discarded by the other two core driver variants (atari_NCR5380.c 
and sun3_NCR5380.c) such as DONT_USE_INTR, UNSAFE, DMA_WORKS_RIGHT, 
AUTOPROBE_IRQ, USLEEP_POLL, NCR53C400.

If it were not for the ISA cards, we may not need three variations on the 
core driver.

-- 

Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Finn,
> On Tue, 17 Jun 2014, Geert Uytterhoeven wrote:
>
>> Hi Finn,
>>
>> On Tue, Jun 17, 2014 at 10:20 AM, Finn Thain wrote:
>>>> now that you've officially taken over NCR5380 is there a chance to
>>>> look into converting the drivers away from scsi_register to the
>>>> modern scsi_host_alloc/scsi_add_host method?
>>> Have any other conversions taken place? I'm curious to see what was
>>> involved. Hopefully not a re-write, like esp_scsi (?)
>> I did a few of them in 2009:
>>
>> commit c2a24a4ca1137473971842461612e56a654e7edb
>> ("m68k: amiga - A3000 SCSI platform device conversion")
>> commit c1d288a58936cd0654844d807e53a203f4838fb4
>> ("m68k: amiga - GVP Series II SCSI zorro_driver conversion")
>> commit c737e22cde37e4e2ad126316e4aab7349a491ab3
>> ("m68k: amiga - A2091/A590 SCSI zorro_driver conversion")
>>
>> All of them without access to the hardware. So far I haven't heard any
>> complaints ;-)
> Thanks Geert. You do make it look easy to avoid regressions!
>
> sun3x_esp and mac_esp are platform devices so I guess I should convert
> sun3_scsi and mac_scsi to platform devices also. I'm not sure about
> atari_scsi, perhaps there's a different bus that can probe it. Michael?

atari_scsi is only used on TT and Falcon Atari variants. 'Testing' for 
hardware presence is just checking bits from a bitmap populated at 
arch_init time, that can easily be moved into a platform device setup, 
much like I already do for the network drivers. Does that answer your 
question?

Cheers,

     Michael


>
> I see that the PCI driver (dmx3191d) and the ARM drivers (oak and
> cumana_1) have already been converted. So the remaining NCR5380 drivers
> that still use scsi_register() are the ISA cards: g_NCR5380, dtc,
> pas16 and t128.
>
> I don't have any ISA hardware. I certainly don't mind if those drivers
> ultimately get removed along with scsi_register() itself.
>
> Those ISA drivers depend on functionality in the NCR5380.c core driver
> that was discarded by the other two core driver variants (atari_NCR5380.c
> and sun3_NCR5380.c) such as DONT_USE_INTR, UNSAFE, DMA_WORKS_RIGHT,
> AUTOPROBE_IRQ, USLEEP_POLL, NCR53C400.
>
> If it were not for the ISA cards, we may not need three variations on the
> core driver.
>

Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Thu, 31 Jul 2014, Michael Schmitz wrote:

> atari_scsi is only used on TT and Falcon Atari variants. 'Testing' for 
> hardware presence is just checking bits from a bitmap populated at 
> arch_init time, that can easily be moved into a platform device setup, 
> much like I already do for the network drivers. Does that answer your 
> question?

Yes. I'll convert {atari,mac,sun3}_scsi to platform devices and 
scsi_host_alloc/scsi_add_host.

-- 

Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Finn,
>> atari_scsi is only used on TT and Falcon Atari variants. 'Testing' for 
>> hardware presence is just checking bits from a bitmap populated at 
>> arch_init time, that can easily be moved into a platform device setup, 
>> much like I already do for the network drivers. Does that answer your 
>> question?
>>     
>
> Yes. I'll convert {atari,mac,sun3}_scsi to platform devices and 
> scsi_host_alloc/scsi_add_host.
>   

Thanks, much appreciated. I can supply the platform device setup in 
arch/m68k/atari/config.c once you have decided what kind of resources or 
platform data will be needed.

If you rather want to do this - either use the ATARIHW_PRESENT() macros 
to test for  ST_SCSI (Falcon, interrupt no. IRQ_MFP_FSCSI) or TT_SCSI 
(TT, interrupt no. IRQ_TT_MFP_SCSI). Or else, replicate the logic from 
config_atari() - the SCSI chip directly mapped only in the TT integration,
the Falcon needs to access SCSI registers through the ST-DMA chip, and 
needs the weird ST-DMA locking scheme plus a few other quirks.

Looking at atari_scsi.c - the code is full of IS_A_TT() macros and other 
Atari specfic macros that could be replaced by testing bits in a feature 
map. One bit (TT or Falcon style SCSI integration) rather - that still 
leaves the register access functions for TT and Falcon to sort out.  Do 
you plan to do all that in one go? Might need another platform device 
for the ST-DMA as well ...

Cheers,

    Michael

Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Fri, 1 Aug 2014, Michael Schmitz wrote:

> 
> Thanks, much appreciated. I can supply the platform device setup in 
> arch/m68k/atari/config.c once you have decided what kind of resources or 
> platform data will be needed.
> 
> If you rather want to do this - either use the ATARIHW_PRESENT() macros 
> to test for ST_SCSI (Falcon, interrupt no. IRQ_MFP_FSCSI) or TT_SCSI 
> (TT, interrupt no. IRQ_TT_MFP_SCSI). Or else, replicate the logic from 
> config_atari()

Yes, that was my plan. A patch that replicates that logic is easier to 
review and less likely to cause regressions (of course, I'd follow the 
existing conventions in arch/m68k/atari/config.c).

Converting three drivers at once is a win because the first conversion is 
always the more difficult one. Therefore I'm planning to use the same 
logic three times over and therefore I'm not intending to address the 
peculiarities of different ports (that would be better done by you, me and 
Sam separately).

However, I'll will need you and Sam to test some patches (if they meet 
with approval once you and Sam get to review them, of course).

> - the SCSI chip directly mapped only in the TT integration, the Falcon 
> needs to access SCSI registers through the ST-DMA chip, and needs the 
> weird ST-DMA locking scheme plus a few other quirks.
> 
> Looking at atari_scsi.c - the code is full of IS_A_TT() macros and other 
> Atari specfic macros that could be replaced by testing bits in a feature 
> map. One bit (TT or Falcon style SCSI integration) rather - that still 
> leaves the register access functions for TT and Falcon to sort out.  Do 
> you plan to do all that in one go?

No, not in one go. That would be a separate patch, so that each patch has 
a single well defined purpose.

I'm not particularly concerned about atari_scsi.c. I am concerned about 
the three forks of the core driver (not least because of the shared 
header) that's why I've sent patches for sun3_NCR5380 and atari_NCR5380 in 
the past.

> Might need another platform device for the ST-DMA as well ...

I don't think that relates to scsi_register() deprecation.

-- 

Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Finn,
>>
>> If you rather want to do this - either use the ATARIHW_PRESENT() macros 
>> to test for ST_SCSI (Falcon, interrupt no. IRQ_MFP_FSCSI) or TT_SCSI 
>> (TT, interrupt no. IRQ_TT_MFP_SCSI). Or else, replicate the logic from 
>> config_atari()
>>     
>
> Yes, that was my plan. A patch that replicates that logic is easier to 
> review and less likely to cause regressions (of course, I'd follow the 
> existing conventions in arch/m68k/atari/config.c).
>   

Sounds good to me.

> Converting three drivers at once is a win because the first conversion is 
> always the more difficult one. Therefore I'm planning to use the same 
> logic three times over and therefore I'm not intending to address the 
> peculiarities of different ports (that would be better done by you, me and 
> Sam separately).
>   

OK, I'll think some more about what could be done to merge the quirks 
into a shared core.

> However, I'll will need you and Sam to test some patches (if they meet 
> with approval once you and Sam get to review them, of course).
>
>   
>> - the SCSI chip directly mapped only in the TT integration, the Falcon 
>> needs to access SCSI registers through the ST-DMA chip, and needs the 
>> weird ST-DMA locking scheme plus a few other quirks.
>>
>> Looking at atari_scsi.c - the code is full of IS_A_TT() macros and other 
>> Atari specfic macros that could be replaced by testing bits in a feature 
>> map. One bit (TT or Falcon style SCSI integration) rather - that still 
>> leaves the register access functions for TT and Falcon to sort out.  Do 
>> you plan to do all that in one go?
>>     
>
> No, not in one go. That would be a separate patch, so that each patch has 
> a single well defined purpose.
>
> I'm not particularly concerned about atari_scsi.c. I am concerned about 
> the three forks of the core driver (not least because of the shared 
> header) that's why I've sent patches for sun3_NCR5380 and atari_NCR5380 in 
> the past.
>   

OK, forget about atari_scsi.c for now. From memory, the major difference 
between atari_NCR5380.c and the others is handling of the ST-DMA/shared 
interrupt locking. We need to preserve that pretty much as is, or risk 
serious regressions. I mean, more serious trouble than I already have 
with the current driver because of the tendency of my Falcon to muck up 
the SCSI chip's clock signal under heavy load. Also note that there is 
still one of my patches unmerged (under review since early this year) 
that is necessary to avoid 'scheduling in interrupt' style panic.

The rest of the differences were tweaks added by the Atari SCSI author 
to make the driver behave with borderline standards compliant hardware 
(long reset delay for some CD drives, tweaks to bus settle delays). 
Probably not that critical in the first instance.

>> Might need another platform device for the ST-DMA as well ...
>>     
>
> I don't think that relates to scsi_register() deprecation.
>   

Not at all - just to the platform device conversion. Not sure I fully 
understand what you have in mind, though.

Cheers,

    Michael

Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Sat, 2 Aug 2014, Michael Schmitz wrote:

> > I'm not particularly concerned about atari_scsi.c. I am concerned 
> > about the three forks of the core driver (not least because of the 
> > shared header) that's why I've sent patches for sun3_NCR5380 and 
> > atari_NCR5380 in the past.
> >   
> 
> OK, forget about atari_scsi.c for now. From memory, the major difference 
> between atari_NCR5380.c and the others is handling of the ST-DMA/shared 
> interrupt locking.

Yes, sun3_NCR5380.c and NCR5380.c lack support for the ST-DMA chip.

And both NCR5380.c and sun3_NCR5380.c dropped the code to support ISA 
cards, as I mentioned.

Both NCR5380.c and sun3_NCR5380.c lack merge_contiguous_buffers().

NCR5380.c lacks support for tagged queueing, while sun3_NCR5380.c and 
atari_NCR5380.c have divergent implementations of this.

sun3_NCR5380.c and atari_NCR5380.c have divergent implementations of 
NCR5380_transfer_dma().

sun3_NCR5380.c and atari_NCR5380.c share the same implementation of the 
NCR5380_main() co-routine, but it differs significantly from the algorithm 
in NCR5380.c.

Each of sun3_NCR5380.c, atari_NCR5380.c and NCR5380.c have had some clean 
up and modernization work that does not appear in any of the others.

Despite all of that, I have a patch series that attempts to unify 
atari_NCR5380.c and sun3_NCR5380.c. This seems like a sensible idea (in 
theory) given that probably only the ISA cards actually need NCR5380.c.

At some point I'll send some patches for comment, so that we might discuss 
the issues with everyone on the same page. Until then, I'd prefer to focus 
on the scsi_register() conversion.

> We need to preserve that pretty much as is, or risk serious regressions. 
> I mean, more serious trouble than I already have with the current driver 
> because of the tendency of my Falcon to muck up the SCSI chip's clock 
> signal under heavy load. Also note that there is still one of my patches 
> unmerged (under review since early this year) that is necessary to avoid 
> 'scheduling in interrupt' style panic.

Can you re-send it please?

> The rest of the differences were tweaks added by the Atari SCSI author 
> to make the driver behave with borderline standards compliant hardware 
> (long reset delay for some CD drives, tweaks to bus settle delays). 
> Probably not that critical in the first instance.
> 
> > > Might need another platform device for the ST-DMA as well ...
> > >     
> >
> > I don't think that relates to scsi_register() deprecation.
> >   
> 
> Not at all - just to the platform device conversion. Not sure I fully 
> understand what you have in mind, though.

With regards to scsi_register() conversion I'll send some patches for you 
to review once complete and tested (in mac_scsi). They will probably need 
revision after review; I'm fine with that.

-- 

Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Finn,
>> OK, forget about atari_scsi.c for now. From memory, the major difference 
>> between atari_NCR5380.c and the others is handling of the ST-DMA/shared 
>> interrupt locking.
>>     
>
> Yes, sun3_NCR5380.c and NCR5380.c lack support for the ST-DMA chip.
>   

As should be.

> And both NCR5380.c and sun3_NCR5380.c dropped the code to support ISA 
> cards, as I mentioned.
>
> Both NCR5380.c and sun3_NCR5380.c lack merge_contiguous_buffers().
>   

I'm sure that was a tweak added by Roman Hodek to improve performance. 
Should not hurt to merge this one.

> 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..

> sun3_NCR5380.c and atari_NCR5380.c have divergent implementations of 
> NCR5380_transfer_dma().
>   

This is due to different DMA hardware in the three cases covered, so 
expected. There's quite a bit of additional DMA mumble in 
NCR5380_information_transfer and NCR5380_reselect.

> sun3_NCR5380.c and atari_NCR5380.c share the same implementation of the 
> NCR5380_main() co-routine, but it differs significantly from the algorithm 
> in NCR5380.c.
>   

atari_NCR5380.c was forked from NCR5380.c as early as 1994, and the 
changes to the NCR5380.c coroutine will be a later addition. NCR5380.c 
has seen development for a few years that we've never picked up.

> Each of sun3_NCR5380.c, atari_NCR5380.c and NCR5380.c have had some clean 
> up and modernization work that does not appear in any of the others.
>
> Despite all of that, I have a patch series that attempts to unify 
> atari_NCR5380.c and sun3_NCR5380.c. This seems like a sensible idea (in 
> theory) given that probably only the ISA cards actually need NCR5380.c.
>   

The diff between the two was amazingly clean and easy to parse (aside 
from the above mentioned DMA setup related stuff). Merging the two seems 
quite possible indeed.

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.

> At some point I'll send some patches for comment, so that we might discuss 
> the issues with everyone on the same page. Until then, I'd prefer to focus 
> on the scsi_register() conversion.
>   

OK, do that first so scsi_register can go away.

>> We need to preserve that pretty much as is, or risk serious regressions. 
>> I mean, more serious trouble than I already have with the current driver 
>> because of the tendency of my Falcon to muck up the SCSI chip's clock 
>> signal under heavy load. Also note that there is still one of my patches 
>> unmerged (under review since early this year) that is necessary to avoid 
>> 'scheduling in interrupt' style panic.
>>     
>
> Can you re-send it please?
>   

I'll do that.

Cheers,

    Michael

Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

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 
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. 
I still have to try to figure out the implications of this change.

#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.

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.

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..)

But one core is hard, even if you have all the hardware. Two cores is 
better than three.

-- 

Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

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

Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Tue, 5 Aug 2014, Michael Schmitz wrote:

> 
> On Sun3, the code is not even configured from what I've seen 
> (SUPPORT_TAGS not defined)
> 
[...]
> 
> 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).

Yes, but why the discrepancy?

Not unlike the use of DMA in NCR5380_reselect(), the sun3 version of 
NCR5380_information_transfer() also uses DMA in one situation where the 
atari version uses PIO. That is, when phase == PHASE_DATA_IN && 
cmd->device->borken.

Can we just use the PIO code from atari_NCR5380, or should we keep the DMA 
versions with #ifdef CONFIG_SUN3? Sam?

> 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.

I guess that's why sun3_scsi leaves SUPPORT_TAGS undefined. I suppose we 
can ignore all the code in sun3_NCR5380.c that is conditional upon #ifdef 
SUPPORT_TAGS. (That's actually how I did my preliminary merge patches.)

I guess having two versions of the NCR5380_transfer_dma() routine is 
reasonable but it would be nice if some of the other discrepancies could 
be resolved. Without knowledge of the Sun 3 DMA hardware it is difficult 
to avoid some questionable #ifdefs.

-- 

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Sam Creasey]

On Wed, Aug 06, 2014 at 11:25:18AM +1000, Finn Thain wrote:
> 
> On Tue, 5 Aug 2014, Michael Schmitz wrote:
> 
> > 
> > On Sun3, the code is not even configured from what I've seen 
> > (SUPPORT_TAGS not defined)
> > 
> [...]
> > 
> > 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).
> 
> Yes, but why the discrepancy?
> 
> Not unlike the use of DMA in NCR5380_reselect(), the sun3 version of 
> NCR5380_information_transfer() also uses DMA in one situation where the 
> atari version uses PIO. That is, when phase == PHASE_DATA_IN && 
> cmd->device->borken.
> 
> Can we just use the PIO code from atari_NCR5380, or should we keep the DMA 
> versions with #ifdef CONFIG_SUN3? Sam?

Oof, my memory is hazy around this one, but I would strongly hesitate
to use the PIO version from the atari driver.  IIRC, the DMA
controller for the sun3's NCR5380 implementation is extremely fussy
about what happens in which phase, and it's quick to anger if you
don't handle everything exactly how it expects.
 
> > 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.
> 
> I guess that's why sun3_scsi leaves SUPPORT_TAGS undefined. I suppose we 
> can ignore all the code in sun3_NCR5380.c that is conditional upon #ifdef 
> SUPPORT_TAGS. (That's actually how I did my preliminary merge patches.)
> 
> I guess having two versions of the NCR5380_transfer_dma() routine is 
> reasonable but it would be nice if some of the other discrepancies could 
> be resolved. Without knowledge of the Sun 3 DMA hardware it is difficult 
> to avoid some questionable #ifdefs.

I definitely remember that the DMA setup logic from the atari version
did not work at all on sun3.  Unfortunatly working from 15 year old
memories, I can't quickly recall what the discrepencies were.  I never
did find any documentation for that DMA chip, so there was a heck of a
lot of trial and error on getting to a working driver.

-- Sam

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Sam,
>>> 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).
>>>       
>> Yes, but why the discrepancy?
>>
>> Not unlike the use of DMA in NCR5380_reselect(), the sun3 version of 
>> NCR5380_information_transfer() also uses DMA in one situation where the 
>> atari version uses PIO. That is, when phase == PHASE_DATA_IN && 
>> cmd->device->borken.
>>
>> Can we just use the PIO code from atari_NCR5380, or should we keep the DMA 
>> versions with #ifdef CONFIG_SUN3? Sam?
>>     
>
> Oof, my memory is hazy around this one, but I would strongly hesitate
> to use the PIO version from the atari driver.  IIRC, the DMA
> controller for the sun3's NCR5380 implementation is extremely fussy
> about what happens in which phase, and it's quick to anger if you
> don't handle everything exactly how it expects.
>   

Does the DMA controller also sit in between the bus and the NCR chip, as 
on Falcon? Otherwise, I can't see how it would matter if we just bypass 
it and use PIO instead.

The PIO code from the Atari driver worked just fine for the Mac 5380 (in 
fact, the first Mac driver used PIO for everything, including data in/out).
>  
>   
>>> 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.
>>>       
>> I guess that's why sun3_scsi leaves SUPPORT_TAGS undefined. I suppose we 
>> can ignore all the code in sun3_NCR5380.c that is conditional upon #ifdef 
>> SUPPORT_TAGS. (That's actually how I did my preliminary merge patches.)
>>
>> I guess having two versions of the NCR5380_transfer_dma() routine is 
>> reasonable but it would be nice if some of the other discrepancies could 
>> be resolved. Without knowledge of the Sun 3 DMA hardware it is difficult 
>> to avoid some questionable #ifdefs.
>>     
>
> I definitely remember that the DMA setup logic from the atari version
> did not work at all on sun3. 

No surprise there - DMA is implementation specific and needs to be 
handled on a per-case basis.

>  Unfortunatly working from 15 year old
> memories, I can't quickly recall what the discrepencies were.  I never
> did find any documentation for that DMA chip, so there was a heck of a
> lot of trial and error on getting to a working driver.
>   

Do you still have the hardware to test (PIO in reselection, to be 
specific)?

Cheers,

    Michael

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Sam Creasey]

On Fri, Aug 08, 2014 at 08:46:08PM +1200, Michael Schmitz wrote:
> Hi Sam,
> >>>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).
> >>>      
> >>Yes, but why the discrepancy?
> >>
> >>Not unlike the use of DMA in NCR5380_reselect(), the sun3 version of 
> >>NCR5380_information_transfer() also uses DMA in one situation where the 
> >>atari version uses PIO. That is, when phase == PHASE_DATA_IN && 
> >>cmd->device->borken.
> >>
> >>Can we just use the PIO code from atari_NCR5380, or should we keep the 
> >>DMA versions with #ifdef CONFIG_SUN3? Sam?
> >>    
> >
> >Oof, my memory is hazy around this one, but I would strongly hesitate
> >to use the PIO version from the atari driver.  IIRC, the DMA
> >controller for the sun3's NCR5380 implementation is extremely fussy
> >about what happens in which phase, and it's quick to anger if you
> >don't handle everything exactly how it expects.
> >  
> 
> Does the DMA controller also sit in between the bus and the NCR chip, as 
> on Falcon? Otherwise, I can't see how it would matter if we just bypass 
> it and use PIO instead.
> 
> The PIO code from the Atari driver worked just fine for the Mac 5380 (in 
> fact, the first Mac driver used PIO for everything, including data in/out).

PIO definitely works on the Sun3 implementation under the right
circumstances, I ran it in PIO mode only for a while (much like the
Mac driver).

I can't remember the bus configuration offhand.

> >  
> >>>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.
> >>>      
> >>I guess that's why sun3_scsi leaves SUPPORT_TAGS undefined. I suppose we 
> >>can ignore all the code in sun3_NCR5380.c that is conditional upon #ifdef 
> >>SUPPORT_TAGS. (That's actually how I did my preliminary merge patches.)
> >>
> >>I guess having two versions of the NCR5380_transfer_dma() routine is 
> >>reasonable but it would be nice if some of the other discrepancies could 
> >>be resolved. Without knowledge of the Sun 3 DMA hardware it is difficult 
> >>to avoid some questionable #ifdefs.
> >>    
> >
> >I definitely remember that the DMA setup logic from the atari version
> >did not work at all on sun3. 
> 
> No surprise there - DMA is implementation specific and needs to be 
> handled on a per-case basis.
> 
> > Unfortunatly working from 15 year old
> >memories, I can't quickly recall what the discrepencies were.  I never
> >did find any documentation for that DMA chip, so there was a heck of a
> >lot of trial and error on getting to a working driver.
> >  
> 
> Do you still have the hardware to test (PIO in reselection, to be 
> specific)?

Yes, I still have hardware.  I'll try to dig something out and make
sure the last kernel I've actually tested still works before trying
anything new...

-- Sam
 
> Cheers,
> 
>    Michael
> 

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Mon, 11 Aug 2014, Sam Creasey wrote:

> On Fri, Aug 08, 2014 at 08:46:08PM +1200, Michael Schmitz wrote:
> > >IIRC, the DMA controller for the sun3's NCR5380 implementation is 
> > >extremely fussy about what happens in which phase, and it's quick to 
> > >anger if you don't handle everything exactly how it expects.
> > >  
> > 
> > Does the DMA controller also sit in between the bus and the NCR chip, 
> > as on Falcon? Otherwise, I can't see how it would matter if we just 
> > bypass it and use PIO instead.
> > 
> > The PIO code from the Atari driver worked just fine for the Mac 5380 
> > (in fact, the first Mac driver used PIO for everything, including data 
> > in/out).
> 
> PIO definitely works on the Sun3 implementation under the right 
> circumstances, I ran it in PIO mode only for a while (much like the Mac 
> driver).
> 
> I can't remember the bus configuration offhand.

It might not be a big problem: sun3_scsi does a lot of PIO a lot as it is.

For NCR5380_reselect(), atari_NCR5380.c apparently uses only PIO whereas 
sun3_NCR5380.c will use PIO up to a 128 byte size limit (beyond which, 
only DMA is used).

For NCR5380_information_transfer() and PHASE_DATAIN, atari will use PIO 
for transfersize <= 31 bytes whereas sun3 will use PIO for <= 128 bytes. 
However, atari never uses DMA here if cmd->device->borken.

When cmd->device->borken, I assume sun3_NCR5380 inhibits PIO because PIO 
was itself expected to be problematic?

OTOH, if PIO works up to 128 bytes in all cases, why not larger transfers? 

Does the sun3_scsi driver still work after removing #define REAL_DMA?

For NCR5380_information_transfer() and PHASE_CMDOUT, the sun3 version 
applies the same size limit but only does DMA setup if cmd type is 
REQ_TYPE_FS (i.e. filesystem request). This command is then sent by PIO, 
so the DMA setup here seems to assume that the next phase will always be 
PHASE_DATAIN...

This would seem to imply that PIO always works when not REQ_TYPE_FS, such 
that no size limit is applicable...

BTW, testing sun3_dma_setup_done against cmd pointers looks very 
unreliable to me: unique cmds only have unique pointers until freed by the 
scsi mid-layer. After that, the same pointer is likely be re-used.

> > >
> > >I definitely remember that the DMA setup logic from the atari version 
> > >did not work at all on sun3.
> > 
> > No surprise there - DMA is implementation specific and needs to be 
> > handled on a per-case basis.

Perhaps atari_NCR5380 could use some of the sun3_NCR5380 code. E.g. atari 
avoids DMA entirely for reselect, and (only in some phases) when 
cmd->device->borken.

-- 

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Finn,
> On Mon, 11 Aug 2014, Sam Creasey wrote:
>
>   
>> On Fri, Aug 08, 2014 at 08:46:08PM +1200, Michael Schmitz wrote:
>>     
>>>> IIRC, the DMA controller for the sun3's NCR5380 implementation is 
>>>> extremely fussy about what happens in which phase, and it's quick to 
>>>> anger if you don't handle everything exactly how it expects.
>>>>  
>>>>         
>>> Does the DMA controller also sit in between the bus and the NCR chip, 
>>> as on Falcon? Otherwise, I can't see how it would matter if we just 
>>> bypass it and use PIO instead.
>>>
>>> The PIO code from the Atari driver worked just fine for the Mac 5380 
>>> (in fact, the first Mac driver used PIO for everything, including data 
>>> in/out).
>>>       
>> PIO definitely works on the Sun3 implementation under the right 
>> circumstances, I ran it in PIO mode only for a while (much like the Mac 
>> driver).
>>
>> I can't remember the bus configuration offhand.
>>     
>
> It might not be a big problem: sun3_scsi does a lot of PIO a lot as it is.
>
> For NCR5380_reselect(), atari_NCR5380.c apparently uses only PIO whereas 
> sun3_NCR5380.c will use PIO up to a 128 byte size limit (beyond which, 
> only DMA is used).
>   

Transfer sizes for message in during reselection are only a few bytes. 
According to atari_dma_xfer_len(), DMA reads must be multiple of 512 
bytes (writes can be rounded up, reads cannot). PIO is the only option 
for Atari, I'm afraid.

> For NCR5380_information_transfer() and PHASE_DATAIN, atari will use PIO 
> for transfersize <= 31 bytes whereas sun3 will use PIO for <= 128 bytes. 
>   

IIRC the ST-DMA can't be programmed for shorter transfers than 512 bytes. 
The condition

(transfersize = NCR5380_dma_xfer_len(instance,cmd,phase)) > 31)

enforces that - transfersize will be 0 on reads that are not multiples 
of 512 bytes (and even some where the advertised transfer size does meet 
that condition, but the command is not guranteed to be a block mode 
command (tape reads).

> However, atari never uses DMA here if cmd->device->borken.
>   

Different semantics of borken, I presume. On Atari, this means a DMA 
transfer has failed earlier.

> When cmd->device->borken, I assume sun3_NCR5380 inhibits PIO because PIO 
> was itself expected to be problematic?
>
> OTOH, if PIO works up to 128 bytes in all cases, why not larger transfers? 
>   

Performance, perhaps. PIO would work, but hog the CPU. In fact, PIO for 
everything _does_ work, on Falcon as well as Mac (I tried the PIO only 
mode on the Falcon before trying on Mac). Did I eve mention how reallly 
incredibly painfully slow that is?

> Does the sun3_scsi driver still work after removing #define REAL_DMA?
>
> For NCR5380_information_transfer() and PHASE_CMDOUT, the sun3 version 
> applies the same size limit but only does DMA setup if cmd type is 
> REQ_TYPE_FS (i.e. filesystem request). This command is then sent by PIO, 
> so the DMA setup here seems to assume that the next phase will always be 
> PHASE_DATAIN...
>   

Might be a similar limitation to certain transfer sizes here. Block mode 
vs. char mode.

> This would seem to imply that PIO always works when not REQ_TYPE_FS, such 
> that no size limit is applicable...
>
> BTW, testing sun3_dma_setup_done against cmd pointers looks very 
> unreliable to me: unique cmds only have unique pointers until freed by the 
> scsi mid-layer. After that, the same pointer is likely be re-used.
>
>   
>>>> I definitely remember that the DMA setup logic from the atari version 
>>>> did not work at all on sun3.
>>>>         
>>> No surprise there - DMA is implementation specific and needs to be 
>>> handled on a per-case basis.
>>>       
>
> Perhaps atari_NCR5380 could use some of the sun3_NCR5380 code. E.g. atari 
> avoids DMA entirely for reselect, and (only in some phases) when 
> cmd->device->borken.
>   

I don't think we can use DMA for the reselect message in transfers. Data 
phase transfer is handled by the coroutine after reselection anyway - 
that one will use DMA.

Cheers,

    Michael

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Wed, 13 Aug 2014, Michael Schmitz wrote:

> > For NCR5380_information_transfer() and PHASE_DATAIN, atari will use 
> > PIO for transfersize <= 31 bytes whereas sun3 will use PIO for <= 128 
> > bytes.
> >   
> 
> IIRC the ST-DMA can't be programmed for shorter transfers than 512 
> bytes. The condition
> 
> (transfersize = NCR5380_dma_xfer_len(instance,cmd,phase)) > 31)
> 
> enforces that - transfersize will be 0 on reads that are not multiples 
> of 512 bytes (and even some where the advertised transfer size does meet 
> that condition, but the command is not guranteed to be a block mode 
> command (tape reads).

I see.

> 
> > However, atari never uses DMA here if cmd->device->borken.
> >   
> 
> Different semantics of borken, I presume. On Atari, this means a DMA 
> transfer has failed earlier.

Right. I suppose the question of using PIO in this situation on sun3 would 
have to be put to the test.

> 
> > When cmd->device->borken, I assume sun3_NCR5380 inhibits PIO because 
> > PIO was itself expected to be problematic?
> >
> > OTOH, if PIO works up to 128 bytes in all cases, why not larger 
> > transfers?
> >   
> 
> Performance, perhaps.

Well, sun3_scsi has this:

static inline unsigned long sun3scsi_dma_xfer_len(unsigned long wanted,
                                                  struct scsi_cmnd *cmd,
                                                  int write_flag)
{
        if (cmd->request->cmd_type == REQ_TYPE_FS)
                return wanted;
        else
                return 0;
}

This does suggest that PIO does work beyond 128 bytes, even if it is 
avoided for filesystem requests. Which suggests that the atari reselect 
code might work on sun3.

(As for performance, I don't know why the REQ_TYPE_FS is significant: why 
not just raise the DMA size limit to 512 instead?)

> PIO would work, but hog the CPU. In fact, PIO for everything _does_ 
> work, on Falcon as well as Mac (I tried the PIO only mode on the Falcon 
> before trying on Mac). Did I eve mention how reallly incredibly 
> painfully slow that is?
> 
> > Does the sun3_scsi driver still work after removing #define REAL_DMA?
> >
> > For NCR5380_information_transfer() and PHASE_CMDOUT, the sun3 version 
> > applies the same size limit but only does DMA setup if cmd type is 
> > REQ_TYPE_FS (i.e. filesystem request). This command is then sent by 
> > PIO, so the DMA setup here seems to assume that the next phase will 
> > always be PHASE_DATAIN...
> >   
> 
> Might be a similar limitation to certain transfer sizes here. Block mode 
> vs. char mode.

That suggests that the test for REQ_TYPE_FS in 
NCR5380_information_transfer() in sun3_NCR5380 could be improved upon by 
using NCR5380_dma_xfer_len() when checking the size limit. That way it 
would work on atari as well, without #ifdefs.

-- 

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Finn,
>>> However, atari never uses DMA here if cmd->device->borken.
>>>   
>>>       
>> Different semantics of borken, I presume. On Atari, this means a DMA 
>> transfer has failed earlier.
>>     
>
> Right. I suppose the question of using PIO in this situation on sun3 would 
> have to be put to the test.
>   

Yep - let's see whether Sam has a working test configuration with 
current kernels.

>   
>>> When cmd->device->borken, I assume sun3_NCR5380 inhibits PIO because 
>>> PIO was itself expected to be problematic?
>>>       

The sun3 driver does set the borken flag but never uses it later - can't 
see where it will inhibit PIO.

>>> OTOH, if PIO works up to 128 bytes in all cases, why not larger 
>>> transfers?
>>>   
>>>       
>> Performance, perhaps.
>>     
>
> Well, sun3_scsi has this:
>
> static inline unsigned long sun3scsi_dma_xfer_len(unsigned long wanted,
>                                                   struct scsi_cmnd *cmd,
>                                                   int write_flag)
> {
>         if (cmd->request->cmd_type == REQ_TYPE_FS)
>                 return wanted;
>         else
>                 return 0;
> }
>
> This does suggest that PIO does work beyond 128 bytes, even if it is 
> avoided for filesystem requests. Which suggests that the atari reselect 
> code might work on sun3.
>
> (As for performance, I don't know why the REQ_TYPE_FS is significant: why 
> not just raise the DMA size limit to 512 instead?)
>   

No idea really - the sun3 DMA allows to read the correct residual so it 
probably is not necessary to strictly avoid DMA on anything not 
guaranteed to transfer the requested size.  Not sure what commands will 
transfer between 128 and 512 bytes ...

>   
>> PIO would work, but hog the CPU. In fact, PIO for everything _does_ 
>> work, on Falcon as well as Mac (I tried the PIO only mode on the Falcon 
>> before trying on Mac). Did I eve mention how reallly incredibly 
>> painfully slow that is?
>>
>>     
>>> Does the sun3_scsi driver still work after removing #define REAL_DMA?
>>>
>>> For NCR5380_information_transfer() and PHASE_CMDOUT, the sun3 version 
>>> applies the same size limit but only does DMA setup if cmd type is 
>>> REQ_TYPE_FS (i.e. filesystem request). This command is then sent by 
>>> PIO, so the DMA setup here seems to assume that the next phase will 
>>> always be PHASE_DATAIN...
>>>   
>>>       
>> Might be a similar limitation to certain transfer sizes here. Block mode 
>> vs. char mode.
>>     

Commands need to be sent by PIO always because of the DMA size limit. 
The DMA setup early on may save time if the next phase is indeed DATAIN 
or DATAOUT - if a command disconnects the DMA just won't be started. I 
would have thought the DMA setup could just as easily be done inside 
NCR5380_transfer_dma instead of splitting it in DMA setup and DMA start 
- any particular reason for this, Sammy?

> That suggests that the test for REQ_TYPE_FS in 
> NCR5380_information_transfer() in sun3_NCR5380 could be improved upon by 
> using NCR5380_dma_xfer_len() when checking the size limit. That way it 
> would work on atari as well, without #ifdefs.
>   

The test for REQ_TYPE_FS could be done in the specific dma_xfer_len() 
function which would need to be used whenever DMA setup is going to be 
used. As it is, the separation of DMA setup from NCR5380_transfer_dma is 
a bit mysterious and makes it hard to merge the drivers without lots of 
ifdefs or runtime tests (think multiplatform kernels) anyway.

Cheers,

    Michael

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Thu, 14 Aug 2014, Michael Schmitz wrote:

> >   
> > > > When cmd->device->borken, I assume sun3_NCR5380 inhibits PIO 
> > > > because PIO was itself expected to be problematic?
> > > >       
> 
> The sun3 driver does set the borken flag but never uses it later - can't 
> see where it will inhibit PIO.

It inhibited PIO when it forked the atari code and commented out the test 
for cmd->device->borken, to ensure that PIO would never be used here.

Hence my suggestion that PIO itself was expected was problematic. OTOH, 
PIO is used up to 128 bytes, and beyond 128 bytes if !REQ_TYPE_FS...

-- 

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Finn,


On Fri, Aug 15, 2014 at 1:46 PM, Finn Thain <fthain@telegraphics.com.au> wrote:
>
> On Thu, 14 Aug 2014, Michael Schmitz wrote:
>
>> >
>> > > > When cmd->device->borken, I assume sun3_NCR5380 inhibits PIO
>> > > > because PIO was itself expected to be problematic?
>> > > >
>>
>> The sun3 driver does set the borken flag but never uses it later - can't
>> see where it will inhibit PIO.
>
> It inhibited PIO when it forked the atari code and commented out the test
> for cmd->device->borken, to ensure that PIO would never be used here.

I see. I just tool that to mean the Sun3 driver never had trouble with
devices failing at that section, so no need to switch from DMA to PIO
(because DMA never fails).

> Hence my suggestion that PIO itself was expected was problematic. OTOH,
> PIO is used up to 128 bytes, and beyond 128 bytes if !REQ_TYPE_FS...

Up to 128 for DMA limitations - the other bit I don't understand either.

Cheers,

  Michael

> --

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Finn Thain]

On Thu, 14 Aug 2014, Michael Schmitz wrote:

> 
> The DMA setup early on may save time if the next phase is indeed DATAIN 
> or DATAOUT - if a command disconnects the DMA just won't be started. I 
> would have thought the DMA setup could just as easily be done inside 
> NCR5380_transfer_dma instead of splitting it in DMA setup and DMA start
> - any particular reason for this, Sammy?

Doing the DMA setup early on may not work for atari_NCR5380 given 
merge_contiguous_buffers() takes place prior to NCR5380_transfer_dma() and 
may alter the length of the tranfer. (sun3_NCR5380 doesn't have 
merge_contiguous_buffers()).

-- 

Re: Sun3 SCSI DMA, was Re: converting the NCR5380 drivers away from scsi_register

[Michael Schmitz]

Hi Finn,


On Fri, Aug 15, 2014 at 2:09 PM, Finn Thain <fthain@telegraphics.com.au> wrote:
>
> On Thu, 14 Aug 2014, Michael Schmitz wrote:
>
>>
>> The DMA setup early on may save time if the next phase is indeed DATAIN
>> or DATAOUT - if a command disconnects the DMA just won't be started. I
>> would have thought the DMA setup could just as easily be done inside
>> NCR5380_transfer_dma instead of splitting it in DMA setup and DMA start
>> - any particular reason for this, Sammy?
>
> Doing the DMA setup early on may not work for atari_NCR5380 given
> merge_contiguous_buffers() takes place prior to NCR5380_transfer_dma() and
> may alter the length of the tranfer. (sun3_NCR5380 doesn't have
> merge_contiguous_buffers()).

Not only that - the DMA setup (on Falcon) will be trashed by
subsequent accesses to the NCR5380 (registers are accessed through the
ST-DMA, the NCR5380 is not directly connected to the CPU bus).

We'd need to do all DMA setup in NCR5380_transfer_dma instead.

Cheers,

  Michael


>
> --