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

[Simon Glass]

Hi Minkyu,

On Thu, May 31, 2012 at 11:41 PM, Minkyu Kang <promsoft at gmail.com> wrote:

> 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.
>

Yes I did - thank you for the explanation. It will complicate dealing with
the config/flags, but that is ok.

Regards,
Simon


>
> 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
>