| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <46A79DE0.8060405@ru.mvista.com>
Date: Wed, 25 Jul 2007 23:00:48 +0400
From: Sergei Shtylyov <sshtylyov@...mvista.com>
To: Vitaly Bordug <vitb@...nel.crashing.org>
Cc: linux-ide@...r.kernel.org, linuxppc-dev@...abs.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
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 <galak@...nel.crashing.org>
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);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists