[U-Boot] [PATCH 1/2 V2] EXYNOS5: PINMUX: Added default pinumx settings

[Minkyu Kang]

Dear Simon Glass,

On 1 June 2012 10:40, Simon Glass <sjg at chromium.org> wrote:
> Hi,
>
> On Thu, May 31, 2012 at 2:57 AM, Minkyu Kang <promsoft at gmail.com> wrote:
>>
>> Dear Rajeshwari Shinde,
>>
>> On 23 May 2012 17:54, Rajeshwari Shinde <rajeshwari.s at samsung.com> wrote:
>> > This patch performs the pinmux configuration in a common file.
>> > As of now only EXYNOS5 pinmux for SDMMC, UART and Ethernet is
>> > supported.
>> >
>> > Signed-off-by: Abhilash Kesavan <a.kesavan at samsung.com>
>> > Signed-off-by: Che-Liang Chiou <clchiou at chromium.org>
>> > Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>
>> > ---
>> > changes in V2:
>> >        - Adding pinmux.c to Makefile moved to this patch.
>> >        - exynos5_pinmux_config made static.
>> >  arch/arm/cpu/armv7/exynos/Makefile        |    2 +-
>> >  arch/arm/cpu/armv7/exynos/pinmux.c        |  189
>> > +++++++++++++++++++++++++++++
>> >  arch/arm/include/asm/arch-exynos/pinmux.h |   77 ++++++++++++
>> >  3 files changed, 267 insertions(+), 1 deletions(-)
>> >  create mode 100644 arch/arm/cpu/armv7/exynos/pinmux.c
>> >  create mode 100644 arch/arm/include/asm/arch-exynos/pinmux.h
>> >
>> > diff --git a/arch/arm/cpu/armv7/exynos/Makefile
>> > b/arch/arm/cpu/armv7/exynos/Makefile
>> > index 90ec2bd..9119961 100644
>> > --- a/arch/arm/cpu/armv7/exynos/Makefile
>> > +++ b/arch/arm/cpu/armv7/exynos/Makefile
>> > @@ -22,7 +22,7 @@ include $(TOPDIR)/config.mk
>> >
>> >  LIB    = $(obj)lib$(SOC).o
>> >
>> > -COBJS  += clock.o power.o soc.o system.o
>> > +COBJS  += clock.o power.o soc.o system.o pinmux.o
>> >
>> >  SRCS   := $(SOBJS:.o=.S) $(COBJS:.o=.c)
>> >  OBJS   := $(addprefix $(obj),$(COBJS) $(SOBJS))
>> > diff --git a/arch/arm/cpu/armv7/exynos/pinmux.c
>> > b/arch/arm/cpu/armv7/exynos/pinmux.c
>> > new file mode 100644
>> > index 0000000..c6392b2
>> > --- /dev/null
>> > +++ b/arch/arm/cpu/armv7/exynos/pinmux.c
>> > @@ -0,0 +1,189 @@
>> > +/*
>> > + * Copyright (c) 2012 Samsung Electronics.
>> > + * Abhilash Kesavan <a.kesavan at samsung.com>
>> > + *
>> > + * See file CREDITS for list of people who contributed to this
>> > + * project.
>> > + *
>> > + * This program is free software; you can redistribute it and/or
>> > + * modify it under the terms of the GNU General Public License as
>> > + * published by the Free Software Foundation; either version 2 of
>> > + * the License, or (at your option) any later version.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> > + * GNU General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU General Public License
>> > + * along with this program; if not, write to the Free Software
>> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> > + * MA 02111-1307 USA
>> > + */
>> > +
>> > +#include <common.h>
>> > +#include <asm/arch/cpu.h>
>>
>> cpu.h is already included.
>>
>> > +#include <asm/arch/gpio.h>
>> > +#include <asm/arch/pinmux.h>
>> > +#include <asm/arch/sromc.h>
>> > +
>> > +static int exynos5_pinmux_config(int peripheral, int flags)
>> > +{
>> > +       struct exynos5_gpio_part1 *gpio1 =
>> > +               (struct exynos5_gpio_part1 *)
>> > samsung_get_base_gpio_part1();
>> > +       struct s5p_gpio_bank *bank, *bank_ext;
>> > +       int i, start, count;
>> > +
>> > +       switch (peripheral) {
>> > +       case PERIPH_ID_UART0:
>> > +       case PERIPH_ID_UART1:
>> > +       case PERIPH_ID_UART2:
>> > +       case PERIPH_ID_UART3:
>> > +               switch (peripheral) {
>> > +               case PERIPH_ID_UART0:
>> > +                       bank = &gpio1->a0;
>> > +                       start = 0; count = 4;
>>
>> Please don't put multiple statement on a single line. fix it globally.
>>
>> > +                       break;
>> > +               case PERIPH_ID_UART1:
>> > +                       bank = &gpio1->a0;
>> > +                       start = 4; count = 4;
>> > +                       break;
>> > +               case PERIPH_ID_UART2:
>> > +                       bank = &gpio1->a1;
>> > +                       start = 0; count = 4;
>> > +                       break;
>> > +               case PERIPH_ID_UART3:
>> > +                       bank = &gpio1->a1;
>> > +                       start = 4; count = 2;
>> > +                       break;
>> > +               }
>> > +               for (i = start; i < start + count; i++) {
>> > +                       s5p_gpio_set_pull(bank, i, GPIO_PULL_NONE);
>> > +                       s5p_gpio_cfg_pin(bank, i, GPIO_FUNC(0x2));
>> > +               }
>> > +               break;
>>
>> Looks confused.
>> Why don't you make function for each peripherals?
>> e.g: exynos5_uart_config, exynos5_mmc_config.
>
>
> The idea here is that later we can configure a peripheral just with the ID.
> Ultimately we might do pinmux in a generic way using device tree.
>

I know.
It seems to me you misunderstand my opinion.
Please see below pseudo code.

static int exynos5_pinmux_config(args) {
       switch (peripheral) {
       case PERIPH_ID_UART0:
       case PERIPH_ID_UART1:
       case PERIPH_ID_UART2:
       case PERIPH_ID_UART3:
               exynos5_uart_config(args);
               break;
       case PERIPH_ID_SDMMC0:
       case PERIPH_ID_SDMMC1:
       case PERIPH_ID_SDMMC2:
       case PERIPH_ID_SDMMC3:
               exynos5_mmc_config(args);
               break;
       }
}

Thanks.
Minkyu Kang.
-- 
from. prom.
www.promsoft.net