Messages in this thread | | | Subject | Re: [PATCH] m68k/mac: Replace macide driver with generic platform driver | From | Michael Schmitz <> | Date | Mon, 26 Apr 2021 10:24:19 +1200 |
| |
Hi Finn,
Am 25.04.2021 um 21:06 schrieb Finn Thain: > This was tested on my Quadra 630. I haven't tested it on my PowerBook 150 > because I don't have a RAM adapter board for it. > > Apparently, the hardware I tested doesn't need macide_clear_irq() or > macide_test_irq() -- if it did, the generic driver would not have worked. > It's possible that those routines are needed for the PowerBook 150 but > we can cross that bridge if and when we come to it. > > BTW, macide_clear_irq() appears to suffer from a race condition. The write > to the interrupt flags register could easily have unintended side effects > as it may alter other flag bits. Fortunately, all of the other bits are > unused by Linux. Moreover, when tested on my Quadra 630, that assignment > (*ide_ifr &= ~0x20) was observed to have no effect on bit 5.
You are worried that the bit clear might not be done atomic?
Regarding the missing effect of clearing bit 5, I suspect this has never before been tested rigorously (I don't remember ever using a Quadra 630). The logic attempted to replicate what the MacOS IDE driver did. The Linux IDE driver has its own way to test and clear a port's interrupt flag, so this extra code can quite probably go.
Thanks for cleaning this up!
Reviewed-by: Michael Schmitz <schmitzmic@gmail.com>
> > Cc: Michael Schmitz <schmitzmic@gmail.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Joshua Thompson <funaho@jurai.org> > Cc: "David S. Miller" <davem@davemloft.net> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au> > --- > arch/m68k/configs/mac_defconfig | 1 - > arch/m68k/configs/multi_defconfig | 1 - > arch/m68k/mac/config.c | 24 +++-- > drivers/ide/Kconfig | 14 --- > drivers/ide/Makefile | 1 - > drivers/ide/macide.c | 161 ------------------------------ > 6 files changed, 14 insertions(+), 188 deletions(-) > delete mode 100644 drivers/ide/macide.c > > diff --git a/arch/m68k/configs/mac_defconfig b/arch/m68k/configs/mac_defconfig > index f6d50b3fe8c2..252596991e4f 100644 > --- a/arch/m68k/configs/mac_defconfig > +++ b/arch/m68k/configs/mac_defconfig > @@ -318,7 +318,6 @@ CONFIG_IDE=y > CONFIG_IDE_GD_ATAPI=y > CONFIG_BLK_DEV_IDECD=y > CONFIG_BLK_DEV_PLATFORM=y > -CONFIG_BLK_DEV_MAC_IDE=y > CONFIG_RAID_ATTRS=m > CONFIG_SCSI=y > CONFIG_BLK_DEV_SD=y > diff --git a/arch/m68k/configs/multi_defconfig b/arch/m68k/configs/multi_defconfig > index 0e067b4320cd..697030472a83 100644 > --- a/arch/m68k/configs/multi_defconfig > +++ b/arch/m68k/configs/multi_defconfig > @@ -350,7 +350,6 @@ CONFIG_BLK_DEV_PLATFORM=y > CONFIG_BLK_DEV_GAYLE=y > CONFIG_BLK_DEV_BUDDHA=y > CONFIG_BLK_DEV_FALCON_IDE=y > -CONFIG_BLK_DEV_MAC_IDE=y > CONFIG_BLK_DEV_Q40IDE=y > CONFIG_RAID_ATTRS=m > CONFIG_SCSI=y > diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c > index 1cdac959bd91..5d16f9b47aa9 100644 > --- a/arch/m68k/mac/config.c > +++ b/arch/m68k/mac/config.c > @@ -933,13 +933,15 @@ static const struct resource mac_scsi_ccl_rsrc[] __initconst = { > }, > }; > > -static const struct resource mac_ide_quadra_rsrc[] __initconst = { > - DEFINE_RES_MEM(0x50F1A000, 0x104), > +static const struct resource mac_pata_quadra_rsrc[] __initconst = { > + DEFINE_RES_MEM(0x50F1A000, 0x38), > + DEFINE_RES_MEM(0x50F1A038, 0x04), > DEFINE_RES_IRQ(IRQ_NUBUS_F), > }; > > -static const struct resource mac_ide_pb_rsrc[] __initconst = { > - DEFINE_RES_MEM(0x50F1A000, 0x104), > +static const struct resource mac_pata_pb_rsrc[] __initconst = { > + DEFINE_RES_MEM(0x50F1A000, 0x38), > + DEFINE_RES_MEM(0x50F1A038, 0x04), > DEFINE_RES_IRQ(IRQ_NUBUS_C), > }; > > @@ -949,7 +951,7 @@ static const struct resource mac_pata_baboon_rsrc[] __initconst = { > DEFINE_RES_IRQ(IRQ_BABOON_1), > }; > > -static const struct pata_platform_info mac_pata_baboon_data __initconst = { > +static const struct pata_platform_info mac_pata_data __initconst = { > .ioport_shift = 2, > }; > > @@ -1067,17 +1069,19 @@ int __init mac_platform_init(void) > > switch (macintosh_config->ide_type) { > case MAC_IDE_QUADRA: > - platform_device_register_simple("mac_ide", -1, > - mac_ide_quadra_rsrc, ARRAY_SIZE(mac_ide_quadra_rsrc)); > + platform_device_register_resndata(NULL, "pata_platform", -1, > + mac_pata_quadra_rsrc, ARRAY_SIZE(mac_pata_quadra_rsrc), > + &mac_pata_data, sizeof(mac_pata_data)); > break; > case MAC_IDE_PB: > - platform_device_register_simple("mac_ide", -1, > - mac_ide_pb_rsrc, ARRAY_SIZE(mac_ide_pb_rsrc)); > + platform_device_register_resndata(NULL, "pata_platform", -1, > + mac_pata_pb_rsrc, ARRAY_SIZE(mac_pata_pb_rsrc), > + &mac_pata_data, sizeof(mac_pata_data)); > break; > case MAC_IDE_BABOON: > platform_device_register_resndata(NULL, "pata_platform", -1, > mac_pata_baboon_rsrc, ARRAY_SIZE(mac_pata_baboon_rsrc), > - &mac_pata_baboon_data, sizeof(mac_pata_baboon_data)); > + &mac_pata_data, sizeof(mac_pata_data)); > break; > } > > diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig > index 19abf11c84c8..8ce4a5878d0c 100644 > --- a/drivers/ide/Kconfig > +++ b/drivers/ide/Kconfig > @@ -739,20 +739,6 @@ config BLK_DEV_FALCON_IDE > disks, CD-ROM drives, etc.) that are connected to the on-board IDE > interface. > > -config BLK_DEV_MAC_IDE > - tristate "Macintosh Quadra/Powerbook IDE interface support" > - depends on MAC > - help > - This is the IDE driver for the on-board IDE interface on some m68k > - Macintosh models, namely Quadra/Centris 630, Performa 588 and > - Powerbook 150. The IDE interface on the Powerbook 190 is not > - supported by this driver and requires BLK_DEV_PLATFORM or > - PATA_PLATFORM. > - > - Say Y if you have such an Macintosh model and want to use IDE > - devices (hard disks, CD-ROM drives, etc.) that are connected to the > - on-board IDE interface. > - > config BLK_DEV_Q40IDE > tristate "Q40/Q60 IDE interface support" > depends on Q40 > diff --git a/drivers/ide/Makefile b/drivers/ide/Makefile > index 2605b3cdaf47..45a1c0463bed 100644 > --- a/drivers/ide/Makefile > +++ b/drivers/ide/Makefile > @@ -29,7 +29,6 @@ obj-$(CONFIG_BLK_DEV_4DRIVES) += ide-4drives.o > > obj-$(CONFIG_BLK_DEV_GAYLE) += gayle.o > obj-$(CONFIG_BLK_DEV_FALCON_IDE) += falconide.o > -obj-$(CONFIG_BLK_DEV_MAC_IDE) += macide.o > obj-$(CONFIG_BLK_DEV_Q40IDE) += q40ide.o > obj-$(CONFIG_BLK_DEV_BUDDHA) += buddha.o > > diff --git a/drivers/ide/macide.c b/drivers/ide/macide.c > deleted file mode 100644 > index 8d2bf73bc548..000000000000 > --- a/drivers/ide/macide.c > +++ /dev/null > @@ -1,161 +0,0 @@ > -/* > - * Macintosh IDE Driver > - * > - * Copyright (C) 1998 by Michael Schmitz > - * > - * This driver was written based on information obtained from the MacOS IDE > - * driver binary by Mikael Forselius > - * > - * 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/types.h> > -#include <linux/mm.h> > -#include <linux/interrupt.h> > -#include <linux/blkdev.h> > -#include <linux/delay.h> > -#include <linux/ide.h> > -#include <linux/module.h> > -#include <linux/platform_device.h> > - > -#include <asm/macintosh.h> > - > -#define DRV_NAME "mac_ide" > - > -#define IDE_BASE 0x50F1A000 /* Base address of IDE controller */ > - > -/* > - * Generic IDE registers as offsets from the base > - * These match MkLinux so they should be correct. > - */ > - > -#define IDE_CONTROL 0x38 /* control/altstatus */ > - > -/* > - * Mac-specific registers > - */ > - > -/* > - * this register is odd; it doesn't seem to do much and it's > - * not word-aligned like virtually every other hardware register > - * on the Mac... > - */ > - > -#define IDE_IFR 0x101 /* (0x101) IDE interrupt flags on Quadra: > - * > - * Bit 0+1: some interrupt flags > - * Bit 2+3: some interrupt enable > - * Bit 4: ?? > - * Bit 5: IDE interrupt flag (any hwif) > - * Bit 6: maybe IDE interrupt enable (any hwif) ?? > - * Bit 7: Any interrupt condition > - */ > - > -volatile unsigned char *ide_ifr = (unsigned char *) (IDE_BASE + IDE_IFR); > - > -int macide_test_irq(ide_hwif_t *hwif) > -{ > - if (*ide_ifr & 0x20) > - return 1; > - return 0; > -} > - > -static void macide_clear_irq(ide_drive_t *drive) > -{ > - *ide_ifr &= ~0x20; > -} > - > -static void __init macide_setup_ports(struct ide_hw *hw, unsigned long base, > - int irq) > -{ > - int i; > - > - memset(hw, 0, sizeof(*hw)); > - > - for (i = 0; i < 8; i++) > - hw->io_ports_array[i] = base + i * 4; > - > - hw->io_ports.ctl_addr = base + IDE_CONTROL; > - > - hw->irq = irq; > -} > - > -static const struct ide_port_ops macide_port_ops = { > - .clear_irq = macide_clear_irq, > - .test_irq = macide_test_irq, > -}; > - > -static const struct ide_port_info macide_port_info = { > - .port_ops = &macide_port_ops, > - .host_flags = IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA, > - .irq_flags = IRQF_SHARED, > - .chipset = ide_generic, > -}; > - > -static const char *mac_ide_name[] = > - { "Quadra", "Powerbook", "Powerbook Baboon" }; > - > -/* > - * Probe for a Macintosh IDE interface > - */ > - > -static int mac_ide_probe(struct platform_device *pdev) > -{ > - struct resource *mem, *irq; > - struct ide_hw hw, *hws[] = { &hw }; > - struct ide_port_info d = macide_port_info; > - struct ide_host *host; > - int rc; > - > - if (!MACH_IS_MAC) > - return -ENODEV; > - > - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (!mem) > - return -ENODEV; > - > - irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > - if (!irq) > - return -ENODEV; > - > - if (!devm_request_mem_region(&pdev->dev, mem->start, > - resource_size(mem), DRV_NAME)) { > - dev_err(&pdev->dev, "resources busy\n"); > - return -EBUSY; > - } > - > - printk(KERN_INFO "ide: Macintosh %s IDE controller\n", > - mac_ide_name[macintosh_config->ide_type - 1]); > - > - macide_setup_ports(&hw, mem->start, irq->start); > - > - rc = ide_host_add(&d, hws, 1, &host); > - if (rc) > - return rc; > - > - platform_set_drvdata(pdev, host); > - return 0; > -} > - > -static int mac_ide_remove(struct platform_device *pdev) > -{ > - struct ide_host *host = platform_get_drvdata(pdev); > - > - ide_host_remove(host); > - return 0; > -} > - > -static struct platform_driver mac_ide_driver = { > - .driver = { > - .name = DRV_NAME, > - }, > - .probe = mac_ide_probe, > - .remove = mac_ide_remove, > -}; > - > -module_platform_driver(mac_ide_driver); > - > -MODULE_ALIAS("platform:" DRV_NAME); > -MODULE_LICENSE("GPL"); >
| |