Port-mac68k archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: DMA support for SCSI adapter of AV Mac
Thank you very much for your kind comments, and sorry for not being to
reply immediately.
I updated the patch in according to your comments. Also, your codes in
arch/arc/jazz/asc.c and dev/pci/pcscp.c are very helpful for me. Let me
thank you again for that :-).
I already committed the patch. I think that it is almost fine, and
remaining problems can be addressed in tree.
I will reply to your each comment below:
On 2018/10/16 22:43, Izumi Tsutsui wrote:
rin@ wrote:
I updated the patch for DMA support for SCSI adapter of AV Mac
(Quadra / Centris 660AV / 840AV), written by Michael Zucca in
PR port-mac68k/24883.
http://www.netbsd.org/~rin/avdma_20181002.patch
:
http://gnats.netbsd.org/24883
I'd like to note some dumb comments:
--- sys/arch/mac68k/include/psc.h 11 Dec 2005 12:18:03 -0000 1.7
+++ sys/arch/mac68k/include/psc.h 2 Oct 2018 06:38:51 -0000
@@ -25,6 +25,8 @@
*
*/
+#include <machine/bus.h>
+
Nowadays <sys/bus.h> is better, but I wonder if it should be included
in C source. (see below)
I replaced it with <sys/bus.h>. This is necessary since there is a
function one of whose argument is bus_addr_t.
--- sys/arch/mac68k/obio/esp.c 18 Feb 2012 23:51:27 -0000 1.55
+++ sys/arch/mac68k/obio/esp.c 2 Oct 2018 06:45:18 -0000
@@ -90,6 +94,7 @@ __KERNEL_RCSID(0, "$NetBSD: esp.c,v 1.55
#include <sys/proc.h>
#include <sys/queue.h>
#include <sys/mutex.h>
+#include <sys/malloc.h>
Is this necessary? (no malloc(9) but only bus_dmamem_alloc(9))
No, I removed it.
@@ -103,6 +108,10 @@ __KERNEL_RCSID(0, "$NetBSD: esp.c,v 1.55
#include <dev/ic/ncr53c9xreg.h>
#include <dev/ic/ncr53c9xvar.h>
+#include <uvm/uvm_extern.h>
+
+#include <machine/bus.h>
+#include <machine/psc.h>
<sys/bus.h> and <machine/psc.h> as noted above?
Fixed.
@@ -134,6 +143,16 @@ int esp_quick_dma_setup(struct ncr53c9x_
size_t *);
void esp_quick_dma_go(struct ncr53c9x_softc *);
+void esp_av_dma_reset(struct ncr53c9x_softc *);
+void esp_av_dma_stop(struct ncr53c9x_softc *);
+void esp_av_write_reg(struct ncr53c9x_softc *, int, u_char);
I prefer uint8_t rather than u_char.
u_char and u_int*_t are replaced by uint*_t.
@@ -321,6 +364,53 @@ espattach(device_t parent, device_t self
:
+ /*
+ * Allocate memory which is friendly to the DMA
+ * engine (16 byte aligned) for use as the
+ * SCSI message buffers.
+ */
+ if (bus_dmamem_alloc(esc->sc_dmat, (bus_size_t)NBPG, 16,
+ (bus_size_t)NBPG, &segs, 1, &rsegs, BUS_DMA_NOWAIT)) {
+ printf("failed to allocate omess buffer!\n") ;
+ }
+
+ if (bus_dmamem_map(esc->sc_dmat, &segs, rsegs,
+ (size_t)NCR_MAX_MSG_LEN, (void **)&sc->sc_omess,
+ BUS_DMA_NOWAIT | BUS_DMA_COHERENT)) {
+ printf("failed to map omess buffer!\n") ;
+ }
+
+ if (bus_dmamem_alloc(esc->sc_dmat, (bus_size_t)NBPG, 16,
+ (bus_size_t)NBPG, &segs, 1, &rsegs, BUS_DMA_NOWAIT)) {
+ printf("failed to allocate imess buffer!\n") ;
+ }
+
+ if (bus_dmamem_map(esc->sc_dmat, &segs, rsegs,
+ (size_t)(NCR_MAX_MSG_LEN + 1), (void **)&sc->sc_imess,
+ BUS_DMA_NOWAIT | BUS_DMA_COHERENT)) {
+ printf("failed to map imess buffer") ;
+ }
+ }
Maybe one NBPG (4KB) buffer is enough for two 16 bytes aligned buffers?
Two buffers are necessary due to m68k-specific behavior of
bus_dmamem_map(9); it rounds size up to NBPG:
https://nxr.netbsd.org/xref/src/sys/arch/m68k/m68k/bus_dma.c#735
I added a comment on it.
@@ -900,3 +990,345 @@ esp_dualbus_intr(void *sc)
ncr53c9x_intr((struct ncr53c9x_softc *)esp1);
}
}
+
+void
+esp_av_dma_stop(struct ncr53c9x_softc *sc)
+{
+ struct esp_softc *esc = (struct esp_softc *)sc;
+ u_int32_t res;
uint32_t is better.
I also wonder if this esp dma_stop glue function should also handle
bus_dmamap_unload() (as pcscp.c does, but I forget details).
bus_dmamap_unload(9) is called from dma_stop function also in
arch/arc/jazz/asp.c. I added it here, and observe no new problems.
(Without it, resource leaks occur in some error paths?)
+esp_av_dma_reset(struct ncr53c9x_softc *sc)
+{
+ struct esp_softc *esc = (struct esp_softc *)sc;
+ u_int32_t res;
Also should be uint32_t.
+void
+esp_av_write_reg(struct ncr53c9x_softc *sc, int reg, u_char val)
+{
+ struct esp_softc *esc = (struct esp_softc *)sc;
+ u_char v;
also uint8_t
+int
+esp_av_pio_intr(struct ncr53c9x_softc *sc)
+{
:
+ if (esc->sc_dmasize == 0)
+ printf("data interrupt, but no count left.");
IIRC some document said this (zero sized DMA) could happen
in the "transfer pad" operation on slow devices. (see pcscp.c)
I handled it. However, this function is used only for transfer data
not fit into 16-byte boundaries, which AV DMA cannot handle. I wonder
whether sc_dmasize == 0 is possible; see esp_av_dma_setup().
+int
+esp_av_dma_intr(struct ncr53c9x_softc *sc)
+{
+ struct esp_softc *esc = (struct esp_softc *)sc;
+ u_int32_t dma_resid;
also uint32_t
+#if DEBUG
+ printf("[av_dma_intr: Servicing Transfer Pad interrupt]\n") ;
+#endif
+ /* A "Transfer Pad" operation completed */
+ return 0;
+ }
See above.
Handled.
+ /* Halt the DMA engine */
+ stop_psc_dma(PSC_DMA_CHANNEL_SCSI, esc->sc_rset, &dma_resid,
+ esc->sc_datain);
+
+ esc->sc_active = 0;
+
+ bus_dmamap_unload(esc->sc_dmat, esc->sc_dmap);
Strictly speaking, bus_dmamap_sync(9) for POSTREAD or POSTWRITE op
(per datain) is necessary before bus_dmamap_unload(9).
I added it.
Thank you again for your helpful comments!
rin
---
Izumi Tsutsui
Home |
Main Index |
Thread Index |
Old Index