2010/6/30 Sekhar Nori <[email protected]>: > This patch adds support to configure the AEMIF interface > with supplied timing values. >
> Signed-off-by: Sekhar Nori <[email protected]> > --- > arch/arm/mach-davinci/Makefile | 2 +- > arch/arm/mach-davinci/aemif.c | 131 > ++++++++++++++++++++++++++++ > arch/arm/mach-davinci/include/mach/aemif.h | 36 ++++++++ > 3 files changed, 168 insertions(+), 1 deletions(-) > create mode 100644 arch/arm/mach-davinci/aemif.c > create mode 100644 arch/arm/mach-davinci/include/mach/aemif.h > > > diff --git a/arch/arm/mach-davinci/aemif.c b/arch/arm/mach-davinci/aemif.c > new file mode 100644 > index 0000000..62d7cce > --- /dev/null > +++ b/arch/arm/mach-davinci/aemif.c > @@ -0,0 +1,131 @@ > +/* > + * AEMIF support for DaVinci SoCs > + * > + * Copyright (C) 2010 Texas Instruments Incorporated. http://www.ti.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 > + * published by the Free Software Foundation. > + */ > + > +#include <linux/kernel.h> > +#include <linux/io.h> > +#include <linux/err.h> > +#include <linux/clk.h> > +#include <linux/module.h> > + > +#include <mach/aemif.h> > + > +/* Timing value configuration */ > + > +#define TA(x) ((x) << 2) > +#define RHOLD(x) ((x) << 4) > +#define RSTROBE(x) ((x) << 7) > +#define RSETUP(x) ((x) << 13) > +#define WHOLD(x) ((x) << 17) > +#define WSTROBE(x) ((x) << 20) > +#define WSETUP(x) ((x) << 26) > + > +#define TA_MAX 0x3 > +#define RHOLD_MAX 0x7 > +#define RSTROBE_MAX 0x3f > +#define RSETUP_MAX 0xf > +#define WHOLD_MAX 0x7 > +#define WSTROBE_MAX 0x3f > +#define WSETUP_MAX 0xf > + > +#define TIMING_MASK (TA(TA_MAX) | \ > + RHOLD(RHOLD_MAX) | \ > + RSTROBE(RSTROBE_MAX) | \ > + RSETUP(RSETUP_MAX) | \ > + WHOLD(WHOLD_MAX) | \ > + WSTROBE(WSTROBE_MAX) | \ > + WSETUP(WSETUP_MAX)) > + > +#define NS_IN_KHZ 1000000 > + Why are the defines splitted like this between the .h and the .c ? If users of this code don't care about these defines, they can be with the code. But I can't see how the defines in the header file are more useful to external code then these ones. > +/* > + * aemif_calc_rate - calculate timing data. > + * @wanted: The cycle time needed in nanoseconds. > + * @clk: The input clock rate in kHz. > + * @max: The maximum divider value that can be programmed. > + * > + * Returns the calculated timing value minus 1 for easy programming into > + * AEMIF timing registers. > + */ > +static int aemif_calc_rate(int wanted, unsigned long clk, int max) > +{ > + int result; > + > + result = DIV_ROUND_UP((wanted * clk), NS_IN_KHZ) - 1; > + > + pr_debug("%s: result %d from %ld, %d\n", __func__, result, clk, > wanted); > + > + if (result < 0) > + result = 0; > + else if (result > max) > + result = max; > + > + return result; > +} I think you should either return a negative value when result > max, or remove the check for negative value in the code below. I think a negative value would make more sense. After all, if one increases the frequency, past the limit of a particular device, then users should be warned that their device will run out of spec. > + > +/** > + * davinci_aemif_setup_timing - setup timing values for a given AEMIF > interface > + * @t: timing values to be progammed > + * @base: The virtual base address of the AEMIF interface > + * @cs: chip-select to program the timing values for > + * > + * This function programs the given timing values (in real clock) into the > + * AEMIF registers taking the AEMIF clock into account. > + * > + * This function does not use any locking while programming the AEMIF > + * because it is expected that there is only one user of a given > + * chip-select. > + * > + * Returns 0 on success, else negative errno. > + */ > +int davinci_aemif_setup_timing(struct davinci_aemif_timing *t, > + void __iomem *base, unsigned cs) > +{ > + unsigned set, val; > + unsigned ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup; > + unsigned offset = A1CR_OFFSET + cs * 4; > + struct clk *aemif_clk; > + unsigned long clkrate; > + > + if (!t) > + return 0; /* Nothing to do */ > + > + aemif_clk = clk_get(NULL, "aemif"); > + if (IS_ERR(aemif_clk)) > + return PTR_ERR(aemif_clk); > + > + clkrate = clk_get_rate(aemif_clk); > + > + clkrate /= 1000; /* turn clock into kHz for ease of use */ > + > + ta = aemif_calc_rate(t->ta, clkrate, TA_MAX); > + rhold = aemif_calc_rate(t->rhold, clkrate, RHOLD_MAX); > + rstrobe = aemif_calc_rate(t->rstrobe, clkrate, RSTROBE_MAX); > + rsetup = aemif_calc_rate(t->rsetup, clkrate, RSETUP_MAX); > + whold = aemif_calc_rate(t->whold, clkrate, WHOLD_MAX); > + wstrobe = aemif_calc_rate(t->wstrobe, clkrate, WSTROBE_MAX); > + wsetup = aemif_calc_rate(t->wsetup, clkrate, WSETUP_MAX); > + > + if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 || > + whold < 0 || wstrobe < 0 || wsetup < 0) { > + pr_err("%s: cannot get suitable timings\n", __func__); > + return -EINVAL; > + } See my comment above about returning negative value. Currently, this test is always false. > + > + set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) | > + WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup); > + > + val = __raw_readl(base + offset); > + val &= ~TIMING_MASK; > + val |= set; > + __raw_writel(val, base + offset); > + > + return 0; > +} > +EXPORT_SYMBOL(davinci_aemif_setup_timing); > diff --git a/arch/arm/mach-davinci/include/mach/aemif.h > b/arch/arm/mach-davinci/include/mach/aemif.h > new file mode 100644 > index 0000000..05b2934 > --- /dev/null > +++ b/arch/arm/mach-davinci/include/mach/aemif.h > @@ -0,0 +1,36 @@ > +/* > + * TI DaVinci AEMIF support > + * > + * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/ > + * > + * This file is licensed under the terms of the GNU General Public License > + * version 2. This program is licensed "as is" without any warranty of any > + * kind, whether express or implied. > + */ > +#ifndef _MACH_DAVINCI_AEMIF_H > +#define _MACH_DAVINCI_AEMIF_H > + > +#define NRCSR_OFFSET 0x00 > +#define AWCCR_OFFSET 0x04 > +#define A1CR_OFFSET 0x10 > + > +#define ACR_ASIZE_MASK 0x3 > +#define ACR_EW_MASK BIT(30) > +#define ACR_SS_MASK BIT(31) > + > +/* All timings in nanoseconds */ > +struct davinci_aemif_timing { > + u8 wsetup; > + u8 wstrobe; > + u8 whold; > + > + u8 rsetup; > + u8 rstrobe; > + u8 rhold; > + > + u8 ta; > +}; > + So, maximum strobe time is 255 ns ? Looks a bit low for a max value. Isn't this a good place to define chip select register base value ? Or are they better defined in the cpu file ? > +int davinci_aemif_setup_timing(struct davinci_aemif_timing *t, > + void __iomem *base, unsigned cs); > +#endif > -- Jean-Philippe Françoi _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
