All of lore.kernel.org
 help / color / mirror / Atom feed
From: Finn Thain <fthain@telegraphics.com.au>
To: Michael Schmitz <schmitzmic@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Tejun Heo <tj@kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-ide@vger.kernel.org,
	Linux/m68k <linux-m68k@lists.linux-m68k.org>,
	Linux Kernel Development <linux-kernel@vger.kernel.org>,
	Andreas Schwab <schwab@linux-m68k.org>
Subject: Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
Date: Sat, 21 Jan 2017 18:37:12 +1100 (AEDT)	[thread overview]
Message-ID: <alpine.LNX.2.00.1701211805050.18276@nippy.intranet> (raw)
In-Reply-To: <b1b409e0-9243-83a6-8379-50f933c97477@gmail.com>


On Fri, 20 Jan 2017, Michael Schmitz wrote:

> Am 15.01.2017 um 17:42 schrieb Finn Thain:
> 
> >> No, we can't check either FDC or SCSI interrupts (or indeed any chip 
> >> registers) without touching the ST-DMA. The moment we select a FDC or 
> >> SCSI register for read, DMA is terminated no questions asked.
> >>
> > 
> > Perhaps we can convert DMA operations to PDMA (by polling with local 
> > irqs disabled) and avoid the whole problem of interrupt handlers 
> > executing during DMA transfers. The docs suggest that it is doable.
> > 
> > "Poll or service the Disk Driver Controller interrupt on the MK68901 
> > MFP General Purpose I/O Register to detect the completion of a WD1772 
> > FDC command. Do not poll the FDC Busy or DMA Sector Count Zero status 
> > bits." -- ST HW Spec, p. 36. 
> > http://dev-docs.atariforge.org/files/ST_HW_Spec_1-7-1986.pdf
> 
> The MFP interrupt in question is the same as the one used by IDE 
> (wired-OR of IDE, FDC and SCSI), so we would still have to figure out 
> where the interrupt originated.

I thought you said the driver you're testing does not use any interrupt -- 
I was assuming that only atari_scsi and ataflop drivers share the 
interupt.

> Polling instead of taking the interrupt does not change that fundamental 
> problem (unless I'm missing something).
> 

Actually, the fundamental problem you are describing is partly solved. By 
polling for DMA completion with local irqs disabled, we mostly avoid the 
need for the stdma.c "lock" because FDC/SCSI/IDE interrupt handlers can 
never interfere with a FDC/SCSI DMA process that might be underway.

> > 
> > On page 18 there is an algorithm for floppy writes which is 
> > interesting.
> 
> That one (and the ACSI algorithm which would apply to SCSI for Falcon) 
> does suggest it won't be possible to peek at the sector count register 
> to detect end of DMA. The addendum (note 841017G) makes it clear that a 
> write to the DMA mode register is required to look at the status 
> register error bit (which might terminate DMA).
> 
> Maybe the DMA address register can be used to check for DMA completion 
> ... it's used to check for residual or lost bytes anyway so that appears 
> to work. And the FDC driver does use the same strategy to check if 
> enough track data have been read.
> 
> Leaves the case where DMA hasn't completed but may have been aborted by 
> a NCR5380 interrupt. I suppose we can detect that by checking for any 
> change in the DMA address while repeatedly reading the DMA address 
> register. No change means the DMA has got stuck. Not exactly pretty but 
> all I can come up with.
> 

We don't have to poll any DMA registers (and I don't believe that it is 
viable to do so). I was talking about polling for end of DMA by polling 
for the interrupt (as per docs) but with local irqs disabled for the 
duration of the transfer (which provides exlusive access to the DMA chip).

> > 
> > I suspect that we will need to keep the FDC idle during SCSI transfers 
> > (and vice versa) much as the present stdma.c lock does.
> > 
> > "The interrupt outputs of the internal floppy disk controller and the 
> > external ACSI DMA port are logically OR'ed. The pin of the MFP GPIP 
> > will read as a '0' if either the FDC or a selected ACSI device 
> > controller is asserting its interrupt request." -- ACSI/DMA 
> > Integration Guide, p.16. 
> > http://dev-docs.atariforge.org/files/ACSI_DMA_Guide_6-28-1991.pdf
> 
> On Falcon, the IDE interrupt is also OR'ed to the above two interrupt 
> lines, hence the need for including IDE in the locking scheme there.
> 

I don't think the IDE/ATA driver needs to be included. atari_scsi and 
ataflop would though (if both drivers need DMA transfers).

