[PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Matteo Fortini]

As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
MODE register has offset 0x10 and at offset 0x0C there is
a TIMINGS register.

Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
---
 arch/arm/mach-at91/include/mach/at91sam9_smc.h | 31 +++++++++++++++++++++++++-
 arch/arm/mach-at91/sam9_smc.c                  | 19 ++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
index d5cf5f7..6412664 100644
--- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
+++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
@@ -43,6 +43,16 @@ struct sam9_smc_config {
 	/* Mode register */
 	u32 mode;
 	u8 tdf_cycles:4;
+
+	/* Timings register */
+	u8 tclr;
+	u8 tadl;
+	u8 tar;
+	u8 ocms;
+	u8 trr;
+	u8 twb;
+	u8 rbnsel;
+	u8 nfsel;
 };
 
 extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
@@ -77,7 +87,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
 #define		AT91_SMC_NRDCYCLE	(0x1ff << 16)			/* Total Read Cycle Length */
 #define			AT91_SMC_NRDCYCLE_(x)	((x) << 16)
 
-#define AT91_SMC_MODE		0x0c				/* Mode Register for CS n */
+#define AT91_SMC_TIMINGS	0x0c				/* Timings register for CS n */
+#define		AT91_SMC_TCLR		(0x0f  <<  0)			/* CLE to REN Low Delay */
+#define			AT91_SMC_TCLR_(x)	((x) << 0)
+#define		AT91_SMC_TADL		(0x0f  <<  4)			/* ALE to Data Start */
+#define			AT91_SMC_TADL_(x)	((x) << 4)
+#define		AT91_SMC_TAR		(0x0f  <<  8)			/* ALE to REN Low Delay */
+#define			AT91_SMC_TAR_(x)	((x) << 8)
+#define		AT91_SMC_OCMS		(0x1   << 12)			/* Off Chip Memory Scrambling Enable */
+#define			AT91_SMC_OCMS_(x)	((x) << 12)
+#define		AT91_SMC_TRR		(0x0f  << 16)			/* Ready to REN Low Delay */
+#define			AT91_SMC_TRR_(x)        ((x) << 16)
+#define		AT91_SMC_TWB		(0x0f  << 24)			/* WEN High to REN to Busy */
+#define			AT91_SMC_TWB_(x)	((x) << 24)
+#define		AT91_SMC_RBNSEL		(0x07  << 28)			/* Ready/Busy Line Selection */
+#define			AT91_SMC_RBNSEL_(x)	((x) << 28)
+#define		AT91_SMC_NFSEL		(0x01  << 31)			/* Nand Flash Selection */
+#define			AT91_SMC_NFSEL_(x)	((x) << 31)
+
+#define AT91_SMC_MODE		((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)				/* Mode Register for CS n */
 #define		AT91_SMC_READMODE	(1 <<  0)			/* Read Mode */
 #define		AT91_SMC_WRITEMODE	(1 <<  1)			/* Write Mode */
 #define		AT91_SMC_EXNWMODE	(3 <<  4)			/* NWAIT Mode */
@@ -101,4 +129,5 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
 #define			AT91_SMC_PS_16			(2 << 28)
 #define			AT91_SMC_PS_32			(3 << 28)
 
+
 #endif
diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
index c7bfdfd..89f73ef 100644
--- a/arch/arm/mach-at91/sam9_smc.c
+++ b/arch/arm/mach-at91/sam9_smc.c
@@ -30,6 +30,20 @@ static void sam9_smc_cs_write_mode(void __iomem *base,
 		   base + AT91_SMC_MODE);
 }
 
+static void sam9_smc_cs_write_timings(void __iomem *base,
+					struct sam9_smc_config *config)
+{
+	__raw_writel(AT91_SMC_TCLR_(config->tclr)
+                   | AT91_SMC_TADL_(config->tadl)
+                   | AT91_SMC_TAR_(config->tar)
+                   | AT91_SMC_OCMS_(config->ocms)
+                   | AT91_SMC_TRR_(config->trr)
+                   | AT91_SMC_TWB_(config->twb)
+                   | AT91_SMC_RBNSEL_(config->rbnsel)
+                   | AT91_SMC_NFSEL_(config->nfsel),
+		   base + AT91_SMC_TIMINGS);
+}
+
 void sam9_smc_write_mode(int id, int cs,
 					struct sam9_smc_config *config)
 {
@@ -61,6 +75,11 @@ static void sam9_smc_cs_configure(void __iomem *base,
 
 	/* Mode register */
 	sam9_smc_cs_write_mode(base, config);
+
+	/* Timings register */
+	if (at91_soc_initdata.type == AT91_SOC_SAMA5D3) {
+		sam9_smc_cs_write_timings(base, config);
+	}
 }
 
 void sam9_smc_configure(int id, int cs,
-- 
2.0.0.rc2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Sascha Hauer]

On Fri, Jun 06, 2014 at 11:12:15AM +0200, Matteo Fortini wrote:
> As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
> MODE register has offset 0x10 and at offset 0x0C there is
> a TIMINGS register.
> 
> Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
> ---
>  arch/arm/mach-at91/include/mach/at91sam9_smc.h | 31 +++++++++++++++++++++++++-
>  arch/arm/mach-at91/sam9_smc.c                  | 19 ++++++++++++++++
>  2 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> index d5cf5f7..6412664 100644
> --- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> +++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> @@ -43,6 +43,16 @@ struct sam9_smc_config {
>  	/* Mode register */
>  	u32 mode;
>  	u8 tdf_cycles:4;
> +
> +	/* Timings register */
> +	u8 tclr;
> +	u8 tadl;
> +	u8 tar;
> +	u8 ocms;
> +	u8 trr;
> +	u8 twb;
> +	u8 rbnsel;
> +	u8 nfsel;
>  };
>  
>  extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
> @@ -77,7 +87,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>  #define		AT91_SMC_NRDCYCLE	(0x1ff << 16)			/* Total Read Cycle Length */
>  #define			AT91_SMC_NRDCYCLE_(x)	((x) << 16)
>  
> -#define AT91_SMC_MODE		0x0c				/* Mode Register for CS n */
> +#define AT91_SMC_TIMINGS	0x0c				/* Timings register for CS n */
> +#define		AT91_SMC_TCLR		(0x0f  <<  0)			/* CLE to REN Low Delay */
> +#define			AT91_SMC_TCLR_(x)	((x) << 0)
> +#define		AT91_SMC_TADL		(0x0f  <<  4)			/* ALE to Data Start */
> +#define			AT91_SMC_TADL_(x)	((x) << 4)
> +#define		AT91_SMC_TAR		(0x0f  <<  8)			/* ALE to REN Low Delay */
> +#define			AT91_SMC_TAR_(x)	((x) << 8)
> +#define		AT91_SMC_OCMS		(0x1   << 12)			/* Off Chip Memory Scrambling Enable */
> +#define			AT91_SMC_OCMS_(x)	((x) << 12)
> +#define		AT91_SMC_TRR		(0x0f  << 16)			/* Ready to REN Low Delay */
> +#define			AT91_SMC_TRR_(x)        ((x) << 16)
> +#define		AT91_SMC_TWB		(0x0f  << 24)			/* WEN High to REN to Busy */
> +#define			AT91_SMC_TWB_(x)	((x) << 24)
> +#define		AT91_SMC_RBNSEL		(0x07  << 28)			/* Ready/Busy Line Selection */
> +#define			AT91_SMC_RBNSEL_(x)	((x) << 28)
> +#define		AT91_SMC_NFSEL		(0x01  << 31)			/* Nand Flash Selection */
> +#define			AT91_SMC_NFSEL_(x)	((x) << 31)
> +
> +#define AT91_SMC_MODE		((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)				/* Mode Register for CS n */
>  #define		AT91_SMC_READMODE	(1 <<  0)			/* Read Mode */
>  #define		AT91_SMC_WRITEMODE	(1 <<  1)			/* Write Mode */
>  #define		AT91_SMC_EXNWMODE	(3 <<  4)			/* NWAIT Mode */
> @@ -101,4 +129,5 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>  #define			AT91_SMC_PS_16			(2 << 28)
>  #define			AT91_SMC_PS_32			(3 << 28)
>  
> +
>  #endif

This hunk shouldn't be here.

> diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
> index c7bfdfd..89f73ef 100644
> --- a/arch/arm/mach-at91/sam9_smc.c
> +++ b/arch/arm/mach-at91/sam9_smc.c
> @@ -30,6 +30,20 @@ static void sam9_smc_cs_write_mode(void __iomem *base,
>  		   base + AT91_SMC_MODE);
>  }
>  
> +static void sam9_smc_cs_write_timings(void __iomem *base,
> +					struct sam9_smc_config *config)
> +{
> +	__raw_writel(AT91_SMC_TCLR_(config->tclr)
> +                   | AT91_SMC_TADL_(config->tadl)
> +                   | AT91_SMC_TAR_(config->tar)
> +                   | AT91_SMC_OCMS_(config->ocms)
> +                   | AT91_SMC_TRR_(config->trr)
> +                   | AT91_SMC_TWB_(config->twb)
> +                   | AT91_SMC_RBNSEL_(config->rbnsel)
> +                   | AT91_SMC_NFSEL_(config->nfsel),
> +		   base + AT91_SMC_TIMINGS);
> +}
> +
>  void sam9_smc_write_mode(int id, int cs,
>  					struct sam9_smc_config *config)
>  {
> @@ -61,6 +75,11 @@ static void sam9_smc_cs_configure(void __iomem *base,
>  
>  	/* Mode register */
>  	sam9_smc_cs_write_mode(base, config);
> +
> +	/* Timings register */
> +	if (at91_soc_initdata.type == AT91_SOC_SAMA5D3) {
> +		sam9_smc_cs_write_timings(base, config);
> +	}

I would pretty much prefer a

sama5d3_smc_cs_configure(int id, int cs, struct sama5d3_smc_config *config)

function so that the older SoCs do not have the overhead which the new
SoC requires.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] sama5d3x: HSMC NAND initialize TIMINGS and import values from U-Boot

[Matteo Fortini]

The configuration for NAND has been aligned with values
from U-Boot and completed with TIMINGS initialization

Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
---
 arch/arm/boards/sama5d3xek/init.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boards/sama5d3xek/init.c b/arch/arm/boards/sama5d3xek/init.c
index e078642..0d1b459 100644
--- a/arch/arm/boards/sama5d3xek/init.c
+++ b/arch/arm/boards/sama5d3xek/init.c
@@ -72,21 +72,29 @@ static struct atmel_nand_data nand_pdata = {
 };
 
 static struct sam9_smc_config cm_nand_smc_config = {
-	.ncs_read_setup		= 0,
-	.nrd_setup		= 1,
-	.ncs_write_setup	= 0,
-	.nwe_setup		= 1,
+	.ncs_read_setup		= 1,
+	.nrd_setup		= 2,
+	.ncs_write_setup	= 1,
+	.nwe_setup		= 2,
 
-	.ncs_read_pulse		= 6,
-	.nrd_pulse		= 4,
+	.ncs_read_pulse		= 5,
+	.nrd_pulse		= 3,
 	.ncs_write_pulse	= 5,
 	.nwe_pulse		= 3,
 
-	.read_cycle		= 6,
-	.write_cycle		= 5,
+	.read_cycle		= 8,
+	.write_cycle		= 8,
 
 	.mode			= AT91_SMC_READMODE | AT91_SMC_WRITEMODE | AT91_SMC_EXNWMODE_DISABLE,
-	.tdf_cycles		= 1,
+	.tdf_cycles		= 3,
+	.tclr			= 3,
+	.tadl			= 10,
+	.tar			= 3,
+	.ocms			= 0,
+	.trr			= 4,
+	.twb			= 5,
+	.rbnsel			= 3,
+	.nfsel			= 1
 };
 
 static void ek_add_device_nand(void)
-- 
2.0.0.rc2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Matteo Fortini]

