On Monday 18 May 2009 15:49:15 João Ramos wrote:
> Hi,
>
> Here is the revised patch according to Bart's suggestions in previous
> emails.
> The patch is against linux-2.6.30-rc6.
Thanks. I see that we can still cleanup the code a bit by folding current
ep93xx_ide_{read,write}() into __ep93xx_ide_{read,write}() and then adding
wrappers on top of it.
I think that the patch would the best to show the idea in this case (it is
completely untested, it may not build).
[ While at it I also changed data transfer functions to use drive's timings
and fixed ->remove method to unregister IDE host before doing host specific
cleanup. These fixes should be applied regardless of the cleanup change. ]
Please review/integrate/test and then resubmit the final patch.
---
drivers/ide/ep93xx-ide.c | 111 ++++++++++++++++++++++-------------------------
1 file changed, 54 insertions(+), 57 deletions(-)
Index: b/drivers/ide/ep93xx-ide.c
===================================================================
--- a/drivers/ide/ep93xx-ide.c
+++ b/drivers/ide/ep93xx-ide.c
@@ -126,50 +126,67 @@ static void ep93xx_ide_clean_regs(void _
writel(0, base + IDEUDMADEBUG);
}
-static u16 ep93xx_ide_read(void __iomem *base, unsigned long addr,
- struct ide_timing *timing)
+static u16 __ep93xx_ide_read(void __iomem *base, unsigned long addr,
+ struct ide_timing *t)
{
u32 reg;
reg = addr | IDECTRL_DIORN | IDECTRL_DIOWN;
writel(reg, base + IDECTRL);
- ndelay(timing->setup);
+ ndelay(t->setup);
reg &= ~IDECTRL_DIORN;
writel(reg, base + IDECTRL);
- ndelay(timing->active);
+ ndelay(t->active);
while (!ep93xx_ide_check_iordy(base))
cpu_relax();
reg |= IDECTRL_DIORN;
writel(reg, base + IDECTRL);
- ndelay(timing->recover);
+ ndelay(t->recover);
return readl(base + IDEDATAIN);
}
-static void ep93xx_ide_write(void __iomem *base, u16 value,
- unsigned long addr, struct ide_timing *timing)
+static inline u16 ep93xx_ide_read(ide_hwif_t *hwif, unsigned long addr)
+{
+ void __iomem *base = hwif->host->host_priv;
+ struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
+
+ return __ep93xx_ide_read(base, addr, t);
+}
+
+static void __ep93xx_ide_write(void __iomem *base, u16 value,
+ unsigned long addr, struct ide_timing *t)
{
u32 reg;
reg = addr | IDECTRL_DIORN | IDECTRL_DIOWN;
writel(reg, base + IDECTRL);
- ndelay(timing->setup);
+ ndelay(t->setup);
writel(value, base + IDEDATAOUT);
reg &= ~IDECTRL_DIOWN;
writel(reg, base + IDECTRL);
- ndelay(timing->active);
+ ndelay(t->active);
while (!ep93xx_ide_check_iordy(base))
cpu_relax();
reg |= IDECTRL_DIOWN;
writel(reg, base + IDECTRL);
- ndelay(timing->recover);
+ ndelay(t->recover);
+}
+
+static inline void ep93xx_ide_write(ide_hwif_t *hwif, u16 value,
+ unsigned long addr)
+{
+ void __iomem *base = hwif->host->host_priv;
+ struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
+
+ __ep93xx_ide_write(base, value, addr, t);
}
/*
@@ -178,88 +195,70 @@ static void ep93xx_ide_write(void __iome
static void ep93xx_ide_exec_command(struct hwif_s *hwif, u8 cmd)
{
- void __iomem *base = hwif->host->host_priv;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
-
- ep93xx_ide_write(base, cmd, hwif->io_ports.command_addr, t);
+ ep93xx_ide_write(hwif, cmd, hwif->io_ports.command_addr);
}
static u8 ep93xx_ide_read_status(ide_hwif_t *hwif)
{
- void __iomem *base = hwif->host->host_priv;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
-
- return ep93xx_ide_read(base, hwif->io_ports.status_addr, t);
+ return ep93xx_ide_read(whif, hwif->io_ports.status_addr);
}
static u8 ep93xx_ide_read_altstatus(ide_hwif_t *hwif)
{
- void __iomem *base = hwif->host->host_priv;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
-
- return ep93xx_ide_read(base, hwif->io_ports.ctl_addr, t);
+ return ep93xx_ide_read(hwif, hwif->io_ports.ctl_addr);
}
static void ep93xx_ide_write_devctl(ide_hwif_t *hwif, u8 ctl)
{
- void __iomem *base = hwif->host->host_priv;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
-
- ep93xx_ide_write(base, ctl, hwif->io_ports.ctl_addr, t);
+ ep93xx_ide_write(hwif, ctl, hwif->io_ports.ctl_addr);
}
static void ep93xx_ide_dev_select(ide_drive_t *drive)
{
ide_hwif_t *hwif = drive->hwif;
- void __iomem *base = hwif->host->host_priv;
u8 select = drive->select | ATA_DEVICE_OBS;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
- ep93xx_ide_write(base, select, hwif->io_ports.device_addr, t);
+ ep93xx_ide_write(hwif, select, hwif->io_ports.device_addr);
}
static void ep93xx_ide_tf_load(ide_drive_t *drive, struct ide_taskfile *tf,
u8 valid)
{
ide_hwif_t *hwif = drive->hwif;
- void __iomem *base = hwif->host->host_priv;
struct ide_io_ports *io_ports = &hwif->io_ports;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
if (valid & IDE_VALID_FEATURE)
- ep93xx_ide_write(base, tf->feature, io_ports->feature_addr, t);
+ ep93xx_ide_write(hwif, tf->feature, io_ports->feature_addr);
if (valid & IDE_VALID_NSECT)
- ep93xx_ide_write(base, tf->nsect, io_ports->nsect_addr, t);
+ ep93xx_ide_write(hwif, tf->nsect, io_ports->nsect_addr);
if (valid & IDE_VALID_LBAL)
- ep93xx_ide_write(base, tf->lbal, io_ports->lbal_addr, t);
+ ep93xx_ide_write(hwif, tf->lbal, io_ports->lbal_addr);
if (valid & IDE_VALID_LBAM)
- ep93xx_ide_write(base, tf->lbam, io_ports->lbam_addr, t);
+ ep93xx_ide_write(hwif, tf->lbam, io_ports->lbam_addr);
if (valid & IDE_VALID_LBAH)
- ep93xx_ide_write(base, tf->lbah, io_ports->lbah_addr, t);
+ ep93xx_ide_write(hwif, tf->lbah, io_ports->lbah_addr);
if (valid & IDE_VALID_DEVICE)
- ep93xx_ide_write(base, tf->device, io_ports->device_addr, t);
+ ep93xx_ide_write(hwif, tf->device, io_ports->device_addr);
}
static void ep93xx_ide_tf_read(ide_drive_t *drive, struct ide_taskfile *tf,
u8 valid)
{
ide_hwif_t *hwif = drive->hwif;
- void __iomem *base = hwif->host->host_priv;
struct ide_io_ports *io_ports = &hwif->io_ports;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
if (valid & IDE_VALID_ERROR)
- tf->error = ep93xx_ide_read(base, io_ports->feature_addr, t);
+ tf->error = ep93xx_ide_read(hwif, io_ports->feature_addr);
if (valid & IDE_VALID_NSECT)
- tf->nsect = ep93xx_ide_read(base, io_ports->nsect_addr, t);
+ tf->nsect = ep93xx_ide_read(hwif, io_ports->nsect_addr);
if (valid & IDE_VALID_LBAL)
- tf->lbal = ep93xx_ide_read(base, io_ports->lbal_addr, t);
+ tf->lbal = ep93xx_ide_read(hwif, io_ports->lbal_addr);
if (valid & IDE_VALID_LBAM)
- tf->lbam = ep93xx_ide_read(base, io_ports->lbam_addr, t);
+ tf->lbam = ep93xx_ide_read(hwif, io_ports->lbam_addr);
if (valid & IDE_VALID_LBAH)
- tf->lbah = ep93xx_ide_read(base, io_ports->lbah_addr, t);
+ tf->lbah = ep93xx_ide_read(hwif, io_ports->lbah_addr);
if (valid & IDE_VALID_DEVICE)
- tf->device = ep93xx_ide_read(base, io_ports->device_addr, t);
+ tf->device = ep93xx_ide_read(hwif, io_ports->device_addr);
}
static void ep93xx_ide_input_data(ide_drive_t *drive, struct ide_cmd *cmd,
@@ -268,13 +267,12 @@ static void ep93xx_ide_input_data(ide_dr
u16 *data = (u16 *)buf;
ide_hwif_t *hwif = drive->hwif;
void __iomem *base = hwif->host->host_priv;
- struct ide_io_ports *io_ports = &hwif->io_ports;
+ unsigned long data_addr = hwif->io_ports.data_addr;
+ struct ide_timing *t = (struct ide_timing *)ide_get_drivedata(drive);
unsigned int words = (len + 1) >> 1;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
while (words--)
- *data++ = cpu_to_le16(ep93xx_ide_read(base,
- io_ports->data_addr, t));
+ *data++ = cpu_to_le16(__ep93xx_ide_read(base, data_addr, t);
}
static void ep93xx_ide_output_data(ide_drive_t *drive, struct ide_cmd *cmd,
@@ -283,13 +281,12 @@ static void ep93xx_ide_output_data(ide_d
u16 *data = (u16 *)buf;
ide_hwif_t *hwif = drive->hwif;
void __iomem *base = hwif->host->host_priv;
- struct ide_io_ports *io_ports = &hwif->io_ports;
+ unsigned long data_addr = hwif->io_ports.data_addr;
+ struct ide_timing *t = (struct ide_timing *)ide_get_drivedata(drive);
unsigned int words = (len + 1) >> 1;
- struct ide_timing *t = (struct ide_timing *)ide_get_hwifdata(hwif);
while (words--)
- ep93xx_ide_write(base, le16_to_cpu(*data++),
- io_ports->data_addr, t);
+ __ep93xx_ide_write(base, le16_to_cpu(*data++), data_addr, t);
}
static struct ide_tp_ops ep93xx_ide_tp_ops = {
@@ -477,18 +474,18 @@ fail_release_mem:
static int __devexit ep93xx_ide_remove(struct platform_device *pdev)
{
- struct resource *mem_res;
struct ide_host *host = pdev->dev.driver_data;
void __iomem *base = host->host_priv;
+ struct resource *mem_res;
+
+ ide_host_remove(host);
/* reset state */
- ep93xx_ide_clean_regs(host->host_priv);
+ ep93xx_ide_clean_regs(base);
/* restore back GPIO port E, G and H for GPIO use */
ep93xx_ide_release_gpios();
- ide_host_remove(host);
-
iounmap(base);
mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
