ARM: at91: smc update
Hi,
this patch series introduce new accessor to manipulate the smc and be allow to abstract it of the soc rm9200 vs sam9
The following changes since commit 5c4ee7a68bc518ec79ac04f7c3259899ecff61db:
Merge branch 'next/cleanup' into for-next (2011-12-01 12:36:29 +0000)
are available in the git repository at:
git://github.com/at91linux/linux-at91.git ..BRANCH.NOT.VERIFIED..
Jean-Christophe PLAGNIOL-VILLARD (3):
ARM: at91: add accessor to manage smc
pata/at91: use new introduce smc accessor
ide/at91: use new introduce smc accessor
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 ------ drivers/ata/pata_at91.c | 43 ++++++------ drivers/ide/at91_ide.c | 65 ++++++++++------- 5 files changed, 173 insertions(+), 80 deletions(-)
Best Regards,
J.
PATCH 1/3 ARM: at91: add accessor to manage smc
this will allow to configure the smc independtly of the register configuration as example on rm9200 different from sam9
--- 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(-)
arch/arm/mach-at91/include/mach/at91sam9_smc.h +29 -0
@@ -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)
arch/arm/mach-at91/sam9_smc.c +86 -7
119 lines not on this page · raw message
arch/arm/mach-at91/sam9_smc.h +0 -23
@@ -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);
Signature · 1 line
1.7.7
PATCH 1/3 ARM: at91: add accessor to manage smc
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
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 <[email protected]>Cc Nicolas Ferre <[email protected]>
Hi Jean,
Couple of comments below.
~Ryan
Quoted 60 lines
--- 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(-)arch/arm/mach-at91/include/mach/at91sam9_smc.h +29
-0@@ -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)arch/arm/mach-at91/sam9_smc.c +1
-0@@ -2,6 +2,7 @@* linux/arch/arm/mach-at91/sam9_smc.c * * Copyright (C) 2008 Andrew Victor +* Copyright (C) 2011 Jean-Christophe PLAGNIOL-VILLARD <[email protected]> * * 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 @@
arch/arm/mach-at91/sam9_smc.c line 26
Quoted 10 lines
arch/arm/mach-at91/sam9_smc.c +2
-2#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?
Quoted 31 lines
arch/arm/mach-at91/sam9_smc.c +31
-0+{ +__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) +{
arch/arm/mach-at91/sam9_smc.c line 67
Quoted 10 lines
arch/arm/mach-at91/sam9_smc.c +9
-0+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.
Quoted 15 lines
arch/arm/mach-at91/sam9_smc.c +5
-3+ +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 );
arch/arm/mach-at91/sam9_smc.c static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_con · line 101
Quoted 10 lines
arch/arm/mach-at91/sam9_smc.c +5
-2+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?
Quoted 68 lines
arch/arm/mach-at91/sam9_smc.c +33
-0+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)arch/arm/mach-at91/sam9_smc.h +0
-23@@ -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);
PATCH 1/3 ARM: at91: add accessor to manage smc
Quoted 82 lines · Ryan Mallon
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
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 <[email protected]>Cc Nicolas Ferre <[email protected]>Hi Jean,
Couple of comments below.
~Ryan
Quoted 67 lines
--- 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(-)arch/arm/mach-at91/include/mach/at91sam9_smc.h +29
-0@@ -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)arch/arm/mach-at91/sam9_smc.c +1
-1@@ -2,6 +2,7 @@* linux/arch/arm/mach-at91/sam9_smc.c * * Copyright (C) 2008 Andrew Victor +* Copyright (C) 2011 Jean-Christophe PLAGNIOL-VILLARD <[email protected]> * * 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 ];
Quoted 10 lines
arch/arm/mach-at91/sam9_smc.c line 26
Quoted 3 lines
arch/arm/mach-at91/sam9_smc.c +2
-1-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.hsays that__init_refokmeans that this function can reference__initcode, without being__inititself 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_refokbeing 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
Quoted 66 lines
Quoted 32 lines
arch/arm/mach-at91/sam9_smc.c +31
-0+{ +__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) +{arch/arm/mach-at91/sam9_smc.c line 67
Quoted 10 lines
arch/arm/mach-at91/sam9_smc.c +9
-0+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.Quoted 20 lines
arch/arm/mach-at91/sam9_smc.c +8
-3+ +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 ; }
Quoted 8 lines
arch/arm/mach-at91/sam9_smc.c static void __init sam9_smc_cs_configure(void __iomem *base, struct sam9_smc_con · line 101
Quoted 5 lines
arch/arm/mach-at91/sam9_smc.c +2
-2-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.
PATCH 1/3 ARM: at91: add accessor to manage smc
Quoted 91 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 81 lines · Ryan Mallon
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
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 <[email protected]>Cc Nicolas Ferre <[email protected]>Hi Jean,
Couple of comments below.
~Ryan
Quoted 67 lines
--- 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(-)arch/arm/mach-at91/include/mach/at91sam9_smc.h +29
-0@@ -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)arch/arm/mach-at91/sam9_smc.c +1
-1@@ -2,6 +2,7 @@* linux/arch/arm/mach-at91/sam9_smc.c * * Copyright (C) 2008 Andrew Victor +* Copyright (C) 2011 Jean-Christophe PLAGNIOL-VILLARD <[email protected]> * * 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 ];Quoted 9 lines
arch/arm/mach-at91/sam9_smc.c line 26
Quoted 3 lines
arch/arm/mach-at91/sam9_smc.c +2
-1-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.hsays that__init_refokmeans that this function can reference__initcode, without being__inititself 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_refokbeing used here and on the other fucntions in this file?
Quoted 2 lines
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
PATCH 2/3 pata/at91: use new introduce smc accessor
In reply to Jean-Christophe PLAGNIOL-VILLARDthis will allow to use the pata_at91 on a single zImage
---
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(-)
drivers/ata/pata_at91.c +21 -22
@@ -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;
Signature · 1 line
1.7.7
PATCH 2/3 pata/at91: use new introduce smc accessor
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
this will allow to use the
pata_at91on a single zImageSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Nicolas Ferre <[email protected]>Cc linux-ide at vger.kernel.org
Some comments below,
~Ryan
Quoted 42 lines
---
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(-)drivers/ata/pata_at91.c +4
-7@@ -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 ;
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 256
Quoted 10 lines
drivers/ata/pata_at91.c +4
-5+ 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.
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 262
Quoted 10 lines
drivers/ata/pata_at91.c +4
-4+ 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.
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 265
Quoted 3 lines
drivers/ata/pata_at91.c +3
-0+ 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?
Quoted 9 lines
drivers/ata/pata_at91.c +1
-1}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;
drivers/ata/pata_at91.c static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev, · line 297
Quoted 10 lines
drivers/ata/pata_at91.c +3
-3local_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?
Quoted 11 lines
drivers/ata/pata_at91.c +2
-2consumed =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;
PATCH 2/3 pata/at91: use new introduce smc accessor
Quoted 116 lines · Ryan Mallon
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
this will allow to use the
pata_at91on a single zImageSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Nicolas Ferre <[email protected]>Cc linux-ide at vger.kernel.orgSome comments below,
~Ryan
Quoted 42 lines
---
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(-)drivers/ata/pata_at91.c +4
-7@@ -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 ;drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 256
Quoted 10 lines
drivers/ata/pata_at91.c +4
-5+ 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.
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 262
Quoted 10 lines
drivers/ata/pata_at91.c +4
-4+ 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.
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 265
Quoted 3 lines
drivers/ata/pata_at91.c +3
-0+ 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?
Quoted 9 lines
drivers/ata/pata_at91.c +1
-1}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;drivers/ata/pata_at91.c static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev, · line 297
Quoted 10 lines
drivers/ata/pata_at91.c +3
-3local_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.
Quoted 4 lines
Also the original code here doesn't write
tdf_cyclesas part of the mode. Perhaps it would be better to havesam9_smc_write_modeto 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
Quoted 2 lines
and in
set_smc_timingabove explicitly or in thetdf_cyclesbits?
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.
PATCH 2/3 pata/at91: use new introduce smc accessor
In reply to Jean-Christophe PLAGNIOL-VILLARDHello.
On 08-12-2011 19:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
Your subject doesn't parse. Maybe you meant "use newly introduced smc
accessor"?
Quoted 38 lines
this will allow to use the
pata_at91on a single zImageSigned-off-by Jean-Christophe PLAGNIOL-VILLARD<[email protected]>Cc Nicolas Ferre<[email protected]>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(-)drivers/ata/pata_at91.c +1
-2@@ -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);
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 252
Quoted 10 lines
drivers/ata/pata_at91.c +4
-5/* 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;
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 256
Quoted 9 lines
drivers/ata/pata_at91.c +3
-5/* 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.
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 257
Quoted 1 line
drivers/ata/pata_at91.c +1
-0+ 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;
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 260
Quoted 6 lines
drivers/ata/pata_at91.c +2
-3/* 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
PATCH 2/3 pata/at91: use new introduce smc accessor
Quoted 6 lines · Sergei Shtylyov
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
Quoted 39 lines
Quoted 39 lines
this will allow to use the
pata_at91on a single zImageSigned-off-by Jean-Christophe PLAGNIOL-VILLARD<[email protected]>Cc Nicolas Ferre<[email protected]>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(-)drivers/ata/pata_at91.c +1
-2@@ -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);
Quoted 15 lines
drivers/ata/pata_at91.c static void set_smc_timing(struct device *dev, struct ata_device *adev, · line 252
Quoted 10 lines
drivers/ata/pata_at91.c +4
-5/* 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.
PATCH 3/3 ide/at91: use new introduce smc accessor
In reply to Jean-Christophe PLAGNIOL-VILLARDthis will allow to use the pata_at91 on a single zImage
---
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(-)
drivers/ide/at91_ide.c +37 -28
@@ -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)
Signature · 1 line
1.7.7
PATCH 3/3 ide/at91: use new introduce smc accessor
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
this will allow to use the
pata_at91on a single zImageSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Nicolas Ferre <[email protected]>Cc linux-ide at vger.kernel.org
Couple of minor comments below,
~Ryan
Quoted 45 lines
---
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(-)drivers/ide/at91_ide.c +14
-6@@ -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 ;
drivers/ide/at91_ide.c line 88
Quoted 10 lines
drivers/ide/at91_ide.c +2
-4/* 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?
drivers/ide/at91_ide.c line 91
Quoted 4 lines
drivers/ide/at91_ide.c +2
-1+ smc.tdf_cycles = data_float; -at91_sys_write (AT91_SMC_MODE (chipselect), mode); +/* write SMC Setup Register */
This comment is no longer correct.
drivers/ide/at91_ide.c line 96
Quoted 5 lines
drivers/ide/at91_ide.c +5
-0+ 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.
drivers/ide/at91_ide.c line 101
Quoted 5 lines
drivers/ide/at91_ide.c +5
-0+ 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.
Quoted 5 lines
drivers/ide/at91_ide.c +2
-2+ 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) |
drivers/ide/at91_ide.c line 105
Quoted 10 lines
drivers/ide/at91_ide.c +1
-9-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?
Quoted 39 lines
drivers/ide/at91_ide.c +6
-6}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)
PATCH 3/3 ide/at91: use new introduce smc accessor
Quoted 101 lines · Ryan Mallon
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
this will allow to use the
pata_at91on a single zImageSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Nicolas Ferre <[email protected]>Cc linux-ide at vger.kernel.orgCouple of minor comments below,
~Ryan
Quoted 45 lines
---
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(-)drivers/ide/at91_ide.c +14
-6@@ -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 ;drivers/ide/at91_ide.c line 88
Quoted 10 lines
drivers/ide/at91_ide.c +2
-4/* 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?
drivers/ide/at91_ide.c line 91
Quoted 4 lines
drivers/ide/at91_ide.c +2
-1+ smc.tdf_cycles = data_float; -at91_sys_write (AT91_SMC_MODE (chipselect), mode); +/* write SMC Setup Register */ This comment is no longer correct.
drivers/ide/at91_ide.c line 96
Quoted 5 lines
drivers/ide/at91_ide.c +5
-0+ 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.
drivers/ide/at91_ide.c line 101
Quoted 5 lines
drivers/ide/at91_ide.c +5
-0+ 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.
Quoted 3 lines
drivers/ide/at91_ide.c +2
-0+ smc.read_cycle = cycle; + smc.write_cycle = smc.read_cycle;
Quoted 15 lines
drivers/ide/at91_ide.c line 105
Quoted 12 lines
drivers/ide/at91_ide.c +1
-11-/* 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.
PATCH 3/3 ide/at91: use new introduce smc accessor
In reply to Jean-Christophe PLAGNIOL-VILLARDHello.
On 08-12-2011 19:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
Same comment about the subject.
Quoted 1 line
this will allow to use the
pata_at91on a single zImage
s/pata_at91/at91_ide/
Quoted 7 lines
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD<[email protected]>Cc Nicolas Ferre<[email protected]>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.
Quoted 5 lines
drivers/ide/at91_ide.c +0
-0@@ -59,41 +59,50 @@
[...]
Quoted 17 lines
static void set_smc_timings(const u8 chipselect, const u16 cycle,
const u16 setup, const u16 pulse, const u16data_float, intuse_iordy)
{drivers/ide/at91_ide.c +4
-4-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)
drivers/ide/at91_ide.c line 95
Quoted 10 lines
drivers/ide/at91_ide.c +7
-2- 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;
drivers/ide/at91_ide.c line 100
Quoted 5 lines
drivers/ide/at91_ide.c +5
-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;
drivers/ide/at91_ide.c line 103
Quoted 5 lines
drivers/ide/at91_ide.c +4
-0+ 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
PATCH 3/3 ide/at91: use new introduce smc accessor
Quoted 16 lines · Sergei Shtylyov
Hello.
On 08-12-2011 19:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
Same comment about the subject.
Quoted 1 line
this will allow to use the
pata_at91on a single zImages/pata_at91/at91_ide/Quoted 5 lines
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD<[email protected]>Cc Nicolas Ferre<[email protected]>Cc linux-ide at vger.kernel.org---
Hi,
Quoted 4 lines
Quoted 1 line
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.
PATCH 3/3 ide/at91: use new introduce smc accessor
In reply to Jean-Christophe PLAGNIOL-VILLARDQuoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
this will allow to use the
pata_at91on a single zImageJ.
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