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.
Cc: Michael Schmitz <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Joshua Thompson <[email protected]>
Cc: "David S. Miller" <[email protected]>
Signed-off-by: Finn Thain <[email protected]>
---
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");
--
2.26.3
On 4/25/21 11:06 AM, Finn Thain wrote:
> 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.
Shouldn't we switch to a libata driver instead with legacy IDE been slated
for removal from the Linux kernel?
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
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 <[email protected]>
>
> Cc: Michael Schmitz <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Joshua Thompson <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Signed-off-by: Finn Thain <[email protected]>
> ---
> 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");
>
On Mon, 26 Apr 2021, Michael Schmitz wrote:
> 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?
>
The edge-triggered interrupt flag bits are usually cleared by writing 1 to
the flag bit. Under this scheme, writing a 0 to a flag bit has no effect.
The assignment statement here is trying to clear bit 5 by writing 0. But
what about the other bits that we're writing 0 to? Some of them may also
be flag bits, and they may have been asserted in between the load and
store. AFAICS this scheme just can't work for edge-triggered interrupts.
So perhaps this is a level-triggered interrupt?
> 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.
Fair enough. Maybe we have found a bug in the MacOS IDE driver.
> 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 <[email protected]>
>
Thanks for your review.
On Sun, 25 Apr 2021, John Paul Adrian Glaubitz wrote:
> On 4/25/21 11:06 AM, Finn Thain wrote:
> > 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.
>
> Shouldn't we switch to a libata driver instead with legacy IDE been slated
> for removal from the Linux kernel?
>
This patch means that the Quadra 630/580 config will work the same as the
Powerbook 190 config. Please see commit 50c5feeea0af ("ide/macide: Convert
Mac IDE driver to platform driver") for some background.
This patch also means that the kernel/drivers/ide/macide.ko driver module
will disappear completely. The module that will replace it is up to you.
If you enable CONFIG_IDE and CONFIG_BLK_DEV_PLATFORM, Quadras and
Powerbooks will use the kernel/drivers/ide/ide_platform.ko module.
If you enable CONFIG_ATA and CONFIG_PATA_PLATFORM, Quadras and
Powerbooks will use the kernel/drivers/ata/pata_platform.ko module.
(If you enable all of the above, you may need to use a blacklist to ensure
that the preferred driver module gets loaded.)
Was macide the only IDE driver in Debian/m68k kernels without a libata
alternative? If so, this patch would allow you to finally drop CONFIG_IDE.
> Adrian
>
> --
> .''`. John Paul Adrian Glaubitz
> : :' : Debian Developer - [email protected]
> `. `' Freie Universitaet Berlin - [email protected]
> `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
>
On 4/26/21 9:37 AM, Finn Thain wrote:> If you enable CONFIG_IDE and CONFIG_BLK_DEV_PLATFORM, Quadras and
> Powerbooks will use the kernel/drivers/ide/ide_platform.ko module.
>
> If you enable CONFIG_ATA and CONFIG_PATA_PLATFORM, Quadras and
> Powerbooks will use the kernel/drivers/ata/pata_platform.ko module.
>
> (If you enable all of the above, you may need to use a blacklist to ensure
> that the preferred driver module gets loaded.)
>
> Was macide the only IDE driver in Debian/m68k kernels without a libata
> alternative? If so, this patch would allow you to finally drop CONFIG_IDE.
That's great to hear. It means that systems using the Mac IDE drivers will
survive the legacy IDE removal if it ever comes.
So, thanks a lot for addressing this!
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi Finn,
On 26/04/21 7:37 pm, Finn Thain wrote:
> Was macide the only IDE driver in Debian/m68k kernels without a libata
> alternative? If so, this patch would allow you to finally drop CONFIG_IDE.
>
There's still q40ide.c (ISA IDE interface, byte-swapped, so would need
treatment similar to Falcon IDE). Hasn't been updated to a platform
device yet.
Cheers,
Michael
On Tue, 27 Apr 2021, Michael Schmitz wrote:
> On 26/04/21 7:37 pm, Finn Thain wrote:
> > Was macide the only IDE driver in Debian/m68k kernels without a libata
> > alternative? If so, this patch would allow you to finally drop
> > CONFIG_IDE.
> >
> There's still q40ide.c (ISA IDE interface, byte-swapped, so would need
> treatment similar to Falcon IDE). Hasn't been updated to a platform
> device yet.
>
AIUI, q40 support is not included in Debian/m68k kernel builds.
I wonder whether q40 could re-use the pata_falcon driver . I suppose
pata_falcon_set_mode() would be undesirable on q40 (?) It could be made
optional using the data parameter passed to
platform_device_register_resndata().
On 27.04.2021 4:51, Michael Schmitz wrote:
>> Was macide the only IDE driver in Debian/m68k kernels without a libata
>> alternative? If so, this patch would allow you to finally drop CONFIG_IDE.
>>
> There's still q40ide.c (ISA IDE interface, byte-swapped, so would need
> treatment similar to Falcon IDE). Hasn't been updated to a platform device yet.
ISA drivers shouldn't be "updated" to the platform drivers. But I don't
see 'struct isa_driver' there either...
> Cheers,
>
> Michael
MBR, Sergei
On 4/27/21 10:11 AM, Sergei Shtylyov wrote:
> On 27.04.2021 4:51, Michael Schmitz wrote:
>
>>> Was macide the only IDE driver in Debian/m68k kernels without a libata
>>> alternative? If so, this patch would allow you to finally drop CONFIG_IDE.
>>>
>> There's still q40ide.c (ISA IDE interface, byte-swapped, so would need treatment similar to Falcon IDE). Hasn't been updated to a platform device yet.
>
> ISA drivers shouldn't be "updated" to the platform drivers.
Why not?
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On 27/04/21 8:11 pm, Sergei Shtylyov wrote:
> On 27.04.2021 4:51, Michael Schmitz wrote:
>
>>> Was macide the only IDE driver in Debian/m68k kernels without a libata
>>> alternative? If so, this patch would allow you to finally drop
>>> CONFIG_IDE.
>>>
>> There's still q40ide.c (ISA IDE interface, byte-swapped, so would
>> need treatment similar to Falcon IDE). Hasn't been updated to a
>> platform device yet.
>
> ISA drivers shouldn't be "updated" to the platform drivers. But I
> don't see 'struct isa_driver' there either...
My bad - while the Q40 has ISA slots, IDE isn't connected to the ISA
bus. Got confused by the base address range matching what I remember
from ISA cards...
Cheers,
Michael
Hi Finn,
On 27/04/21 3:47 pm, Finn Thain wrote:
> On Tue, 27 Apr 2021, Michael Schmitz wrote:
>
>> On 26/04/21 7:37 pm, Finn Thain wrote:
>>> Was macide the only IDE driver in Debian/m68k kernels without a libata
>>> alternative? If so, this patch would allow you to finally drop
>>> CONFIG_IDE.
>>>
>> There's still q40ide.c (ISA IDE interface, byte-swapped, so would need
>> treatment similar to Falcon IDE). Hasn't been updated to a platform
>> device yet.
>>
> AIUI, q40 support is not included in Debian/m68k kernel builds.
I see.
> I wonder whether q40 could re-use the pata_falcon driver . I suppose
I'm pretty sure it could, but there is no reason why it would have to be
crippled in that way. Interrupts should work perfectly fine with IDE on
Q40.
There is another reason why using the same module binary for both might
fail - the awkward address translation code in io_mm.h. Not certain at
all whether we can even have Q40 and Atari in the same kernel binary...
> pata_falcon_set_mode() would be undesirable on q40 (?) It could be made
Not sure what the defaults are - pata_buddha.c and pata_gayle.c use the
same code in their _set_mode(). I suspect we'd need it for Q40, too.
Cheers,
Michael
Hi Michael,
On Tue, Apr 27, 2021 at 9:55 PM Michael Schmitz <[email protected]> wrote:
> On 27/04/21 3:47 pm, Finn Thain wrote:
> > On Tue, 27 Apr 2021, Michael Schmitz wrote:
> >> On 26/04/21 7:37 pm, Finn Thain wrote:
> >>> Was macide the only IDE driver in Debian/m68k kernels without a libata
> >>> alternative? If so, this patch would allow you to finally drop
> >>> CONFIG_IDE.
> >>>
> >> There's still q40ide.c (ISA IDE interface, byte-swapped, so would need
> >> treatment similar to Falcon IDE). Hasn't been updated to a platform
> >> device yet.
> >>
> > AIUI, q40 support is not included in Debian/m68k kernel builds.
> I see.
> > I wonder whether q40 could re-use the pata_falcon driver . I suppose
>
> I'm pretty sure it could, but there is no reason why it would have to be
> crippled in that way. Interrupts should work perfectly fine with IDE on
> Q40.
>
> There is another reason why using the same module binary for both might
> fail - the awkward address translation code in io_mm.h. Not certain at
> all whether we can even have Q40 and Atari in the same kernel binary...
That's supposed to work, else our multi_defconfig is broken.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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