> > 
> > Polling the logically OR'ed interrupt sources to detect end-of-DMA 
> > will not be reliable unless we disable those sources that aren't 
> > relevant. Otherwise we access the DMA registers too early (which IIUC 
> > would kill the transfer). I'm afraid we shall have to expect that a 
> > few transfers will be interrupted by other devices in this way, and 
> > carefully check for this.
> > 
> > For example, the 5380 SCSI bus reset interrupt is not maskable, which 
> > could affect FDC transfers. If this terminated the polling for DMA 
> > completion, the FDC driver then has to access the FDC registers and 
> > confirm that the transfer was not terminated early.
> > 
> 
> We'll have to make sure FDC and SCSI don't clash in their DMA and 
> interrupt use.
> 

The point I was trying to make above is that stdma lock only gets you so 
far: if SCSI or FDC generate an interrupt that can't be disabled, it could 
mess up the interrupt polling (and the interrupt polling is a necessary 
consequence of IDE operating without stdma lock). This would lead to a 
short transfer (which could be easily detected).

So the chips clash in their interrupt line use (rarely). The drivers need 
not clash at all.

Anyway, we seem to be talking past each other somewhat. I suggest we start 
coding and discuss actual patches ... unless you can convince me that this 
won't work ...

-- 

> Cheers,
> 
> 	Michael
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

WARNING: multiple messages have this Message-ID (diff)
From: Finn Thain <fthain@telegraphics.com.au>
To: Michael Schmitz <schmitzmic@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Tejun Heo <tj@kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-ide@vger.kernel.org,
	Linux/m68k <linux-m68k@vger.kernel.org>,
	Linux Kernel Development <linux-kernel@vger.kernel.org>,
	Andreas Schwab <schwab@linux-m68k.org>
Subject: Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
Date: Sat, 21 Jan 2017 18:37:12 +1100 (AEDT)	[thread overview]
Message-ID: <alpine.LNX.2.00.1701211805050.18276@nippy.intranet> (raw)
In-Reply-To: <b1b409e0-9243-83a6-8379-50f933c97477@gmail.com>


On Fri, 20 Jan 2017, Michael Schmitz wrote:

> Am 15.01.2017 um 17:42 schrieb Finn Thain:
> 
> >> No, we can't check either FDC or SCSI interrupts (or indeed any chip 
> >> registers) without touching the ST-DMA. The moment we select a FDC or 
> >> SCSI register for read, DMA is terminated no questions asked.
> >>
> > 
> > Perhaps we can convert DMA operations to PDMA (by polling with local 
> > irqs disabled) and avoid the whole problem of interrupt handlers 
> > executing during DMA transfers. The docs suggest that it is doable.
> > 
> > "Poll or service the Disk Driver Controller interrupt on the MK68901 
> > MFP General Purpose I/O Register to detect the completion of a WD1772 
> > FDC command. Do not poll the FDC Busy or DMA Sector Count Zero status 
> > bits." -- ST HW Spec, p. 36. 
> > http://dev-docs.atariforge.org/files/ST_HW_Spec_1-7-1986.pdf
> 
> The MFP interrupt in question is the same as the one used by IDE 
> (wired-OR of IDE, FDC and SCSI), so we would still have to figure out 
> where the interrupt originated.

I thought you said the driver you're testing does not use any interrupt -- 
I was assuming that only atari_scsi and ataflop drivers share the 
interupt.

> Polling instead of taking the interrupt does not change that fundamental 
> problem (unless I'm missing something).
> 

Actually, the fundamental problem you are describing is partly solved. By 
polling for DMA completion with local irqs disabled, we mostly avoid the 
need for the stdma.c "lock" because FDC/SCSI/IDE interrupt handlers can 
never interfere with a FDC/SCSI DMA process that might be underway.

> > 
> > On page 18 there is an algorithm for floppy writes which is 
> > interesting.
> 
> That one (and the ACSI algorithm which would apply to SCSI for Falcon) 
> does suggest it won't be possible to peek at the sector count register 
> to detect end of DMA. The addendum (note 841017G) makes it clear that a 
> write to the DMA mode register is required to look at the status 
> register error bit (which might terminate DMA).
> 
> Maybe the DMA address register can be used to check for DMA completion 
> ... it's used to check for residual or lost bytes anyway so that appears 
> to work. And the FDC driver does use the same strategy to check if 
> enough track data have been read.
> 
> Leaves the case where DMA hasn't completed but may have been aborted by 
> a NCR5380 interrupt. I suppose we can detect that by checking for any 
> change in the DMA address while repeatedly reading the DMA address 
> register. No change means the DMA has got stuck. Not exactly pretty but 
> all I can come up with.
> 

We don't have to poll any DMA registers (and I don't believe that it is 
viable to do so). I was talking about polling for end of DMA by polling 
for the interrupt (as per docs) but with local irqs disabled for the 
duration of the transfer (which provides exlusive access to the DMA chip).

