On 09/12/11 17:27, Jean-Christophe PLAGNIOL-VILLARD wrote:

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

quoted

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

quoted

this will allow to configure the smc independtly of the register configuration
as example on rm9200 different from sam9

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>
Cc: Nicolas Ferre <redacted>

Hi Jean,

Couple of comments below.

~Ryan

quoted

---
 arch/arm/mach-at91/include/mach/at91sam9_smc.h |   29 ++++++++
 arch/arm/mach-at91/sam9_smc.c                  |   93 ++++++++++++++++++++++--
 arch/arm/mach-at91/sam9_smc.h                  |   23 ------
 3 files changed, 115 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
index eb18a70..b5eff0e 100644
--- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
+++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h

@@ -18,6 +18,35 @@
 
 #include <mach/cpu.h>
 
+#ifndef __ASSEMBLY__
+struct sam9_smc_config {
+	/* Setup register */
+	u8 ncs_read_setup;
+	u8 nrd_setup;
+	u8 ncs_write_setup;
+	u8 nwe_setup;
+
+	/* Pulse register */
+	u8 ncs_read_pulse;
+	u8 nrd_pulse;
+	u8 ncs_write_pulse;
+	u8 nwe_pulse;
+
+	/* Cycle register */
+	u16 read_cycle;
+	u16 write_cycle;
+
+	/* Mode register */
+	u32 mode;
+	u8 tdf_cycles:4;
+};
+
+extern int sam9_smc_configure(int id, int cs, struct sam9_smc_config* config);
+extern int sam9_smc_read(int id, int cs, struct sam9_smc_config* config);
+extern int sam9_smc_read_mode(int id, int cs, struct sam9_smc_config* config);
+extern int sam9_smc_write_mode(int id, int cs, struct sam9_smc_config* config);
+#endif
+
 #define AT91_SMC_SETUP		0x00				/* Setup Register for CS n */
 #define		AT91_SMC_NWESETUP	(0x3f << 0)			/* NWE Setup Length */
 #define			AT91_SMC_NWESETUP_(x)	((x) << 0)

diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
index 8294783..e5936dc 100644
--- a/arch/arm/mach-at91/sam9_smc.c
+++ b/arch/arm/mach-at91/sam9_smc.c

@@ -2,6 +2,7 @@
  * linux/arch/arm/mach-at91/sam9_smc.c
  *
  * Copyright (C) 2008 Andrew Victor
+ * Copyright (C) 2011 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as

@@ -17,13 +18,58 @@
 
 #include "sam9_smc.h"
 
-
 #define AT91_SMC_CS(id, n)	(smc_base_addr[id] + ((n) * 0x10))
 
 static void __iomem *smc_base_addr[2];
 
-static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_config* config)
+static void __init_refok sam9_smc_cs_write_mode(void __iomem *base,
+					struct sam9_smc_config* config)

include/linux/kernel.h says that __init_refok means that this function
can reference __init code, without being __init itself and without
generating a modpost warning. It also says that such cases should be
documented as to why the reference is ok. Why is __init_refok being used
here and on the other fucntions in this file?

because you can you this function before and after init and this need to not
be free by the kernel after boot

Lots of functions are used before and after init without needing special
markers. __init_refok means that this function references __init code
even though the function itself is not __init (which would usually
generate a modpost warning). This function doesn't reference any __init
code, so the __init_refok should not be needed?

~Ryan