Hi Amar,
On Fri, Jan 4, 2013 at 1:34 AM, Amar <amarendra...@samsung.com> wrote:
> This patch adds FDT support for DWMMC, by reading the DWMMC node data
> from the device tree and initialising DWMMC channels as per data
> obtained from the node.
>
> Changes from V1:
> 1)Updated code to have same signature for the function
> exynos_dwmci_init() for both FDT and non-FDT versions.
> 2)Updated code to pass device_id parameter to the function
> exynos5_mmc_set_clk_div() instead of index.
> 3)Updated code to decode the value of "samsung,width" from FDT.
> 4)Channel index is computed instead of getting from FDT.
>
> Changes from V2:
> 1)Updation of commit message and resubmition of proper patch set.
>
> Changes from V3:
> 1)Replaced the new function exynos5_mmc_set_clk_div() with the
> existing function set_mmc_clk(). set_mmc_clk() will do the purpose.
> 2)Computation of FSYS block clock divisor (pre-ratio) is added.
>
> Signed-off-by: Vivek Gautam <gautam.vi...@samsung.com>
> Signed-off-by: Amar <amarendra...@samsung.com>
> ---
> arch/arm/include/asm/arch-exynos/dwmmc.h | 4 +
> drivers/mmc/exynos_dw_mmc.c | 129
> +++++++++++++++++++++++++++++--
> include/dwmmc.h | 4 +
> 3 files changed, 130 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h
> b/arch/arm/include/asm/arch-exynos/dwmmc.h
> index 8acdf9b..40dcc7b 100644
> --- a/arch/arm/include/asm/arch-exynos/dwmmc.h
> +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h
> @@ -29,8 +29,12 @@
>
> int exynos_dwmci_init(u32 regbase, int bus_width, int index);
>
> +#ifdef CONFIG_OF_CONTROL
> +unsigned int exynos_dwmmc_init(const void *blob);
> +#else
> static inline unsigned int exynos_dwmmc_init(int index, int bus_width)
Why unsigned?
I'm really not that keen on functions which change their signature
based on an #ifdef. Can we perhaps have
int exynos_dwmmc_init(const void *blob);
which will pass NULL when there is no FDT, and
int exynos_dwmmc_add_port(int index, int bus_width)
for use by non-FDT boards?
> {
> unsigned int base = samsung_get_base_mmc() + (0x10000 * index);
> return exynos_dwmci_init(base, bus_width, index);
> }
> +#endif
> diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
> index 72a31b7..d7ca7d0 100644
> --- a/drivers/mmc/exynos_dw_mmc.c
> +++ b/drivers/mmc/exynos_dw_mmc.c
> @@ -19,39 +19,154 @@
> */
>
> #include <common.h>
> -#include <malloc.h>
> #include <dwmmc.h>
> +#include <fdtdec.h>
> +#include <libfdt.h>
> +#include <malloc.h>
> #include <asm/arch/dwmmc.h>
> #include <asm/arch/clk.h>
> +#include <asm/arch/pinmux.h>
> +
> +#define DWMMC_MAX_CH_NUM 4
> +#define DWMMC_MAX_FREQ 52000000
> +#define DWMMC_MIN_FREQ 400000
> +#define DWMMC_MMC0_CLKSEL_VAL 0x03030001
> +#define DWMMC_MMC2_CLKSEL_VAL 0x03020001
> +#define ONE_MEGA_HZ 1000000
> +#define SCALED_VAL_FOUR_HUNDRED 400
I don't think you need these last two - you can just write the number
in the code
>
> static char *EXYNOS_NAME = "EXYNOS DWMMC";
Same with this I think
> +u32 timing[3];
>
> +/*
> + * Function used as callback function to initialise the
> + * CLKSEL register for every mmc channel.
> + */
> static void exynos_dwmci_clksel(struct dwmci_host *host)
> {
> - u32 val;
> - val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
> - DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0);
> + dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);
> +}
>
> - dwmci_writel(host, DWMCI_CLKSEL, val);
> +unsigned int exynos_dwmci_get_clk(int dev_index)
> +{
> + return get_mmc_clk(dev_index);
> }
>
> int exynos_dwmci_init(u32 regbase, int bus_width, int index)
> {
> struct dwmci_host *host = NULL;
> + unsigned int clock, div;
> host = malloc(sizeof(struct dwmci_host));
> if (!host) {
> printf("dwmci_host malloc fail!\n");
> return 1;
> }
>
> + /*
> + * The max operating freq of FSYS block is 400MHz.
> + * Scale down the 400MHz to number 400.
> + * Scale down the MPLL clock by dividing MPLL_CLK with ONE_MEGA_HZ.
> + * Arrive at the divisor value taking 400 as the reference.
> + */
> +
> + /* get mpll clock and divide it by ONE_MEGA_HZ */
> + clock = get_pll_clk(MPLL) / ONE_MEGA_HZ;
> +
> + /* Arrive at the divisor value. */
> + for (div = 0; div <= 0xf; div++) {
> + if ((clock / (div + 1)) <= SCALED_VAL_FOUR_HUNDRED)
> + break;
> + }
What if you don't find the right divisor?
> +
> + /* set the clock divisor for mmc */
> + set_mmc_clk(index, div);
> +
> host->name = EXYNOS_NAME;
> host->ioaddr = (void *)regbase;
> host->buswidth = bus_width;
> +#ifdef CONFIG_OF_CONTROL
> + host->clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |
> + DWMCI_SET_DRV_CLK(timing[1]) |
> + DWMCI_SET_DIV_RATIO(timing[2]));
timing should be a parameter, not a global. For the non-FDT case
perhaps you can accept NULL, meaning default? Then:
if (timing)
do the code above
else
do the code below
> +#else
> + if (0 == index)
> + host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;
> + if (2 == index)
> + host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;
> +#endif
> host->clksel = exynos_dwmci_clksel;
> host->dev_index = index;
> -
> - add_dwmci(host, 52000000, 400000);
> + host->mmc_clk = exynos_dwmci_get_clk;
> + /* Add the mmc chennel to be registered with mmc core */
> + add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ);
Is error checking needed here?
>
> return 0;
> }
>
> +#ifdef CONFIG_OF_CONTROL
> +unsigned int exynos_dwmmc_init(const void *blob)
> +{
> + u32 base;
> + int index, bus_width;
> + int node_list[DWMMC_MAX_CH_NUM];
> + int err = 0;
> + int dev_id, flag;
> + int count, i;
> +
> + count = fdtdec_find_aliases_for_id(blob, "dwmmc",
> + COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list,
> + DWMMC_MAX_CH_NUM);
> +
> + for (i = 0; i < count; i++) {
> + int node = node_list[i];
> +
> + if (node <= 0)
> + continue;
> +
> + /* Extract device id for each mmc channel */
> + dev_id = pinmux_decode_periph_id(blob, node);
> +
> + /* Get the bus width from the device node */
> + bus_width = fdtdec_get_int(blob, node, "samsung,bus-width",
> 0);
> + if (bus_width < 0) {
<= 0? The function will return 0 if there is no property.
> + debug("DWMMC: Can't get bus-width\n");
> + return -1;
> + }
> + if (8 == bus_width)
> + flag = PINMUX_FLAG_8BIT_MODE;
> + else
> + flag = PINMUX_FLAG_NONE;
> +
> + /* config pinmux for each mmc channel */
> + err = exynos_pinmux_config(dev_id, flag);
> + if (err) {
> + debug("DWMMC not configured\n");
> + return err;
> + }
> +
> + index = dev_id - PERIPH_ID_SDMMC0;
> +
> + /* Get the base address from the device node */
> + base = fdtdec_get_addr(blob, node, "reg");
> + if (!base) {
> + debug("DWMMC: Can't get base address\n");
> + return -1;
> + }
> + /* Extract the timing info from the node */
> + err = fdtdec_get_int_array(blob, node, "samsung,timing",
> + timing, 3);
> + if (err) {
> + debug("Can't get sdr-timings for divider\n");
> + return -1;
> + }
> + /* Initialise each mmc channel */
> + err = exynos_dwmci_init(base, bus_width, index);
> + if (err) {
> + debug("Can't do dwmci init\n");
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +#endif
> diff --git a/include/dwmmc.h b/include/dwmmc.h
> index c8b1d40..4a42849 100644
> --- a/include/dwmmc.h
> +++ b/include/dwmmc.h
> @@ -123,6 +123,9 @@
> #define MSIZE(x) ((x) << 28)
> #define RX_WMARK(x) ((x) << 16)
> #define TX_WMARK(x) (x)
> +#define RX_WMARK_SHIFT 16
> +#define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)
> +
Remove this extra line?
>
> #define DWMCI_IDMAC_OWN (1 << 31)
> #define DWMCI_IDMAC_CH (1 << 4)
> @@ -144,6 +147,7 @@ struct dwmci_host {
> unsigned int bus_hz;
> int dev_index;
> int buswidth;
> + u32 clksel_val;
> u32 fifoth_val;
> struct mmc *mmc;
>
> --
> 1.8.0
>
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