> > 
> > I suspect that we will need to keep the FDC idle during SCSI transfers 
> > (and vice versa) much as the present stdma.c lock does.
> > 
> > "The interrupt outputs of the internal floppy disk controller and the 
> > external ACSI DMA port are logically OR'ed. The pin of the MFP GPIP 
> > will read as a '0' if either the FDC or a selected ACSI device 
> > controller is asserting its interrupt request." -- ACSI/DMA 
> > Integration Guide, p.16. 
> > http://dev-docs.atariforge.org/files/ACSI_DMA_Guide_6-28-1991.pdf
> 
> On Falcon, the IDE interrupt is also OR'ed to the above two interrupt 
> lines, hence the need for including IDE in the locking scheme there.
> 

I don't think the IDE/ATA driver needs to be included. atari_scsi and 
ataflop would though (if both drivers need DMA transfers).

> > 
> > Polling the logically OR'ed interrupt sources to detect end-of-DMA 
> > will not be reliable unless we disable those sources that aren't 
> > relevant. Otherwise we access the DMA registers too early (which IIUC 
> > would kill the transfer). I'm afraid we shall have to expect that a 
> > few transfers will be interrupted by other devices in this way, and 
> > carefully check for this.
> > 
> > For example, the 5380 SCSI bus reset interrupt is not maskable, which 
> > could affect FDC transfers. If this terminated the polling for DMA 
> > completion, the FDC driver then has to access the FDC registers and 
> > confirm that the transfer was not terminated early.
> > 
> 
> We'll have to make sure FDC and SCSI don't clash in their DMA and 
> interrupt use.
> 

The point I was trying to make above is that stdma lock only gets you so 
far: if SCSI or FDC generate an interrupt that can't be disabled, it could 
mess up the interrupt polling (and the interrupt polling is a necessary 
consequence of IDE operating without stdma lock). This would lead to a 
short transfer (which could be easily detected).

So the chips clash in their interrupt line use (rarely). The drivers need 
not clash at all.

Anyway, we seem to be talking past each other somewhat. I suggest we start 
coding and discuss actual patches ... unless you can convince me that this 
won't work ...

-- 

> Cheers,
> 
> 	Michael
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

  parent reply	other threads:[~2017-01-21  7:36 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20161230140139epcas5p160eda5a6a77be084e21f12002c85cc2a@epcas5p1.samsung.com>
2016-12-30 14:01 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Bartlomiej Zolnierkiewicz
2016-12-30 14:01   ` Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20161230140141epcas5p161d9467e10e294f502863b5347e351d5@epcas5p1.samsung.com>
2016-12-30 14:01     ` [PATCH 1/3] ata: allow subsystem to be used on m68k arch Bartlomiej Zolnierkiewicz
2016-12-30 14:01     ` Bartlomiej Zolnierkiewicz
2016-12-30 14:01       ` Bartlomiej Zolnierkiewicz
2016-12-30 14:12       ` Christoph Hellwig
2016-12-30 14:12       ` Christoph Hellwig
2016-12-30 14:12         ` Christoph Hellwig
     [not found]         ` <CGME20161230171438epcas1p3c1d8ea8e4c77d796b81f7130c5e61d3f@epcas1p3.samsung.com>
2016-12-30 17:14           ` Bartlomiej Zolnierkiewicz
2016-12-30 17:14             ` Bartlomiej Zolnierkiewicz
2017-01-08 10:08             ` Christoph Hellwig
2017-01-08 10:08               ` Christoph Hellwig
     [not found]               ` <CGME20170109160128epcas1p36a0a8f79b32e5024ffa480fd848e3a79@epcas1p3.samsung.com>
2017-01-09 16:01                 ` Bartlomiej Zolnierkiewicz
2017-01-09 16:01                   ` Bartlomiej Zolnierkiewicz
2017-01-09 16:01                 ` Bartlomiej Zolnierkiewicz
2017-01-08 10:08             ` Christoph Hellwig
2016-12-30 17:14           ` Bartlomiej Zolnierkiewicz
2017-01-09 16:15       ` Geert Uytterhoeven
2017-01-09 16:15       ` Geert Uytterhoeven
2017-01-09 16:15         ` Geert Uytterhoeven
     [not found]   ` <CGME20161230140144epcas1p2ada13244f4ba5b45ed903ab7d614f6db@epcas1p2.samsung.com>
2016-12-30 14:01     ` [PATCH 2/3] ata: pass queued command to ->sff_data_xfer method Bartlomiej Zolnierkiewicz
2016-12-30 14:01       ` Bartlomiej Zolnierkiewicz
2016-12-30 14:01     ` Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20161230140147epcas5p1fa7e99f39921a8ee90aabd59ff7b7645@epcas5p1.samsung.com>
2016-12-30 14:01     ` [PATCH 3/3] ata: add Atari Falcon PATA controller driver Bartlomiej Zolnierkiewicz
2016-12-30 14:01       ` Bartlomiej Zolnierkiewicz
2017-01-03 10:49   ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Geert Uytterhoeven
2017-01-03 10:49     ` Geert Uytterhoeven
2017-01-09 16:11     ` Bartlomiej Zolnierkiewicz
2017-01-09 16:11     ` Bartlomiej Zolnierkiewicz
2017-01-09 16:11       ` Bartlomiej Zolnierkiewicz
2017-01-10 16:09       ` Tejun Heo
2017-01-10 16:09         ` Tejun Heo
2017-01-03 10:49   ` Geert Uytterhoeven
2017-01-05 21:01   ` Michael Schmitz
2017-01-05 21:01     ` Michael Schmitz
2017-01-10 12:53     ` Bartlomiej Zolnierkiewicz
2017-01-10 12:53       ` Bartlomiej Zolnierkiewicz
2017-01-10 20:02       ` Michael Schmitz
2017-01-10 20:02         ` Michael Schmitz
2017-01-13  2:33         ` Finn Thain
2017-01-13  2:33           ` Finn Thain
2017-01-14  8:55           ` Michael Schmitz
2017-01-14  8:55           ` Michael Schmitz
2017-01-14  8:55             ` Michael Schmitz
2017-01-14 23:47             ` Finn Thain
2017-01-14 23:47             ` Finn Thain
2017-01-14 23:47               ` Finn Thain
2017-01-15  1:48               ` Michael Schmitz
2017-01-15  1:48               ` Michael Schmitz
2017-01-15  1:48                 ` Michael Schmitz
2017-01-15  4:42                 ` Finn Thain
2017-01-15  4:42                   ` Finn Thain
2017-01-20  7:49                   ` Michael Schmitz
2017-01-20  7:49                   ` Michael Schmitz
2017-01-20  7:49                     ` Michael Schmitz
2017-01-21  7:37                     ` Finn Thain
2017-01-21  7:37                     ` Finn Thain [this message]
2017-01-21  7:37                       ` Finn Thain
2017-01-23  8:04                       ` Michael Schmitz
2017-01-23  8:04                         ` Michael Schmitz
2017-01-26  8:47                         ` Finn Thain
2017-01-26  8:47                         ` Finn Thain
2017-01-26  8:47                           ` Finn Thain
2017-01-26  9:03                           ` Geert Uytterhoeven
2017-01-26  9:03                             ` Geert Uytterhoeven
2017-01-27  1:41                             ` Finn Thain
2017-01-27  1:41                               ` Finn Thain
2017-01-27  1:41                             ` Finn Thain
2017-01-27  4:28                           ` Michael Schmitz
2017-01-27  4:28                           ` Michael Schmitz
2017-01-27  4:28                             ` Michael Schmitz
2017-02-01  8:40                             ` Finn Thain
2017-02-01  8:40                             ` Finn Thain
2017-02-01  8:40                               ` Finn Thain
2017-02-01  8:45                               ` Geert Uytterhoeven
2017-02-01  8:45                                 ` Geert Uytterhoeven
2017-02-02  7:48                               ` Michael Schmitz
2017-02-02  7:48                                 ` Michael Schmitz
2017-02-02  7:48                               ` Michael Schmitz
2017-01-23  8:04                       ` Michael Schmitz
2017-01-15  4:42                 ` Finn Thain
2017-01-10 16:11   ` Tejun Heo
2017-01-10 16:11   ` Tejun Heo
2017-01-10 16:11     ` Tejun Heo
2017-02-15  8:45   ` Geert Uytterhoeven
2017-02-15  8:45   ` Geert Uytterhoeven
2017-02-15  8:45     ` Geert Uytterhoeven
2017-02-20 18:15     ` Bartlomiej Zolnierkiewicz
2017-02-20 18:15     ` Bartlomiej Zolnierkiewicz
2017-02-20 18:15       ` Bartlomiej Zolnierkiewicz
2017-02-21 22:18       ` Tejun Heo
2017-02-21 22:18         ` Tejun Heo

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=alpine.LNX.2.00.1701211805050.18276@nippy.intranet \
    --to=fthain@telegraphics.com.au \
    --cc=b.zolnierkie@samsung.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=schmitzmic@gmail.com \
    --cc=schwab@linux-m68k.org \
    --cc=tj@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.