mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
       [not found] <1403609192-5862-1-git-send-email-matteo.fortini@gmail.com>
@ 2014-06-24 11:26 ` Matteo Fortini
  2014-06-25  1:45   ` Bo Shen
                     ` (2 more replies)
  2014-06-24 11:26 ` [PATCH 2/2] sama5d3x: HSMC NAND initialize TIMINGS and import values from U-Boot Matteo Fortini
  1 sibling, 3 replies; 13+ messages in thread
From: Matteo Fortini @ 2014-06-24 11:26 UTC (permalink / raw)
  To: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] sama5d3x: HSMC NAND initialize TIMINGS and import values from U-Boot
       [not found] <1403609192-5862-1-git-send-email-matteo.fortini@gmail.com>
  2014-06-24 11:26 ` [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register Matteo Fortini
@ 2014-06-24 11:26 ` Matteo Fortini
  1 sibling, 0 replies; 13+ messages in thread
From: Matteo Fortini @ 2014-06-24 11:26 UTC (permalink / raw)
  To: barebox

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 | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boards/sama5d3xek/init.c b/arch/arm/boards/sama5d3xek/init.c
index e078642..06e354c 100644
--- a/arch/arm/boards/sama5d3xek/init.c
+++ b/arch/arm/boards/sama5d3xek/init.c
@@ -72,21 +72,32 @@ 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,
+};
+
+static struct sam9_smc_sama5d3_extra_config cm_nand_smc_sama5d_extra_config = {
+	.tclr			= 3,
+	.tadl			= 10,
+	.tar			= 3,
+	.ocms			= 0,
+	.trr			= 4,
+	.twb			= 5,
+	.rbnsel			= 3,
+	.nfsel			= 1
 };
 
 static void ek_add_device_nand(void)
@@ -102,7 +113,7 @@ static void ek_add_device_nand(void)
 		cm_nand_smc_config.mode |= AT91_SMC_DBW_8;
 
 	/* configure chip-select 3 (NAND) */
-	sam9_smc_configure(0, 3, &cm_nand_smc_config);
+	sam9_smc_sama5d3_configure(0, 3, &cm_nand_smc_config, &cm_nand_smc_sama5d_extra_config);
 
 	at91_add_device_nand(&nand_pdata);
 }
-- 
2.0.0


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-06-24 11:26 ` [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register Matteo Fortini
@ 2014-06-25  1:45   ` Bo Shen
  2014-06-25  6:42     ` Sascha Hauer
  2014-07-04  7:47   ` Jean-Christophe PLAGNIOL-VILLARD
  2014-07-04  7:49   ` Jean-Christophe PLAGNIOL-VILLARD
  2 siblings, 1 reply; 13+ messages in thread
From: Bo Shen @ 2014-06-25  1:45 UTC (permalink / raw)
  To: Matteo Fortini, Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-06-25  1:45   ` Bo Shen
@ 2014-06-25  6:42     ` Sascha Hauer
  2014-07-02 10:12       ` Raphaël Poggi
  0 siblings, 1 reply; 13+ messages in thread
From: Sascha Hauer @ 2014-06-25  6:42 UTC (permalink / raw)
  To: Bo Shen; +Cc: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-06-25  6:42     ` Sascha Hauer
@ 2014-07-02 10:12       ` Raphaël Poggi
  0 siblings, 0 replies; 13+ messages in thread
From: Raphaël Poggi @ 2014-07-02 10:12 UTC (permalink / raw)
  To: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-06-24 11:26 ` [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register Matteo Fortini
  2014-06-25  1:45   ` Bo Shen
@ 2014-07-04  7:47   ` Jean-Christophe PLAGNIOL-VILLARD
  2014-07-04  8:28     ` Matteo Fortini
  2014-07-07  6:20     ` Sascha Hauer
  2014-07-04  7:49   ` Jean-Christophe PLAGNIOL-VILLARD
  2 siblings, 2 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-07-04  7:47 UTC (permalink / raw)
  To: Matteo Fortini; +Cc: barebox


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-06-24 11:26 ` [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register Matteo Fortini
  2014-06-25  1:45   ` Bo Shen
  2014-07-04  7:47   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2014-07-04  7:49   ` Jean-Christophe PLAGNIOL-VILLARD
  2 siblings, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-07-04  7:49 UTC (permalink / raw)
  To: Matteo Fortini; +Cc: barebox


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-07-04  7:47   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2014-07-04  8:28     ` Matteo Fortini
  2014-07-07  6:20     ` Sascha Hauer
  1 sibling, 0 replies; 13+ messages in thread
From: Matteo Fortini @ 2014-07-04  8:28 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-07-04  7:47   ` Jean-Christophe PLAGNIOL-VILLARD
  2014-07-04  8:28     ` Matteo Fortini
@ 2014-07-07  6:20     ` Sascha Hauer
  2014-07-07 18:17       ` Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 1 reply; 13+ messages in thread
From: Sascha Hauer @ 2014-07-07  6:20 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-07-07  6:20     ` Sascha Hauer
@ 2014-07-07 18:17       ` Jean-Christophe PLAGNIOL-VILLARD
  2014-07-08  5:39         ` Sascha Hauer
  0 siblings, 1 reply; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-07-07 18:17 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register
  2014-07-07 18:17       ` Jean-Christophe PLAGNIOL-VILLARD
@ 2014-07-08  5:39         ` Sascha Hauer
  0 siblings, 0 replies; 13+ messages in thread
From: Sascha Hauer @ 2014-07-08  5:39 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] sama5d3x: HSMC NAND initialize TIMINGS and import values from U-Boot
       [not found] <1405959207-21839-1-git-send-email-matteo.fortini@gmail.com>
@ 2014-07-21 16:13 ` Matteo Fortini
  0 siblings, 0 replies; 13+ messages in thread
From: Matteo Fortini @ 2014-07-21 16:13 UTC (permalink / raw)
  To: barebox

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 | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boards/sama5d3xek/init.c b/arch/arm/boards/sama5d3xek/init.c
index e078642..743197f 100644
--- a/arch/arm/boards/sama5d3xek/init.c
+++ b/arch/arm/boards/sama5d3xek/init.c
@@ -72,21 +72,30 @@ 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)
@@ -102,7 +111,7 @@ static void ek_add_device_nand(void)
 		cm_nand_smc_config.mode |= AT91_SMC_DBW_8;
 
 	/* configure chip-select 3 (NAND) */
-	sam9_smc_configure(0, 3, &cm_nand_smc_config);
+	sama5_smc_configure(0, 3, &cm_nand_smc_config);
 
 	at91_add_device_nand(&nand_pdata);
 }
-- 
2.0.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] sama5d3x: HSMC NAND initialize TIMINGS and import values from U-Boot
       [not found] <1402045936-18733-1-git-send-email-matteo.fortini@gmail.com>
@ 2014-06-06  9:12 ` Matteo Fortini
  0 siblings, 0 replies; 13+ messages in thread
From: Matteo Fortini @ 2014-06-06  9:12 UTC (permalink / raw)
  To: barebox

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-07-21 16:14 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1403609192-5862-1-git-send-email-matteo.fortini@gmail.com>
2014-06-24 11:26 ` [PATCH 1/2] sama5d3x: fix HSMC MODE register offset and add TIMINGS register Matteo Fortini
2014-06-25  1:45   ` Bo Shen
2014-06-25  6:42     ` Sascha Hauer
2014-07-02 10:12       ` Raphaël Poggi
2014-07-04  7:47   ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-04  8:28     ` Matteo Fortini
2014-07-07  6:20     ` Sascha Hauer
2014-07-07 18:17       ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-08  5:39         ` Sascha Hauer
2014-07-04  7:49   ` Jean-Christophe PLAGNIOL-VILLARD
2014-06-24 11:26 ` [PATCH 2/2] sama5d3x: HSMC NAND initialize TIMINGS and import values from U-Boot Matteo Fortini
     [not found] <1405959207-21839-1-git-send-email-matteo.fortini@gmail.com>
2014-07-21 16:13 ` Matteo Fortini
     [not found] <1402045936-18733-1-git-send-email-matteo.fortini@gmail.com>
2014-06-06  9:12 ` Matteo Fortini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox