Hi Minkyu Kang,
Thank you for comments.
On Fri, Jun 1, 2012 at 12:11 PM, Minkyu Kang <proms...@gmail.com> wrote:
> Dear Simon Glass,
>
> On 1 June 2012 10:40, Simon Glass <s...@chromium.org> wrote:
>> Hi,
>>
>> On Thu, May 31, 2012 at 2:57 AM, Minkyu Kang <proms...@gmail.com> wrote:
>>>
>>> Dear Rajeshwari Shinde,
>>>
>>> On 23 May 2012 17:54, Rajeshwari Shinde <rajeshwar...@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.kesa...@samsung.com>
>>> > Signed-off-by: Che-Liang Chiou <clch...@chromium.org>
>>> > Signed-off-by: Rajeshwari Shinde <rajeshwar...@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.kesa...@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.
-- will do this modification will create separate function for each.
>
> 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
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Regards,
Rajeshwari Shinde.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
