Re: EP93xx PIO IDE driver proposal

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

 

Ok, I will apply the changes, test them and then resubmit the final patch.
By the way, I will submit the final patch through 'arm-linux' mailing list, if that is OK to you, since the full patch has platform-specific dependencies. I mean, as long as we iterate with the IDE part of it untill it is good for submission from you, I can submit it in 'arm-linux' with your acknowledge, right? 

Thank you for your revisions.

Best regards,
João Ramos

Bartlomiej Zolnierkiewicz escreveu:

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); 





--
************************************************************************

   João Ramos        <joao.ramos@xxxxxxx>

   INOV INESC Inovação - ESTG Leiria
   Escola Superior de Tecnologia e Gestão de Leiria
   Edíficio C1, Campus 2
   Morro do Lena, Alto do Vieiro
   Leiria
   2411-901 Leiria
   Portugal

   Tel: +351244843424
   Fax: +351244843424

************************************************************************

--
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
[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux