[PATCH v5 06/14] ARM: OMAP2+: gpmc: CS configuration helper
flat view
STALE5229d
SUPERSEDED
From: Jon Hunter <hidden>
Date: 2012-06-11 21:43:02
Also in:
linux-omap
Revision v5 of 3 in this series.
Revisions (3)
- v5
- v5 current
- v6 [diff vs current]
On 06/11/2012 09:26 AM, Afzal Mohammed wrote:
quoted hunk ↗ jump to hunk
Helper for configuring given CS based on flags. Signed-off-by: Afzal Mohammed <redacted> --- arch/arm/mach-omap2/gpmc.c | 33 ++++++++++++++++++++++++++++++++ arch/arm/plat-omap/include/plat/gpmc.h | 5 +++++ 2 files changed, 38 insertions(+)diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c index 652b245..4e19010 100644 --- a/arch/arm/mach-omap2/gpmc.c +++ b/arch/arm/mach-omap2/gpmc.c@@ -927,6 +927,39 @@ static __devinit int gpmc_setup_cs_mem(struct gpmc_cs_data *cs, return 1; } +static void gpmc_setup_cs_config(unsigned cs, unsigned conf) +{ + u32 l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
Why is it necessary to read the register first? I thought you wanted to get away from relying on bootloader settings?
quoted hunk ↗ jump to hunk
+ l &= ~(GPMC_CONFIG1_MUXADDDATA | + GPMC_CONFIG1_WRITETYPE_SYNC | + GPMC_CONFIG1_WRITEMULTIPLE_SUPP | + GPMC_CONFIG1_READTYPE_SYNC | + GPMC_CONFIG1_READMULTIPLE_SUPP | + GPMC_CONFIG1_WRAPBURST_SUPP | + GPMC_CONFIG1_DEVICETYPE(~0) | + GPMC_CONFIG1_DEVICESIZE(~0) | + GPMC_CONFIG1_PAGE_LEN(~0)); + + l |= conf & GPMC_CONFIG1_DEVICETYPE_NAND; + l |= conf & GPMC_CONFIG1_DEVICESIZE_16;
I can see that it works to use the above definitions as masks because of the possible values that can be programmed into these fields. However, from a read-ability standpoint this is unclear and requires people to review the documentation to understand what you are doing here. Furthermore, if any new device-types or sizes were added in the future this could lead to bugs. Hence, it would be better to define a mask for these fields. Now you may say what happens if someone pass a bad device-type or device-size that would equate to a reserved value being programmed. Well if someone passes a bad value we don't know what they intended to program and that raises the question should we be checking the conf value of bad device types, size, and page lengths here? Jon