As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
MODE register has offset 0x10 and at offset 0x0C there is
a TIMINGS register.

Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
---
 arch/arm/mach-at91/include/mach/at91sam9_smc.h | 35 +++++++++++++++++++++++++-
 arch/arm/mach-at91/sam9_smc.c                  | 21 ++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
index d5cf5f7..e4f0f54 100644
--- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
+++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
@@ -45,10 +45,24 @@ struct sam9_smc_config {
 	u8 tdf_cycles:4;
 };
 
+struct sam9_smc_sama5d3_extra_config {
+	/* Timings register */
+	u8 tclr;
+	u8 tadl;
+	u8 tar;
+	u8 ocms;
+	u8 trr;
+	u8 twb;
+	u8 rbnsel;
+	u8 nfsel;
+};
+
 extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
 extern void sam9_smc_read(int id, int cs, struct sam9_smc_config *config);
 extern void sam9_smc_read_mode(int id, int cs, struct sam9_smc_config *config);
 extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
+
+extern void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config);
 #endif
 
 #define AT91_SMC_SETUP		0x00				/* Setup Register for CS n */
@@ -77,7 +91,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
 #define		AT91_SMC_NRDCYCLE	(0x1ff << 16)			/* Total Read Cycle Length */
 #define			AT91_SMC_NRDCYCLE_(x)	((x) << 16)
 
-#define AT91_SMC_MODE		0x0c				/* Mode Register for CS n */
+#define AT91_SMC_TIMINGS	0x0c				/* Timings register for CS n */
+#define		AT91_SMC_TCLR		(0x0f  <<  0)			/* CLE to REN Low Delay */
+#define			AT91_SMC_TCLR_(x)	((x) << 0)
+#define		AT91_SMC_TADL		(0x0f  <<  4)			/* ALE to Data Start */
+#define			AT91_SMC_TADL_(x)	((x) << 4)
+#define		AT91_SMC_TAR		(0x0f  <<  8)			/* ALE to REN Low Delay */
+#define			AT91_SMC_TAR_(x)	((x) << 8)
+#define		AT91_SMC_OCMS		(0x1   << 12)			/* Off Chip Memory Scrambling Enable */
+#define			AT91_SMC_OCMS_(x)	((x) << 12)
+#define		AT91_SMC_TRR		(0x0f  << 16)			/* Ready to REN Low Delay */
+#define			AT91_SMC_TRR_(x)        ((x) << 16)
+#define		AT91_SMC_TWB		(0x0f  << 24)			/* WEN High to REN to Busy */
+#define			AT91_SMC_TWB_(x)	((x) << 24)
+#define		AT91_SMC_RBNSEL		(0x07  << 28)			/* Ready/Busy Line Selection */
+#define			AT91_SMC_RBNSEL_(x)	((x) << 28)
+#define		AT91_SMC_NFSEL		(0x01  << 31)			/* Nand Flash Selection */
+#define			AT91_SMC_NFSEL_(x)	((x) << 31)
+
+#define AT91_SMC_MODE		((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)				/* Mode Register for CS n */
 #define		AT91_SMC_READMODE	(1 <<  0)			/* Read Mode */
 #define		AT91_SMC_WRITEMODE	(1 <<  1)			/* Write Mode */
 #define		AT91_SMC_EXNWMODE	(3 <<  4)			/* NWAIT Mode */
@@ -101,4 +133,5 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
 #define			AT91_SMC_PS_16			(2 << 28)
 #define			AT91_SMC_PS_32			(3 << 28)
 
+
 #endif
diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
index c7bfdfd..a068d89 100644
--- a/arch/arm/mach-at91/sam9_smc.c
+++ b/arch/arm/mach-at91/sam9_smc.c
@@ -30,6 +30,20 @@ static void sam9_smc_cs_write_mode(void __iomem *base,
 		   base + AT91_SMC_MODE);
 }
 
+static void sam9_smc_cs_write_timings(void __iomem *base,
+					struct sam9_smc_sama5d3_extra_config *config)
+{
+	__raw_writel(AT91_SMC_TCLR_(config->tclr)
+                   | AT91_SMC_TADL_(config->tadl)
+                   | AT91_SMC_TAR_(config->tar)
+                   | AT91_SMC_OCMS_(config->ocms)
+                   | AT91_SMC_TRR_(config->trr)
+                   | AT91_SMC_TWB_(config->twb)
+                   | AT91_SMC_RBNSEL_(config->rbnsel)
+                   | AT91_SMC_NFSEL_(config->nfsel),
+		   base + AT91_SMC_TIMINGS);
+}
+
 void sam9_smc_write_mode(int id, int cs,
 					struct sam9_smc_config *config)
 {
@@ -120,6 +134,13 @@ void sam9_smc_read(int id, int cs, struct sam9_smc_config *config)
 	sam9_smc_cs_read(AT91_SMC_CS(id, cs), config);
 }
 
+void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config)
+{
+        sam9_smc_configure(id, cs, config);
+
+        sam9_smc_cs_write_timings(AT91_SMC_CS(id, cs), sama5d3_extra_config);
+}
+
 static int at91sam9_smc_probe(struct device_d *dev)
 {
 	int id = dev->id;
-- 
2.0.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Bo Shen]

Hi Matteo,
   Thanks for your patch.

Hi Jean-Christophe PLAGNIOL-VILLARD,
   For this patch series, can you give some comments (maybe the question 
from I need more discussion)? Thanks.

On 06/24/2014 07:26 PM, Matteo Fortini wrote:
> As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
> MODE register has offset 0x10 and at offset 0x0C there is
> a TIMINGS register.
>
> Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
> ---
>   arch/arm/mach-at91/include/mach/at91sam9_smc.h | 35 +++++++++++++++++++++++++-
>   arch/arm/mach-at91/sam9_smc.c                  | 21 ++++++++++++++++
>   2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> index d5cf5f7..e4f0f54 100644
> --- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> +++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> @@ -45,10 +45,24 @@ struct sam9_smc_config {
>   	u8 tdf_cycles:4;
>   };
>
> +struct sam9_smc_sama5d3_extra_config {

Nitpick: I am thinking another name, maybe: sama5d3_timing_config (?)

> +	/* Timings register */
> +	u8 tclr;
> +	u8 tadl;
> +	u8 tar;
> +	u8 ocms;
> +	u8 trr;
> +	u8 twb;
> +	u8 rbnsel;
> +	u8 nfsel;
> +};
> +
>   extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
>   extern void sam9_smc_read(int id, int cs, struct sam9_smc_config *config);
>   extern void sam9_smc_read_mode(int id, int cs, struct sam9_smc_config *config);
>   extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> +
> +extern void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config);

Ditto

>   #endif
>
>   #define AT91_SMC_SETUP		0x00				/* Setup Register for CS n */
> @@ -77,7 +91,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>   #define		AT91_SMC_NRDCYCLE	(0x1ff << 16)			/* Total Read Cycle Length */
>   #define			AT91_SMC_NRDCYCLE_(x)	((x) << 16)
>
> -#define AT91_SMC_MODE		0x0c				/* Mode Register for CS n */
> +#define AT91_SMC_TIMINGS	0x0c				/* Timings register for CS n */
> +#define		AT91_SMC_TCLR		(0x0f  <<  0)			/* CLE to REN Low Delay */
> +#define			AT91_SMC_TCLR_(x)	((x) << 0)
> +#define		AT91_SMC_TADL		(0x0f  <<  4)			/* ALE to Data Start */
> +#define			AT91_SMC_TADL_(x)	((x) << 4)
> +#define		AT91_SMC_TAR		(0x0f  <<  8)			/* ALE to REN Low Delay */
> +#define			AT91_SMC_TAR_(x)	((x) << 8)
> +#define		AT91_SMC_OCMS		(0x1   << 12)			/* Off Chip Memory Scrambling Enable */
> +#define			AT91_SMC_OCMS_(x)	((x) << 12)
> +#define		AT91_SMC_TRR		(0x0f  << 16)			/* Ready to REN Low Delay */
> +#define			AT91_SMC_TRR_(x)        ((x) << 16)
> +#define		AT91_SMC_TWB		(0x0f  << 24)			/* WEN High to REN to Busy */
> +#define			AT91_SMC_TWB_(x)	((x) << 24)
> +#define		AT91_SMC_RBNSEL		(0x07  << 28)			/* Ready/Busy Line Selection */
> +#define			AT91_SMC_RBNSEL_(x)	((x) << 28)
> +#define		AT91_SMC_NFSEL		(0x01  << 31)			/* Nand Flash Selection */
> +#define			AT91_SMC_NFSEL_(x)	((x) << 31)
> +
> +#define AT91_SMC_MODE		((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)				/* Mode Register for CS n */

Here make me thinking more, if new SoC added and MODE register's offset 
is the same as sama5d3, then it will be:
(at91_soc_initdata.type == AT91_SOC_SAMA5D3) || (at91_soc_initdata.type 
== AT91_SOC_NEW1) || (at91_soc_initdata.type == AT91_SOC_NEW2)

Will this be acceptable?

>   #define		AT91_SMC_READMODE	(1 <<  0)			/* Read Mode */
>   #define		AT91_SMC_WRITEMODE	(1 <<  1)			/* Write Mode */
>   #define		AT91_SMC_EXNWMODE	(3 <<  4)			/* NWAIT Mode */
> @@ -101,4 +133,5 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>   #define			AT91_SMC_PS_16			(2 << 28)
>   #define			AT91_SMC_PS_32			(3 << 28)
>
> +

Nitpick: remove this unintentionally added extra blank line.

>   #endif
> diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
> index c7bfdfd..a068d89 100644
> --- a/arch/arm/mach-at91/sam9_smc.c
> +++ b/arch/arm/mach-at91/sam9_smc.c
> @@ -30,6 +30,20 @@ static void sam9_smc_cs_write_mode(void __iomem *base,
>   		   base + AT91_SMC_MODE);
>   }
>
> +static void sam9_smc_cs_write_timings(void __iomem *base,
> +					struct sam9_smc_sama5d3_extra_config *config)
> +{
> +	__raw_writel(AT91_SMC_TCLR_(config->tclr)
> +                   | AT91_SMC_TADL_(config->tadl)
> +                   | AT91_SMC_TAR_(config->tar)
> +                   | AT91_SMC_OCMS_(config->ocms)
> +                   | AT91_SMC_TRR_(config->trr)
> +                   | AT91_SMC_TWB_(config->twb)
> +                   | AT91_SMC_RBNSEL_(config->rbnsel)
> +                   | AT91_SMC_NFSEL_(config->nfsel),
> +		   base + AT91_SMC_TIMINGS);
> +}
> +
>   void sam9_smc_write_mode(int id, int cs,
>   					struct sam9_smc_config *config)
>   {
> @@ -120,6 +134,13 @@ void sam9_smc_read(int id, int cs, struct sam9_smc_config *config)
>   	sam9_smc_cs_read(AT91_SMC_CS(id, cs), config);
>   }
>
> +void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config)
> +{
> +        sam9_smc_configure(id, cs, config);
> +
> +        sam9_smc_cs_write_timings(AT91_SMC_CS(id, cs), sama5d3_extra_config);
> +}
> +
>   static int at91sam9_smc_probe(struct device_d *dev)
>   {
>   	int id = dev->id;
>

Otherwise, it seems good.
Thanks again.

Best Regards,
Bo Shen

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Sascha Hauer]

On Wed, Jun 25, 2014 at 09:45:49AM +0800, Bo Shen wrote:
> Hi Matteo,
>   Thanks for your patch.
> 
> Hi Jean-Christophe PLAGNIOL-VILLARD,
>   For this patch series, can you give some comments (maybe the
> question from I need more discussion)? Thanks.
> 
> On 06/24/2014 07:26 PM, Matteo Fortini wrote:
> >As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
> >MODE register has offset 0x10 and at offset 0x0C there is
> >a TIMINGS register.
> >
> >Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
> >---
> >  arch/arm/mach-at91/include/mach/at91sam9_smc.h | 35 +++++++++++++++++++++++++-
> >  arch/arm/mach-at91/sam9_smc.c                  | 21 ++++++++++++++++
> >  2 files changed, 55 insertions(+), 1 deletion(-)
> >
> >diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> >index d5cf5f7..e4f0f54 100644
> >--- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> >+++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> >@@ -45,10 +45,24 @@ struct sam9_smc_config {
> >  	u8 tdf_cycles:4;
> >  };
> >
> >+struct sam9_smc_sama5d3_extra_config {
> 
> Nitpick: I am thinking another name, maybe: sama5d3_timing_config (?)
> 
> >+	/* Timings register */
> >+	u8 tclr;
> >+	u8 tadl;
> >+	u8 tar;
> >+	u8 ocms;
> >+	u8 trr;
> >+	u8 twb;
> >+	u8 rbnsel;
> >+	u8 nfsel;
> >+};
> >+
> >  extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
> >  extern void sam9_smc_read(int id, int cs, struct sam9_smc_config *config);
> >  extern void sam9_smc_read_mode(int id, int cs, struct sam9_smc_config *config);
> >  extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> >+
> >+extern void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config);
> 
> Ditto
> 
> >  #endif
> >
> >  #define AT91_SMC_SETUP		0x00				/* Setup Register for CS n */
> >@@ -77,7 +91,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> >  #define		AT91_SMC_NRDCYCLE	(0x1ff << 16)			/* Total Read Cycle Length */
> >  #define			AT91_SMC_NRDCYCLE_(x)	((x) << 16)
> >
> >-#define AT91_SMC_MODE		0x0c				/* Mode Register for CS n */
> >+#define AT91_SMC_TIMINGS	0x0c				/* Timings register for CS n */
> >+#define		AT91_SMC_TCLR		(0x0f  <<  0)			/* CLE to REN Low Delay */
> >+#define			AT91_SMC_TCLR_(x)	((x) << 0)
> >+#define		AT91_SMC_TADL		(0x0f  <<  4)			/* ALE to Data Start */
> >+#define			AT91_SMC_TADL_(x)	((x) << 4)
> >+#define		AT91_SMC_TAR		(0x0f  <<  8)			/* ALE to REN Low Delay */
> >+#define			AT91_SMC_TAR_(x)	((x) << 8)
> >+#define		AT91_SMC_OCMS		(0x1   << 12)			/* Off Chip Memory Scrambling Enable */
> >+#define			AT91_SMC_OCMS_(x)	((x) << 12)
> >+#define		AT91_SMC_TRR		(0x0f  << 16)			/* Ready to REN Low Delay */
> >+#define			AT91_SMC_TRR_(x)        ((x) << 16)
> >+#define		AT91_SMC_TWB		(0x0f  << 24)			/* WEN High to REN to Busy */
> >+#define			AT91_SMC_TWB_(x)	((x) << 24)
> >+#define		AT91_SMC_RBNSEL		(0x07  << 28)			/* Ready/Busy Line Selection */
> >+#define			AT91_SMC_RBNSEL_(x)	((x) << 28)
> >+#define		AT91_SMC_NFSEL		(0x01  << 31)			/* Nand Flash Selection */
> >+#define			AT91_SMC_NFSEL_(x)	((x) << 31)
> >+
> >+#define AT91_SMC_MODE		((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)				/* Mode Register for CS n */
> 
> Here make me thinking more, if new SoC added and MODE register's
> offset is the same as sama5d3, then it will be:
> (at91_soc_initdata.type == AT91_SOC_SAMA5D3) ||
> (at91_soc_initdata.type == AT91_SOC_NEW1) || (at91_soc_initdata.type
> == AT91_SOC_NEW2)
> 
> Will this be acceptable?

No.

I think two SoC specific defines, for example

#define AT91_SMC_MODE 0xc
#define AT91_SAMA5_SMC_MODE 0x10

and handling the differences in the code will scale better. I don't like
hiding SoC differences in defines like done in this patch. If there show
up even more differences a SoC specific struct with register offsets
might help.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Raphaël Poggi]

Hi all !

So what's the status of these patch? Matteo, are you working on a v2
patch series ?

Raphaël

2014-06-25 8:42 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Wed, Jun 25, 2014 at 09:45:49AM +0800, Bo Shen wrote:
>> Hi Matteo,
>>   Thanks for your patch.
>>
>> Hi Jean-Christophe PLAGNIOL-VILLARD,
>>   For this patch series, can you give some comments (maybe the
>> question from I need more discussion)? Thanks.
>>
>> On 06/24/2014 07:26 PM, Matteo Fortini wrote:
>> >As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
>> >MODE register has offset 0x10 and at offset 0x0C there is
>> >a TIMINGS register.
>> >
>> >Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
>> >---
>> >  arch/arm/mach-at91/include/mach/at91sam9_smc.h | 35 +++++++++++++++++++++++++-
>> >  arch/arm/mach-at91/sam9_smc.c                  | 21 ++++++++++++++++
>> >  2 files changed, 55 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>> >index d5cf5f7..e4f0f54 100644
>> >--- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>> >+++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>> >@@ -45,10 +45,24 @@ struct sam9_smc_config {
>> >     u8 tdf_cycles:4;
>> >  };
>> >
>> >+struct sam9_smc_sama5d3_extra_config {
>>
>> Nitpick: I am thinking another name, maybe: sama5d3_timing_config (?)
>>
>> >+    /* Timings register */
>> >+    u8 tclr;
>> >+    u8 tadl;
>> >+    u8 tar;
>> >+    u8 ocms;
>> >+    u8 trr;
>> >+    u8 twb;
>> >+    u8 rbnsel;
>> >+    u8 nfsel;
>> >+};
>> >+
>> >  extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
>> >  extern void sam9_smc_read(int id, int cs, struct sam9_smc_config *config);
>> >  extern void sam9_smc_read_mode(int id, int cs, struct sam9_smc_config *config);
>> >  extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>> >+
>> >+extern void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config);
>>
>> Ditto
>>
>> >  #endif
>> >
>> >  #define AT91_SMC_SETUP             0x00                            /* Setup Register for CS n */
>> >@@ -77,7 +91,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>> >  #define            AT91_SMC_NRDCYCLE       (0x1ff << 16)                   /* Total Read Cycle Length */
>> >  #define                    AT91_SMC_NRDCYCLE_(x)   ((x) << 16)
>> >
>> >-#define AT91_SMC_MODE               0x0c                            /* Mode Register for CS n */
>> >+#define AT91_SMC_TIMINGS    0x0c                            /* Timings register for CS n */
>> >+#define             AT91_SMC_TCLR           (0x0f  <<  0)                   /* CLE to REN Low Delay */
>> >+#define                     AT91_SMC_TCLR_(x)       ((x) << 0)
>> >+#define             AT91_SMC_TADL           (0x0f  <<  4)                   /* ALE to Data Start */
>> >+#define                     AT91_SMC_TADL_(x)       ((x) << 4)
>> >+#define             AT91_SMC_TAR            (0x0f  <<  8)                   /* ALE to REN Low Delay */
>> >+#define                     AT91_SMC_TAR_(x)        ((x) << 8)
>> >+#define             AT91_SMC_OCMS           (0x1   << 12)                   /* Off Chip Memory Scrambling Enable */
>> >+#define                     AT91_SMC_OCMS_(x)       ((x) << 12)
>> >+#define             AT91_SMC_TRR            (0x0f  << 16)                   /* Ready to REN Low Delay */
>> >+#define                     AT91_SMC_TRR_(x)        ((x) << 16)
>> >+#define             AT91_SMC_TWB            (0x0f  << 24)                   /* WEN High to REN to Busy */
>> >+#define                     AT91_SMC_TWB_(x)        ((x) << 24)
>> >+#define             AT91_SMC_RBNSEL         (0x07  << 28)                   /* Ready/Busy Line Selection */
>> >+#define                     AT91_SMC_RBNSEL_(x)     ((x) << 28)
>> >+#define             AT91_SMC_NFSEL          (0x01  << 31)                   /* Nand Flash Selection */
>> >+#define                     AT91_SMC_NFSEL_(x)      ((x) << 31)
>> >+
>> >+#define AT91_SMC_MODE               ((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)                            /* Mode Register for CS n */
>>
>> Here make me thinking more, if new SoC added and MODE register's
>> offset is the same as sama5d3, then it will be:
>> (at91_soc_initdata.type == AT91_SOC_SAMA5D3) ||
>> (at91_soc_initdata.type == AT91_SOC_NEW1) || (at91_soc_initdata.type
>> == AT91_SOC_NEW2)
>>
>> Will this be acceptable?
>
> No.
>
> I think two SoC specific defines, for example
>
> #define AT91_SMC_MODE 0xc
> #define AT91_SAMA5_SMC_MODE 0x10
>
> and handling the differences in the code will scale better. I don't like
> hiding SoC differences in defines like done in this patch. If there show
> up even more differences a SoC specific struct with register offsets
> might help.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Jean-Christophe PLAGNIOL-VILLARD]

On Jun 24, 2014, at 7:26 PM, Matteo Fortini <matteo.fortini@gmail.com> wrote:

> 
> As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
> MODE register has offset 0x10 and at offset 0x0C there is
> a TIMINGS register.
> 
> Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
> ---
> arch/arm/mach-at91/include/mach/at91sam9_smc.h | 35 +++++++++++++++++++++++++-
> arch/arm/mach-at91/sam9_smc.c                  | 21 ++++++++++++++++
> 2 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> index d5cf5f7..e4f0f54 100644
> --- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> +++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> @@ -45,10 +45,24 @@ struct sam9_smc_config {
> 	u8 tdf_cycles:4;
> };
> 
> +struct sam9_smc_sama5d3_extra_config {
> +	/* Timings register */
> +	u8 tclr;
> +	u8 tadl;
> +	u8 tar;
> +	u8 ocms;
> +	u8 trr;
> +	u8 twb;
> +	u8 rbnsel;
> +	u8 nfsel;
> +};
> +
> extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
> extern void sam9_smc_read(int id, int cs, struct sam9_smc_config *config);
> extern void sam9_smc_read_mode(int id, int cs, struct sam9_smc_config *config);
> extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> +
> +extern void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config);
> #endif
> 
> #define AT91_SMC_SETUP		0x00				/* Setup Register for CS n */
> @@ -77,7 +91,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> #define		AT91_SMC_NRDCYCLE	(0x1ff << 16)			/* Total Read Cycle Length */
> #define			AT91_SMC_NRDCYCLE_(x)	((x) << 16)
> 
> -#define AT91_SMC_MODE		0x0c				/* Mode Register for CS n */
> +#define AT91_SMC_TIMINGS	0x0c				/* Timings register for CS n */
> +#define		AT91_SMC_TCLR		(0x0f  <<  0)			/* CLE to REN Low Delay */
> +#define			AT91_SMC_TCLR_(x)	((x) << 0)
> +#define		AT91_SMC_TADL		(0x0f  <<  4)			/* ALE to Data Start */
> +#define			AT91_SMC_TADL_(x)	((x) << 4)
> +#define		AT91_SMC_TAR		(0x0f  <<  8)			/* ALE to REN Low Delay */
> +#define			AT91_SMC_TAR_(x)	((x) << 8)
> +#define		AT91_SMC_OCMS		(0x1   << 12)			/* Off Chip Memory Scrambling Enable */
> +#define			AT91_SMC_OCMS_(x)	((x) << 12)
> +#define		AT91_SMC_TRR		(0x0f  << 16)			/* Ready to REN Low Delay */
> +#define			AT91_SMC_TRR_(x)        ((x) << 16)
> +#define		AT91_SMC_TWB		(0x0f  << 24)			/* WEN High to REN to Busy */
> +#define			AT91_SMC_TWB_(x)	((x) << 24)
> +#define		AT91_SMC_RBNSEL		(0x07  << 28)			/* Ready/Busy Line Selection */
> +#define			AT91_SMC_RBNSEL_(x)	((x) << 28)
> +#define		AT91_SMC_NFSEL		(0x01  << 31)			/* Nand Flash Selection */
> +#define			AT91_SMC_NFSEL_(x)	((x) << 31)
> +
> +#define AT91_SMC_MODE		((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)				/* Mode Register for CS n */
> #define		AT91_SMC_READMODE	(1 <<  0)			/* Read Mode */
> #define		AT91_SMC_WRITEMODE	(1 <<  1)			/* Write Mode */
> #define		AT91_SMC_EXNWMODE	(3 <<  4)			/* NWAIT Mode */
> @@ -101,4 +133,5 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> #define			AT91_SMC_PS_16			(2 << 28)
> #define			AT91_SMC_PS_32			(3 << 28)
> 
> +
> #endif
> diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
> index c7bfdfd..a068d89 100644
> --- a/arch/arm/mach-at91/sam9_smc.c
> +++ b/arch/arm/mach-at91/sam9_smc.c
> @@ -30,6 +30,20 @@ static void sam9_smc_cs_write_mode(void __iomem *base,
> 		   base + AT91_SMC_MODE);
> }
> 
> +static void sam9_smc_cs_write_timings(void __iomem *base,
> +					struct sam9_smc_sama5d3_extra_config *config)
> +{
> +	__raw_writel(AT91_SMC_TCLR_(config->tclr)
> +                   | AT91_SMC_TADL_(config->tadl)
> +                   | AT91_SMC_TAR_(config->tar)
> +                   | AT91_SMC_OCMS_(config->ocms)
> +                   | AT91_SMC_TRR_(config->trr)
> +                   | AT91_SMC_TWB_(config->twb)
> +                   | AT91_SMC_RBNSEL_(config->rbnsel)
> +                   | AT91_SMC_NFSEL_(config->nfsel),
> +		   base + AT91_SMC_TIMINGS);
> +}
> +
> void sam9_smc_write_mode(int id, int cs,
> 					struct sam9_smc_config *config)
> {
> @@ -120,6 +134,13 @@ void sam9_smc_read(int id, int cs, struct sam9_smc_config *config)
> 	sam9_smc_cs_read(AT91_SMC_CS(id, cs), config);
> }
> 
> +void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config)
2 structures no

just extend the current one for sam9 we just ignore the additional config

Best Regards,
J.
> +{
> +        sam9_smc_configure(id, cs, config);
> +
> +        sam9_smc_cs_write_timings(AT91_SMC_CS(id, cs), sama5d3_extra_config);
> +}
> +
> static int at91sam9_smc_probe(struct device_d *dev)
> {
> 	int id = dev->id;
> -- 
> 2.0.0
> 


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Matteo Fortini]

Ok guys, who calls the shots? My V1 of the patch had the big structure
and ignored the timings part, and Sascha said he preferred not to
incur in the overhead of managing the extra part. In V2 I split the
structures and had some minor naming issues and I am currently
preparing a V3 with the split structure and the names for the
structure, the defines and the functions that were suggested by Bo and
Sascha.

I think we should move on with this patch, currently barebox is not
able to properly use NAND memory on SAMA5D3 (besides smearing some
other wrong registers in the meantime), and this is a major bug. For
my board I already fixed it in some way, but it would be useful for
everyone.

Thank you for your comments.

Regards,
Matteo


2014-07-04 9:47 GMT+02:00 Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com>:
>
> On Jun 24, 2014, at 7:26 PM, Matteo Fortini <matteo.fortini@gmail.com> wrote:
>
>>
>> As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
>> MODE register has offset 0x10 and at offset 0x0C there is
>> a TIMINGS register.
>>
>> Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
>> ---
>> arch/arm/mach-at91/include/mach/at91sam9_smc.h | 35 +++++++++++++++++++++++++-
>> arch/arm/mach-at91/sam9_smc.c                  | 21 ++++++++++++++++
>> 2 files changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>> index d5cf5f7..e4f0f54 100644
>> --- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>> +++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>> @@ -45,10 +45,24 @@ struct sam9_smc_config {
>>       u8 tdf_cycles:4;
>> };
>>
>> +struct sam9_smc_sama5d3_extra_config {
>> +     /* Timings register */
>> +     u8 tclr;
>> +     u8 tadl;
>> +     u8 tar;
>> +     u8 ocms;
>> +     u8 trr;
>> +     u8 twb;
>> +     u8 rbnsel;
>> +     u8 nfsel;
>> +};
>> +
>> extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
>> extern void sam9_smc_read(int id, int cs, struct sam9_smc_config *config);
>> extern void sam9_smc_read_mode(int id, int cs, struct sam9_smc_config *config);
>> extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>> +
>> +extern void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config);
>> #endif
>>
>> #define AT91_SMC_SETUP                0x00                            /* Setup Register for CS n */
>> @@ -77,7 +91,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>> #define               AT91_SMC_NRDCYCLE       (0x1ff << 16)                   /* Total Read Cycle Length */
>> #define                       AT91_SMC_NRDCYCLE_(x)   ((x) << 16)
>>
>> -#define AT91_SMC_MODE                0x0c                            /* Mode Register for CS n */
>> +#define AT91_SMC_TIMINGS     0x0c                            /* Timings register for CS n */
>> +#define              AT91_SMC_TCLR           (0x0f  <<  0)                   /* CLE to REN Low Delay */
>> +#define                      AT91_SMC_TCLR_(x)       ((x) << 0)
>> +#define              AT91_SMC_TADL           (0x0f  <<  4)                   /* ALE to Data Start */
>> +#define                      AT91_SMC_TADL_(x)       ((x) << 4)
>> +#define              AT91_SMC_TAR            (0x0f  <<  8)                   /* ALE to REN Low Delay */
>> +#define                      AT91_SMC_TAR_(x)        ((x) << 8)
>> +#define              AT91_SMC_OCMS           (0x1   << 12)                   /* Off Chip Memory Scrambling Enable */
>> +#define                      AT91_SMC_OCMS_(x)       ((x) << 12)
>> +#define              AT91_SMC_TRR            (0x0f  << 16)                   /* Ready to REN Low Delay */
>> +#define                      AT91_SMC_TRR_(x)        ((x) << 16)
>> +#define              AT91_SMC_TWB            (0x0f  << 24)                   /* WEN High to REN to Busy */
>> +#define                      AT91_SMC_TWB_(x)        ((x) << 24)
>> +#define              AT91_SMC_RBNSEL         (0x07  << 28)                   /* Ready/Busy Line Selection */
>> +#define                      AT91_SMC_RBNSEL_(x)     ((x) << 28)
>> +#define              AT91_SMC_NFSEL          (0x01  << 31)                   /* Nand Flash Selection */
>> +#define                      AT91_SMC_NFSEL_(x)      ((x) << 31)
>> +
>> +#define AT91_SMC_MODE                ((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)                            /* Mode Register for CS n */
>> #define               AT91_SMC_READMODE       (1 <<  0)                       /* Read Mode */
>> #define               AT91_SMC_WRITEMODE      (1 <<  1)                       /* Write Mode */
>> #define               AT91_SMC_EXNWMODE       (3 <<  4)                       /* NWAIT Mode */
>> @@ -101,4 +133,5 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
>> #define                       AT91_SMC_PS_16                  (2 << 28)
>> #define                       AT91_SMC_PS_32                  (3 << 28)
>>
>> +
>> #endif
>> diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
>> index c7bfdfd..a068d89 100644
>> --- a/arch/arm/mach-at91/sam9_smc.c
>> +++ b/arch/arm/mach-at91/sam9_smc.c
>> @@ -30,6 +30,20 @@ static void sam9_smc_cs_write_mode(void __iomem *base,
>>                  base + AT91_SMC_MODE);
>> }
>>
>> +static void sam9_smc_cs_write_timings(void __iomem *base,
>> +                                     struct sam9_smc_sama5d3_extra_config *config)
>> +{
>> +     __raw_writel(AT91_SMC_TCLR_(config->tclr)
>> +                   | AT91_SMC_TADL_(config->tadl)
>> +                   | AT91_SMC_TAR_(config->tar)
>> +                   | AT91_SMC_OCMS_(config->ocms)
>> +                   | AT91_SMC_TRR_(config->trr)
>> +                   | AT91_SMC_TWB_(config->twb)
>> +                   | AT91_SMC_RBNSEL_(config->rbnsel)
>> +                   | AT91_SMC_NFSEL_(config->nfsel),
>> +                base + AT91_SMC_TIMINGS);
>> +}
>> +
>> void sam9_smc_write_mode(int id, int cs,
>>                                       struct sam9_smc_config *config)
>> {
>> @@ -120,6 +134,13 @@ void sam9_smc_read(int id, int cs, struct sam9_smc_config *config)
>>       sam9_smc_cs_read(AT91_SMC_CS(id, cs), config);
>> }
>>
>> +void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config)
> 2 structures no
>
> just extend the current one for sam9 we just ignore the additional config
>
> Best Regards,
> J.
>> +{
>> +        sam9_smc_configure(id, cs, config);
>> +
>> +        sam9_smc_cs_write_timings(AT91_SMC_CS(id, cs), sama5d3_extra_config);
>> +}
>> +
>> static int at91sam9_smc_probe(struct device_d *dev)
>> {
>>       int id = dev->id;
>> --
>> 2.0.0
>>
>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Sascha Hauer]

On Fri, Jul 04, 2014 at 03:47:58PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> On Jun 24, 2014, at 7:26 PM, Matteo Fortini <matteo.fortini@gmail.com> wrote:
> 
> > 
> > +void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config)
> 2 structures no
> 
> just extend the current one for sam9 we just ignore the additional config

sam9_smc_sama5d3_configure() is called from code which knows it runs on
sama5d3. IMO it doesn't make much sense to call a generic SoC function
from special board code when the generic SoC function has to
differentiate between SoCs in the next step.

I'm ok with merging this patch or the original one, but if you are still
interested in the AT91 port please not only complain about a patch
two weeks later but also ACK a patch when you are fine with it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Jean-Christophe PLAGNIOL-VILLARD]

On 08:20 Mon 07 Jul     , Sascha Hauer wrote:
> 
> On Fri, Jul 04, 2014 at 03:47:58PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > 
> > On Jun 24, 2014, at 7:26 PM, Matteo Fortini <matteo.fortini@gmail.com> wrote:
> > 
> > > 
> > > +void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config)
> > 2 structures no
> > 
> > just extend the current one for sam9 we just ignore the additional config
> 
> sam9_smc_sama5d3_configure() is called from code which knows it runs on

I do agree to call sam9_smc_sama5d3_configure from sama5 directly

or sama5_smc_configure(xx) will be better and shorter
> sama5d3. IMO it doesn't make much sense to call a generic SoC function
> from special board code when the generic SoC function has to
> differentiate between SoCs in the next step.

except you need both for the sama5 so no need to store on 2 struct
one struct should be fine

and few more bytes will not matter much on the current boards.

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Sascha Hauer]

On Mon, Jul 07, 2014 at 08:17:44PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 08:20 Mon 07 Jul     , Sascha Hauer wrote:
> > 
> > On Fri, Jul 04, 2014 at 03:47:58PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > 
> > > On Jun 24, 2014, at 7:26 PM, Matteo Fortini <matteo.fortini@gmail.com> wrote:
> > > 
> > > > 
> > > > +void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config)
> > > 2 structures no
> > > 
> > > just extend the current one for sam9 we just ignore the additional config
> > 
> > sam9_smc_sama5d3_configure() is called from code which knows it runs on
> 
> I do agree to call sam9_smc_sama5d3_configure from sama5 directly
> 
> or sama5_smc_configure(xx) will be better and shorter
> > sama5d3. IMO it doesn't make much sense to call a generic SoC function
> > from special board code when the generic SoC function has to
> > differentiate between SoCs in the next step.
> 
> except you need both for the sama5 so no need to store on 2 struct
> one struct should be fine
> 
> and few more bytes will not matter much on the current boards.

Ok, so we agree on two functions, but a single struct.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Jean-Christophe PLAGNIOL-VILLARD]

On Jun 24, 2014, at 7:26 PM, Matteo Fortini <matteo.fortini@gmail.com> wrote:

> 
> As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
> MODE register has offset 0x10 and at offset 0x0C there is
> a TIMINGS register.
> 
> Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
> ---
> arch/arm/mach-at91/include/mach/at91sam9_smc.h | 35 +++++++++++++++++++++++++-
> arch/arm/mach-at91/sam9_smc.c                  | 21 ++++++++++++++++
> 2 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> index d5cf5f7..e4f0f54 100644
> --- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> +++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> @@ -45,10 +45,24 @@ struct sam9_smc_config {
> 	u8 tdf_cycles:4;
> };
> 
> +struct sam9_smc_sama5d3_extra_config {
> +	/* Timings register */
> +	u8 tclr;
> +	u8 tadl;
> +	u8 tar;
> +	u8 ocms;
> +	u8 trr;
> +	u8 twb;
> +	u8 rbnsel;
> +	u8 nfsel;
> +};
> +
> extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
> extern void sam9_smc_read(int id, int cs, struct sam9_smc_config *config);
> extern void sam9_smc_read_mode(int id, int cs, struct sam9_smc_config *config);
> extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> +
> +extern void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config);
> #endif
> 
> #define AT91_SMC_SETUP		0x00				/* Setup Register for CS n */
> @@ -77,7 +91,25 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> #define		AT91_SMC_NRDCYCLE	(0x1ff << 16)			/* Total Read Cycle Length */
> #define			AT91_SMC_NRDCYCLE_(x)	((x) << 16)
> 
> -#define AT91_SMC_MODE		0x0c				/* Mode Register for CS n */
> +#define AT91_SMC_TIMINGS	0x0c				/* Timings register for CS n */
> +#define		AT91_SMC_TCLR		(0x0f  <<  0)			/* CLE to REN Low Delay */
> +#define			AT91_SMC_TCLR_(x)	((x) << 0)
> +#define		AT91_SMC_TADL		(0x0f  <<  4)			/* ALE to Data Start */
> +#define			AT91_SMC_TADL_(x)	((x) << 4)
> +#define		AT91_SMC_TAR		(0x0f  <<  8)			/* ALE to REN Low Delay */
> +#define			AT91_SMC_TAR_(x)	((x) << 8)
> +#define		AT91_SMC_OCMS		(0x1   << 12)			/* Off Chip Memory Scrambling Enable */
> +#define			AT91_SMC_OCMS_(x)	((x) << 12)
> +#define		AT91_SMC_TRR		(0x0f  << 16)			/* Ready to REN Low Delay */
> +#define			AT91_SMC_TRR_(x)        ((x) << 16)
> +#define		AT91_SMC_TWB		(0x0f  << 24)			/* WEN High to REN to Busy */
> +#define			AT91_SMC_TWB_(x)	((x) << 24)
> +#define		AT91_SMC_RBNSEL		(0x07  << 28)			/* Ready/Busy Line Selection */
> +#define			AT91_SMC_RBNSEL_(x)	((x) << 28)
> +#define		AT91_SMC_NFSEL		(0x01  << 31)			/* Nand Flash Selection */
> +#define			AT91_SMC_NFSEL_(x)	((x) << 31)
> +
> +#define AT91_SMC_MODE		((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x10 : 0x0c)				/* Mode Register for CS n */
> #define		AT91_SMC_READMODE	(1 <<  0)			/* Read Mode */
> #define		AT91_SMC_WRITEMODE	(1 <<  1)			/* Write Mode */
> #define		AT91_SMC_EXNWMODE	(3 <<  4)			/* NWAIT Mode */
> @@ -101,4 +133,5 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
> #define			AT91_SMC_PS_16			(2 << 28)
> #define			AT91_SMC_PS_32			(3 << 28)
> 
> +
> #endif
> diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
> index c7bfdfd..a068d89 100644
> --- a/arch/arm/mach-at91/sam9_smc.c
> +++ b/arch/arm/mach-at91/sam9_smc.c
> @@ -30,6 +30,20 @@ static void sam9_smc_cs_write_mode(void __iomem *base,
> 		   base + AT91_SMC_MODE);
> }
> 
> +static void sam9_smc_cs_write_timings(void __iomem *base,
> +					struct sam9_smc_sama5d3_extra_config *config)
> +{
> +	__raw_writel(AT91_SMC_TCLR_(config->tclr)
> +                   | AT91_SMC_TADL_(config->tadl)
> +                   | AT91_SMC_TAR_(config->tar)
> +                   | AT91_SMC_OCMS_(config->ocms)
> +                   | AT91_SMC_TRR_(config->trr)
> +                   | AT91_SMC_TWB_(config->twb)
> +                   | AT91_SMC_RBNSEL_(config->rbnsel)
> +                   | AT91_SMC_NFSEL_(config->nfsel),
> +		   base + AT91_SMC_TIMINGS);
> +}
> +
> void sam9_smc_write_mode(int id, int cs,
> 					struct sam9_smc_config *config)
> {
> @@ -120,6 +134,13 @@ void sam9_smc_read(int id, int cs, struct sam9_smc_config *config)
> 	sam9_smc_cs_read(AT91_SMC_CS(id, cs), config);
> }
> 
> +void sam9_smc_sama5d3_configure(int id, int cs, struct sam9_smc_config *config, struct sam9_smc_sama5d3_extra_config *sama5d3_extra_config)
2 structures no

just extend the current one for sam9 we just ignore the additional config

Best Regards,
J.
> +{
> +        sam9_smc_configure(id, cs, config);
> +
> +        sam9_smc_cs_write_timings(AT91_SMC_CS(id, cs), sama5d3_extra_config);
> +}
> +
> static int at91sam9_smc_probe(struct device_d *dev)
> {
> 	int id = dev->id;
> -- 
> 2.0.0
> 


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Matteo Fortini]

As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
MODE register has offset 0x10 and at offset 0x0C there is
a TIMINGS register.

Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
---
 arch/arm/mach-at91/include/mach/at91sam9_smc.h | 34 ++++++++++++++++++++++-
 arch/arm/mach-at91/sam9_smc.c                  | 38 ++++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
index d5cf5f7..29ae643 100644
--- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
+++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
@@ -43,12 +43,24 @@ struct sam9_smc_config {
 	/* Mode register */
 	u32 mode;
 	u8 tdf_cycles:4;
+
+	/* Timings register */
+	u8 tclr;
+	u8 tadl;
+	u8 tar;
+	u8 ocms;
+	u8 trr;
+	u8 twb;
+	u8 rbnsel;
+	u8 nfsel;
 };
 
 extern void sam9_smc_configure(int id, int cs, struct sam9_smc_config *config);
 extern void sam9_smc_read(int id, int cs, struct sam9_smc_config *config);
 extern void sam9_smc_read_mode(int id, int cs, struct sam9_smc_config *config);
 extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
+
+extern void sama5_smc_configure(int id, int cs, struct sam9_smc_config *config);
 #endif
 
 #define AT91_SMC_SETUP		0x00				/* Setup Register for CS n */
@@ -77,7 +89,26 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
 #define		AT91_SMC_NRDCYCLE	(0x1ff << 16)			/* Total Read Cycle Length */
 #define			AT91_SMC_NRDCYCLE_(x)	((x) << 16)
 
-#define AT91_SMC_MODE		0x0c				/* Mode Register for CS n */
+#define AT91_SAMA5_SMC_TIMINGS	0x0c				/* Timings register for CS n */
+#define		AT91_SMC_TCLR		(0x0f  <<  0)			/* CLE to REN Low Delay */
+#define			AT91_SMC_TCLR_(x)	((x) << 0)
+#define		AT91_SMC_TADL		(0x0f  <<  4)			/* ALE to Data Start */
+#define			AT91_SMC_TADL_(x)	((x) << 4)
+#define		AT91_SMC_TAR		(0x0f  <<  8)			/* ALE to REN Low Delay */
+#define			AT91_SMC_TAR_(x)	((x) << 8)
+#define		AT91_SMC_OCMS		(0x1   << 12)			/* Off Chip Memory Scrambling Enable */
+#define			AT91_SMC_OCMS_(x)	((x) << 12)
+#define		AT91_SMC_TRR		(0x0f  << 16)			/* Ready to REN Low Delay */
+#define			AT91_SMC_TRR_(x)        ((x) << 16)
+#define		AT91_SMC_TWB		(0x0f  << 24)			/* WEN High to REN to Busy */
+#define			AT91_SMC_TWB_(x)	((x) << 24)
+#define		AT91_SMC_RBNSEL		(0x07  << 28)			/* Ready/Busy Line Selection */
+#define			AT91_SMC_RBNSEL_(x)	((x) << 28)
+#define		AT91_SMC_NFSEL		(0x01  << 31)			/* Nand Flash Selection */
+#define			AT91_SMC_NFSEL_(x)	((x) << 31)
+
+#define AT91_SAM9_SMC_MODE 0xc
+#define AT91_SAMA5_SMC_MODE 0x10
 #define		AT91_SMC_READMODE	(1 <<  0)			/* Read Mode */
 #define		AT91_SMC_WRITEMODE	(1 <<  1)			/* Write Mode */
 #define		AT91_SMC_EXNWMODE	(3 <<  4)			/* NWAIT Mode */
@@ -101,4 +132,5 @@ extern void sam9_smc_write_mode(int id, int cs, struct sam9_smc_config *config);
 #define			AT91_SMC_PS_16			(2 << 28)
 #define			AT91_SMC_PS_32			(3 << 28)
 
+
 #endif
diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
index c7bfdfd..9f02807 100644
--- a/arch/arm/mach-at91/sam9_smc.c
+++ b/arch/arm/mach-at91/sam9_smc.c
@@ -17,7 +17,9 @@
 
 #include <mach/at91sam9_smc.h>
 
-#define AT91_SMC_CS_STRIDE      ((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? 0x14 : 0x10)
+#define AT91_SAM9_SMC_CS_STRIDE		0x10
+#define AT91_SAMA5_SMC_CS_STRIDE	0x14
+#define AT91_SMC_CS_STRIDE      ((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? AT91_SAMA5_SMC_CS_STRIDE : AT91_SAM9_SMC_CS_STRIDE)
 #define AT91_SMC_CS(id, n)	(smc_base_addr[id] + ((n) * AT91_SMC_CS_STRIDE))
 
 static void __iomem *smc_base_addr[2];
@@ -25,9 +27,27 @@ static void __iomem *smc_base_addr[2];
 static void sam9_smc_cs_write_mode(void __iomem *base,
 					struct sam9_smc_config *config)
 {
+	void __iomem *mode_reg;
+
+	mode_reg = base + ((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? AT91_SAMA5_SMC_MODE : AT91_SAM9_SMC_MODE);
+
 	__raw_writel(config->mode
 		   | AT91_SMC_TDF_(config->tdf_cycles),
-		   base + AT91_SMC_MODE);
+		   mode_reg);
+}
+
+static void sam9_smc_cs_write_timings(void __iomem *base,
+					struct sam9_smc_config *config)
+{
+	__raw_writel(AT91_SMC_TCLR_(config->tclr)
+                   | AT91_SMC_TADL_(config->tadl)
+                   | AT91_SMC_TAR_(config->tar)
+                   | AT91_SMC_OCMS_(config->ocms)
+                   | AT91_SMC_TRR_(config->trr)
+                   | AT91_SMC_TWB_(config->twb)
+                   | AT91_SMC_RBNSEL_(config->rbnsel)
+                   | AT91_SMC_NFSEL_(config->nfsel),
+		   base + AT91_SAMA5_SMC_TIMINGS);
 }
 
 void sam9_smc_write_mode(int id, int cs,
@@ -72,7 +92,12 @@ void sam9_smc_configure(int id, int cs,
 static void sam9_smc_cs_read_mode(void __iomem *base,
 					struct sam9_smc_config *config)
 {
-	u32 val = __raw_readl(base + AT91_SMC_MODE);
+	u32 val;
+	void __iomem *mode_reg;
+
+	mode_reg = base + ((at91_soc_initdata.type == AT91_SOC_SAMA5D3) ? AT91_SAMA5_SMC_MODE : AT91_SAM9_SMC_MODE);
+
+	val = __raw_readl(mode_reg);
 
 	config->mode = (val & ~AT91_SMC_NWECYCLE);
 	config->tdf_cycles = (val & AT91_SMC_NWECYCLE) >> 16 ;
@@ -120,6 +145,13 @@ void sam9_smc_read(int id, int cs, struct sam9_smc_config *config)
 	sam9_smc_cs_read(AT91_SMC_CS(id, cs), config);
 }
 
+void sama5_smc_configure(int id, int cs, struct sam9_smc_config *config)
+{
+        sam9_smc_configure(id, cs, config);
+
+        sam9_smc_cs_write_timings(AT91_SMC_CS(id, cs), config);
+}
+
 static int at91sam9_smc_probe(struct device_d *dev)
 {
 	int id = dev->id;
-- 
2.0.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Sascha Hauer]

Hi Matteo,

On Mon, Jul 21, 2014 at 06:13:26PM +0200, Matteo Fortini wrote:
> As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
> MODE register has offset 0x10 and at offset 0x0C there is
> a TIMINGS register.
> 
> Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>

I had to fix up this patch with:

From 43a67c4260488beefa8fab8181bf342048b083f2 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Thu, 31 Jul 2014 14:37:29 +0200
Subject: [PATCH] fixup! sama5d3x: fix HSMC MODE register offset and add
 TIMINGS register

---
 arch/arm/mach-at91/at91sam926x_lowlevel_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/at91sam926x_lowlevel_init.c b/arch/arm/mach-at91/at91sam926x_lowlevel_init.c
index 985203a..e69e4a8 100644
--- a/arch/arm/mach-at91/at91sam926x_lowlevel_init.c
+++ b/arch/arm/mach-at91/at91sam926x_lowlevel_init.c
@@ -130,7 +130,7 @@ void __bare_init at91sam926x_lowlevel_init(struct at91sam926x_lowlevel_cfg *cfg)
 	at91_sys_write(cfg->matrix_csa, cfg->ebi_csa);
 
 	/* flash */
-	at91_smc_write(cfg->smc_cs, AT91_SMC_MODE, cfg->smc_mode);
+	at91_smc_write(cfg->smc_cs, AT91_SAM9_SMC_MODE, cfg->smc_mode);
 
 	at91_smc_write(cfg->smc_cs, AT91_SMC_CYCLE, cfg->smc_cycle);
 

Otherwise several boards fail with:

arch/arm/mach-at91/at91sam926x_lowlevel_init.c: In function 'at91sam926x_lowlevel_init':
arch/arm/mach-at91/at91sam926x_lowlevel_init.c:133:2: error: 'AT91_SMC_MODE' undeclared (first use in this function)
arch/arm/mach-at91/at91sam926x_lowlevel_init.c:133:2: note: each undeclared identifier is reported only once for each function it appears in
scripts/Makefile.build:245: recipe for target 'arch/arm/mach-at91/pbl-at91sam926x_lowlevel_init.o' failed
make[1]: *** [arch/arm/mach-at91/pbl-at91sam926x_lowlevel_init.o] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:769: recipe for target 'arch/arm/mach-at91' failed
make: *** [arch/arm/mach-at91] Error 2
make: *** Waiting for unfinished jobs....

Can you confirm my fix is correct?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register

[Matteo Fortini]

Yes confirmed Sascha, bag of paper for me for not checking it all.

I renamed AT91_SMC_MODE->AT91_SAM9_SMC_MODE because I introduced 
AT91_SAMA5_SMC_MODE and AT91_SMC_MODE was ambiguous.

Thank you and my apologies,
Matteo

Il 31/07/2014 14:39, Sascha Hauer ha scritto:
> Hi Matteo,
>
> On Mon, Jul 21, 2014 at 06:13:26PM +0200, Matteo Fortini wrote:
>> As stated in section 29.19.35 of SAMA5D3 Series Datasheet,
>> MODE register has offset 0x10 and at offset 0x0C there is
>> a TIMINGS register.
>>
>> Signed-off-by: Matteo Fortini <matteo.fortini@gmail.com>
> I had to fix up this patch with:
>
>  From 43a67c4260488beefa8fab8181bf342048b083f2 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Thu, 31 Jul 2014 14:37:29 +0200
> Subject: [PATCH] fixup! sama5d3x: fix HSMC MODE register offset and add
>   TIMINGS register
>
> ---
>   arch/arm/mach-at91/at91sam926x_lowlevel_init.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-at91/at91sam926x_lowlevel_init.c b/arch/arm/mach-at91/at91sam926x_lowlevel_init.c
> index 985203a..e69e4a8 100644
> --- a/arch/arm/mach-at91/at91sam926x_lowlevel_init.c
> +++ b/arch/arm/mach-at91/at91sam926x_lowlevel_init.c
> @@ -130,7 +130,7 @@ void __bare_init at91sam926x_lowlevel_init(struct at91sam926x_lowlevel_cfg *cfg)
>   	at91_sys_write(cfg->matrix_csa, cfg->ebi_csa);
>   
>   	/* flash */
> -	at91_smc_write(cfg->smc_cs, AT91_SMC_MODE, cfg->smc_mode);
> +	at91_smc_write(cfg->smc_cs, AT91_SAM9_SMC_MODE, cfg->smc_mode);
>   
>   	at91_smc_write(cfg->smc_cs, AT91_SMC_CYCLE, cfg->smc_cycle);
>   
>
> Otherwise several boards fail with:
>
> arch/arm/mach-at91/at91sam926x_lowlevel_init.c: In function 'at91sam926x_lowlevel_init':
> arch/arm/mach-at91/at91sam926x_lowlevel_init.c:133:2: error: 'AT91_SMC_MODE' undeclared (first use in this function)
> arch/arm/mach-at91/at91sam926x_lowlevel_init.c:133:2: note: each undeclared identifier is reported only once for each function it appears in
> scripts/Makefile.build:245: recipe for target 'arch/arm/mach-at91/pbl-at91sam926x_lowlevel_init.o' failed
> make[1]: *** [arch/arm/mach-at91/pbl-at91sam926x_lowlevel_init.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:769: recipe for target 'arch/arm/mach-at91' failed
> make: *** [arch/arm/mach-at91] Error 2
> make: *** Waiting for unfinished jobs....
>
> Can you confirm my fix is correct?
>
> Sascha
>


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox