* [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