Subject: [PATCH 1/3] ARM: at91: add accessor to manage smc

From: [email-masked] (Ryan Mallon)
Date: Fri, 09 Dec 2011 18:06:52 +1100

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 <[email-masked]>
>>> Cc: Nicolas Ferre <[email-masked]>
>>
>> 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 <[email-masked]>
>>>   *
>>>   * 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