* [PATCH 1/3] ARM: at91: add accessor to manage smc
2011-12-08 15:03 ARM: at91: smc update Jean-Christophe PLAGNIOL-VILLARD
@ 2011-12-08 15:23 ` Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 22:20 ` Ryan Mallon
2011-12-08 15:23 ` [PATCH 2/3] pata/at91: use new introduce smc accessor Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 15:23 ` [PATCH 3/3] ide/at91: " Jean-Christophe PLAGNIOL-VILLARD
2 siblings, 1 reply; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-12-08 15:23 UTC (permalink / raw)
To: linux-arm-kernel
this will allow to configure the smc independtly of the register configuration
as example on rm9200 different from sam9
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
---
arch/arm/mach-at91/include/mach/at91sam9_smc.h | 29 ++++++++
arch/arm/mach-at91/sam9_smc.c | 93 ++++++++++++++++++++++--
arch/arm/mach-at91/sam9_smc.h | 23 ------
3 files changed, 115 insertions(+), 30 deletions(-)
diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
index eb18a70..b5eff0e 100644
--- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
+++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
@@ -18,6 +18,35 @@
#include <mach/cpu.h>
+#ifndef __ASSEMBLY__
+struct sam9_smc_config {
+ /* Setup register */
+ u8 ncs_read_setup;
+ u8 nrd_setup;
+ u8 ncs_write_setup;
+ u8 nwe_setup;
+
+ /* Pulse register */
+ u8 ncs_read_pulse;
+ u8 nrd_pulse;
+ u8 ncs_write_pulse;
+ u8 nwe_pulse;
+
+ /* Cycle register */
+ u16 read_cycle;
+ u16 write_cycle;
+
+ /* Mode register */
+ u32 mode;
+ u8 tdf_cycles:4;
+};
+
+extern int sam9_smc_configure(int id, int cs, struct sam9_smc_config* config);
+extern int sam9_smc_read(int id, int cs, struct sam9_smc_config* config);
+extern int sam9_smc_read_mode(int id, int cs, struct sam9_smc_config* config);
+extern int sam9_smc_write_mode(int id, int cs, struct sam9_smc_config* config);
+#endif
+
#define AT91_SMC_SETUP 0x00 /* Setup Register for CS n */
#define AT91_SMC_NWESETUP (0x3f << 0) /* NWE Setup Length */
#define AT91_SMC_NWESETUP_(x) ((x) << 0)
diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
index 8294783..e5936dc 100644
--- a/arch/arm/mach-at91/sam9_smc.c
+++ b/arch/arm/mach-at91/sam9_smc.c
@@ -2,6 +2,7 @@
* linux/arch/arm/mach-at91/sam9_smc.c
*
* Copyright (C) 2008 Andrew Victor
+ * Copyright (C) 2011 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -17,13 +18,58 @@
#include "sam9_smc.h"
-
#define AT91_SMC_CS(id, n) (smc_base_addr[id] + ((n) * 0x10))
static void __iomem *smc_base_addr[2];
-static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_config* config)
+static void __init_refok sam9_smc_cs_write_mode(void __iomem *base,
+ struct sam9_smc_config* config)
+{
+ __raw_writel(config->mode
+ | AT91_SMC_TDF_(config->tdf_cycles),
+ base + AT91_SMC_MODE);
+}
+
+int __init_refok sam9_smc_write_mode(int id, int cs,
+ struct sam9_smc_config* config)
+{
+ if (!smc_base_addr[id])
+ return -EINVAL;
+
+ sam9_smc_cs_write_mode(AT91_SMC_CS(id, cs), config);
+
+ return 0;
+}
+
+int __init_refok sam9_smc_configure(int id, int cs,
+ struct sam9_smc_config* config)
+{
+ if (!smc_base_addr[id])
+ return -EINVAL;
+
+ sam9_smc_cs_configure(AT91_SMC_CS(id, cs), config);
+
+ return 0;
+}
+
+static void __init_refok sam9_smc_cs_read_mode(void __iomem *base,
+ struct sam9_smc_config* config)
+{
+ u32 val = __raw_readl(base + AT91_SMC_MODE);
+
+ config->mode = (val & ~AT91_SMC_NWECYCLE);
+ config->tdf_cycles = (val & AT91_SMC_NWECYCLE) >> 16 ;
+}
+
+int __init_refok sam9_smc_read_mode(int id, int cs,
+ struct sam9_smc_config* config)
{
+ void __iomen *base;
+
+ if (!smc_base_addr[id])
+ return -EINVAL;
+
+ base = AT91_SMC_CS(id, cs);
/* Setup register */
__raw_writel(AT91_SMC_NWESETUP_(config->nwe_setup)
@@ -45,14 +91,47 @@ static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_con
base + AT91_SMC_CYCLE);
/* Mode register */
- __raw_writel(config->mode
- | AT91_SMC_TDF_(config->tdf_cycles),
- base + AT91_SMC_MODE);
+ sam9_smc_cs_write_mode(base, config);
+
+ return 0;
}
-void __init sam9_smc_configure(int id, int cs, struct sam9_smc_config* config)
+int __init_refok sam9_smc_read(int id, int cs, struct sam9_smc_config* config)
{
- sam9_smc_cs_configure(AT91_SMC_CS(id, cs), config);
+ void __iomen *base;
+ u32 val;
+
+ if (!smc_base_addr[id])
+ return -EINVAL;
+
+ base = AT91_SMC_CS(id, cs);
+
+ /* Setup register */
+ val = __raw_readl(base + AT91_SMC_SETUP);
+
+ config->nwe_setup = val & AT91_SMC_NWESETUP;
+ config->ncs_write_setup = (val & AT91_SMC_NCS_WRSETUP) >> 8;
+ config->nrd_setup = (val & AT91_SMC_NRDSETUP) >> 16;
+ config->ncs_read_setup = (val & AT91_SMC_NCS_RDSETUP) >> 24;
+
+ /* Pulse register */
+ val = __raw_readl(base + AT91_SMC_PULSE);
+
+ config->nwe_setup = val & AT91_SMC_NWEPULSE;
+ config->ncs_write_pulse = (val & AT91_SMC_NCS_WRPULSE) >> 8;
+ config->nrd_pulse = (val & AT91_SMC_NRDPULSE) >> 16;
+ config->ncs_read_pulse = (val & AT91_SMC_NCS_RDPULSE) >> 24;
+
+ /* Cycle register */
+ val = __raw_readl(base + AT91_SMC_CYCLE);
+
+ config->write_cycle = val & AT91_SMC_NWECYCLE;
+ config->read_cycle = (val & AT91_SMC_NRDCYCLE) >> 16;
+
+ /* Mode register */
+ sam9_smc_cs_read_mode(base, config);
+
+ return 0;
}
void __init at91sam9_ioremap_smc(int id, u32 addr)
diff --git a/arch/arm/mach-at91/sam9_smc.h b/arch/arm/mach-at91/sam9_smc.h
index 039c5ce..3e52dcd4 100644
--- a/arch/arm/mach-at91/sam9_smc.h
+++ b/arch/arm/mach-at91/sam9_smc.h
@@ -8,27 +8,4 @@
* published by the Free Software Foundation.
*/
-struct sam9_smc_config {
- /* Setup register */
- u8 ncs_read_setup;
- u8 nrd_setup;
- u8 ncs_write_setup;
- u8 nwe_setup;
-
- /* Pulse register */
- u8 ncs_read_pulse;
- u8 nrd_pulse;
- u8 ncs_write_pulse;
- u8 nwe_pulse;
-
- /* Cycle register */
- u16 read_cycle;
- u16 write_cycle;
-
- /* Mode register */
- u32 mode;
- u8 tdf_cycles:4;
-};
-
-extern void __init sam9_smc_configure(int id, int cs, struct sam9_smc_config* config);
extern void __init at91sam9_ioremap_smc(int id, u32 addr);
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 1/3] ARM: at91: add accessor to manage smc
2011-12-08 15:23 ` [PATCH 1/3] ARM: at91: add accessor to manage smc Jean-Christophe PLAGNIOL-VILLARD
@ 2011-12-08 22:20 ` Ryan Mallon
2011-12-09 6:27 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 1 reply; 16+ messages in thread
From: Ryan Mallon @ 2011-12-08 22:20 UTC (permalink / raw)
To: linux-arm-kernel
On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
> this will allow to configure the smc independtly of the register configuration
> as example on rm9200 different from sam9
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Hi Jean,
Couple of comments below.
~Ryan
> ---
> arch/arm/mach-at91/include/mach/at91sam9_smc.h | 29 ++++++++
> arch/arm/mach-at91/sam9_smc.c | 93 ++++++++++++++++++++++--
> arch/arm/mach-at91/sam9_smc.h | 23 ------
> 3 files changed, 115 insertions(+), 30 deletions(-)
>
> diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> index eb18a70..b5eff0e 100644
> --- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> +++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> @@ -18,6 +18,35 @@
>
> #include <mach/cpu.h>
>
> +#ifndef __ASSEMBLY__
> +struct sam9_smc_config {
> + /* Setup register */
> + u8 ncs_read_setup;
> + u8 nrd_setup;
> + u8 ncs_write_setup;
> + u8 nwe_setup;
> +
> + /* Pulse register */
> + u8 ncs_read_pulse;
> + u8 nrd_pulse;
> + u8 ncs_write_pulse;
> + u8 nwe_pulse;
> +
> + /* Cycle register */
> + u16 read_cycle;
> + u16 write_cycle;
> +
> + /* Mode register */
> + u32 mode;
> + u8 tdf_cycles:4;
> +};
> +
> +extern int sam9_smc_configure(int id, int cs, struct sam9_smc_config* config);
> +extern int sam9_smc_read(int id, int cs, struct sam9_smc_config* config);
> +extern int sam9_smc_read_mode(int id, int cs, struct sam9_smc_config* config);
> +extern int sam9_smc_write_mode(int id, int cs, struct sam9_smc_config* config);
> +#endif
> +
> #define AT91_SMC_SETUP 0x00 /* Setup Register for CS n */
> #define AT91_SMC_NWESETUP (0x3f << 0) /* NWE Setup Length */
> #define AT91_SMC_NWESETUP_(x) ((x) << 0)
> diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
> index 8294783..e5936dc 100644
> --- a/arch/arm/mach-at91/sam9_smc.c
> +++ b/arch/arm/mach-at91/sam9_smc.c
> @@ -2,6 +2,7 @@
> * linux/arch/arm/mach-at91/sam9_smc.c
> *
> * Copyright (C) 2008 Andrew Victor
> + * Copyright (C) 2011 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -17,13 +18,58 @@
>
> #include "sam9_smc.h"
>
> -
> #define AT91_SMC_CS(id, n) (smc_base_addr[id] + ((n) * 0x10))
>
> static void __iomem *smc_base_addr[2];
>
> -static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_config* config)
> +static void __init_refok sam9_smc_cs_write_mode(void __iomem *base,
> + struct sam9_smc_config* config)
include/linux/kernel.h says that __init_refok means that this function
can reference __init code, without being __init itself and without
generating a modpost warning. It also says that such cases should be
documented as to why the reference is ok. Why is __init_refok being used
here and on the other fucntions in this file?
> +{
> + __raw_writel(config->mode
> + | AT91_SMC_TDF_(config->tdf_cycles),
> + base + AT91_SMC_MODE);
> +}
> +
> +int __init_refok sam9_smc_write_mode(int id, int cs,
> + struct sam9_smc_config* config)
> +{
> + if (!smc_base_addr[id])
> + return -EINVAL;
> +
> + sam9_smc_cs_write_mode(AT91_SMC_CS(id, cs), config);
> +
> + return 0;
> +}
> +
> +int __init_refok sam9_smc_configure(int id, int cs,
> + struct sam9_smc_config* config)
> +{
> + if (!smc_base_addr[id])
> + return -EINVAL;
> +
> + sam9_smc_cs_configure(AT91_SMC_CS(id, cs), config);
> +
> + return 0;
> +}
> +
> +static void __init_refok sam9_smc_cs_read_mode(void __iomem *base,
> + struct sam9_smc_config* config)
> +{
> + u32 val = __raw_readl(base + AT91_SMC_MODE);
> +
> + config->mode = (val & ~AT91_SMC_NWECYCLE);
> + config->tdf_cycles = (val & AT91_SMC_NWECYCLE) >> 16 ;
> +}
> +
> +int __init_refok sam9_smc_read_mode(int id, int cs,
> + struct sam9_smc_config* config)
> {
> + void __iomen *base;
Typo: __iomem.
> +
> + if (!smc_base_addr[id])
> + return -EINVAL;
> +
> + base = AT91_SMC_CS(id, cs);
>
> /* Setup register */
> __raw_writel(AT91_SMC_NWESETUP_(config->nwe_setup)
> @@ -45,14 +91,47 @@ static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_con
> base + AT91_SMC_CYCLE);
>
> /* Mode register */
> - __raw_writel(config->mode
> - | AT91_SMC_TDF_(config->tdf_cycles),
> - base + AT91_SMC_MODE);
> + sam9_smc_cs_write_mode(base, config);
> +
> + return 0;
> }
>
> -void __init sam9_smc_configure(int id, int cs, struct sam9_smc_config* config)
> +int __init_refok sam9_smc_read(int id, int cs, struct sam9_smc_config* config)
> {
> - sam9_smc_cs_configure(AT91_SMC_CS(id, cs), config);
> + void __iomen *base;
Typo: __iomem. Has this been compile tested?
> + u32 val;
> +
> + if (!smc_base_addr[id])
> + return -EINVAL;
> +
> + base = AT91_SMC_CS(id, cs);
> +
> + /* Setup register */
> + val = __raw_readl(base + AT91_SMC_SETUP);
> +
> + config->nwe_setup = val & AT91_SMC_NWESETUP;
> + config->ncs_write_setup = (val & AT91_SMC_NCS_WRSETUP) >> 8;
> + config->nrd_setup = (val & AT91_SMC_NRDSETUP) >> 16;
> + config->ncs_read_setup = (val & AT91_SMC_NCS_RDSETUP) >> 24;
> +
> + /* Pulse register */
> + val = __raw_readl(base + AT91_SMC_PULSE);
> +
> + config->nwe_setup = val & AT91_SMC_NWEPULSE;
> + config->ncs_write_pulse = (val & AT91_SMC_NCS_WRPULSE) >> 8;
> + config->nrd_pulse = (val & AT91_SMC_NRDPULSE) >> 16;
> + config->ncs_read_pulse = (val & AT91_SMC_NCS_RDPULSE) >> 24;
> +
> + /* Cycle register */
> + val = __raw_readl(base + AT91_SMC_CYCLE);
> +
> + config->write_cycle = val & AT91_SMC_NWECYCLE;
> + config->read_cycle = (val & AT91_SMC_NRDCYCLE) >> 16;
> +
> + /* Mode register */
> + sam9_smc_cs_read_mode(base, config);
> +
> + return 0;
> }
>
> void __init at91sam9_ioremap_smc(int id, u32 addr)
> diff --git a/arch/arm/mach-at91/sam9_smc.h b/arch/arm/mach-at91/sam9_smc.h
> index 039c5ce..3e52dcd4 100644
> --- a/arch/arm/mach-at91/sam9_smc.h
> +++ b/arch/arm/mach-at91/sam9_smc.h
> @@ -8,27 +8,4 @@
> * published by the Free Software Foundation.
> */
>
> -struct sam9_smc_config {
> - /* Setup register */
> - u8 ncs_read_setup;
> - u8 nrd_setup;
> - u8 ncs_write_setup;
> - u8 nwe_setup;
> -
> - /* Pulse register */
> - u8 ncs_read_pulse;
> - u8 nrd_pulse;
> - u8 ncs_write_pulse;
> - u8 nwe_pulse;
> -
> - /* Cycle register */
> - u16 read_cycle;
> - u16 write_cycle;
> -
> - /* Mode register */
> - u32 mode;
> - u8 tdf_cycles:4;
> -};
> -
> -extern void __init sam9_smc_configure(int id, int cs, struct sam9_smc_config* config);
> extern void __init at91sam9_ioremap_smc(int id, u32 addr);
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] ARM: at91: add accessor to manage smc
2011-12-08 22:20 ` Ryan Mallon
@ 2011-12-09 6:27 ` Jean-Christophe PLAGNIOL-VILLARD
2011-12-09 7:06 ` Ryan Mallon
0 siblings, 1 reply; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-12-09 6:27 UTC (permalink / raw)
To: linux-arm-kernel
On 09:20 Fri 09 Dec , Ryan Mallon wrote:
> On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> > this will allow to configure the smc independtly of the register configuration
> > as example on rm9200 different from sam9
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>
>
> Hi Jean,
>
> Couple of comments below.
>
> ~Ryan
>
> > ---
> > arch/arm/mach-at91/include/mach/at91sam9_smc.h | 29 ++++++++
> > arch/arm/mach-at91/sam9_smc.c | 93 ++++++++++++++++++++++--
> > arch/arm/mach-at91/sam9_smc.h | 23 ------
> > 3 files changed, 115 insertions(+), 30 deletions(-)
> >
> > diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> > index eb18a70..b5eff0e 100644
> > --- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> > +++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
> > @@ -18,6 +18,35 @@
> >
> > #include <mach/cpu.h>
> >
> > +#ifndef __ASSEMBLY__
> > +struct sam9_smc_config {
> > + /* Setup register */
> > + u8 ncs_read_setup;
> > + u8 nrd_setup;
> > + u8 ncs_write_setup;
> > + u8 nwe_setup;
> > +
> > + /* Pulse register */
> > + u8 ncs_read_pulse;
> > + u8 nrd_pulse;
> > + u8 ncs_write_pulse;
> > + u8 nwe_pulse;
> > +
> > + /* Cycle register */
> > + u16 read_cycle;
> > + u16 write_cycle;
> > +
> > + /* Mode register */
> > + u32 mode;
> > + u8 tdf_cycles:4;
> > +};
> > +
> > +extern int sam9_smc_configure(int id, int cs, struct sam9_smc_config* config);
> > +extern int sam9_smc_read(int id, int cs, struct sam9_smc_config* config);
> > +extern int sam9_smc_read_mode(int id, int cs, struct sam9_smc_config* config);
> > +extern int sam9_smc_write_mode(int id, int cs, struct sam9_smc_config* config);
> > +#endif
> > +
> > #define AT91_SMC_SETUP 0x00 /* Setup Register for CS n */
> > #define AT91_SMC_NWESETUP (0x3f << 0) /* NWE Setup Length */
> > #define AT91_SMC_NWESETUP_(x) ((x) << 0)
> > diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
> > index 8294783..e5936dc 100644
> > --- a/arch/arm/mach-at91/sam9_smc.c
> > +++ b/arch/arm/mach-at91/sam9_smc.c
> > @@ -2,6 +2,7 @@
> > * linux/arch/arm/mach-at91/sam9_smc.c
> > *
> > * Copyright (C) 2008 Andrew Victor
> > + * Copyright (C) 2011 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > *
> > * This program is free software; you can redistribute it and/or modify
> > * it under the terms of the GNU General Public License version 2 as
> > @@ -17,13 +18,58 @@
> >
> > #include "sam9_smc.h"
> >
> > -
> > #define AT91_SMC_CS(id, n) (smc_base_addr[id] + ((n) * 0x10))
> >
> > static void __iomem *smc_base_addr[2];
> >
> > -static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_config* config)
> > +static void __init_refok sam9_smc_cs_write_mode(void __iomem *base,
> > + struct sam9_smc_config* config)
>
>
> include/linux/kernel.h says that __init_refok means that this function
> can reference __init code, without being __init itself and without
> generating a modpost warning. It also says that such cases should be
> documented as to why the reference is ok. Why is __init_refok being used
> here and on the other fucntions in this file?
because you can you this function before and after init and this need to not
be free by the kernel after boot
>
> > +{
> > + __raw_writel(config->mode
> > + | AT91_SMC_TDF_(config->tdf_cycles),
> > + base + AT91_SMC_MODE);
> > +}
> > +
> > +int __init_refok sam9_smc_write_mode(int id, int cs,
> > + struct sam9_smc_config* config)
> > +{
> > + if (!smc_base_addr[id])
> > + return -EINVAL;
> > +
> > + sam9_smc_cs_write_mode(AT91_SMC_CS(id, cs), config);
> > +
> > + return 0;
> > +}
> > +
> > +int __init_refok sam9_smc_configure(int id, int cs,
> > + struct sam9_smc_config* config)
> > +{
> > + if (!smc_base_addr[id])
> > + return -EINVAL;
> > +
> > + sam9_smc_cs_configure(AT91_SMC_CS(id, cs), config);
> > +
> > + return 0;
> > +}
> > +
> > +static void __init_refok sam9_smc_cs_read_mode(void __iomem *base,
> > + struct sam9_smc_config* config)
> > +{
> > + u32 val = __raw_readl(base + AT91_SMC_MODE);
> > +
> > + config->mode = (val & ~AT91_SMC_NWECYCLE);
> > + config->tdf_cycles = (val & AT91_SMC_NWECYCLE) >> 16 ;
> > +}
> > +
> > +int __init_refok sam9_smc_read_mode(int id, int cs,
> > + struct sam9_smc_config* config)
> > {
> > + void __iomen *base;
>
>
> Typo: __iomem.
>
> > +
> > + if (!smc_base_addr[id])
> > + return -EINVAL;
> > +
> > + base = AT91_SMC_CS(id, cs);
> >
> > /* Setup register */
> > __raw_writel(AT91_SMC_NWESETUP_(config->nwe_setup)
> > @@ -45,14 +91,47 @@ static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_con
> > base + AT91_SMC_CYCLE);
> >
> > /* Mode register */
> > - __raw_writel(config->mode
> > - | AT91_SMC_TDF_(config->tdf_cycles),
> > - base + AT91_SMC_MODE);
> > + sam9_smc_cs_write_mode(base, config);
> > +
> > + return 0;
> > }
> >
> > -void __init sam9_smc_configure(int id, int cs, struct sam9_smc_config* config)
> > +int __init_refok sam9_smc_read(int id, int cs, struct sam9_smc_config* config)
> > {
> > - sam9_smc_cs_configure(AT91_SMC_CS(id, cs), config);
> > + void __iomen *base;
>
>
> Typo: __iomem. Has this been compile tested?
yes and tested but rebased on for-next where the typo could have beed introduced
Best Regards,
J.
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] ARM: at91: add accessor to manage smc
2011-12-09 6:27 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2011-12-09 7:06 ` Ryan Mallon
0 siblings, 0 replies; 16+ messages in thread
From: Ryan Mallon @ 2011-12-09 7:06 UTC (permalink / raw)
To: linux-arm-kernel
On 09/12/11 17:27, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:20 Fri 09 Dec , Ryan Mallon wrote:
>> On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>>> this will allow to configure the smc independtly of the register configuration
>>> as example on rm9200 different from sam9
>>>
>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>
>> Hi Jean,
>>
>> Couple of comments below.
>>
>> ~Ryan
>>
>>> ---
>>> arch/arm/mach-at91/include/mach/at91sam9_smc.h | 29 ++++++++
>>> arch/arm/mach-at91/sam9_smc.c | 93 ++++++++++++++++++++++--
>>> arch/arm/mach-at91/sam9_smc.h | 23 ------
>>> 3 files changed, 115 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-at91/include/mach/at91sam9_smc.h b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>>> index eb18a70..b5eff0e 100644
>>> --- a/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>>> +++ b/arch/arm/mach-at91/include/mach/at91sam9_smc.h
>>> @@ -18,6 +18,35 @@
>>>
>>> #include <mach/cpu.h>
>>>
>>> +#ifndef __ASSEMBLY__
>>> +struct sam9_smc_config {
>>> + /* Setup register */
>>> + u8 ncs_read_setup;
>>> + u8 nrd_setup;
>>> + u8 ncs_write_setup;
>>> + u8 nwe_setup;
>>> +
>>> + /* Pulse register */
>>> + u8 ncs_read_pulse;
>>> + u8 nrd_pulse;
>>> + u8 ncs_write_pulse;
>>> + u8 nwe_pulse;
>>> +
>>> + /* Cycle register */
>>> + u16 read_cycle;
>>> + u16 write_cycle;
>>> +
>>> + /* Mode register */
>>> + u32 mode;
>>> + u8 tdf_cycles:4;
>>> +};
>>> +
>>> +extern int sam9_smc_configure(int id, int cs, struct sam9_smc_config* config);
>>> +extern int sam9_smc_read(int id, int cs, struct sam9_smc_config* config);
>>> +extern int sam9_smc_read_mode(int id, int cs, struct sam9_smc_config* config);
>>> +extern int sam9_smc_write_mode(int id, int cs, struct sam9_smc_config* config);
>>> +#endif
>>> +
>>> #define AT91_SMC_SETUP 0x00 /* Setup Register for CS n */
>>> #define AT91_SMC_NWESETUP (0x3f << 0) /* NWE Setup Length */
>>> #define AT91_SMC_NWESETUP_(x) ((x) << 0)
>>> diff --git a/arch/arm/mach-at91/sam9_smc.c b/arch/arm/mach-at91/sam9_smc.c
>>> index 8294783..e5936dc 100644
>>> --- a/arch/arm/mach-at91/sam9_smc.c
>>> +++ b/arch/arm/mach-at91/sam9_smc.c
>>> @@ -2,6 +2,7 @@
>>> * linux/arch/arm/mach-at91/sam9_smc.c
>>> *
>>> * Copyright (C) 2008 Andrew Victor
>>> + * Copyright (C) 2011 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> *
>>> * This program is free software; you can redistribute it and/or modify
>>> * it under the terms of the GNU General Public License version 2 as
>>> @@ -17,13 +18,58 @@
>>>
>>> #include "sam9_smc.h"
>>>
>>> -
>>> #define AT91_SMC_CS(id, n) (smc_base_addr[id] + ((n) * 0x10))
>>>
>>> static void __iomem *smc_base_addr[2];
>>>
>>> -static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_config* config)
>>> +static void __init_refok sam9_smc_cs_write_mode(void __iomem *base,
>>> + struct sam9_smc_config* config)
>>
>> include/linux/kernel.h says that __init_refok means that this function
>> can reference __init code, without being __init itself and without
>> generating a modpost warning. It also says that such cases should be
>> documented as to why the reference is ok. Why is __init_refok being used
>> here and on the other fucntions in this file?
> because you can you this function before and after init and this need to not
> be free by the kernel after boot
Lots of functions are used before and after init without needing special
markers. __init_refok means that this function references __init code
even though the function itself is not __init (which would usually
generate a modpost warning). This function doesn't reference any __init
code, so the __init_refok should not be needed?
~Ryan
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] pata/at91: use new introduce smc accessor
2011-12-08 15:03 ARM: at91: smc update Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 15:23 ` [PATCH 1/3] ARM: at91: add accessor to manage smc Jean-Christophe PLAGNIOL-VILLARD
@ 2011-12-08 15:23 ` Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 22:34 ` Ryan Mallon
2011-12-09 10:59 ` Sergei Shtylyov
2011-12-08 15:23 ` [PATCH 3/3] ide/at91: " Jean-Christophe PLAGNIOL-VILLARD
2 siblings, 2 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-12-08 15:23 UTC (permalink / raw)
To: linux-arm-kernel
this will allow to use the pata_at91 on a single zImage
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: linux-ide at vger.kernel.org
---
Hi,
it's depends on other patch for AT91 can we apply via at91
Best Regards,
J.
drivers/ata/pata_at91.c | 43 +++++++++++++++++++++----------------------
1 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
index 5249e6d..c8d1154 100644
--- a/drivers/ata/pata_at91.c
+++ b/drivers/ata/pata_at91.c
@@ -207,11 +207,11 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
{
int ret = 0;
int use_iordy;
+ struct sam9_smc_config smc;
unsigned int t6z; /* data tristate time in ns */
unsigned int cycle; /* SMC Cycle width in MCK ticks */
unsigned int setup; /* SMC Setup width in MCK ticks */
unsigned int pulse; /* CFIOR and CFIOW pulse width in MCK ticks */
- unsigned int cs_setup = 0;/* CS4 or CS5 setup width in MCK ticks */
unsigned int cs_pulse; /* CS4 or CS5 pulse width in MCK ticks*/
unsigned int tdf_cycles; /* SMC TDF MCK ticks */
unsigned long mck_hz; /* MCK frequency in Hz */
@@ -244,26 +244,25 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
}
dev_dbg(dev, "Use IORDY=%u, TDF Cycles=%u\n", use_iordy, tdf_cycles);
- info->mode |= AT91_SMC_TDF_(tdf_cycles);
/* write SMC Setup Register */
- at91_sys_write(AT91_SMC_SETUP(info->cs),
- AT91_SMC_NWESETUP_(setup) |
- AT91_SMC_NRDSETUP_(setup) |
- AT91_SMC_NCS_WRSETUP_(cs_setup) |
- AT91_SMC_NCS_RDSETUP_(cs_setup));
+ smc.nrd_setup = setup;
+ smc.nwe_setup = smc.nrd_setup;
+ smc.ncs_read_setup = 0;
+ smc.ncs_write_setup = smc.ncs_read_setup;
/* write SMC Pulse Register */
- at91_sys_write(AT91_SMC_PULSE(info->cs),
- AT91_SMC_NWEPULSE_(pulse) |
- AT91_SMC_NRDPULSE_(pulse) |
- AT91_SMC_NCS_WRPULSE_(cs_pulse) |
- AT91_SMC_NCS_RDPULSE_(cs_pulse));
+ smc.nrd_pulse = pulse;
+ smc.nwe_pulse = smc.nrd_pulse;
+ smc.ncs_read_pulse =cs_pulse;
+ smc.ncs_write_pulse = smc.ncs_read_pulse;
/* write SMC Cycle Register */
- at91_sys_write(AT91_SMC_CYCLE(info->cs),
- AT91_SMC_NWECYCLE_(cycle) |
- AT91_SMC_NRDCYCLE_(cycle));
+ smc.read_cycle = cycle;
+ smc.write_cycle = smc.read_cycle;
/* write SMC Mode Register*/
- at91_sys_write(AT91_SMC_MODE(info->cs), info->mode);
+ smc.tdf_cycles = tdf_cycles;
+ smc.mode = info->mode;
+
+ sam9_smc_configure(0, info->cs, &smc);
}
static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev)
@@ -288,20 +287,20 @@ static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev,
struct at91_ide_info *info = dev->link->ap->host->private_data;
unsigned int consumed;
unsigned long flags;
- unsigned int mode;
+ struct sam9_smc_config smc;
local_irq_save(flags);
- mode = at91_sys_read(AT91_SMC_MODE(info->cs));
+ sam9_smc_read_mode(0, info->cs, &smc);
/* set 16bit mode before writing data */
- at91_sys_write(AT91_SMC_MODE(info->cs),
- (mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16);
+ smc.mode = (smc.mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16;
+ sam9_smc_write_mode(0, info->cs, &smc);
consumed = ata_sff_data_xfer(dev, buf, buflen, rw);
/* restore 8bit mode after data is written */
- at91_sys_write(AT91_SMC_MODE(info->cs),
- (mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8);
+ smc.mode = (smc.mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8;
+ sam9_smc_write_mode(0, info->cs, &smc);
local_irq_restore(flags);
return consumed;
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/3] pata/at91: use new introduce smc accessor
2011-12-08 15:23 ` [PATCH 2/3] pata/at91: use new introduce smc accessor Jean-Christophe PLAGNIOL-VILLARD
@ 2011-12-08 22:34 ` Ryan Mallon
2011-12-09 6:24 ` Jean-Christophe PLAGNIOL-VILLARD
2011-12-09 10:59 ` Sergei Shtylyov
1 sibling, 1 reply; 16+ messages in thread
From: Ryan Mallon @ 2011-12-08 22:34 UTC (permalink / raw)
To: linux-arm-kernel
On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
> this will allow to use the pata_at91 on a single zImage
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: linux-ide at vger.kernel.org
Some comments below,
~Ryan
> ---
> Hi,
>
> it's depends on other patch for AT91 can we apply via at91
>
> Best Regards,
> J.
> drivers/ata/pata_at91.c | 43 +++++++++++++++++++++----------------------
> 1 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
> index 5249e6d..c8d1154 100644
> --- a/drivers/ata/pata_at91.c
> +++ b/drivers/ata/pata_at91.c
> @@ -207,11 +207,11 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
> {
> int ret = 0;
> int use_iordy;
> + struct sam9_smc_config smc;
> unsigned int t6z; /* data tristate time in ns */
> unsigned int cycle; /* SMC Cycle width in MCK ticks */
> unsigned int setup; /* SMC Setup width in MCK ticks */
> unsigned int pulse; /* CFIOR and CFIOW pulse width in MCK ticks */
> - unsigned int cs_setup = 0;/* CS4 or CS5 setup width in MCK ticks */
> unsigned int cs_pulse; /* CS4 or CS5 pulse width in MCK ticks*/
> unsigned int tdf_cycles; /* SMC TDF MCK ticks */
> unsigned long mck_hz; /* MCK frequency in Hz */
> @@ -244,26 +244,25 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
> }
>
> dev_dbg(dev, "Use IORDY=%u, TDF Cycles=%u\n", use_iordy, tdf_cycles);
> - info->mode |= AT91_SMC_TDF_(tdf_cycles);
>
> /* write SMC Setup Register */
> - at91_sys_write(AT91_SMC_SETUP(info->cs),
> - AT91_SMC_NWESETUP_(setup) |
> - AT91_SMC_NRDSETUP_(setup) |
> - AT91_SMC_NCS_WRSETUP_(cs_setup) |
> - AT91_SMC_NCS_RDSETUP_(cs_setup));
> + smc.nrd_setup = setup;
> + smc.nwe_setup = smc.nrd_setup;
> + smc.ncs_read_setup = 0;
> + smc.ncs_write_setup = smc.ncs_read_setup;
> /* write SMC Pulse Register */
> - at91_sys_write(AT91_SMC_PULSE(info->cs),
> - AT91_SMC_NWEPULSE_(pulse) |
> - AT91_SMC_NRDPULSE_(pulse) |
> - AT91_SMC_NCS_WRPULSE_(cs_pulse) |
> - AT91_SMC_NCS_RDPULSE_(cs_pulse));
> + smc.nrd_pulse = pulse;
> + smc.nwe_pulse = smc.nrd_pulse;
> + smc.ncs_read_pulse =cs_pulse;
Nitpick: Whitespace around the = operator.
> + smc.ncs_write_pulse = smc.ncs_read_pulse;
> /* write SMC Cycle Register */
> - at91_sys_write(AT91_SMC_CYCLE(info->cs),
> - AT91_SMC_NWECYCLE_(cycle) |
> - AT91_SMC_NRDCYCLE_(cycle));
> + smc.read_cycle = cycle;
> + smc.write_cycle = smc.read_cycle;
> /* write SMC Mode Register*/
> - at91_sys_write(AT91_SMC_MODE(info->cs), info->mode);
> + smc.tdf_cycles = tdf_cycles;
The "write SMC Mode Register" comment should be removed.
> + smc.mode = info->mode;
> +
> + sam9_smc_configure(0, info->cs, &smc);
This new function returns an int, should we be checking the return value
here?
> }
>
> static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev)
> @@ -288,20 +287,20 @@ static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev,
> struct at91_ide_info *info = dev->link->ap->host->private_data;
> unsigned int consumed;
> unsigned long flags;
> - unsigned int mode;
> + struct sam9_smc_config smc;
>
> local_irq_save(flags);
> - mode = at91_sys_read(AT91_SMC_MODE(info->cs));
> + sam9_smc_read_mode(0, info->cs, &smc);
>
> /* set 16bit mode before writing data */
> - at91_sys_write(AT91_SMC_MODE(info->cs),
> - (mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16);
> + smc.mode = (smc.mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16;
> + sam9_smc_write_mode(0, info->cs, &smc);
Do sam9_smc_read/write_mode really need to pass the whole smc structure?
The only fields used are mode and tdf_cycles. It might be clearer to
pass those directly.
Also the original code here doesn't write tdf_cycles as part of the
mode. Perhaps it would be better to have sam9_smc_write_mode to be:
int sam9_smc_write_mode(int id, int cs, unsigned mode);
and in set_smc_timing above explicitly or in the tdf_cycles bits?
>
> consumed = ata_sff_data_xfer(dev, buf, buflen, rw);
>
> /* restore 8bit mode after data is written */
> - at91_sys_write(AT91_SMC_MODE(info->cs),
> - (mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8);
> + smc.mode = (smc.mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8;
> + sam9_smc_write_mode(0, info->cs, &smc);
>
> local_irq_restore(flags);
> return consumed;
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 2/3] pata/at91: use new introduce smc accessor
2011-12-08 22:34 ` Ryan Mallon
@ 2011-12-09 6:24 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-12-09 6:24 UTC (permalink / raw)
To: linux-arm-kernel
On 09:34 Fri 09 Dec , Ryan Mallon wrote:
> On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> > this will allow to use the pata_at91 on a single zImage
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > Cc: linux-ide at vger.kernel.org
>
>
> Some comments below,
>
> ~Ryan
>
> > ---
> > Hi,
> >
> > it's depends on other patch for AT91 can we apply via at91
> >
> > Best Regards,
> > J.
> > drivers/ata/pata_at91.c | 43 +++++++++++++++++++++----------------------
> > 1 files changed, 21 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
> > index 5249e6d..c8d1154 100644
> > --- a/drivers/ata/pata_at91.c
> > +++ b/drivers/ata/pata_at91.c
> > @@ -207,11 +207,11 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
> > {
> > int ret = 0;
> > int use_iordy;
> > + struct sam9_smc_config smc;
> > unsigned int t6z; /* data tristate time in ns */
> > unsigned int cycle; /* SMC Cycle width in MCK ticks */
> > unsigned int setup; /* SMC Setup width in MCK ticks */
> > unsigned int pulse; /* CFIOR and CFIOW pulse width in MCK ticks */
> > - unsigned int cs_setup = 0;/* CS4 or CS5 setup width in MCK ticks */
> > unsigned int cs_pulse; /* CS4 or CS5 pulse width in MCK ticks*/
> > unsigned int tdf_cycles; /* SMC TDF MCK ticks */
> > unsigned long mck_hz; /* MCK frequency in Hz */
> > @@ -244,26 +244,25 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
> > }
> >
> > dev_dbg(dev, "Use IORDY=%u, TDF Cycles=%u\n", use_iordy, tdf_cycles);
> > - info->mode |= AT91_SMC_TDF_(tdf_cycles);
> >
> > /* write SMC Setup Register */
> > - at91_sys_write(AT91_SMC_SETUP(info->cs),
> > - AT91_SMC_NWESETUP_(setup) |
> > - AT91_SMC_NRDSETUP_(setup) |
> > - AT91_SMC_NCS_WRSETUP_(cs_setup) |
> > - AT91_SMC_NCS_RDSETUP_(cs_setup));
> > + smc.nrd_setup = setup;
> > + smc.nwe_setup = smc.nrd_setup;
> > + smc.ncs_read_setup = 0;
> > + smc.ncs_write_setup = smc.ncs_read_setup;
> > /* write SMC Pulse Register */
> > - at91_sys_write(AT91_SMC_PULSE(info->cs),
> > - AT91_SMC_NWEPULSE_(pulse) |
> > - AT91_SMC_NRDPULSE_(pulse) |
> > - AT91_SMC_NCS_WRPULSE_(cs_pulse) |
> > - AT91_SMC_NCS_RDPULSE_(cs_pulse));
> > + smc.nrd_pulse = pulse;
> > + smc.nwe_pulse = smc.nrd_pulse;
> > + smc.ncs_read_pulse =cs_pulse;
>
>
> Nitpick: Whitespace around the = operator.
>
> > + smc.ncs_write_pulse = smc.ncs_read_pulse;
> > /* write SMC Cycle Register */
> > - at91_sys_write(AT91_SMC_CYCLE(info->cs),
> > - AT91_SMC_NWECYCLE_(cycle) |
> > - AT91_SMC_NRDCYCLE_(cycle));
> > + smc.read_cycle = cycle;
> > + smc.write_cycle = smc.read_cycle;
> > /* write SMC Mode Register*/
> > - at91_sys_write(AT91_SMC_MODE(info->cs), info->mode);
> > + smc.tdf_cycles = tdf_cycles;
>
>
> The "write SMC Mode Register" comment should be removed.
>
> > + smc.mode = info->mode;
> > +
> > + sam9_smc_configure(0, info->cs, &smc);
>
>
> This new function returns an int, should we be checking the return value
> here?
>
> > }
> >
> > static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev)
> > @@ -288,20 +287,20 @@ static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev,
> > struct at91_ide_info *info = dev->link->ap->host->private_data;
> > unsigned int consumed;
> > unsigned long flags;
> > - unsigned int mode;
> > + struct sam9_smc_config smc;
> >
> > local_irq_save(flags);
> > - mode = at91_sys_read(AT91_SMC_MODE(info->cs));
> > + sam9_smc_read_mode(0, info->cs, &smc);
> >
> > /* set 16bit mode before writing data */
> > - at91_sys_write(AT91_SMC_MODE(info->cs),
> > - (mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16);
> > + smc.mode = (smc.mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16;
> > + sam9_smc_write_mode(0, info->cs, &smc);
>
>
> Do sam9_smc_read/write_mode really need to pass the whole smc structure?
> The only fields used are mode and tdf_cycles. It might be clearer to
> pass those directly.
>
> Also the original code here doesn't write tdf_cycles as part of the
> mode. Perhaps it would be better to have sam9_smc_write_mode to be:
>
> int sam9_smc_write_mode(int id, int cs, unsigned mode);
no as you will force the write to read to content before updating it
which I do not want the the need to you update the mode
>
> and in set_smc_timing above explicitly or in the tdf_cycles bits?
the mode and tdf_cycles are closely related so I choose to manipulate them
together
and as it's supposed to be register independant you always pass the struct
and the implemetation manage the write
Best Regards,
J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] pata/at91: use new introduce smc accessor
2011-12-08 15:23 ` [PATCH 2/3] pata/at91: use new introduce smc accessor Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 22:34 ` Ryan Mallon
@ 2011-12-09 10:59 ` Sergei Shtylyov
2011-12-09 18:42 ` Jean-Christophe PLAGNIOL-VILLARD
1 sibling, 1 reply; 16+ messages in thread
From: Sergei Shtylyov @ 2011-12-09 10:59 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 08-12-2011 19:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
Your subject doesn't parse. Maybe you meant "use newly introduced smc
accessor"?
> this will allow to use the pata_at91 on a single zImage
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre<nicolas.ferre@atmel.com>
> Cc: linux-ide at vger.kernel.org
> ---
> Hi,
> it's depends on other patch for AT91 can we apply via at91
> Best Regards,
> J.
> drivers/ata/pata_at91.c | 43 +++++++++++++++++++++----------------------
> 1 files changed, 21 insertions(+), 22 deletions(-)
> diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
> index 5249e6d..c8d1154 100644
> --- a/drivers/ata/pata_at91.c
> +++ b/drivers/ata/pata_at91.c
> @@ -207,11 +207,11 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
> {
> int ret = 0;
> int use_iordy;
> + struct sam9_smc_config smc;
> unsigned int t6z; /* data tristate time in ns */
> unsigned int cycle; /* SMC Cycle width in MCK ticks */
> unsigned int setup; /* SMC Setup width in MCK ticks */
> unsigned int pulse; /* CFIOR and CFIOW pulse width in MCK ticks */
> - unsigned int cs_setup = 0;/* CS4 or CS5 setup width in MCK ticks */
> unsigned int cs_pulse; /* CS4 or CS5 pulse width in MCK ticks*/
> unsigned int tdf_cycles; /* SMC TDF MCK ticks */
> unsigned long mck_hz; /* MCK frequency in Hz */
> @@ -244,26 +244,25 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
> }
>
> dev_dbg(dev, "Use IORDY=%u, TDF Cycles=%u\n", use_iordy, tdf_cycles);
> - info->mode |= AT91_SMC_TDF_(tdf_cycles);
>
> /* write SMC Setup Register */
> - at91_sys_write(AT91_SMC_SETUP(info->cs),
> - AT91_SMC_NWESETUP_(setup) |
> - AT91_SMC_NRDSETUP_(setup) |
> - AT91_SMC_NCS_WRSETUP_(cs_setup) |
> - AT91_SMC_NCS_RDSETUP_(cs_setup));
> + smc.nrd_setup = setup;
> + smc.nwe_setup = smc.nrd_setup;
> + smc.ncs_read_setup = 0;
> + smc.ncs_write_setup = smc.ncs_read_setup;
Why not:
smc.nrd_setup = smc.nwe_setup = setup;
smc.ncs_read_setup = smc.ncs_write_setup = 0;
> /* write SMC Pulse Register */
> - at91_sys_write(AT91_SMC_PULSE(info->cs),
> - AT91_SMC_NWEPULSE_(pulse) |
> - AT91_SMC_NRDPULSE_(pulse) |
> - AT91_SMC_NCS_WRPULSE_(cs_pulse) |
> - AT91_SMC_NCS_RDPULSE_(cs_pulse));
> + smc.nrd_pulse = pulse;
> + smc.nwe_pulse = smc.nrd_pulse;
> + smc.ncs_read_pulse =cs_pulse;
Add space after =, please.
> + smc.ncs_write_pulse = smc.ncs_read_pulse;
Why not:
smc.nrd_pulse = smc.nwe_pulse = pulse;
smc.ncs_read_pulse = smc.ncs_write_pulse = cs_pulse;
> /* write SMC Cycle Register */
> - at91_sys_write(AT91_SMC_CYCLE(info->cs),
> - AT91_SMC_NWECYCLE_(cycle) |
> - AT91_SMC_NRDCYCLE_(cycle));
> + smc.read_cycle = cycle;
> + smc.write_cycle = smc.read_cycle;
Why not:
smc.read_cycle = smc.write_cycle = cycle;
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 2/3] pata/at91: use new introduce smc accessor
2011-12-09 10:59 ` Sergei Shtylyov
@ 2011-12-09 18:42 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-12-09 18:42 UTC (permalink / raw)
To: linux-arm-kernel
On 14:59 Fri 09 Dec , Sergei Shtylyov wrote:
> Hello.
>
> On 08-12-2011 19:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> Your subject doesn't parse. Maybe you meant "use newly introduced
> smc accessor"?
yeap
>
> >this will allow to use the pata_at91 on a single zImage
>
> >Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
> >Cc: Nicolas Ferre<nicolas.ferre@atmel.com>
> >Cc: linux-ide at vger.kernel.org
> >---
> >Hi,
>
> > it's depends on other patch for AT91 can we apply via at91
>
> >Best Regards,
> >J.
> > drivers/ata/pata_at91.c | 43 +++++++++++++++++++++----------------------
> > 1 files changed, 21 insertions(+), 22 deletions(-)
>
> >diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
> >index 5249e6d..c8d1154 100644
> >--- a/drivers/ata/pata_at91.c
> >+++ b/drivers/ata/pata_at91.c
> >@@ -207,11 +207,11 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
> > {
> > int ret = 0;
> > int use_iordy;
> >+ struct sam9_smc_config smc;
> > unsigned int t6z; /* data tristate time in ns */
> > unsigned int cycle; /* SMC Cycle width in MCK ticks */
> > unsigned int setup; /* SMC Setup width in MCK ticks */
> > unsigned int pulse; /* CFIOR and CFIOW pulse width in MCK ticks */
> >- unsigned int cs_setup = 0;/* CS4 or CS5 setup width in MCK ticks */
> > unsigned int cs_pulse; /* CS4 or CS5 pulse width in MCK ticks*/
> > unsigned int tdf_cycles; /* SMC TDF MCK ticks */
> > unsigned long mck_hz; /* MCK frequency in Hz */
> >@@ -244,26 +244,25 @@ static void set_smc_timing(struct device *dev, struct ata_device *adev,
> > }
> >
> > dev_dbg(dev, "Use IORDY=%u, TDF Cycles=%u\n", use_iordy, tdf_cycles);
> >- info->mode |= AT91_SMC_TDF_(tdf_cycles);
> >
> > /* write SMC Setup Register */
> >- at91_sys_write(AT91_SMC_SETUP(info->cs),
> >- AT91_SMC_NWESETUP_(setup) |
> >- AT91_SMC_NRDSETUP_(setup) |
> >- AT91_SMC_NCS_WRSETUP_(cs_setup) |
> >- AT91_SMC_NCS_RDSETUP_(cs_setup));
> >+ smc.nrd_setup = setup;
> >+ smc.nwe_setup = smc.nrd_setup;
> >+ smc.ncs_read_setup = 0;
> >+ smc.ncs_write_setup = smc.ncs_read_setup;
>
> Why not:
>
> smc.nrd_setup = smc.nwe_setup = setup;
> smc.ncs_read_setup = smc.ncs_write_setup = 0;
why not
Best Regards,
J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] ide/at91: use new introduce smc accessor
2011-12-08 15:03 ARM: at91: smc update Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 15:23 ` [PATCH 1/3] ARM: at91: add accessor to manage smc Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 15:23 ` [PATCH 2/3] pata/at91: use new introduce smc accessor Jean-Christophe PLAGNIOL-VILLARD
@ 2011-12-08 15:23 ` Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 22:38 ` Ryan Mallon
` (2 more replies)
2 siblings, 3 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-12-08 15:23 UTC (permalink / raw)
To: linux-arm-kernel
this will allow to use the pata_at91 on a single zImage
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: linux-ide at vger.kernel.org
---
Hi,
it's depends on other patch for AT91 can we apply via at91
Best Regards,
J.
drivers/ide/at91_ide.c | 65 +++++++++++++++++++++++++++--------------------
1 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c
index 41d4155..407595b 100644
--- a/drivers/ide/at91_ide.c
+++ b/drivers/ide/at91_ide.c
@@ -59,41 +59,50 @@
#define ALT_MODE 0x00e00000
#define REGS_SIZE 8
-#define enter_16bit(cs, mode) do { \
- mode = at91_sys_read(AT91_SMC_MODE(cs)); \
- at91_sys_write(AT91_SMC_MODE(cs), mode | AT91_SMC_DBW_16); \
-} while (0)
+static inline void enter_16bit(int cs, struct sam9_smc_config *smc)
+{
+ sam9_smc_read_mode(0, cs, smc);
+ smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16;
+ sam9_smc_write_mode(0, cs, smc);
+}
-#define leave_16bit(cs, mode) at91_sys_write(AT91_SMC_MODE(cs), mode);
+static inline void leave_16bit(int cs, struct sam9_smc_config *smc)
+{
+ smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8;
+ sam9_smc_write_mode(0, cs, smc);
+}
static void set_smc_timings(const u8 chipselect, const u16 cycle,
const u16 setup, const u16 pulse,
const u16 data_float, int use_iordy)
{
- unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
+ struct sam9_smc_config smc;
+
+ smc.mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
AT91_SMC_BAT_SELECT;
/* disable or enable waiting for IORDY signal */
if (use_iordy)
- mode |= AT91_SMC_EXNWMODE_READY;
+ smc.mode |= AT91_SMC_EXNWMODE_READY;
- /* add data float cycles if needed */
- if (data_float)
- mode |= AT91_SMC_TDF_(data_float);
+ /* add data ofloat cycles if needed */
+ smc.tdf_cycles = data_float;
- at91_sys_write(AT91_SMC_MODE(chipselect), mode);
+ /* write SMC Setup Register */
+ smc.nrd_setup = setup;
+ smc.nwe_setup = smc.nrd_setup;
+ smc.ncs_read_setup = 0;
+ smc.ncs_write_setup = smc.ncs_read_setup;
+ /* write SMC Pulse Register */
+ smc.nrd_pulse = pulse;
+ smc.nwe_pulse = smc.nrd_pulse;
+ smc.ncs_read_pulse = cycle;
+ smc.ncs_write_pulse = smc.ncs_read_pulse;
+ /* write SMC Cycle Register */
+ smc.read_cycle = cycle;
+ smc.write_cycle = smc.read_cycle;
- /* setup timings in SMC */
- at91_sys_write(AT91_SMC_SETUP(chipselect), AT91_SMC_NWESETUP_(setup) |
- AT91_SMC_NCS_WRSETUP_(0) |
- AT91_SMC_NRDSETUP_(setup) |
- AT91_SMC_NCS_RDSETUP_(0));
- at91_sys_write(AT91_SMC_PULSE(chipselect), AT91_SMC_NWEPULSE_(pulse) |
- AT91_SMC_NCS_WRPULSE_(cycle) |
- AT91_SMC_NRDPULSE_(pulse) |
- AT91_SMC_NCS_RDPULSE_(cycle));
- at91_sys_write(AT91_SMC_CYCLE(chipselect), AT91_SMC_NWECYCLE_(cycle) |
- AT91_SMC_NRDCYCLE_(cycle));
+ sam9_smc_configure(0, chipselect, &smc);
}
static unsigned int calc_mck_cycles(unsigned int ns, unsigned int mck_hz)
@@ -146,15 +155,15 @@ static void at91_ide_input_data(ide_drive_t *drive, struct ide_cmd *cmd,
ide_hwif_t *hwif = drive->hwif;
struct ide_io_ports *io_ports = &hwif->io_ports;
u8 chipselect = hwif->select_data;
- unsigned long mode;
+ struct sam9_smc_config smc;
pdbg("cs %u buf %p len %d\n", chipselect, buf, len);
len++;
- enter_16bit(chipselect, mode);
+ enter_16bit(chipselect, &smc);
readsw((void __iomem *)io_ports->data_addr, buf, len / 2);
- leave_16bit(chipselect, mode);
+ leave_16bit(chipselect, &smc);
}
static void at91_ide_output_data(ide_drive_t *drive, struct ide_cmd *cmd,
@@ -163,13 +172,13 @@ static void at91_ide_output_data(ide_drive_t *drive, struct ide_cmd *cmd,
ide_hwif_t *hwif = drive->hwif;
struct ide_io_ports *io_ports = &hwif->io_ports;
u8 chipselect = hwif->select_data;
- unsigned long mode;
+ struct sam9_smc_config smc;
pdbg("cs %u buf %p len %d\n", chipselect, buf, len);
- enter_16bit(chipselect, mode);
+ enter_16bit(chipselect, &smc);
writesw((void __iomem *)io_ports->data_addr, buf, len / 2);
- leave_16bit(chipselect, mode);
+ leave_16bit(chipselect, &smc);
}
static void at91_ide_set_pio_mode(ide_hwif_t *hwif, ide_drive_t *drive)
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 3/3] ide/at91: use new introduce smc accessor
2011-12-08 15:23 ` [PATCH 3/3] ide/at91: " Jean-Christophe PLAGNIOL-VILLARD
@ 2011-12-08 22:38 ` Ryan Mallon
2011-12-09 6:19 ` Jean-Christophe PLAGNIOL-VILLARD
2011-12-09 11:06 ` Sergei Shtylyov
2011-12-09 13:59 ` Arnd Bergmann
2 siblings, 1 reply; 16+ messages in thread
From: Ryan Mallon @ 2011-12-08 22:38 UTC (permalink / raw)
To: linux-arm-kernel
On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
> this will allow to use the pata_at91 on a single zImage
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: linux-ide at vger.kernel.org
Couple of minor comments below,
~Ryan
> ---
> Hi,
>
> it's depends on other patch for AT91 can we apply via at91
>
> Best Regards,
> J.
> drivers/ide/at91_ide.c | 65 +++++++++++++++++++++++++++--------------------
> 1 files changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c
> index 41d4155..407595b 100644
> --- a/drivers/ide/at91_ide.c
> +++ b/drivers/ide/at91_ide.c
> @@ -59,41 +59,50 @@
> #define ALT_MODE 0x00e00000
> #define REGS_SIZE 8
>
> -#define enter_16bit(cs, mode) do { \
> - mode = at91_sys_read(AT91_SMC_MODE(cs)); \
> - at91_sys_write(AT91_SMC_MODE(cs), mode | AT91_SMC_DBW_16); \
> -} while (0)
> +static inline void enter_16bit(int cs, struct sam9_smc_config *smc)
> +{
> + sam9_smc_read_mode(0, cs, smc);
> + smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16;
> + sam9_smc_write_mode(0, cs, smc);
> +}
>
> -#define leave_16bit(cs, mode) at91_sys_write(AT91_SMC_MODE(cs), mode);
> +static inline void leave_16bit(int cs, struct sam9_smc_config *smc)
> +{
> + smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8;
> + sam9_smc_write_mode(0, cs, smc);
> +}
>
> static void set_smc_timings(const u8 chipselect, const u16 cycle,
> const u16 setup, const u16 pulse,
> const u16 data_float, int use_iordy)
> {
> - unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
> + struct sam9_smc_config smc;
> +
> + smc.mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
> AT91_SMC_BAT_SELECT;
>
> /* disable or enable waiting for IORDY signal */
> if (use_iordy)
> - mode |= AT91_SMC_EXNWMODE_READY;
> + smc.mode |= AT91_SMC_EXNWMODE_READY;
>
> - /* add data float cycles if needed */
> - if (data_float)
> - mode |= AT91_SMC_TDF_(data_float);
> + /* add data ofloat cycles if needed */
Typo: ofloat -> float?
> + smc.tdf_cycles = data_float;
>
> - at91_sys_write(AT91_SMC_MODE(chipselect), mode);
> + /* write SMC Setup Register */
This comment is no longer correct.
> + smc.nrd_setup = setup;
> + smc.nwe_setup = smc.nrd_setup;
> + smc.ncs_read_setup = 0;
> + smc.ncs_write_setup = smc.ncs_read_setup;
> + /* write SMC Pulse Register */
Nor is this one.
> + smc.nrd_pulse = pulse;
> + smc.nwe_pulse = smc.nrd_pulse;
> + smc.ncs_read_pulse = cycle;
> + smc.ncs_write_pulse = smc.ncs_read_pulse;
> + /* write SMC Cycle Register */
or this one.
> + smc.read_cycle = cycle;
> + smc.write_cycle = smc.read_cycle;
>
> - /* setup timings in SMC */
> - at91_sys_write(AT91_SMC_SETUP(chipselect), AT91_SMC_NWESETUP_(setup) |
> - AT91_SMC_NCS_WRSETUP_(0) |
> - AT91_SMC_NRDSETUP_(setup) |
> - AT91_SMC_NCS_RDSETUP_(0));
> - at91_sys_write(AT91_SMC_PULSE(chipselect), AT91_SMC_NWEPULSE_(pulse) |
> - AT91_SMC_NCS_WRPULSE_(cycle) |
> - AT91_SMC_NRDPULSE_(pulse) |
> - AT91_SMC_NCS_RDPULSE_(cycle));
> - at91_sys_write(AT91_SMC_CYCLE(chipselect), AT91_SMC_NWECYCLE_(cycle) |
> - AT91_SMC_NRDCYCLE_(cycle));
> + sam9_smc_configure(0, chipselect, &smc);
Check return value?
> }
>
> static unsigned int calc_mck_cycles(unsigned int ns, unsigned int mck_hz)
> @@ -146,15 +155,15 @@ static void at91_ide_input_data(ide_drive_t *drive, struct ide_cmd *cmd,
> ide_hwif_t *hwif = drive->hwif;
> struct ide_io_ports *io_ports = &hwif->io_ports;
> u8 chipselect = hwif->select_data;
> - unsigned long mode;
> + struct sam9_smc_config smc;
>
> pdbg("cs %u buf %p len %d\n", chipselect, buf, len);
>
> len++;
>
> - enter_16bit(chipselect, mode);
> + enter_16bit(chipselect, &smc);
> readsw((void __iomem *)io_ports->data_addr, buf, len / 2);
> - leave_16bit(chipselect, mode);
> + leave_16bit(chipselect, &smc);
> }
>
> static void at91_ide_output_data(ide_drive_t *drive, struct ide_cmd *cmd,
> @@ -163,13 +172,13 @@ static void at91_ide_output_data(ide_drive_t *drive, struct ide_cmd *cmd,
> ide_hwif_t *hwif = drive->hwif;
> struct ide_io_ports *io_ports = &hwif->io_ports;
> u8 chipselect = hwif->select_data;
> - unsigned long mode;
> + struct sam9_smc_config smc;
>
> pdbg("cs %u buf %p len %d\n", chipselect, buf, len);
>
> - enter_16bit(chipselect, mode);
> + enter_16bit(chipselect, &smc);
> writesw((void __iomem *)io_ports->data_addr, buf, len / 2);
> - leave_16bit(chipselect, mode);
> + leave_16bit(chipselect, &smc);
> }
>
> static void at91_ide_set_pio_mode(ide_hwif_t *hwif, ide_drive_t *drive)
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 3/3] ide/at91: use new introduce smc accessor
2011-12-08 22:38 ` Ryan Mallon
@ 2011-12-09 6:19 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-12-09 6:19 UTC (permalink / raw)
To: linux-arm-kernel
On 09:38 Fri 09 Dec , Ryan Mallon wrote:
> On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> > this will allow to use the pata_at91 on a single zImage
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > Cc: linux-ide at vger.kernel.org
>
>
> Couple of minor comments below,
>
> ~Ryan
>
> > ---
> > Hi,
> >
> > it's depends on other patch for AT91 can we apply via at91
> >
> > Best Regards,
> > J.
> > drivers/ide/at91_ide.c | 65 +++++++++++++++++++++++++++--------------------
> > 1 files changed, 37 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c
> > index 41d4155..407595b 100644
> > --- a/drivers/ide/at91_ide.c
> > +++ b/drivers/ide/at91_ide.c
> > @@ -59,41 +59,50 @@
> > #define ALT_MODE 0x00e00000
> > #define REGS_SIZE 8
> >
> > -#define enter_16bit(cs, mode) do { \
> > - mode = at91_sys_read(AT91_SMC_MODE(cs)); \
> > - at91_sys_write(AT91_SMC_MODE(cs), mode | AT91_SMC_DBW_16); \
> > -} while (0)
> > +static inline void enter_16bit(int cs, struct sam9_smc_config *smc)
> > +{
> > + sam9_smc_read_mode(0, cs, smc);
> > + smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16;
> > + sam9_smc_write_mode(0, cs, smc);
> > +}
> >
> > -#define leave_16bit(cs, mode) at91_sys_write(AT91_SMC_MODE(cs), mode);
> > +static inline void leave_16bit(int cs, struct sam9_smc_config *smc)
> > +{
> > + smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8;
> > + sam9_smc_write_mode(0, cs, smc);
> > +}
> >
> > static void set_smc_timings(const u8 chipselect, const u16 cycle,
> > const u16 setup, const u16 pulse,
> > const u16 data_float, int use_iordy)
> > {
> > - unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
> > + struct sam9_smc_config smc;
> > +
> > + smc.mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
> > AT91_SMC_BAT_SELECT;
> >
> > /* disable or enable waiting for IORDY signal */
> > if (use_iordy)
> > - mode |= AT91_SMC_EXNWMODE_READY;
> > + smc.mode |= AT91_SMC_EXNWMODE_READY;
> >
> > - /* add data float cycles if needed */
> > - if (data_float)
> > - mode |= AT91_SMC_TDF_(data_float);
> > + /* add data ofloat cycles if needed */
>
>
> Typo: ofloat -> float?
>
> > + smc.tdf_cycles = data_float;
> >
> > - at91_sys_write(AT91_SMC_MODE(chipselect), mode);
> > + /* write SMC Setup Register */
>
>
> This comment is no longer correct.
>
> > + smc.nrd_setup = setup;
> > + smc.nwe_setup = smc.nrd_setup;
> > + smc.ncs_read_setup = 0;
> > + smc.ncs_write_setup = smc.ncs_read_setup;
> > + /* write SMC Pulse Register */
>
>
> Nor is this one.
>
> > + smc.nrd_pulse = pulse;
> > + smc.nwe_pulse = smc.nrd_pulse;
> > + smc.ncs_read_pulse = cycle;
> > + smc.ncs_write_pulse = smc.ncs_read_pulse;
> > + /* write SMC Cycle Register */
>
>
> or this one.
>
> > + smc.read_cycle = cycle;
> > + smc.write_cycle = smc.read_cycle;
> >
> > - /* setup timings in SMC */
> > - at91_sys_write(AT91_SMC_SETUP(chipselect), AT91_SMC_NWESETUP_(setup) |
> > - AT91_SMC_NCS_WRSETUP_(0) |
> > - AT91_SMC_NRDSETUP_(setup) |
> > - AT91_SMC_NCS_RDSETUP_(0));
> > - at91_sys_write(AT91_SMC_PULSE(chipselect), AT91_SMC_NWEPULSE_(pulse) |
> > - AT91_SMC_NCS_WRPULSE_(cycle) |
> > - AT91_SMC_NRDPULSE_(pulse) |
> > - AT91_SMC_NCS_RDPULSE_(cycle));
> > - at91_sys_write(AT91_SMC_CYCLE(chipselect), AT91_SMC_NWECYCLE_(cycle) |
> > - AT91_SMC_NRDCYCLE_(cycle));
> > + sam9_smc_configure(0, chipselect, &smc);
>
>
> Check return value?
set_timings don't care about it
so no need
Best Regards,
J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] ide/at91: use new introduce smc accessor
2011-12-08 15:23 ` [PATCH 3/3] ide/at91: " Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 22:38 ` Ryan Mallon
@ 2011-12-09 11:06 ` Sergei Shtylyov
2011-12-09 18:40 ` Jean-Christophe PLAGNIOL-VILLARD
2011-12-09 13:59 ` Arnd Bergmann
2 siblings, 1 reply; 16+ messages in thread
From: Sergei Shtylyov @ 2011-12-09 11:06 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 08-12-2011 19:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
Same comment about the subject.
> this will allow to use the pata_at91 on a single zImage
s/pata_at91/at91_ide/
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre<nicolas.ferre@atmel.com>
> Cc: linux-ide at vger.kernel.org
> ---
> Hi,
> it's depends on other patch for AT91 can we apply via at91
I'm afraid David won't accept this, as the IDE tree is frozen for anything
but the fixes.
> diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c
> index 41d4155..407595b 100644
> --- a/drivers/ide/at91_ide.c
> +++ b/drivers/ide/at91_ide.c
> @@ -59,41 +59,50 @@
[...]
> static void set_smc_timings(const u8 chipselect, const u16 cycle,
> const u16 setup, const u16 pulse,
> const u16 data_float, int use_iordy)
> {
> - unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
> + struct sam9_smc_config smc;
> +
> + smc.mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
> AT91_SMC_BAT_SELECT;
>
> /* disable or enable waiting for IORDY signal */
> if (use_iordy)
> - mode |= AT91_SMC_EXNWMODE_READY;
> + smc.mode |= AT91_SMC_EXNWMODE_READY;
>
> - /* add data float cycles if needed */
> - if (data_float)
> - mode |= AT91_SMC_TDF_(data_float);
> + /* add data ofloat cycles if needed */
> + smc.tdf_cycles = data_float;
>
> - at91_sys_write(AT91_SMC_MODE(chipselect), mode);
> + /* write SMC Setup Register */
> + smc.nrd_setup = setup;
> + smc.nwe_setup = smc.nrd_setup;
> + smc.ncs_read_setup = 0;
> + smc.ncs_write_setup = smc.ncs_read_setup;
smc.nrd_setup = smc.nwe_setup = setup;
smc.ncs_read_setup = smc.ncs_write_setup = 0;
> + /* write SMC Pulse Register */
> + smc.nrd_pulse = pulse;
> + smc.nwe_pulse = smc.nrd_pulse;
> + smc.ncs_read_pulse = cycle;
> + smc.ncs_write_pulse = smc.ncs_read_pulse;
smc.nrd_pulse = smc.nwe_pulse = pulse;
> + smc.ncs_read_pulse = smc.ncs_write_pulse = cycle;
> + /* write SMC Cycle Register */
> + smc.read_cycle = cycle;
> + smc.write_cycle = smc.read_cycle;
smc.read_cycle = smc.write_cycle = cycle;
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 3/3] ide/at91: use new introduce smc accessor
2011-12-09 11:06 ` Sergei Shtylyov
@ 2011-12-09 18:40 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-12-09 18:40 UTC (permalink / raw)
To: linux-arm-kernel
On 15:06 Fri 09 Dec , Sergei Shtylyov wrote:
> Hello.
>
> On 08-12-2011 19:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> Same comment about the subject.
>
> >this will allow to use the pata_at91 on a single zImage
>
> s/pata_at91/at91_ide/
>
> >Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
> >Cc: Nicolas Ferre<nicolas.ferre@atmel.com>
> >Cc: linux-ide at vger.kernel.org
> >---
> >Hi,
>
> > it's depends on other patch for AT91 can we apply via at91
>
> I'm afraid David won't accept this, as the IDE tree is frozen for
> anything but the fixes.
basicaly it's a fix on at91 not ide. we can not compile all at91 together because of the
at91_sys_read/write so I need to fix it on at91 this one of the place I need
to touch.
I'll not touch it otherwise
Best Regards,
J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] ide/at91: use new introduce smc accessor
2011-12-08 15:23 ` [PATCH 3/3] ide/at91: " Jean-Christophe PLAGNIOL-VILLARD
2011-12-08 22:38 ` Ryan Mallon
2011-12-09 11:06 ` Sergei Shtylyov
@ 2011-12-09 13:59 ` Arnd Bergmann
2 siblings, 0 replies; 16+ messages in thread
From: Arnd Bergmann @ 2011-12-09 13:59 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 08 December 2011, Jean-Christophe PLAGNIOL-VILLARD wrote:
> this will allow to use the pata_at91 on a single zImage
>
> J.
> drivers/ide/at91_ide.c | 65 +++++++++++++++++++++++++++--------------------
> 1 files changed, 37 insertions(+), 28 deletions(-)
The description doesn't match the file you are changing. Is there still
a strong reason to keep using the at91_ide driver instead of migrating
everybody to pata_at91 now?
Arnd
^ permalink raw reply [flat|nested] 16+ messages in thread