On 09:38 Fri 09 Dec     , Ryan Mallon wrote:

On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

this will allow to use the pata_at91 on a single zImage

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>
Cc: Nicolas Ferre <redacted>
Cc: linux-ide at vger.kernel.org

Couple of minor comments below,

~Ryan

quoted

---
Hi,

	it's depends on other patch for AT91 can we apply via at91

Best Regards,
J.
 drivers/ide/at91_ide.c |   65 +++++++++++++++++++++++++++--------------------
 1 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c
index 41d4155..407595b 100644
--- a/drivers/ide/at91_ide.c
+++ b/drivers/ide/at91_ide.c

@@ -59,41 +59,50 @@
 #define ALT_MODE	0x00e00000
 #define REGS_SIZE	8
 
-#define enter_16bit(cs, mode) do {					\
-	mode = at91_sys_read(AT91_SMC_MODE(cs));			\
-	at91_sys_write(AT91_SMC_MODE(cs), mode | AT91_SMC_DBW_16);	\
-} while (0)
+static inline void enter_16bit(int cs, struct sam9_smc_config *smc)
+{
+	sam9_smc_read_mode(0, cs, smc);
+	smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16;
+	sam9_smc_write_mode(0, cs, smc);
+}
 
-#define leave_16bit(cs, mode) at91_sys_write(AT91_SMC_MODE(cs), mode);
+static inline void leave_16bit(int cs, struct sam9_smc_config *smc)
+{
+	smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8;
+	sam9_smc_write_mode(0, cs, smc);
+}
 
 static void set_smc_timings(const u8 chipselect, const u16 cycle,
 			    const u16 setup, const u16 pulse,
 			    const u16 data_float, int use_iordy)
 {
-	unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
+	struct sam9_smc_config smc;
+
+	smc.mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
 			     AT91_SMC_BAT_SELECT;
 
 	/* disable or enable waiting for IORDY signal */
 	if (use_iordy)
-		mode |= AT91_SMC_EXNWMODE_READY;
+		smc.mode |= AT91_SMC_EXNWMODE_READY;
 
-	/* add data float cycles if needed */
-	if (data_float)
-		mode |= AT91_SMC_TDF_(data_float);
+	/* add data ofloat cycles if needed */

Typo: ofloat -> float?

quoted

+	smc.tdf_cycles = data_float;
 
-	at91_sys_write(AT91_SMC_MODE(chipselect), mode);
+	/* write SMC Setup Register */

This comment is no longer correct.

quoted

+	smc.nrd_setup = setup;
+	smc.nwe_setup = smc.nrd_setup;
+	smc.ncs_read_setup = 0;
+	smc.ncs_write_setup = smc.ncs_read_setup;
+	/* write SMC Pulse Register */

Nor is this one.

quoted

+	smc.nrd_pulse = pulse;
+	smc.nwe_pulse = smc.nrd_pulse;
+	smc.ncs_read_pulse = cycle;
+	smc.ncs_write_pulse = smc.ncs_read_pulse;
+	/* write SMC Cycle Register */

or this one.

quoted

+	smc.read_cycle = cycle;
+	smc.write_cycle = smc.read_cycle;
 
-	/* setup timings in SMC */
-	at91_sys_write(AT91_SMC_SETUP(chipselect), AT91_SMC_NWESETUP_(setup) |
-						   AT91_SMC_NCS_WRSETUP_(0) |
-						   AT91_SMC_NRDSETUP_(setup) |
-						   AT91_SMC_NCS_RDSETUP_(0));
-	at91_sys_write(AT91_SMC_PULSE(chipselect), AT91_SMC_NWEPULSE_(pulse) |
-						   AT91_SMC_NCS_WRPULSE_(cycle) |
-						   AT91_SMC_NRDPULSE_(pulse) |
-						   AT91_SMC_NCS_RDPULSE_(cycle));
-	at91_sys_write(AT91_SMC_CYCLE(chipselect), AT91_SMC_NWECYCLE_(cycle) |
-						   AT91_SMC_NRDCYCLE_(cycle));
+	sam9_smc_configure(0, chipselect, &smc);

Check return value?

set_timings don't care about it

so no need

Best Regards,
J.