* [PATCH 0/3] ata: add m68k/Atari Falcon PATA support [not found] <CGME20161230140139epcas5p160eda5a6a77be084e21f12002c85cc2a@epcas5p1.samsung.com> @ 2016-12-30 14:01 ` Bartlomiej Zolnierkiewicz [not found] ` <CGME20161230140141epcas5p161d9467e10e294f502863b5347e351d5@epcas5p1.samsung.com> ` (6 more replies) 0 siblings, 7 replies; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2016-12-30 14:01 UTC (permalink / raw) To: Tejun Heo Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel, b.zolnierkie Hi, This patchset adds m68k/Atari Falcon PATA support to libata. The major difference in the new libata's pata_falcon host driver when compared to legacy IDE's falconide host driver is that we are using polled PIO mode and thus avoiding the need for STDMA locking magic altogether. Tested under ARAnyM emulator. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics Bartlomiej Zolnierkiewicz (3): ata: allow subsystem to be used on m68k arch ata: pass queued command to ->sff_data_xfer method ata: add Atari Falcon PATA controller driver drivers/ata/Kconfig | 11 ++- drivers/ata/Makefile | 1 + drivers/ata/libata-sff.c | 29 +++---- drivers/ata/pata_at91.c | 6 +- drivers/ata/pata_bf54x.c | 7 +- drivers/ata/pata_ep93xx.c | 4 +- drivers/ata/pata_falcon.c | 184 ++++++++++++++++++++++++++++++++++++++++++ drivers/ata/pata_ixp4xx_cf.c | 4 +- drivers/ata/pata_legacy.c | 15 ++-- drivers/ata/pata_octeon_cf.c | 12 +-- drivers/ata/pata_pcmcia.c | 6 +- drivers/ata/pata_samsung_cf.c | 4 +- drivers/ata/sata_rcar.c | 4 +- include/linux/libata.h | 8 +- 14 files changed, 247 insertions(+), 48 deletions(-) create mode 100644 drivers/ata/pata_falcon.c -- 1.9.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20161230140141epcas5p161d9467e10e294f502863b5347e351d5@epcas5p1.samsung.com>]
* [PATCH 1/3] ata: allow subsystem to be used on m68k arch [not found] ` <CGME20161230140141epcas5p161d9467e10e294f502863b5347e351d5@epcas5p1.samsung.com> @ 2016-12-30 14:01 ` Bartlomiej Zolnierkiewicz 2016-12-30 14:12 ` Christoph Hellwig 2017-01-09 16:15 ` Geert Uytterhoeven 0 siblings, 2 replies; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2016-12-30 14:01 UTC (permalink / raw) To: Tejun Heo Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel, b.zolnierkie When libata was merged m68k lacked IOMAP support. This has not been true for a long time now so allow subsystem to be used on m68k. Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- drivers/ata/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index 2c8be74..da19fc9 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -14,7 +14,7 @@ menuconfig ATA tristate "Serial ATA and Parallel ATA drivers (libata)" depends on HAS_IOMEM depends on BLOCK - depends on !(M32R || M68K || S390) || BROKEN + depends on !(M32R || S390) || BROKEN select SCSI select GLOB ---help--- -- 1.9.1 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch 2016-12-30 14:01 ` [PATCH 1/3] ata: allow subsystem to be used on m68k arch Bartlomiej Zolnierkiewicz @ 2016-12-30 14:12 ` Christoph Hellwig [not found] ` <CGME20161230171438epcas1p3c1d8ea8e4c77d796b81f7130c5e61d3f@epcas1p3.samsung.com> 2017-01-09 16:15 ` Geert Uytterhoeven 1 sibling, 1 reply; 34+ messages in thread From: Christoph Hellwig @ 2016-12-30 14:12 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Tejun Heo, Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel On Fri, Dec 30, 2016 at 03:01:16PM +0100, Bartlomiej Zolnierkiewicz wrote: > When libata was merged m68k lacked IOMAP support. This has not been > true for a long time now so allow subsystem to be used on m68k. Is that dependency needed at all anymore? What exact functionality is missing on m32r and s390? ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20161230171438epcas1p3c1d8ea8e4c77d796b81f7130c5e61d3f@epcas1p3.samsung.com>]
* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch [not found] ` <CGME20161230171438epcas1p3c1d8ea8e4c77d796b81f7130c5e61d3f@epcas1p3.samsung.com> @ 2016-12-30 17:14 ` Bartlomiej Zolnierkiewicz 2017-01-08 10:08 ` Christoph Hellwig 0 siblings, 1 reply; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2016-12-30 17:14 UTC (permalink / raw) To: Christoph Hellwig Cc: Tejun Heo, Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel Hi, On Friday, December 30, 2016 06:12:49 AM Christoph Hellwig wrote: > On Fri, Dec 30, 2016 at 03:01:16PM +0100, Bartlomiej Zolnierkiewicz wrote: > > When libata was merged m68k lacked IOMAP support. This has not been > > true for a long time now so allow subsystem to be used on m68k. > > Is that dependency needed at all anymore? What exact functionality > is missing on m32r and s390? * m32r: I don't know why it was restricted but it builds fine nowadays (I will fix it later in incremental patch) * s390: no PATA hardware, legacy IDE subsystem is also not supported Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch 2016-12-30 17:14 ` Bartlomiej Zolnierkiewicz @ 2017-01-08 10:08 ` Christoph Hellwig [not found] ` <CGME20170109160128epcas1p36a0a8f79b32e5024ffa480fd848e3a79@epcas1p3.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Christoph Hellwig @ 2017-01-08 10:08 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Christoph Hellwig, Tejun Heo, Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel On Fri, Dec 30, 2016 at 06:14:36PM +0100, Bartlomiej Zolnierkiewicz wrote: > * m32r: I don't know why it was restricted but it builds fine nowadays > (I will fix it later in incremental patch) > > * s390: no PATA hardware, legacy IDE subsystem is also not supported s390 now has PCIe support, so PATA could be attached through a PCIe to PCI bridge in theory. And while that is very theoretical not having arch ifdefs is still something we should strive for, so I'd suggest removing the condition entirely. ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20170109160128epcas1p36a0a8f79b32e5024ffa480fd848e3a79@epcas1p3.samsung.com>]
* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch [not found] ` <CGME20170109160128epcas1p36a0a8f79b32e5024ffa480fd848e3a79@epcas1p3.samsung.com> @ 2017-01-09 16:01 ` Bartlomiej Zolnierkiewicz 0 siblings, 0 replies; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2017-01-09 16:01 UTC (permalink / raw) To: Christoph Hellwig Cc: Tejun Heo, Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel Hi, On Sunday, January 08, 2017 02:08:23 AM Christoph Hellwig wrote: > On Fri, Dec 30, 2016 at 06:14:36PM +0100, Bartlomiej Zolnierkiewicz wrote: > > * m32r: I don't know why it was restricted but it builds fine nowadays > > (I will fix it later in incremental patch) > > > > * s390: no PATA hardware, legacy IDE subsystem is also not supported > > s390 now has PCIe support, so PATA could be attached through a PCIe to > PCI bridge in theory. And while that is very theoretical not having > arch ifdefs is still something we should strive for, so I'd suggest > removing the condition entirely. I agree. I will do it later unless someone beats me to it. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch 2016-12-30 14:01 ` [PATCH 1/3] ata: allow subsystem to be used on m68k arch Bartlomiej Zolnierkiewicz 2016-12-30 14:12 ` Christoph Hellwig @ 2017-01-09 16:15 ` Geert Uytterhoeven 1 sibling, 0 replies; 34+ messages in thread From: Geert Uytterhoeven @ 2017-01-09 16:15 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Tejun Heo, Michael Schmitz, linux-ide, linux-m68k, linux-kernel On Fri, Dec 30, 2016 at 3:01 PM, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote: > When libata was merged m68k lacked IOMAP support. This has not been > true for a long time now so allow subsystem to be used on m68k. > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20161230140144epcas1p2ada13244f4ba5b45ed903ab7d614f6db@epcas1p2.samsung.com>]
* [PATCH 2/3] ata: pass queued command to ->sff_data_xfer method [not found] ` <CGME20161230140144epcas1p2ada13244f4ba5b45ed903ab7d614f6db@epcas1p2.samsung.com> @ 2016-12-30 14:01 ` Bartlomiej Zolnierkiewicz 0 siblings, 0 replies; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2016-12-30 14:01 UTC (permalink / raw) To: Tejun Heo Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel, b.zolnierkie For Atari Falcon PATA support we need to check the current command in its ->sff_data_xfer method. Update core code and all users accordingly. There should be no functional changes caused by this patch. Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- drivers/ata/libata-sff.c | 29 +++++++++++++++-------------- drivers/ata/pata_at91.c | 6 +++--- drivers/ata/pata_bf54x.c | 7 ++++--- drivers/ata/pata_ep93xx.c | 4 ++-- drivers/ata/pata_ixp4xx_cf.c | 4 ++-- drivers/ata/pata_legacy.c | 15 +++++++++------ drivers/ata/pata_octeon_cf.c | 12 ++++++------ drivers/ata/pata_pcmcia.c | 6 +++--- drivers/ata/pata_samsung_cf.c | 4 ++-- drivers/ata/sata_rcar.c | 4 ++-- include/linux/libata.h | 8 ++++---- 11 files changed, 52 insertions(+), 47 deletions(-) diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index 051b615..4441b5c 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -542,7 +542,7 @@ static inline void ata_tf_to_host(struct ata_port *ap, /** * ata_sff_data_xfer - Transfer data by PIO - * @dev: device to target + * @qc: queued command * @buf: data buffer * @buflen: buffer length * @rw: read/write @@ -555,10 +555,10 @@ static inline void ata_tf_to_host(struct ata_port *ap, * RETURNS: * Bytes consumed. */ -unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, +unsigned int ata_sff_data_xfer(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { - struct ata_port *ap = dev->link->ap; + struct ata_port *ap = qc->dev->link->ap; void __iomem *data_addr = ap->ioaddr.data_addr; unsigned int words = buflen >> 1; @@ -595,7 +595,7 @@ unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, /** * ata_sff_data_xfer32 - Transfer data by PIO - * @dev: device to target + * @qc: queued command * @buf: data buffer * @buflen: buffer length * @rw: read/write @@ -610,16 +610,17 @@ unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, * Bytes consumed. */ -unsigned int ata_sff_data_xfer32(struct ata_device *dev, unsigned char *buf, +unsigned int ata_sff_data_xfer32(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { + struct ata_device *dev = qc->dev; struct ata_port *ap = dev->link->ap; void __iomem *data_addr = ap->ioaddr.data_addr; unsigned int words = buflen >> 2; int slop = buflen & 3; if (!(ap->pflags & ATA_PFLAG_PIO32)) - return ata_sff_data_xfer(dev, buf, buflen, rw); + return ata_sff_data_xfer(qc, buf, buflen, rw); /* Transfer multiple of 4 bytes */ if (rw == READ) @@ -658,7 +659,7 @@ unsigned int ata_sff_data_xfer32(struct ata_device *dev, unsigned char *buf, /** * ata_sff_data_xfer_noirq - Transfer data by PIO - * @dev: device to target + * @qc: queued command * @buf: data buffer * @buflen: buffer length * @rw: read/write @@ -672,14 +673,14 @@ unsigned int ata_sff_data_xfer32(struct ata_device *dev, unsigned char *buf, * RETURNS: * Bytes consumed. */ -unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev, unsigned char *buf, +unsigned int ata_sff_data_xfer_noirq(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { unsigned long flags; unsigned int consumed; local_irq_save(flags); - consumed = ata_sff_data_xfer32(dev, buf, buflen, rw); + consumed = ata_sff_data_xfer32(qc, buf, buflen, rw); local_irq_restore(flags); return consumed; @@ -723,14 +724,14 @@ static void ata_pio_sector(struct ata_queued_cmd *qc) buf = kmap_atomic(page); /* do the actual data transfer */ - ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size, + ap->ops->sff_data_xfer(qc, buf + offset, qc->sect_size, do_write); kunmap_atomic(buf); local_irq_restore(flags); } else { buf = page_address(page); - ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size, + ap->ops->sff_data_xfer(qc, buf + offset, qc->sect_size, do_write); } @@ -791,7 +792,7 @@ static void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc) DPRINTK("send cdb\n"); WARN_ON_ONCE(qc->dev->cdb_len < 12); - ap->ops->sff_data_xfer(qc->dev, qc->cdb, qc->dev->cdb_len, 1); + ap->ops->sff_data_xfer(qc, qc->cdb, qc->dev->cdb_len, 1); ata_sff_sync(ap); /* FIXME: If the CDB is for DMA do we need to do the transition delay or is bmdma_start guaranteed to do it ? */ @@ -868,14 +869,14 @@ static int __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes) buf = kmap_atomic(page); /* do the actual data transfer */ - consumed = ap->ops->sff_data_xfer(dev, buf + offset, + consumed = ap->ops->sff_data_xfer(qc, buf + offset, count, rw); kunmap_atomic(buf); local_irq_restore(flags); } else { buf = page_address(page); - consumed = ap->ops->sff_data_xfer(dev, buf + offset, + consumed = ap->ops->sff_data_xfer(qc, buf + offset, count, rw); } diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c index 1611e0e..fd5b34f 100644 --- a/drivers/ata/pata_at91.c +++ b/drivers/ata/pata_at91.c @@ -286,10 +286,10 @@ static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev) set_smc_timing(ap->dev, adev, info, &timing); } -static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev, +static unsigned int pata_at91_data_xfer_noirq(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { - struct at91_ide_info *info = dev->link->ap->host->private_data; + struct at91_ide_info *info = qc->dev->link->ap->host->private_data; unsigned int consumed; unsigned int mode; unsigned long flags; @@ -301,7 +301,7 @@ static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev, regmap_fields_write(fields.mode, info->cs, (mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16); - consumed = ata_sff_data_xfer(dev, buf, buflen, rw); + consumed = ata_sff_data_xfer(qc, buf, buflen, rw); /* restore 8bit mode after data is written */ regmap_fields_write(fields.mode, info->cs, (mode & ~AT91_SMC_DBW) | diff --git a/drivers/ata/pata_bf54x.c b/drivers/ata/pata_bf54x.c index ec748d3..9c5780a 100644 --- a/drivers/ata/pata_bf54x.c +++ b/drivers/ata/pata_bf54x.c @@ -1143,7 +1143,7 @@ static unsigned char bfin_bmdma_status(struct ata_port *ap) /** * bfin_data_xfer - Transfer data by PIO - * @adev: device for this I/O + * @qc: queued command * @buf: data buffer * @buflen: buffer length * @write_data: read/write @@ -1151,10 +1151,11 @@ static unsigned char bfin_bmdma_status(struct ata_port *ap) * Note: Original code is ata_sff_data_xfer(). */ -static unsigned int bfin_data_xfer(struct ata_device *dev, unsigned char *buf, +static unsigned int bfin_data_xfer(struct ata_queued_cmd *qc, + unsigned char *buf, unsigned int buflen, int rw) { - struct ata_port *ap = dev->link->ap; + struct ata_port *ap = qc->dev->link->ap; void __iomem *base = (void __iomem *)ap->ioaddr.ctl_addr; unsigned int words = buflen >> 1; unsigned short *buf16 = (u16 *)buf; diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c index bd6b089..bf1b910 100644 --- a/drivers/ata/pata_ep93xx.c +++ b/drivers/ata/pata_ep93xx.c @@ -474,11 +474,11 @@ static void ep93xx_pata_set_devctl(struct ata_port *ap, u8 ctl) } /* Note: original code is ata_sff_data_xfer */ -static unsigned int ep93xx_pata_data_xfer(struct ata_device *adev, +static unsigned int ep93xx_pata_data_xfer(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { - struct ata_port *ap = adev->link->ap; + struct ata_port *ap = qc->dev->link->ap; struct ep93xx_pata_data *drv_data = ap->host->private_data; u16 *data = (u16 *)buf; unsigned int words = buflen >> 1; diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c index abda441..0b0d930 100644 --- a/drivers/ata/pata_ixp4xx_cf.c +++ b/drivers/ata/pata_ixp4xx_cf.c @@ -40,13 +40,13 @@ static int ixp4xx_set_mode(struct ata_link *link, struct ata_device **error) return 0; } -static unsigned int ixp4xx_mmio_data_xfer(struct ata_device *dev, +static unsigned int ixp4xx_mmio_data_xfer(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { unsigned int i; unsigned int words = buflen >> 1; u16 *buf16 = (u16 *) buf; - struct ata_port *ap = dev->link->ap; + struct ata_port *ap = qc->dev->link->ap; void __iomem *mmio = ap->ioaddr.data_addr; struct ixp4xx_pata_data *data = dev_get_platdata(ap->host->dev); diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c index bce2a8c..53828b6c 100644 --- a/drivers/ata/pata_legacy.c +++ b/drivers/ata/pata_legacy.c @@ -303,11 +303,12 @@ static void pdc20230_set_piomode(struct ata_port *ap, struct ata_device *adev) } -static unsigned int pdc_data_xfer_vlb(struct ata_device *dev, +static unsigned int pdc_data_xfer_vlb(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { - int slop = buflen & 3; + struct ata_device *dev = qc->dev; struct ata_port *ap = dev->link->ap; + int slop = buflen & 3; /* 32bit I/O capable *and* we need to write a whole number of dwords */ if (ata_id_has_dword_io(dev->id) && (slop == 0 || slop == 3) @@ -340,7 +341,7 @@ static unsigned int pdc_data_xfer_vlb(struct ata_device *dev, } local_irq_restore(flags); } else - buflen = ata_sff_data_xfer_noirq(dev, buf, buflen, rw); + buflen = ata_sff_data_xfer_noirq(qc, buf, buflen, rw); return buflen; } @@ -702,9 +703,11 @@ static unsigned int qdi_qc_issue(struct ata_queued_cmd *qc) return ata_sff_qc_issue(qc); } -static unsigned int vlb32_data_xfer(struct ata_device *adev, unsigned char *buf, - unsigned int buflen, int rw) +static unsigned int vlb32_data_xfer(struct ata_queued_cmd *qc, + unsigned char *buf, + unsigned int buflen, int rw) { + struct ata_device *adev = qc->dev; struct ata_port *ap = adev->link->ap; int slop = buflen & 3; @@ -727,7 +730,7 @@ static unsigned int vlb32_data_xfer(struct ata_device *adev, unsigned char *buf, } return (buflen + 3) & ~3; } else - return ata_sff_data_xfer(adev, buf, buflen, rw); + return ata_sff_data_xfer(qc, buf, buflen, rw); } static int qdi_port(struct platform_device *dev, diff --git a/drivers/ata/pata_octeon_cf.c b/drivers/ata/pata_octeon_cf.c index 475a006..e94e7ca 100644 --- a/drivers/ata/pata_octeon_cf.c +++ b/drivers/ata/pata_octeon_cf.c @@ -293,17 +293,17 @@ static void octeon_cf_set_dmamode(struct ata_port *ap, struct ata_device *dev) /** * Handle an 8 bit I/O request. * - * @dev: Device to access + * @qc: Queued command * @buffer: Data buffer * @buflen: Length of the buffer. * @rw: True to write. */ -static unsigned int octeon_cf_data_xfer8(struct ata_device *dev, +static unsigned int octeon_cf_data_xfer8(struct ata_queued_cmd *qc, unsigned char *buffer, unsigned int buflen, int rw) { - struct ata_port *ap = dev->link->ap; + struct ata_port *ap = qc->dev->link->ap; void __iomem *data_addr = ap->ioaddr.data_addr; unsigned long words; int count; @@ -332,17 +332,17 @@ static unsigned int octeon_cf_data_xfer8(struct ata_device *dev, /** * Handle a 16 bit I/O request. * - * @dev: Device to access + * @qc: Queued command * @buffer: Data buffer * @buflen: Length of the buffer. * @rw: True to write. */ -static unsigned int octeon_cf_data_xfer16(struct ata_device *dev, +static unsigned int octeon_cf_data_xfer16(struct ata_queued_cmd *qc, unsigned char *buffer, unsigned int buflen, int rw) { - struct ata_port *ap = dev->link->ap; + struct ata_port *ap = qc->dev->link->ap; void __iomem *data_addr = ap->ioaddr.data_addr; unsigned long words; int count; diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c index bcc4b96..a541eac 100644 --- a/drivers/ata/pata_pcmcia.c +++ b/drivers/ata/pata_pcmcia.c @@ -90,7 +90,7 @@ static int pcmcia_set_mode_8bit(struct ata_link *link, /** * ata_data_xfer_8bit - Transfer data by 8bit PIO - * @dev: device to target + * @qc: queued command * @buf: data buffer * @buflen: buffer length * @rw: read/write @@ -101,10 +101,10 @@ static int pcmcia_set_mode_8bit(struct ata_link *link, * Inherited from caller. */ -static unsigned int ata_data_xfer_8bit(struct ata_device *dev, +static unsigned int ata_data_xfer_8bit(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { - struct ata_port *ap = dev->link->ap; + struct ata_port *ap = qc->dev->link->ap; if (rw == READ) ioread8_rep(ap->ioaddr.data_addr, buf, buflen); diff --git a/drivers/ata/pata_samsung_cf.c b/drivers/ata/pata_samsung_cf.c index f6facd6..431c7de 100644 --- a/drivers/ata/pata_samsung_cf.c +++ b/drivers/ata/pata_samsung_cf.c @@ -263,10 +263,10 @@ static u8 pata_s3c_check_altstatus(struct ata_port *ap) /* * pata_s3c_data_xfer - Transfer data by PIO */ -static unsigned int pata_s3c_data_xfer(struct ata_device *dev, +static unsigned int pata_s3c_data_xfer(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { - struct ata_port *ap = dev->link->ap; + struct ata_port *ap = qc->dev->link->ap; struct s3c_ide_info *info = ap->host->private_data; void __iomem *data_addr = ap->ioaddr.data_addr; unsigned int words = buflen >> 1, i; diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c index f72d601..5d38245 100644 --- a/drivers/ata/sata_rcar.c +++ b/drivers/ata/sata_rcar.c @@ -447,11 +447,11 @@ static void sata_rcar_exec_command(struct ata_port *ap, ata_sff_pause(ap); } -static unsigned int sata_rcar_data_xfer(struct ata_device *dev, +static unsigned int sata_rcar_data_xfer(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw) { - struct ata_port *ap = dev->link->ap; + struct ata_port *ap = qc->dev->link->ap; void __iomem *data_addr = ap->ioaddr.data_addr; unsigned int words = buflen >> 1; diff --git a/include/linux/libata.h b/include/linux/libata.h index c170be5..0e8a800 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -968,7 +968,7 @@ struct ata_port_operations { void (*sff_tf_read)(struct ata_port *ap, struct ata_taskfile *tf); void (*sff_exec_command)(struct ata_port *ap, const struct ata_taskfile *tf); - unsigned int (*sff_data_xfer)(struct ata_device *dev, + unsigned int (*sff_data_xfer)(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw); void (*sff_irq_on)(struct ata_port *); bool (*sff_irq_check)(struct ata_port *); @@ -1823,11 +1823,11 @@ extern int ata_sff_busy_sleep(struct ata_port *ap, extern void ata_sff_tf_read(struct ata_port *ap, struct ata_taskfile *tf); extern void ata_sff_exec_command(struct ata_port *ap, const struct ata_taskfile *tf); -extern unsigned int ata_sff_data_xfer(struct ata_device *dev, +extern unsigned int ata_sff_data_xfer(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw); -extern unsigned int ata_sff_data_xfer32(struct ata_device *dev, +extern unsigned int ata_sff_data_xfer32(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw); -extern unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev, +extern unsigned int ata_sff_data_xfer_noirq(struct ata_queued_cmd *qc, unsigned char *buf, unsigned int buflen, int rw); extern void ata_sff_irq_on(struct ata_port *ap); extern void ata_sff_irq_clear(struct ata_port *ap); -- 1.9.1 ^ permalink raw reply related [flat|nested] 34+ messages in thread
[parent not found: <CGME20161230140147epcas5p1fa7e99f39921a8ee90aabd59ff7b7645@epcas5p1.samsung.com>]
* [PATCH 3/3] ata: add Atari Falcon PATA controller driver [not found] ` <CGME20161230140147epcas5p1fa7e99f39921a8ee90aabd59ff7b7645@epcas5p1.samsung.com> @ 2016-12-30 14:01 ` Bartlomiej Zolnierkiewicz 0 siblings, 0 replies; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2016-12-30 14:01 UTC (permalink / raw) To: Tejun Heo Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel, b.zolnierkie Add Atari Falcon PATA controller driver. The major difference when compared to legacy IDE's falconide host driver is that we are using polled PIO mode and thus avoiding the need for STDMA locking magic altogether. Tested under ARAnyM emulator. Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- drivers/ata/Kconfig | 9 +++ drivers/ata/Makefile | 1 + drivers/ata/pata_falcon.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+) create mode 100644 drivers/ata/pata_falcon.c diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index da19fc9..cbd1019 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -895,6 +895,15 @@ config PATA_CMD640_PCI If unsure, say N. +config PATA_FALCON + tristate "Atari Falcon PATA support" + depends on M68K && ATARI + help + This option enables support for the on-board IDE + interface on the Atari Falcon. + + If unsure, say N. + config PATA_ISAPNP tristate "ISA Plug and Play PATA support" depends on ISAPNP diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile index a46e6b7..89a0a19 100644 --- a/drivers/ata/Makefile +++ b/drivers/ata/Makefile @@ -93,6 +93,7 @@ obj-$(CONFIG_PATA_WINBOND) += pata_sl82c105.o obj-$(CONFIG_PATA_AT32) += pata_at32.o obj-$(CONFIG_PATA_AT91) += pata_at91.o obj-$(CONFIG_PATA_CMD640_PCI) += pata_cmd640.o +obj-$(CONFIG_PATA_FALCON) += pata_falcon.o obj-$(CONFIG_PATA_ISAPNP) += pata_isapnp.o obj-$(CONFIG_PATA_IXP4XX_CF) += pata_ixp4xx_cf.o obj-$(CONFIG_PATA_MPIIX) += pata_mpiix.o diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c new file mode 100644 index 0000000..7826408 --- /dev/null +++ b/drivers/ata/pata_falcon.c @@ -0,0 +1,184 @@ +/* + * Atari Falcon PATA controller driver + * + * Copyright (c) 2016 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Based on falconide.c: + * + * Created 12 Jul 1997 by Geert Uytterhoeven + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/blkdev.h> +#include <linux/delay.h> +#include <scsi/scsi_host.h> +#include <scsi/scsi_cmnd.h> +#include <linux/ata.h> +#include <linux/libata.h> +#include <linux/mm.h> +#include <linux/interrupt.h> +#include <linux/platform_device.h> + +#include <asm/setup.h> +#include <asm/atarihw.h> +#include <asm/atariints.h> +#include <asm/atari_stdma.h> +#include <asm/ide.h> + +#define DRV_NAME "pata_falcon" +#define DRV_VERSION "0.1.0" + +#define ATA_HD_BASE 0xfff00000 +#define ATA_HD_CONTROL 0x39 + +static struct scsi_host_template pata_falcon_sht = { + ATA_PIO_SHT(DRV_NAME), +}; + +static unsigned int pata_falcon_data_xfer(struct ata_queued_cmd *qc, + unsigned char *buf, + unsigned int buflen, int rw) +{ + struct ata_device *dev = qc->dev; + struct ata_port *ap = dev->link->ap; + void __iomem *data_addr = ap->ioaddr.data_addr; + unsigned int words = buflen >> 1; + struct scsi_cmnd *cmd = qc->scsicmd; + bool swap = 1; + + if (dev->class == ATA_DEV_ATA && cmd && cmd->request && + cmd->request->cmd_type == REQ_TYPE_FS) + swap = 0; + + /* Transfer multiple of 2 bytes */ + if (rw == READ) { + if (swap) + raw_insw_swapw((u16 *)data_addr, (u16 *)buf, words); + else + raw_insw((u16 *)data_addr, (u16 *)buf, words); + } else { + if (swap) + raw_outsw_swapw((u16 *)data_addr, (u16 *)buf, words); + else + raw_outsw((u16 *)data_addr, (u16 *)buf, words); + } + + /* Transfer trailing byte, if any. */ + if (unlikely(buflen & 0x01)) { + unsigned char pad[2] = { }; + + /* Point buf to the tail of buffer */ + buf += buflen - 1; + + if (rw == READ) { + if (swap) + raw_insw_swapw((u16 *)data_addr, (u16 *)pad, 1); + else + raw_insw((u16 *)data_addr, (u16 *)pad, 1); + *buf = pad[0]; + } else { + pad[0] = *buf; + if (swap) + raw_outsw_swapw((u16 *)data_addr, (u16 *)pad, 1); + else + raw_outsw((u16 *)data_addr, (u16 *)pad, 1); + } + words++; + } + + return words << 1; +} + +/* + * Provide our own set_mode() as we don't want to change anything that has + * already been configured.. + */ +static int pata_falcon_set_mode(struct ata_link *link, + struct ata_device **unused) +{ + struct ata_device *dev; + + ata_for_each_dev(dev, link, ENABLED) { + /* We don't really care */ + dev->pio_mode = dev->xfer_mode = XFER_PIO_0; + dev->xfer_shift = ATA_SHIFT_PIO; + dev->flags |= ATA_DFLAG_PIO; + ata_dev_info(dev, "configured for PIO\n"); + } + return 0; +} + +static struct ata_port_operations pata_falcon_ops = { + .inherits = &ata_sff_port_ops, + .sff_data_xfer = pata_falcon_data_xfer, + .cable_detect = ata_cable_unknown, + .set_mode = pata_falcon_set_mode, +}; + +static int pata_falcon_init_one(void) +{ + struct ata_host *host; + struct ata_port *ap; + struct platform_device *pdev; + void __iomem *base; + + if (!MACH_IS_ATARI || !ATARIHW_PRESENT(IDE)) + return -ENODEV; + + pr_info(DRV_NAME ": Atari Falcon PATA controller\n"); + + pdev = platform_device_register_simple(DRV_NAME, 0, NULL, 0); + if (IS_ERR(pdev)) + return PTR_ERR(pdev); + + if (!devm_request_mem_region(&pdev->dev, ATA_HD_BASE, 0x40, DRV_NAME)) { + pr_err(DRV_NAME ": resources busy\n"); + return -EBUSY; + } + + /* allocate host */ + host = ata_host_alloc(&pdev->dev, 1); + if (!host) + return -ENOMEM; + ap = host->ports[0]; + + ap->ops = &pata_falcon_ops; + ap->pio_mask = ATA_PIO4; + ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY; + ap->flags |= ATA_FLAG_PIO_POLLING; + + base = (void __iomem *)ATA_HD_BASE; + ap->ioaddr.data_addr = base; + ap->ioaddr.error_addr = base + 1 + 1 * 4; + ap->ioaddr.feature_addr = base + 1 + 1 * 4; + ap->ioaddr.nsect_addr = base + 1 + 2 * 4; + ap->ioaddr.lbal_addr = base + 1 + 3 * 4; + ap->ioaddr.lbam_addr = base + 1 + 4 * 4; + ap->ioaddr.lbah_addr = base + 1 + 5 * 4; + ap->ioaddr.device_addr = base + 1 + 6 * 4; + ap->ioaddr.status_addr = base + 1 + 7 * 4; + ap->ioaddr.command_addr = base + 1 + 7 * 4; + + ap->ioaddr.altstatus_addr = base + ATA_HD_CONTROL; + ap->ioaddr.ctl_addr = base + ATA_HD_CONTROL; + + ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base, + (unsigned long)base + ATA_HD_CONTROL); + + /* activate */ + return ata_host_activate(host, 0, NULL, 0, &pata_falcon_sht); +} + +module_init(pata_falcon_init_one); + +MODULE_AUTHOR("Bartlomiej Zolnierkiewicz"); +MODULE_DESCRIPTION("low-level driver for Atari Falcon PATA"); +MODULE_LICENSE("GPL"); +MODULE_VERSION(DRV_VERSION); -- 1.9.1 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2016-12-30 14:01 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Bartlomiej Zolnierkiewicz ` (2 preceding siblings ...) [not found] ` <CGME20161230140147epcas5p1fa7e99f39921a8ee90aabd59ff7b7645@epcas5p1.samsung.com> @ 2017-01-03 10:49 ` Geert Uytterhoeven 2017-01-09 16:11 ` Bartlomiej Zolnierkiewicz 2017-01-05 21:01 ` Michael Schmitz ` (2 subsequent siblings) 6 siblings, 1 reply; 34+ messages in thread From: Geert Uytterhoeven @ 2017-01-03 10:49 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Tejun Heo, Michael Schmitz, linux-ide, linux-m68k, linux-kernel Hi Bartlomiej, On Fri, Dec 30, 2016 at 3:01 PM, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote: > This patchset adds m68k/Atari Falcon PATA support to libata. Thanks for your series! That leaves us with 4 to go ;-) CONFIG_BLK_DEV_GAYLE CONFIG_BLK_DEV_BUDDHA CONFIG_BLK_DEV_MAC_IDE CONFIG_BLK_DEV_Q40IDE Note that using libata instead of the legacy IDE driver increases kernel size. After enabling libata: CONFIG_ATA=y CONFIG_ATA_VERBOSE_ERROR=y CONFIG_ATA_SFF=y CONFIG_PATA_FALCON=y an atari_defconfig kernel grew by: add/remove: 775/0 grow/shrink: 753/41 up/down: 98999/-242 (98757) After disabling CONFIG_IDE: add/remove: 0/589 grow/shrink: 0/12 up/down: 0/-62835 (-62835) So the net result is: add/remove: 775/589 grow/shrink: 749/51 up/down: 98886/-62964 (35922) Disabling CONFIG_ATA_VERBOSE_ERROR saved 1380 bytes, which is less than the value advertised by Kconfig (6KB). > The major difference in the new libata's pata_falcon host > driver when compared to legacy IDE's falconide host driver is > that we are using polled PIO mode and thus avoiding the need > for STDMA locking magic altogether. I'll let the Atari experts (Michael?) comment on that... > Tested under ARAnyM emulator. Works indeed fine on ARAnyM. Unfortunately I can't test it on real hardware... 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-03 10:49 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Geert Uytterhoeven @ 2017-01-09 16:11 ` Bartlomiej Zolnierkiewicz 2017-01-10 16:09 ` Tejun Heo 0 siblings, 1 reply; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2017-01-09 16:11 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Tejun Heo, Michael Schmitz, linux-ide, linux-m68k, linux-kernel Hi, On Tuesday, January 03, 2017 11:49:16 AM Geert Uytterhoeven wrote: > Hi Bartlomiej, > > On Fri, Dec 30, 2016 at 3:01 PM, Bartlomiej Zolnierkiewicz > <b.zolnierkie@samsung.com> wrote: > > This patchset adds m68k/Atari Falcon PATA support to libata. > > Thanks for your series! > > That leaves us with 4 to go ;-) > > CONFIG_BLK_DEV_GAYLE > CONFIG_BLK_DEV_BUDDHA > CONFIG_BLK_DEV_MAC_IDE > CONFIG_BLK_DEV_Q40IDE They should be easy to port to libata, one just needs to remember to use ->qc_defer for ports that require serialization. > Note that using libata instead of the legacy IDE driver increases kernel size. > > After enabling libata: > > CONFIG_ATA=y > CONFIG_ATA_VERBOSE_ERROR=y > CONFIG_ATA_SFF=y > CONFIG_PATA_FALCON=y > > an atari_defconfig kernel grew by: > > add/remove: 775/0 grow/shrink: 753/41 up/down: 98999/-242 (98757) > > After disabling CONFIG_IDE: > > add/remove: 0/589 grow/shrink: 0/12 up/down: 0/-62835 (-62835) > > So the net result is: > > add/remove: 775/589 grow/shrink: 749/51 up/down: 98886/-62964 (35922) SATA-specific code is not needed on legacy PATA systems so it should be possible to reduce the size by adding new config option (i.e. SATA_HOST) and: - covering SATA only code with CONFIG_SATA_HOST ifdefs - making SATA_HOST to be selected by SATA host drivers in drivers/ata/KConfig > Disabling CONFIG_ATA_VERBOSE_ERROR saved 1380 bytes, which is less than the > value advertised by Kconfig (6KB). If it is only ~1kB nowadays I would vote for making the error logging always verbose and just removing CONFIG_ATA_VERBOSE_ERROR, Tejun? Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-09 16:11 ` Bartlomiej Zolnierkiewicz @ 2017-01-10 16:09 ` Tejun Heo 0 siblings, 0 replies; 34+ messages in thread From: Tejun Heo @ 2017-01-10 16:09 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel Hello, On Mon, Jan 09, 2017 at 05:11:12PM +0100, Bartlomiej Zolnierkiewicz wrote: > > Disabling CONFIG_ATA_VERBOSE_ERROR saved 1380 bytes, which is less than the > > value advertised by Kconfig (6KB). > > If it is only ~1kB nowadays I would vote for making the error logging always > verbose and just removing CONFIG_ATA_VERBOSE_ERROR, Tejun? No objection from me. Thanks. -- tejun ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2016-12-30 14:01 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Bartlomiej Zolnierkiewicz ` (3 preceding siblings ...) 2017-01-03 10:49 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Geert Uytterhoeven @ 2017-01-05 21:01 ` Michael Schmitz 2017-01-10 12:53 ` Bartlomiej Zolnierkiewicz 2017-01-10 16:11 ` Tejun Heo 2017-02-15 8:45 ` Geert Uytterhoeven 6 siblings, 1 reply; 34+ messages in thread From: Michael Schmitz @ 2017-01-05 21:01 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development Hi Bartlomiej, thanks for caring to support our legacy PATA systems! On Sat, Dec 31, 2016 at 3:01 AM, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote: > Hi, > > This patchset adds m68k/Atari Falcon PATA support to libata. > The major difference in the new libata's pata_falcon host > driver when compared to legacy IDE's falconide host driver is > that we are using polled PIO mode and thus avoiding the need > for STDMA locking magic altogether. I don't suppose this is the default libata mode for PIO? How is polling implemented in libata? Sleeping for something approximating the average seek latency shouldn't hurt but spinning wont be acceptable for a low performance single CPU architecture like the Falcon. > Tested under ARAnyM emulator. Not sure that the emulator is really feature complete enough - I'll get this tested on my Falcon in the next few weeks. I'm a bit worried about IDE still generating interrupts on seek completion (did you spot anything like that, Geert?). Cheers, Michael ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-05 21:01 ` Michael Schmitz @ 2017-01-10 12:53 ` Bartlomiej Zolnierkiewicz 2017-01-10 20:02 ` Michael Schmitz 0 siblings, 1 reply; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2017-01-10 12:53 UTC (permalink / raw) To: Michael Schmitz Cc: Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development Hi, On Friday, January 06, 2017 10:01:49 AM Michael Schmitz wrote: > Hi Bartlomiej, > > thanks for caring to support our legacy PATA systems! > > On Sat, Dec 31, 2016 at 3:01 AM, Bartlomiej Zolnierkiewicz > <b.zolnierkie@samsung.com> wrote: > > Hi, > > > > This patchset adds m68k/Atari Falcon PATA support to libata. > > The major difference in the new libata's pata_falcon host > > driver when compared to legacy IDE's falconide host driver is > > that we are using polled PIO mode and thus avoiding the need > > for STDMA locking magic altogether. > > I don't suppose this is the default libata mode for PIO? No, by default it is used only for some commands (i.e. IDENTIFY, SET FEATURES - XFER MODE). > How is polling implemented in libata? Sleeping for something > approximating the average seek latency shouldn't hurt but spinning > wont be acceptable for a low performance single CPU architecture like > the Falcon. You can find actual implementation in libata-sff.c. Please see ata_sff_pio_task() for the main polling logic: fsm_start: WARN_ON_ONCE(ap->hsm_task_state == HSM_ST_IDLE); /* * This is purely heuristic. This is a fast path. * Sometimes when we enter, BSY will be cleared in * a chk-status or two. If not, the drive is probably seeking * or something. Snooze for a couple msecs, then * chk-status again. If still busy, queue delayed work. */ status = ata_sff_busy_wait(ap, ATA_BUSY, 5); if (status & ATA_BUSY) { spin_unlock_irq(ap->lock); ata_msleep(ap, 2); spin_lock_irq(ap->lock); status = ata_sff_busy_wait(ap, ATA_BUSY, 10); if (status & ATA_BUSY) { ata_sff_queue_pio_task(link, ATA_SHORT_PAUSE); goto out_unlock; } } /* * hsm_move() may trigger another command to be processed. * clean the link beforehand. */ ap->sff_pio_task_link = NULL; /* move the HSM */ poll_next = ata_sff_hsm_move(ap, qc, status, 1); /* another command or interrupt handler * may be running at this point. */ if (poll_next) goto fsm_start; out_unlock: ata_sff_busy_wait()'s last argument is number of 10uS waits so first check (50uS) should be quite quick. If device is still busy we sleep for 2ms. Then we quickly (100uS) busy wait and if needed queue delayed work (with ATA_SHORT_PAUSE == 16 ms delay). Overall the current heuristic looks fine and spinning should be minimal. > > Tested under ARAnyM emulator. > > Not sure that the emulator is really feature complete enough - I'll > get this tested on my Falcon in the next few weeks. I'm a bit worried Great! > about IDE still generating interrupts on seek completion (did you spot > anything like that, Geert?). > > Cheers, > > Michael BTW according to comment in arch/m68k/atari/stdma.c: /* On the Falcon, the IDE bus uses just the ACSI/Floppy interrupt, but */ /* not the ST-DMA chip itself. So falhd.c needs not to lock the */ /* chip. The interrupt is routed to falhd.c if IDE is configured, the */ /* model is a Falcon and the interrupt was caused by the HD controller */ /* (can be determined by looking at its status register). */ it should be okay to use IDE at the same time as SCSI/Floppy which is what the new driver does (the old one is serialized operations by ST-DMA related IRQ handling magic). Also the comment itself may need some fixups as on Falcon it is SCSI not ACSI (according to the earlier comment in same file) and the old IDE host driver name is not falhd.c but falconide.c. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-10 12:53 ` Bartlomiej Zolnierkiewicz @ 2017-01-10 20:02 ` Michael Schmitz 2017-01-13 2:33 ` Finn Thain 0 siblings, 1 reply; 34+ messages in thread From: Michael Schmitz @ 2017-01-10 20:02 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Bartlomiej, >> How is polling implemented in libata? Sleeping for something >> approximating the average seek latency shouldn't hurt but spinning >> wont be acceptable for a low performance single CPU architecture like >> the Falcon. > > You can find actual implementation in libata-sff.c. > > Please see ata_sff_pio_task() for the main polling logic: > > fsm_start: > WARN_ON_ONCE(ap->hsm_task_state == HSM_ST_IDLE); > > /* > * This is purely heuristic. This is a fast path. > * Sometimes when we enter, BSY will be cleared in > * a chk-status or two. If not, the drive is probably seeking > * or something. Snooze for a couple msecs, then > * chk-status again. If still busy, queue delayed work. > */ > status = ata_sff_busy_wait(ap, ATA_BUSY, 5); > if (status & ATA_BUSY) { > spin_unlock_irq(ap->lock); > ata_msleep(ap, 2); > spin_lock_irq(ap->lock); > > status = ata_sff_busy_wait(ap, ATA_BUSY, 10); > if (status & ATA_BUSY) { > ata_sff_queue_pio_task(link, ATA_SHORT_PAUSE); > goto out_unlock; > } > } > > /* > * hsm_move() may trigger another command to be processed. > * clean the link beforehand. > */ > ap->sff_pio_task_link = NULL; > /* move the HSM */ > poll_next = ata_sff_hsm_move(ap, qc, status, 1); > > /* another command or interrupt handler > * may be running at this point. > */ > if (poll_next) > goto fsm_start; > out_unlock: > > > ata_sff_busy_wait()'s last argument is number of 10uS waits so > first check (50uS) should be quite quick. If device is still > busy we sleep for 2ms. Then we quickly (100uS) busy wait and > if needed queue delayed work (with ATA_SHORT_PAUSE == 16 ms > delay). > > Overall the current heuristic looks fine and spinning should be > minimal. Thanks, that looks entirely reasonable. >> > Tested under ARAnyM emulator. >> >> Not sure that the emulator is really feature complete enough - I'll >> get this tested on my Falcon in the next few weeks. I'm a bit worried > > Great! > >> about IDE still generating interrupts on seek completion (did you spot >> anything like that, Geert?). >> >> Cheers, >> >> Michael > > BTW according to comment in arch/m68k/atari/stdma.c: > > /* On the Falcon, the IDE bus uses just the ACSI/Floppy interrupt, but */ > /* not the ST-DMA chip itself. So falhd.c needs not to lock the */ > /* chip. The interrupt is routed to falhd.c if IDE is configured, the */ > /* model is a Falcon and the interrupt was caused by the HD controller */ > /* (can be determined by looking at its status register). */ That comment is probably incorrect in part. Blame Linus :-) Seriously though - that comment dates back to the dark ages (when m68k was an entirely separate port and the IDE driver was indeed named falhd.c). That would have been even before 2.2 or 2.4 times. The comment just never got updated. What is still correct is that the IDE driver does use the interrupt only, not the ST-DMA chip. And a single IDE interrupt can be correctly assigned to IDE by looking at the status register. With the SCSI (and IIRC also floppy) interrupts, we don't have direct access to the status registers without disturbing the state of the DMA though. Unless we know for definite that either chips have raised the interrupt (and DMA ops are in flight), we must not touch the DMA chip at all. The case I'm worried about is both IDE and SCSI raising an interrupt. We don't currently mask the IDE/ST-DMA interrupt so a stacked interrupt must be processed in the same pass as the initial interrupt or it will get dropped. We'd have to peek at the DMA registers to check the SCSI or floppy interrupt status, and we just can't safely do that. So races of this kind are currently prevented by including IDE in the IRQ locking process. Whether it's possible to mask the interrupt, do one pass, unmask and process the second interrupt I don't know. Maybe Andreas does? Cheers, Michael > it should be okay to use IDE at the same time as SCSI/Floppy which > is what the new driver does (the old one is serialized operations by > ST-DMA related IRQ handling magic). > > Also the comment itself may need some fixups as on Falcon it is SCSI > not ACSI (according to the earlier comment in same file) and the old > IDE host driver name is not falhd.c but falconide.c. > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-10 20:02 ` Michael Schmitz @ 2017-01-13 2:33 ` Finn Thain 2017-01-14 8:55 ` Michael Schmitz 0 siblings, 1 reply; 34+ messages in thread From: Finn Thain @ 2017-01-13 2:33 UTC (permalink / raw) To: Michael Schmitz Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab On Wed, 11 Jan 2017, Michael Schmitz wrote: > What is still correct is that the IDE driver does use the interrupt > only, not the ST-DMA chip. And a single IDE interrupt can be correctly > assigned to IDE by looking at the status register. > > With the SCSI (and IIRC also floppy) interrupts, we don't have direct > access to the status registers without disturbing the state of the DMA > though. Unless we know for definite that either chips have raised the > interrupt (and DMA ops are in flight), we must not touch the DMA chip at > all. > > The case I'm worried about is both IDE and SCSI raising an interrupt. We > don't currently mask the IDE/ST-DMA interrupt so a stacked interrupt > must be processed in the same pass as the initial interrupt or it will > get dropped. We'd have to peek at the DMA registers to check the SCSI or > floppy interrupt status, and we just can't safely do that. So races of > this kind are currently prevented by including IDE in the IRQ locking > process. > > Whether it's possible to mask the interrupt, do one pass, unmask and > process the second interrupt I don't know. Would that require handling the SCSI DMA interrupt in the first pass? Or handling IDE first, and ensuring that the IDE handler does not access ST-DMA registers? What about FDC? The atari_scsi handler accesses the ST-DMA registers; it can do so because it knows that any DMA must have completed -- it can infer this because a simultaneous pending interrupt from FDC or IDE is impossible due to stdma_lock(). Your suggestion would seem to allow other pending interrupts, hence the atari_scsi interrupt handler logic has to be tossed out. What logic would replace it? If all else fails, perhaps we could inhibit DMA entirely when the new ATA driver is loaded. Then we can just dispatch the ST-DMA irq like a shared irq. I'm sure that atari_scsi can work without DMA. No idea about the FDC driver though (ataflop.c). Another solution would be to dedicate the DMA function to atari_scsi, and then mask the FDC and IDE interrupts during each DMA transfer. But once again, this would mean changing the FDC driver to eliminate DMA, if that is possible. From the schematic it looks the the FDC chip, "AJAX", is another custom ... http://dev-docs.atariforge.org/files/Falcon030_Schematic.pdf Unfortunately my grasp of the ST hardware reflects my inability to read German; those who can may want to take a look at "ATARI Profibuch ST-STE-TT.pdf". -- > Maybe Andreas does? > > Cheers, > > Michael > > > > it should be okay to use IDE at the same time as SCSI/Floppy which is > > what the new driver does (the old one is serialized operations by > > ST-DMA related IRQ handling magic). > > > > Also the comment itself may need some fixups as on Falcon it is SCSI > > not ACSI (according to the earlier comment in same file) and the old > > IDE host driver name is not falhd.c but falconide.c. > > > > Best regards, > > -- > > Bartlomiej Zolnierkiewicz > > Samsung R&D Institute Poland > > Samsung Electronics > > > -- > 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 > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-13 2:33 ` Finn Thain @ 2017-01-14 8:55 ` Michael Schmitz 2017-01-14 23:47 ` Finn Thain 0 siblings, 1 reply; 34+ messages in thread From: Michael Schmitz @ 2017-01-14 8:55 UTC (permalink / raw) To: Finn Thain Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Hi Finn, Am 13.01.2017 um 15:33 schrieb Finn Thain: >> The case I'm worried about is both IDE and SCSI raising an interrupt. We >> don't currently mask the IDE/ST-DMA interrupt so a stacked interrupt >> must be processed in the same pass as the initial interrupt or it will >> get dropped. We'd have to peek at the DMA registers to check the SCSI or >> floppy interrupt status, and we just can't safely do that. So races of >> this kind are currently prevented by including IDE in the IRQ locking >> process. >> >> Whether it's possible to mask the interrupt, do one pass, unmask and >> process the second interrupt I don't know. > > Would that require handling the SCSI DMA interrupt in the first pass? Or > handling IDE first, and ensuring that the IDE handler does not access > ST-DMA registers? What about FDC? Handling the IDE interrupt first I think, then looking at the DMA (for SCSI or FDC). > The atari_scsi handler accesses the ST-DMA registers; it can do so because > it knows that any DMA must have completed -- it can infer this because a > simultaneous pending interrupt from FDC or IDE is impossible due to > stdma_lock(). libata dropped the locking (and does not use IDE interrupts at present so it seems to be safe. Still testing - I've seen IO errors, and that's a bit of a worry). > Your suggestion would seem to allow other pending interrupts, hence the > atari_scsi interrupt handler logic has to be tossed out. What logic would > replace it? I need to think about that some more - if no DMA is in progress we can safely peek at the SCSI registers. So the logic could be changed to test for DMA operation first, and just try and service the interrupt if DMA wasn't active. If DMA has been in progress, I'm not sure that we can figure out if it's still active from looking at the status register (that is, whether bits 0 or 1 are set while DMA is ongoing). We'd have to peek at the DMA status register (or DMA address registers) without first stopping DMA, which the current driver does. The docs seem to advise against that. If DMA was in progress, stopping it would likely leave us with residual bytes to be transferred - we'd have to handle that transfer as we would any other DMA error (from memory, probably best to retry the entire command, or transfer the remaining bytes using PIO if we're sure no bytes have been lost). > If all else fails, perhaps we could inhibit DMA entirely when the new ATA > driver is loaded. Then we can just dispatch the ST-DMA irq like a shared > irq. I'm sure that atari_scsi can work without DMA. No idea about the FDC > driver though (ataflop.c). Yes, SCSI can work using PIO but's it a real dog. Been there, done that (about 20 years ago). I know nothing about the FDC chip though. > Another solution would be to dedicate the DMA function to atari_scsi, and > then mask the FDC and IDE interrupts during each DMA transfer. But once > again, this would mean changing the FDC driver to eliminate DMA, if that > is possible. From the schematic it looks the the FDC chip, "AJAX", is > another custom ... > http://dev-docs.atariforge.org/files/Falcon030_Schematic.pdf > > Unfortunately my grasp of the ST hardware reflects my inability to read > German; those who can may want to take a look at "ATARI Profibuch > ST-STE-TT.pdf". I'll reread the ST-DMA description (and the FDC one). Let me know if you think this could work ... Cheers, Michael ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-14 8:55 ` Michael Schmitz @ 2017-01-14 23:47 ` Finn Thain 2017-01-15 1:48 ` Michael Schmitz 0 siblings, 1 reply; 34+ messages in thread From: Finn Thain @ 2017-01-14 23:47 UTC (permalink / raw) To: Michael Schmitz Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab On Sat, 14 Jan 2017, Michael Schmitz wrote: > Am 13.01.2017 um 15:33 schrieb Finn Thain: > > >> The case I'm worried about is both IDE and SCSI raising an interrupt. > >> We don't currently mask the IDE/ST-DMA interrupt so a stacked > >> interrupt must be processed in the same pass as the initial interrupt > >> or it will get dropped. We'd have to peek at the DMA registers to > >> check the SCSI or floppy interrupt status, and we just can't safely > >> do that. So races of this kind are currently prevented by including > >> IDE in the IRQ locking process. > >> > >> Whether it's possible to mask the interrupt, do one pass, unmask and > >> process the second interrupt I don't know. > > > > Would that require handling the SCSI DMA interrupt in the first pass? > > Or handling IDE first, and ensuring that the IDE handler does not > > access ST-DMA registers? What about FDC? > > Handling the IDE interrupt first I think, then looking at the DMA (for > SCSI or FDC). > For the sake of discussion, I'll assume that the FDC driver will not be using DMA. (Perhaps FDC and SCSI can share the ST-DMA chip, using the present locking mechanism, but it would not simplify things much: when IDE no longer participates in that locking mechanism then both FDC and SCSI drivers have to solve the same issues.) > > The atari_scsi handler accesses the ST-DMA registers; it can do so > > because it knows that any DMA must have completed -- it can infer this > > because a simultaneous pending interrupt from FDC or IDE is impossible > > due to stdma_lock(). > > libata dropped the locking (and does not use IDE interrupts at present > so it seems to be safe. Still testing - I've seen IO errors, and that's > a bit of a worry). > What compiler are you using, BTW? Are you still using the gcc-4.6.3 m68k cross-compiler from kernel.org? I had to abandon it in order to get my SCSI driver patches to work. > > Your suggestion would seem to allow other pending interrupts, hence > > the atari_scsi interrupt handler logic has to be tossed out. What > > logic would replace it? > > I need to think about that some more - if no DMA is in progress we can > safely peek at the SCSI registers. So the logic could be changed to test > for DMA operation first, and just try and service the interrupt if DMA > wasn't active. > OK, so based on the above, we handle the possible IDE interrupt (without checking DMA registers), handle the possible FDC interrupt (again without checking DMA registers) and finally handle the possible SCSI interrupt. The core 5380 driver knows whether or not it has started a DMA. The atari_scsi driver also knows that no other Falcon driver uses DMA. So the atari_scsi handler only has to figure out whether the interrupt was asserted by the ST-DMA chip or the 5380 chip, or neither. (The "neither" possility arises when IDE ditches the the stdma.c lock mechanism.) Without the stdma.c lock, any or all of these interrupts could be asserted simultaneously, so the IDE and FDC drivers need to be able to do the right thing in the presence of the other interrupts and do so without accessing the ST-DMA chip. And the SCSI interrupt handler needs to do the right thing when there is no DMA interrupt, and yet a DMA is running. Perhaps we could reverse the algorithm in scsi_falcon_intr(): if NCR5380_intr() completes with IRQ_HANDLED and the core 5380 driver is no longer in DMA, then check the ST-DMA registers for errors etc. Alternatively, if NCR5380_intr() returns IRQ_NONE, then do nothing at all, on the basis that the interrupt was handled by FDC or IDE. In this situation, I gather that atari_scsi could miss out on a DMA completion interrupt from the ST-DMA chip, which could lead to command timeout? > If DMA has been in progress, I'm not sure that we can figure out if it's > still active from looking at the status register (that is, whether bits > 0 or 1 are set while DMA is ongoing). We'd have to peek at the DMA > status register (or DMA address registers) without first stopping DMA, > which the current driver does. The docs seem to advise against that. If > DMA was in progress, stopping it would likely leave us with residual > bytes to be transferred - I can't comment on the Profibuch doc or the ST-DMA chip details (Andreas?) I suspect it has to be tried. > we'd have to handle that transfer as we would any other DMA error (from > memory, probably best to retry the entire command, or transfer the > remaining bytes using PIO if we're sure no bytes have been lost). > Allowing the command to fail should be fine so long as the 5380 driver sends the correct result code to the mid-layer. To attempt to complete the command after a failed DMA is needless complexity, and it's a trick that probably can't be pulled off reliably anyway. -- > > If all else fails, perhaps we could inhibit DMA entirely when the new > > ATA driver is loaded. Then we can just dispatch the ST-DMA irq like a > > shared irq. I'm sure that atari_scsi can work without DMA. No idea > > about the FDC driver though (ataflop.c). > > Yes, SCSI can work using PIO but's it a real dog. Been there, done that > (about 20 years ago). I know nothing about the FDC chip though. > > > Another solution would be to dedicate the DMA function to atari_scsi, > > and then mask the FDC and IDE interrupts during each DMA transfer. But > > once again, this would mean changing the FDC driver to eliminate DMA, > > if that is possible. From the schematic it looks the the FDC chip, > > "AJAX", is another custom ... > > http://dev-docs.atariforge.org/files/Falcon030_Schematic.pdf > > > > Unfortunately my grasp of the ST hardware reflects my inability to > > read German; those who can may want to take a look at "ATARI Profibuch > > ST-STE-TT.pdf". > > I'll reread the ST-DMA description (and the FDC one). > > Let me know if you think this could work ... > > Cheers, > > Michael > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-14 23:47 ` Finn Thain @ 2017-01-15 1:48 ` Michael Schmitz 2017-01-15 4:42 ` Finn Thain 0 siblings, 1 reply; 34+ messages in thread From: Michael Schmitz @ 2017-01-15 1:48 UTC (permalink / raw) To: Finn Thain Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Hi Finn, Am 15.01.2017 um 12:47 schrieb Finn Thain: > For the sake of discussion, I'll assume that the FDC driver will not > be using DMA. (Perhaps FDC and SCSI can share the ST-DMA chip, using > the present locking mechanism, but it would not simplify things much: > when IDE no longer participates in that locking mechanism then both > FDC and SCSI drivers have to solve the same issues.) Correct - IIRC the FDC registers are also accessible 'through' the ST-DMA registers only so the same problem WRT DMA status arises. > What compiler are you using, BTW? Are you still using the gcc-4.6.3 > m68k cross-compiler from kernel.org? I had to abandon it in order to > get my SCSI driver patches to work. 4.6.3 is the version I still use. You had trouble with that one? I recall some discussion on gcc versions on the m68k list a while back, just never seemed to see any problems... >> I need to think about that some more - if no DMA is in progress we >> can safely peek at the SCSI registers. So the logic could be >> changed to test for DMA operation first, and just try and service >> the interrupt if DMA wasn't active. >> > > OK, so based on the above, we handle the possible IDE interrupt > (without checking DMA registers), handle the possible FDC interrupt > (again without checking DMA registers) and finally handle the > possible SCSI interrupt. 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. > The core 5380 driver knows whether or not it has started a DMA. The > atari_scsi driver also knows that no other Falcon driver uses DMA. So > the atari_scsi handler only has to figure out whether the interrupt > was asserted by the ST-DMA chip or the 5380 chip, or neither. (The > "neither" possility arises when IDE ditches the the stdma.c lock > mechanism.) > > Without the stdma.c lock, any or all of these interrupts could be > asserted simultaneously, so the IDE and FDC drivers need to be able > to do the right thing in the presence of the other interrupts and do > so without accessing the ST-DMA chip. And the SCSI interrupt handler > needs to do the right thing when there is no DMA interrupt, and yet a > DMA is running. Again, whenever DMA was running (and it might still be), we have to stop it in order to look at FDC or SCSI registers. Utter braindamage, but > Perhaps we could reverse the algorithm in scsi_falcon_intr(): if > NCR5380_intr() completes with IRQ_HANDLED and the core 5380 driver is > no longer in DMA, then check the ST-DMA registers for errors etc. > > Alternatively, if NCR5380_intr() returns IRQ_NONE, then do nothing at > all, on the basis that the interrupt was handled by FDC or IDE. We may only call NCR5380_intr() if DMA hadn't been active (or we are sure it's completed, i.e. transfer address == end address. If that's even possible). If the DMA is still ongoing, we have the choice of punting (hoping for a command timeout to happen and clean up the mess), or terminate DMA (by selecting the SCSI chip registers instead of the DMA ones) and deal with the fallout. > In this situation, I gather that atari_scsi could miss out on a DMA > completion interrupt from the ST-DMA chip, which could lead to > command timeout? Doing nothing if DMA is enabled (and IDE had successfully handled an interrupt!) would cause us to miss a stacked interrupt, yes. Timeout would likely ensue. Not sure it's wise to kludge around that using a watchdog timer activated in case we're not sure of the DMA completion state... > >> If DMA has been in progress, I'm not sure that we can figure out if >> it's still active from looking at the status register (that is, >> whether bits 0 or 1 are set while DMA is ongoing). We'd have to >> peek at the DMA status register (or DMA address registers) without >> first stopping DMA, which the current driver does. The docs seem to >> advise against that. If DMA was in progress, stopping it would >> likely leave us with residual bytes to be transferred - > > I can't comment on the Profibuch doc or the ST-DMA chip details > (Andreas?) > > I suspect it has to be tried. Yes, I fear I'll have to just try what happens if SCSI and IDE raise an interrupt at the same time. For now, polled IDE might be working well enough (haven't seen a huge impact in IDE-only test workloads, I'll have to check impact on lots of seeks across the whole disk a lot harder though). I need to recheck the old IDE driver with my current combined test workload though (my second 4 GB Seagate disk has finally kicked the bucket, after the latest power brownout). > >> we'd have to handle that transfer as we would any other DMA error >> (from memory, probably best to retry the entire command, or >> transfer the remaining bytes using PIO if we're sure no bytes have >> been lost). >> > > Allowing the command to fail should be fine so long as the 5380 > driver sends the correct result code to the mid-layer. To attempt to > complete the command after a failed DMA is needless complexity, and > it's a trick that probably can't be pulled off reliably anyway. Yep, we've been there before (when my CT60 caused SCSI DMA errors and lost bytes). Not sure anymore what the correct result code was... Cheers, Michael ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-15 1:48 ` Michael Schmitz @ 2017-01-15 4:42 ` Finn Thain 2017-01-20 7:49 ` Michael Schmitz 0 siblings, 1 reply; 34+ messages in thread From: Finn Thain @ 2017-01-15 4:42 UTC (permalink / raw) To: Michael Schmitz Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab On Sun, 15 Jan 2017, Michael Schmitz wrote: > Am 15.01.2017 um 12:47 schrieb Finn Thain: > > > For the sake of discussion, I'll assume that the FDC driver will not > > be using DMA. (Perhaps FDC and SCSI can share the ST-DMA chip, using > > the present locking mechanism, but it would not simplify things much: > > when IDE no longer participates in that locking mechanism then both > > FDC and SCSI drivers have to solve the same issues.) > > Correct - IIRC the FDC registers are also accessible 'through' the > ST-DMA registers only so the same problem WRT DMA status arises. > I had not considered that limitation. > > What compiler are you using, BTW? Are you still using the gcc-4.6.3 > > m68k cross-compiler from kernel.org? I had to abandon it in order to > > get my SCSI driver patches to work. > > 4.6.3 is the version I still use. You had trouble with that one? I > recall some discussion on gcc versions on the m68k list a while back, > just never seemed to see any problems... > ... none that could be easily blamed on the compiler, anyway. The gcc 4.6.3 issue affecting my builds was discussed back in November. There are alternative compilers available: http://marc.info/?l=linux-m68k&m=147859596303294&w=2 http://marc.info/?l=linux-m68k&m=147859567903210&w=2 > >> I need to think about that some more - if no DMA is in progress we > >> can safely peek at the SCSI registers. So the logic could be changed > >> to test for DMA operation first, and just try and service the > >> interrupt if DMA wasn't active. > >> > > > > OK, so based on the above, we handle the possible IDE interrupt > > (without checking DMA registers), handle the possible FDC interrupt > > (again without checking DMA registers) and finally handle the possible > > SCSI interrupt. > > 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 On page 18 there is an algorithm for floppy writes which is interesting. 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 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. -- ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-15 4:42 ` Finn Thain @ 2017-01-20 7:49 ` Michael Schmitz 2017-01-21 7:37 ` Finn Thain 0 siblings, 1 reply; 34+ messages in thread From: Michael Schmitz @ 2017-01-20 7:49 UTC (permalink / raw) To: Finn Thain Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Hi Finn, 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. Polling instead of taking the interrupt does not change that fundamental problem (unless I'm missing something). > > 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. > > 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. > > 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. Cheers, Michael ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-20 7:49 ` Michael Schmitz @ 2017-01-21 7:37 ` Finn Thain 2017-01-23 8:04 ` Michael Schmitz 0 siblings, 1 reply; 34+ messages in thread From: Finn Thain @ 2017-01-21 7:37 UTC (permalink / raw) To: Michael Schmitz Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab 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 > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-21 7:37 ` Finn Thain @ 2017-01-23 8:04 ` Michael Schmitz 2017-01-26 8:47 ` Finn Thain 0 siblings, 1 reply; 34+ messages in thread From: Michael Schmitz @ 2017-01-23 8:04 UTC (permalink / raw) To: Finn Thain Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Hi Finn, Am 21.01.2017 um 20:37 schrieb Finn Thain: >> 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. My mistake - I was considering options to allow IDE to share the interrupt, without the complexities of the old locking scheme. > >> 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. I hadn't considered that. Can PDMA for Falcon SCSI coexist with interrupt-using DMA for TT SCSI in the same driver (i.e. as runtime options)? How much overhead and latency would polling for DMA completion add? >> 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). atari_irq_pending(IRQ_MFP_FSCSI) should show the interrupt pending condition if you want to poll for it. That's actually given me another idea to pursue - if we can ensure the IDE interrupt handler is always run first, and check whether the interrupt is still pending when the SCSI or floppy interrupt handler runs and DMA has been in progress, we should be able to avoid calling the respective handlers unnecessarily. (The output of atari_irq_pending() does not directly reflect the status of the MFP IRQ inputs - that would require testing bits in st_mfp.par_dt_reg instead. ) > I don't think the IDE/ATA driver needs to be included. atari_scsi and > ataflop would though (if both drivers need DMA transfers). If we manage to separate interrupt sharing from DMA access locking, IDE would not need to take part in the locking. I'm assuming that IDE can cope with spurious interrupts and won't get confused by a SCSI interrupt. >>> 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). Point taken - that problem still remains but is already being detected (though not properly handled IIRC), > 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 ... I think it could work both ways - polling for DMA completion or avoiding to call the SCSI interrupt handler the interrupt was caused by IDE only. But it's indeed time to put that to the test. Cheers, Michael ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-23 8:04 ` Michael Schmitz @ 2017-01-26 8:47 ` Finn Thain 2017-01-26 9:03 ` Geert Uytterhoeven 2017-01-27 4:28 ` Michael Schmitz 0 siblings, 2 replies; 34+ messages in thread From: Finn Thain @ 2017-01-26 8:47 UTC (permalink / raw) To: Michael Schmitz Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab On Mon, 23 Jan 2017, Michael Schmitz wrote: > > Am 21.01.2017 um 20:37 schrieb Finn Thain: > > > > > 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. > > I hadn't considered that. Can PDMA for Falcon SCSI coexist with > interrupt-using DMA for TT SCSI in the same driver (i.e. as runtime > options)? Sure, why not? > How much overhead and latency would polling for DMA completion add? > A polled DMA transfer should be faster than PDMA (i.e. mac_scsi, g_NCR5380 etc). mac_scsi gets about 0.5 MBps from PDMA with sg_tablesize == 1, and I hope that DMA could get twice that (notwithstanding dumb hardware design). This would imply CPU overhead that is half of that which mac_scsi incurs. That's the best case, but I see no reason to expect worse performance than PDMA gets. > atari_irq_pending(IRQ_MFP_FSCSI) should show the interrupt pending > condition if you want to poll for it. The difficulty will be arranging for disabled FDC & IDE interrupt sources during SCSI DMA, and disabled SCSI & IDE interrupt sources during FDC DMA. (Not all 5380 interrupts can be disabled; no idea about the IDE device or WD1772 FDC.) But if that is impossible, we just have to detect the short DMA that might result from an undesired interrupt. > That's actually given me another idea to pursue - if we can ensure the > IDE interrupt handler is always run first, There are no interrupts from the ATA driver you're testing, right? If you would re-introduce them, the whole polled DMA idea is moot. > and check whether the interrupt is still pending when the SCSI or floppy > interrupt handler runs and DMA has been in progress, we should be able > to avoid calling the respective handlers unnecessarily. > > (The output of atari_irq_pending() does not directly reflect the status > of the MFP IRQ inputs - that would require testing bits in > st_mfp.par_dt_reg instead. ) > > > I don't think the IDE/ATA driver needs to be included. atari_scsi and > > ataflop would though (if both drivers need DMA transfers). > > If we manage to separate interrupt sharing from DMA access locking, IDE > would not need to take part in the locking. I'm assuming that IDE can > cope with spurious interrupts and won't get confused by a SCSI > interrupt. > The ATA driver will never have to cope with a spurious interrupt under my simplifying assumptions discussed earlier, so the spurious interrupt question seems to belong to some alternative approach... > I think it could work both ways - polling for DMA completion or avoiding > to call the SCSI interrupt handler the interrupt was caused by IDE only. > But it's indeed time to put that to the test. > ... "Both ways"? I don't follow. I don't see how IDE can share the FDC and SCSI interrupt line without sharing the stdma.c locking scheme. What is the alternative approach (i.e not polled DMA) that you alude to? -- ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-26 8:47 ` Finn Thain @ 2017-01-26 9:03 ` Geert Uytterhoeven 2017-01-27 1:41 ` Finn Thain 2017-01-27 4:28 ` Michael Schmitz 1 sibling, 1 reply; 34+ messages in thread From: Geert Uytterhoeven @ 2017-01-26 9:03 UTC (permalink / raw) To: Finn Thain Cc: Michael Schmitz, Bartlomiej Zolnierkiewicz, Tejun Heo, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Hi Finn, On Thu, Jan 26, 2017 at 9:47 AM, Finn Thain <fthain@telegraphics.com.au> wrote: > The difficulty will be arranging for disabled FDC & IDE interrupt sources > during SCSI DMA, and disabled SCSI & IDE interrupt sources during FDC DMA. > (Not all 5380 interrupts can be disabled; no idea about the IDE device or > WD1772 FDC.) IDE interrupts are disabled at the device level. Unfortunately some hard drives (e.g. Western Digital Caviar) didn't honour the ATA disable IRQ bit, so they caused an interrupt deadlock if you probed for them on Amiga with the IDE interrupt enabled. The problem didn't show up on PC because they had no shared interrupts, while on A4000 the IDE interrupt is shared with Zorro Ethernet, which was still enabled. That was fixed (in 1995 or 1996?) by disabling the IDE interrupt at the IRQ controller level. 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-26 9:03 ` Geert Uytterhoeven @ 2017-01-27 1:41 ` Finn Thain 0 siblings, 0 replies; 34+ messages in thread From: Finn Thain @ 2017-01-27 1:41 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Schmitz, Bartlomiej Zolnierkiewicz, Tejun Heo, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab On Thu, 26 Jan 2017, Geert Uytterhoeven wrote: > Hi Finn, > > On Thu, Jan 26, 2017 at 9:47 AM, Finn Thain <fthain@telegraphics.com.au> > wrote: > > The difficulty will be arranging for disabled FDC & IDE interrupt > > sources during SCSI DMA, and disabled SCSI & IDE interrupt sources > > during FDC DMA. (Not all 5380 interrupts can be disabled; no idea > > about the IDE device or WD1772 FDC.) > > IDE interrupts are disabled at the device level. Unfortunately some hard > drives (e.g. Western Digital Caviar) didn't honour the ATA disable IRQ > bit, so they caused an interrupt deadlock if you probed for them on > Amiga with the IDE interrupt enabled. The problem didn't show up on PC > because they had no shared interrupts, while on A4000 the IDE interrupt > is shared with Zorro Ethernet, which was still enabled. > > That was fixed (in 1995 or 1996?) by disabling the IDE interrupt at the > IRQ controller level. > As I undersand it, masking these interrupts at the IRQ controller level won't work because these interrupt sources are all logically-OR'd together to generate the input that is to be polled during DMA. (And accessing the individual device registers is impossible during DMA!) Anyway, the end result is that both IDE and SCSI have the potential to (occasionally) mess up the DMA polling during and FDC or SCSI transfer. For SCSI, I believe that we can detect this when it happens (by checking the sector count registers in the DMA chip) and return the SCSI command with an error result, so that the mid-layer will retry it. I don't know how it would be handled in the case of an FDC transfer but I presume that a similar mechanism is available in the block layer. -- > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-26 8:47 ` Finn Thain 2017-01-26 9:03 ` Geert Uytterhoeven @ 2017-01-27 4:28 ` Michael Schmitz 2017-02-01 8:40 ` Finn Thain 1 sibling, 1 reply; 34+ messages in thread From: Michael Schmitz @ 2017-01-27 4:28 UTC (permalink / raw) To: Finn Thain Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Hi Finn, Am 26.01.2017 um 21:47 schrieb Finn Thain: >> I hadn't considered that. Can PDMA for Falcon SCSI coexist with >> interrupt-using DMA for TT SCSI in the same driver (i.e. as runtime >> options)? > > Sure, why not? > >> How much overhead and latency would polling for DMA completion add? >> > > A polled DMA transfer should be faster than PDMA (i.e. mac_scsi, g_NCR5380 > etc). mac_scsi gets about 0.5 MBps from PDMA with sg_tablesize == 1, and I > hope that DMA could get twice that (notwithstanding dumb hardware design). DMA contends with the processor use of the data bus but that's true for many DMA designs. Don't think Atari made any more dumb decisions (the opaque DMA FIFO is probably the worst feature there but that could be worked around at considerable expense by programming the DMA and the SCSI bus transfer for one additional 512 byte block. Let's not go there.) > This would imply CPU overhead that is half of that which mac_scsi incurs. > That's the best case, but I see no reason to expect worse performance than > PDMA gets. But how much more overhead would we have compared to using the SCSI interrupt to signal DMA completion? >> atari_irq_pending(IRQ_MFP_FSCSI) should show the interrupt pending >> condition if you want to poll for it. > > The difficulty will be arranging for disabled FDC & IDE interrupt sources > during SCSI DMA, and disabled SCSI & IDE interrupt sources during FDC DMA. > (Not all 5380 interrupts can be disabled; no idea about the IDE device or > WD1772 FDC.) > > But if that is impossible, we just have to detect the short DMA that might > result from an undesired interrupt. I think that's infeasible - IDE interrupts could be disabled at disk level as Geert suggests but I don't think there is a kernel API for other drivers to do so? At the IRQ controller level, it's all or none due to the wired-OR design as you pointed out in the reply to Geert's mail. >> That's actually given me another idea to pursue - if we can ensure the >> IDE interrupt handler is always run first, > > There are no interrupts from the ATA driver you're testing, right? If you > would re-introduce them, the whole polled DMA idea is moot. The libata driver currently does disable the IDE interrupt and uses polling, but I'd like to change that if at all possible. Sorry I didn't make that clear. As far as I could see during my testing, the current libata driver coexists just fine with interrupt driven SCSI operation. I've once seen a 'lost arbitration' message in the very first test when loading the SCSI driver, but that's not been repeated. No problems seen otherwise. >> If we manage to separate interrupt sharing from DMA access locking, IDE >> would not need to take part in the locking. I'm assuming that IDE can >> cope with spurious interrupts and won't get confused by a SCSI >> interrupt. >> > > The ATA driver will never have to cope with a spurious interrupt under my > simplifying assumptions discussed earlier, so the spurious interrupt > question seems to belong to some alternative approach... I was assuming IDE could have interrupts reenabled in libata for that discussion. >> I think it could work both ways - polling for DMA completion or avoiding >> to call the SCSI interrupt handler the interrupt was caused by IDE only. >> But it's indeed time to put that to the test. >> > > ... "Both ways"? I don't follow. I don't see how IDE can share the FDC and > SCSI interrupt line without sharing the stdma.c locking scheme. What is > the alternative approach (i.e not polled DMA) that you alude to? Since IDE does not use the ST-DMA and does not share any registers with ST-DMA, peeking at the IDE status register in order to decide whether the interrupt was raised by the IDE interface won't hurt the running DMA process (regardless of whether FDC or SCSI started it). Nor will servicing the IDE interrupt. If at the end of the IDE interrupt processing the interrupt status is cleared in the IDE interface, the interrupt line should go high again before the IDE inthandler returns. If we can ensure that the FDC/SCSI interrupt handler runs after the IDE handler, we can then make that handler check the interrupt line status and bail out if there's nothing to be done. (For the sake of simplicity, this check can be done in stdma_int() since we need to retain mutual locking of the DMA interface by SCSI and FDC anyway.) We can ensure the IDE interrupt is called first by using a special interrupt controller to register separate IDE and FDC/SCSI interrupts with (I've done that to provide distinct interrupt numbers and handlers for the timer D interrupt that's used to poll ethernet and USB interface status on the ROM port). That way, we can ensure IDE interrupts do not step on the ST-DMA state, and all that remains are premature SCSI interrupts terminating DMA transfer (which we already face anyway). Am I missing a potential race here? Does IDE send the next request off to the disk from inside the interrupt handler so we could see IDE immediately raise the next interrupt? In that case, we'd also need to check the IDE interrupt status in the interface status register, and bail out for another pass through the IDE/FDC/SCSI handlers until IDE is no longer posting an interrupt... Cheers, Michael ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-01-27 4:28 ` Michael Schmitz @ 2017-02-01 8:40 ` Finn Thain 2017-02-01 8:45 ` Geert Uytterhoeven 2017-02-02 7:48 ` Michael Schmitz 0 siblings, 2 replies; 34+ messages in thread From: Finn Thain @ 2017-02-01 8:40 UTC (permalink / raw) To: Michael Schmitz Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab On Fri, 27 Jan 2017, Michael Schmitz wrote: > Am 26.01.2017 um 21:47 schrieb Finn Thain: > > > This would imply CPU overhead that is half of that which mac_scsi > > incurs. That's the best case, but I see no reason to expect worse > > performance than PDMA gets. > > But how much more overhead would we have compared to using the SCSI > interrupt to signal DMA completion? > I imagine that contention for the CPU bus would be a problem if we polled the interrupt flag without any delay between iterations. With a small delay I think the overhead would be comparable with PDMA and therefore tolerable. > The libata driver currently does disable the IDE interrupt and uses > polling, but I'd like to change that if at all possible. Sorry I didn't > make that clear. > [snip] > Since IDE does not use the ST-DMA and does not share any registers with > ST-DMA, peeking at the IDE status register in order to decide whether > the interrupt was raised by the IDE interface won't hurt the running DMA > process (regardless of whether FDC or SCSI started it). Nor will > servicing the IDE interrupt. > Maybe we can just call the IDE handler from the ST-DMA handler regardless of the status register. For a shared interrupt handler this should work okay. (BTW, where is the IDE status register found anyway?) > If at the end of the IDE interrupt processing the interrupt status is > cleared in the IDE interface, the interrupt line should go high again > before the IDE inthandler returns. > On page 2 of the schematic, MFP pin I5 is wired to the output of the logical OR combination of the IDEIRQ and XDISKINT signals (actually active-low signals fed into an AND gate). The pin is edge-triggered. This is just like the wired-OR Nubus slot IRQs connected to the Mac's VIA pin. The handler must ack all asserted IRQs. Otherwise there will be no more edges and no more interrupts. This means looping over the IDE, FDC/SCSI DMA handlers until they all return IRQ_NONE. (Or equivalently, looping over the IRQ flags in the device registers until they are all de-asserted.) BTW, this makes me think that the stdma.c mechanism is already flawed, since stdma_int() can cause only one of IDEIRQ and XDISKINT to become inactive, but not both. That's fine as long as no device raises IRQ until it's driver acquires the stdma lock -- but we know this is not true for the 5380 bus reset interrupt and it isn't true for IDE devices either (based on Geert's email in this thread). > If we can ensure that the FDC/SCSI interrupt handler runs after the IDE > handler, we can then make that handler check the interrupt line status > and bail out if there's nothing to be done. (For the sake of simplicity, > this check can be done in stdma_int() since we need to retain mutual > locking of the DMA interface by SCSI and FDC anyway.) > > We can ensure the IDE interrupt is called first by using a special > interrupt controller to register separate IDE and FDC/SCSI interrupts > with (I've done that to provide distinct interrupt numbers and handlers > for the timer D interrupt that's used to poll ethernet and USB interface > status on the ROM port). > > That way, we can ensure IDE interrupts do not step on the ST-DMA state, > and all that remains are premature SCSI interrupts terminating DMA > transfer (which we already face anyway). > > Am I missing a potential race here? Does IDE send the next request off > to the disk from inside the interrupt handler so we could see IDE > immediately raise the next interrupt? In that case, we'd also need to > check the IDE interrupt status in the interface status register, and > bail out for another pass through the IDE/FDC/SCSI handlers until IDE is > no longer posting an interrupt... > I don't know anything about IDE so I can't comment on this particular scenario (IDE interrupt handler causing IDE interrupt). The race condition may be only theoretical. What you seem to be aiming at is an algorithm to ensure that no DMA interrupt is handled whilst an IDE interrupt is pending. Taking into account the logical OR issue, one could imagine a handler for the IRQ_MFP_FSCSI interrupt something like the following. (This code is probably useless for implementing your interrupt controller, but I hope it illustrates some of the issues.) do { handled = ata_handler(irq, ata_dev); if (handled == IRQ_NONE && atari_irq_pending(IRQ_MFP_FSCSI)) handled |= stdma_int(irq, stdma_dev); } while (handled != IRQ_NONE); Clearly this is not free from race conditions. The other problem is the use of atari_irq_pending(IRQ_MFP_FSCSI). It tells us when an edge appears but doesn't tell us about the present state of the IRQ output pins on the NCR5380 or the WD1772. We can't access the device registers so st_mfp.par_dt_reg & BIT(5) must be used instead of atari_irq_pending(IRQ_MFP_FSCSI); The simplest approach is to treat it like a shared interrupt, with a loop to account for the logical OR: do { handled = ata_handler(irq, ata_dev) | scsi_falcon_intr(irq, scsi_dev) | fdc_handler(irq, fdc_dev); } while (handled != IRQ_NONE); This should work fine with polled DMA (and might even allow the flawed stdma.c lock mechanism to be eliminated) but it can't work with your scheme because scsi_falcon_intr() assumes exclusive access to the IRQ; hence it must not be called unless there is an actual 5380 or DMA interrupt. I don't know which scheme is better. Mine is simpler and probably free of race conditions but does burn some CPU time. Your scheme is more complicated. -- > Cheers, > > Michael > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-02-01 8:40 ` Finn Thain @ 2017-02-01 8:45 ` Geert Uytterhoeven 2017-02-02 7:48 ` Michael Schmitz 1 sibling, 0 replies; 34+ messages in thread From: Geert Uytterhoeven @ 2017-02-01 8:45 UTC (permalink / raw) To: Finn Thain Cc: Michael Schmitz, Bartlomiej Zolnierkiewicz, Tejun Heo, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Hi Finn, On Wed, Feb 1, 2017 at 9:40 AM, Finn Thain <fthain@telegraphics.com.au> wrote: > okay. (BTW, where is the IDE status register found anyway?) In the IDE device. 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-02-01 8:40 ` Finn Thain 2017-02-01 8:45 ` Geert Uytterhoeven @ 2017-02-02 7:48 ` Michael Schmitz 1 sibling, 0 replies; 34+ messages in thread From: Michael Schmitz @ 2017-02-02 7:48 UTC (permalink / raw) To: Finn Thain Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab Hi Finn, Am 01.02.2017 um 21:40 schrieb Finn Thain: > > On Fri, 27 Jan 2017, Michael Schmitz wrote: > >> Am 26.01.2017 um 21:47 schrieb Finn Thain: >> >>> This would imply CPU overhead that is half of that which mac_scsi >>> incurs. That's the best case, but I see no reason to expect worse >>> performance than PDMA gets. >> >> But how much more overhead would we have compared to using the SCSI >> interrupt to signal DMA completion? >> > > I imagine that contention for the CPU bus would be a problem if we polled > the interrupt flag without any delay between iterations. With a small > delay I think the overhead would be comparable with PDMA and therefore > tolerable. The first delay can even be quite long, estimated based on the transfer size. >> Since IDE does not use the ST-DMA and does not share any registers with >> ST-DMA, peeking at the IDE status register in order to decide whether >> the interrupt was raised by the IDE interface won't hurt the running DMA >> process (regardless of whether FDC or SCSI started it). Nor will >> servicing the IDE interrupt. >> > > Maybe we can just call the IDE handler from the ST-DMA handler regardless > of the status register. For a shared interrupt handler this should work > okay. (BTW, where is the IDE status register found anyway?) We could do that as well, true. Easier to implement on the quick. >> If at the end of the IDE interrupt processing the interrupt status is >> cleared in the IDE interface, the interrupt line should go high again >> before the IDE inthandler returns. >> > > On page 2 of the schematic, MFP pin I5 is wired to the output of the > logical OR combination of the IDEIRQ and XDISKINT signals (actually > active-low signals fed into an AND gate). The pin is edge-triggered. > > This is just like the wired-OR Nubus slot IRQs connected to the Mac's VIA > pin. The handler must ack all asserted IRQs. Otherwise there will be no > more edges and no more interrupts. Quite right - that's why I mentioned monitoring the IRQ signal status in the GPIO register. > This means looping over the IDE, FDC/SCSI DMA handlers until they all > return IRQ_NONE. (Or equivalently, looping over the IRQ flags in the > device registers until they are all de-asserted.) Looping over the handlers risks stopping the DMA without need (except for IDE). > BTW, this makes me think that the stdma.c mechanism is already flawed, > since stdma_int() can cause only one of IDEIRQ and XDISKINT to become > inactive, but not both. That's fine as long as no device raises IRQ until > it's driver acquires the stdma lock -- but we know this is not true for > the 5380 bus reset interrupt and it isn't true for IDE devices either > (based on Geert's email in this thread). The initial bus reset code had safeguards against raising an interrupt - the IRQ was 'turned off' while the bus reset was executed. Maybe we need something like that, at least in the case where SCSI does not hold the ST-DMA lock. >> If we can ensure that the FDC/SCSI interrupt handler runs after the IDE >> handler, we can then make that handler check the interrupt line status >> and bail out if there's nothing to be done. (For the sake of simplicity, >> this check can be done in stdma_int() since we need to retain mutual >> locking of the DMA interface by SCSI and FDC anyway.) >> >> We can ensure the IDE interrupt is called first by using a special >> interrupt controller to register separate IDE and FDC/SCSI interrupts >> with (I've done that to provide distinct interrupt numbers and handlers >> for the timer D interrupt that's used to poll ethernet and USB interface >> status on the ROM port). >> >> That way, we can ensure IDE interrupts do not step on the ST-DMA state, >> and all that remains are premature SCSI interrupts terminating DMA >> transfer (which we already face anyway). >> >> Am I missing a potential race here? Does IDE send the next request off >> to the disk from inside the interrupt handler so we could see IDE >> immediately raise the next interrupt? In that case, we'd also need to >> check the IDE interrupt status in the interface status register, and >> bail out for another pass through the IDE/FDC/SCSI handlers until IDE is >> no longer posting an interrupt... >> > > I don't know anything about IDE so I can't comment on this particular > scenario (IDE interrupt handler causing IDE interrupt). The race condition > may be only theoretical. > > What you seem to be aiming at is an algorithm to ensure that no DMA > interrupt is handled whilst an IDE interrupt is pending. Taking into Whilst the IDE interrupt is the only one pending, to be precise. > account the logical OR issue, one could imagine a handler for the > IRQ_MFP_FSCSI interrupt something like the following. (This code is > probably useless for implementing your interrupt controller, but I hope it > illustrates some of the issues.) > > do { > handled = ata_handler(irq, ata_dev); > if (handled == IRQ_NONE && atari_irq_pending(IRQ_MFP_FSCSI)) > handled |= stdma_int(irq, stdma_dev); > } while (handled != IRQ_NONE); Not quite - we can't be certain that there aren't actually two interrupts pending (say IDE interrupts first, and while we service that interrupt, SCSI interrupts next but since the IRQ signal remains low, we don't trigger another interrupt). So we must run the loop for both IDE and stdma_int for as long as the IRQ signal remains low. I had missed the case where SCSI interrupts first and the IDE interrupt only comes in while the SCSI inthandler is running - need to think a bit more about this. > Clearly this is not free from race conditions. The other problem is the > use of atari_irq_pending(IRQ_MFP_FSCSI). It tells us when an edge appears > but doesn't tell us about the present state of the IRQ output pins on the > NCR5380 or the WD1772. We can't access the device registers so > st_mfp.par_dt_reg & BIT(5) must be used instead of > atari_irq_pending(IRQ_MFP_FSCSI); I've written a function to that effect, and tested it in the current Falcon inthandler (without sharing the interrupt truly yet). > The simplest approach is to treat it like a shared interrupt, with a loop > to account for the logical OR: > > do { > handled = ata_handler(irq, ata_dev) | > scsi_falcon_intr(irq, scsi_dev) | > fdc_handler(irq, fdc_dev); > } while (handled != IRQ_NONE); We can never have both SCSI and FDC interrupt at the same time (both register access and DMA setup are shared between the two). Best retain stdma_lock() for these two. > This should work fine with polled DMA (and might even allow the flawed > stdma.c lock mechanism to be eliminated) but it can't work with your > scheme because scsi_falcon_intr() assumes exclusive access to the IRQ; > hence it must not be called unless there is an actual 5380 or DMA > interrupt. Correct - polled DMA might be easier to handle here. > I don't know which scheme is better. Mine is simpler and probably free of > race conditions but does burn some CPU time. Your scheme is more > complicated. Meaning likely to race... I'll have to test this with IDE removed from the locking scheme, and we'll hopefully see races pretty quick. Got a few new ideas to try now, thanks! Cheers, Michael ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2016-12-30 14:01 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Bartlomiej Zolnierkiewicz ` (4 preceding siblings ...) 2017-01-05 21:01 ` Michael Schmitz @ 2017-01-10 16:11 ` Tejun Heo 2017-02-15 8:45 ` Geert Uytterhoeven 6 siblings, 0 replies; 34+ messages in thread From: Tejun Heo @ 2017-01-10 16:11 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel On Fri, Dec 30, 2016 at 03:01:15PM +0100, Bartlomiej Zolnierkiewicz wrote: > Hi, > > This patchset adds m68k/Atari Falcon PATA support to libata. > The major difference in the new libata's pata_falcon host > driver when compared to legacy IDE's falconide host driver is > that we are using polled PIO mode and thus avoiding the need > for STDMA locking magic altogether. > > Tested under ARAnyM emulator. Applied 1-3 to libata/for-4.11. Thanks. -- tejun ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2016-12-30 14:01 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Bartlomiej Zolnierkiewicz ` (5 preceding siblings ...) 2017-01-10 16:11 ` Tejun Heo @ 2017-02-15 8:45 ` Geert Uytterhoeven 2017-02-20 18:15 ` Bartlomiej Zolnierkiewicz 6 siblings, 1 reply; 34+ messages in thread From: Geert Uytterhoeven @ 2017-02-15 8:45 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Tejun Heo, Michael Schmitz, linux-ide, linux-m68k, linux-kernel, Jens Axboe On Fri, Dec 30, 2016 at 3:01 PM, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote: > This patchset adds m68k/Atari Falcon PATA support to libata. > The major difference in the new libata's pata_falcon host > driver when compared to legacy IDE's falconide host driver is > that we are using polled PIO mode and thus avoiding the need > for STDMA locking magic altogether. > > Tested under ARAnyM emulator. > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > > > Bartlomiej Zolnierkiewicz (3): > ata: allow subsystem to be used on m68k arch > ata: pass queued command to ->sff_data_xfer method > ata: add Atari Falcon PATA controller driver drivers/ata/pata_falcon.c:57:18: error: 'struct request' has no member named 'cmd_type' drivers/ata/pata_falcon.c:57:32: error: 'REQ_TYPE_FS' undeclared (first use in this function) http://kisskb.ellerman.id.au/kisskb/buildresult/12936876/ 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-02-15 8:45 ` Geert Uytterhoeven @ 2017-02-20 18:15 ` Bartlomiej Zolnierkiewicz 2017-02-21 22:18 ` Tejun Heo 0 siblings, 1 reply; 34+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2017-02-20 18:15 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Tejun Heo, Michael Schmitz, linux-ide, linux-m68k, linux-kernel, Jens Axboe, Christoph Hellwig Hi, On Wednesday, February 15, 2017 09:45:53 AM Geert Uytterhoeven wrote: > On Fri, Dec 30, 2016 at 3:01 PM, Bartlomiej Zolnierkiewicz > <b.zolnierkie@samsung.com> wrote: > > This patchset adds m68k/Atari Falcon PATA support to libata. > > The major difference in the new libata's pata_falcon host > > driver when compared to legacy IDE's falconide host driver is > > that we are using polled PIO mode and thus avoiding the need > > for STDMA locking magic altogether. > > > > Tested under ARAnyM emulator. > > > > Best regards, > > -- > > Bartlomiej Zolnierkiewicz > > Samsung R&D Institute Poland > > Samsung Electronics > > > > > > Bartlomiej Zolnierkiewicz (3): > > ata: allow subsystem to be used on m68k arch > > ata: pass queued command to ->sff_data_xfer method > > ata: add Atari Falcon PATA controller driver > > drivers/ata/pata_falcon.c:57:18: error: 'struct request' has no member > named 'cmd_type' > drivers/ata/pata_falcon.c:57:32: error: 'REQ_TYPE_FS' undeclared > (first use in this function) > > http://kisskb.ellerman.id.au/kisskb/buildresult/12936876/ From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Subject: [PATCH] pata_falcon: build fix for block layer changes commit aebf526b53ae ("block: fold cmd_type into the REQ_OP_ space") from the block tree removes cmd_type so pata_falcon needs the following trivial update to make it build again. Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- Tejun, I guess that you may need to fold this fix into your pull request for 4.11 (block layer pull request has been already sent by Jens so it will be most likely merged first). drivers/ata/pata_falcon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/drivers/ata/pata_falcon.c =================================================================== --- a/drivers/ata/pata_falcon.c 2017-02-20 18:40:04.174989455 +0100 +++ b/drivers/ata/pata_falcon.c 2017-02-20 18:42:59.482993870 +0100 @@ -54,7 +54,7 @@ static unsigned int pata_falcon_data_xfe bool swap = 1; if (dev->class == ATA_DEV_ATA && cmd && cmd->request && - cmd->request->cmd_type == REQ_TYPE_FS) + !blk_rq_is_passthrough(cmd->request)) swap = 0; /* Transfer multiple of 2 bytes */ ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 2017-02-20 18:15 ` Bartlomiej Zolnierkiewicz @ 2017-02-21 22:18 ` Tejun Heo 0 siblings, 0 replies; 34+ messages in thread From: Tejun Heo @ 2017-02-21 22:18 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k, linux-kernel, Jens Axboe, Christoph Hellwig On Mon, Feb 20, 2017 at 07:15:34PM +0100, Bartlomiej Zolnierkiewicz wrote: > From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Subject: [PATCH] pata_falcon: build fix for block layer changes > > commit aebf526b53ae ("block: fold cmd_type into the REQ_OP_ > space") from the block tree removes cmd_type so pata_falcon > needs the following trivial update to make it build again. > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > Tejun, I guess that you may need to fold this fix into > your pull request for 4.11 (block layer pull request has > been already sent by Jens so it will be most likely merged > first). > > drivers/ata/pata_falcon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: b/drivers/ata/pata_falcon.c > =================================================================== > --- a/drivers/ata/pata_falcon.c 2017-02-20 18:40:04.174989455 +0100 > +++ b/drivers/ata/pata_falcon.c 2017-02-20 18:42:59.482993870 +0100 > @@ -54,7 +54,7 @@ static unsigned int pata_falcon_data_xfe > bool swap = 1; > > if (dev->class == ATA_DEV_ATA && cmd && cmd->request && > - cmd->request->cmd_type == REQ_TYPE_FS) > + !blk_rq_is_passthrough(cmd->request)) Sent it along with the pull request. Thanks. -- tejun ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2017-02-21 22:18 UTC | newest] Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20161230140139epcas5p160eda5a6a77be084e21f12002c85cc2a@epcas5p1.samsung.com> 2016-12-30 14:01 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support 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:12 ` Christoph Hellwig [not found] ` <CGME20161230171438epcas1p3c1d8ea8e4c77d796b81f7130c5e61d3f@epcas1p3.samsung.com> 2016-12-30 17:14 ` Bartlomiej Zolnierkiewicz 2017-01-08 10:08 ` Christoph Hellwig [not found] ` <CGME20170109160128epcas1p36a0a8f79b32e5024ffa480fd848e3a79@epcas1p3.samsung.com> 2017-01-09 16:01 ` Bartlomiej Zolnierkiewicz 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 [not found] ` <CGME20161230140147epcas5p1fa7e99f39921a8ee90aabd59ff7b7645@epcas5p1.samsung.com> 2016-12-30 14:01 ` [PATCH 3/3] ata: add Atari Falcon PATA controller driver Bartlomiej Zolnierkiewicz 2017-01-03 10:49 ` [PATCH 0/3] ata: add m68k/Atari Falcon PATA support Geert Uytterhoeven 2017-01-09 16:11 ` Bartlomiej Zolnierkiewicz 2017-01-10 16:09 ` Tejun Heo 2017-01-05 21:01 ` Michael Schmitz 2017-01-10 12:53 ` Bartlomiej Zolnierkiewicz 2017-01-10 20:02 ` Michael Schmitz 2017-01-13 2:33 ` Finn Thain 2017-01-14 8:55 ` Michael Schmitz 2017-01-14 23:47 ` Finn Thain 2017-01-15 1:48 ` Michael Schmitz 2017-01-15 4:42 ` Finn Thain 2017-01-20 7:49 ` Michael Schmitz 2017-01-21 7:37 ` Finn Thain 2017-01-23 8:04 ` Michael Schmitz 2017-01-26 8:47 ` Finn Thain 2017-01-26 9:03 ` Geert Uytterhoeven 2017-01-27 1:41 ` Finn Thain 2017-01-27 4:28 ` Michael Schmitz 2017-02-01 8:40 ` Finn Thain 2017-02-01 8:45 ` Geert Uytterhoeven 2017-02-02 7:48 ` Michael Schmitz 2017-01-10 16:11 ` Tejun Heo 2017-02-15 8:45 ` Geert Uytterhoeven 2017-02-20 18:15 ` Bartlomiej Zolnierkiewicz 2017-02-21 22:18 ` Tejun Heo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).