Re: [PATCH v2] ide/macide: Convert Mac IDE driver to platform driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Finn,

On Tue, Sep 15, 2020 at 3:17 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
On Mon, 14 Sep 2020, Geert Uytterhoeven wrote:
On Mon, Sep 14, 2020 at 4:47 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
Add platform devices for the Mac IDE controller variants. Convert the
macide module into a platform driver to support two of those variants.
For the third, use a generic "pata_platform" driver instead.
This enables automatic loading of the appropriate module and begins
the process of replacing the driver with libata alternatives.

Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx>
Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Cc: Joshua Thompson <funaho@xxxxxxxxx>
References: commit 5ed0794cde593 ("m68k/atari: Convert Falcon IDE drivers to platform drivers")
References: commit 7ad19a99ad431 ("ide: officially deprecated the legacy IDE driver")
Tested-by: Stan Johnson <userm57@xxxxxxxxx>
Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>
---
This patch was tested successfully on a Powerbook 190 (MAC_IDE_BABOON)
using both pata_platform and ide_platform drivers.
The next step will be to try using these generic drivers with the other
IDE controller variants (MAC_IDE_QUADRA or MAC_IDE_PB) so that the macide
driver can be entirely replaced with libata drivers.

Changed since v1:
 - Adopted DEFINE_RES_MEM and DEFINE_RES_IRQ macros.
 - Dropped IORESOURCE_IRQ_SHAREABLE flag as it is ignored by pata_platform.c
   and IRQF_SHARED makes no difference in this case.
 - Removed redundant release_mem_region() call.
 - Enabled CONFIG_BLK_DEV_PLATFORM in mac_defconfig. We might also enable
   CONFIG_PATA_PLATFORM but IMO migration to libata should be a separate
   patch (as this patch has some unrelated benefits).

Thanks for the update!

BTW, who do you envision taking this (or the next version)? IDE or m68k?


It's unclear from the diff stat. But the focus is on the platform which
probably means it is more interesting to you, as the arch maintainer.

Fair enough.

Looking for an Acked-by from the IDE maintainer...

--- a/drivers/ide/macide.c
+++ b/drivers/ide/macide.c

@@ -109,42 +110,61 @@ static const char *mac_ide_name[] =
  * Probe for a Macintosh IDE interface
  */

-static int __init macide_init(void)
+static int mac_ide_probe(struct platform_device *pdev)
 {

        printk(KERN_INFO "ide: Macintosh %s IDE controller\n",
                         mac_ide_name[macintosh_config->ide_type - 1]);

-       macide_setup_ports(&hw, base, irq);
+       macide_setup_ports(&hw, mem->start, irq->start);

-       return ide_host_add(&d, hws, 1, NULL);
+       rc = ide_host_add(&d, hws, 1, &host);
+       if (rc)
+               return rc;
+
+       platform_set_drvdata(pdev, host);

Move one up, to play it safe?


You mean, before calling ide_host_add? The 'host' pointer is uninitialized
prior to that call.

Oh right, so the IDE subsystem doesn't let you use the drvdata inside
your driver (besides in remove()) in a safe way :-(

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux