Vitaly Bordug wrote: > This is now very similar to pata_platform.c, they both use > same platform data structure and same resources.
> To achieve that, byte_lanes_swapping platform data variable > and platform specified iops removed from that driver. It's fine, > since those were never used anyway. Hopefully, PPC will never need them. > pata_platform and ide_platform are carrying same driver names, > to easily switch between these drivers, without need to touch > platform code. > diff --git a/drivers/ide/legacy/ide_platform.c > b/drivers/ide/legacy/ide_platform.c > new file mode 100644 > index 0000000..0b3f5b5 > --- /dev/null > +++ b/drivers/ide/legacy/ide_platform.c > @@ -0,0 +1,181 @@ > +/* > + * Platform IDE driver > + * > + * Copyright (C) 2007 MontaVista Software > + * > + * Maintainer: Kumar Gala <[EMAIL PROTECTED]> Poor Kumar, I guess he was surprised be being assigned a maintainer of the driver he didn't write (well, even if it borrowed some code from his earlier one ;-)... > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#include <linux/types.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/ide.h> > +#include <linux/ioport.h> > +#include <linux/module.h> > +#include <linux/pata_platform.h> > +#include <linux/io.h> > + > +static struct { > + void __iomem *plat_ide_mapbase; > + void __iomem *plat_ide_alt_mapbase; > + ide_hwif_t *hwif; > + int index; > +} hwif_prop; > + > +static ide_hwif_t *__devinit plat_ide_locate_hwif(void __iomem *base, > + void __iomem *ctrl, struct pata_platform_info *pdata, int irq, > + int mmio) > +{ > + unsigned long port = (unsigned long)base; > + ide_hwif_t *hwif; > + int index, i; > + > + for (index = 0; index < MAX_HWIFS; ++index) { > + hwif = ide_hwifs + index; > + if (hwif->io_ports[IDE_DATA_OFFSET] == port) > + goto found; > + } > + > + for (index = 0; index < MAX_HWIFS; ++index) { > + hwif = ide_hwifs + index; > + if (hwif->io_ports[IDE_DATA_OFFSET] == 0) > + goto found; > + } > + > + return NULL; > + > +found: > + > + hwif->hw.io_ports[IDE_DATA_OFFSET] = port; > + > + port += (1 << pdata->ioport_shift); > + for (i = IDE_ERROR_OFFSET; i <= IDE_STATUS_OFFSET; > + i++, port += (1 << pdata->ioport_shift)) Looks like shift doesn't buy as anything, why not just use stride? > + hwif->hw.io_ports[i] = port; > + > + hwif->hw.io_ports[IDE_CONTROL_OFFSET] = (unsigned long)ctrl; > + > + memcpy(hwif->io_ports, hwif->hw.io_ports, sizeof(hwif->hw.io_ports)); > + hwif->hw.irq = hwif->irq = irq; > + > + hwif->hw.dma = NO_DMA; > + hwif->hw.chipset = ide_generic; > + > + if (mmio) { > + hwif->mmio = 1; > + default_hwif_mmiops(hwif); > + } And why default_hwif_iops(hwif) is not called else? And I remember the previos variant was overriding INSW()/OUTSW() methods -- turned out to be unnecessary? :-) > + > + hwif_prop.hwif = hwif; > + hwif_prop.index = index; > + > + return hwif; > +} > + > +static int __devinit plat_ide_probe(struct platform_device *pdev) > +{ > + struct resource *res_base, *res_alt, *res_irq; > + ide_hwif_t *hwif; > + struct pata_platform_info *pdata; > + int ret = 0; > + int mmio = 0; > + > + pdata = pdev->dev.platform_data; > + > + /* get a pointer to the register memory */ > + res_base = platform_get_resource(pdev, IORESOURCE_IO, 0); > + res_alt = platform_get_resource(pdev, IORESOURCE_IO, 1); > + > + if (!res_base || !res_alt) { > + res_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + res_alt = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (!res_base || !res_alt) { > + ret = -ENOMEM; ENODEV or EINVAL you mean? :-) > + goto out; Bleh... just return please. > + } > + mmio = 1; > + } > + > + res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > + if (!res_irq) { > + ret = -EINVAL; > + goto out; Bleh... return -EINVAL, please. Or maybe -ENODEV. > + } > + [...] > + hwif = plat_ide_locate_hwif(hwif_prop.plat_ide_mapbase, > + hwif_prop.plat_ide_alt_mapbase, pdata, res_irq->start, mmio); > + > + if (!hwif) { > + ret = -ENODEV; > + goto out; Bleh... please just return -ENODEV. > + } > + hwif->gendev.parent = &pdev->dev; > + hwif->noprobe = 0; > + > + probe_hwif_init(hwif); > + > + platform_set_drvdata(pdev, hwif); > + ide_proc_register_port(hwif); > + > + return 0; > + > +out: No need to abuse the function cleanup style when you have nothing to cleanup. :-/ > + return ret; > +} > + > +static int __devexit plat_ide_remove(struct platform_device *pdev) > +{ > + ide_hwif_t *hwif = pdev->dev.driver_data; > + > + if (hwif != hwif_prop.hwif) { > + dev_printk(KERN_DEBUG, &pdev->dev, "%s: hwif value error", > + pdev->name); > + } else { > + ide_unregister(hwif_prop.index); > + hwif_prop.index = 0; > + hwif_prop.hwif = NULL; > + } > + I'd have interchanged the success/failure branches... > + return 0; > +} > + > +static struct platform_driver platform_ide_driver = { > + .driver { > + .name = "pata_platform", > + }, > + .probe = plat_ide_probe, > + .remove = __devexit_p(plat_ide_remove), > +}; > + > +static int __init platform_ide_init(void) > +{ > + return platform_driver_register(&platform_ide_driver); > +} > + > +static void __exit platform_ide_exit(void) > +{ > + platform_driver_unregister(&platform_ide_driver); > +} > + > +MODULE_DESCRIPTION("Platform IDE driver"); > +MODULE_LICENSE("GPL"); > + > +module_init(platform_ide_init); > +module_exit(platform_ide_exit); _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev