Re: [PATCH 1/5] ARM: S5PV310: Add HSMMC support and SDHCI configuration

Kyungmin Park
2010-09-17T04:15:21Z

On Fri, Sep 17, 2010 at 9:57 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Kyungmin Park wrote:
>>
>> On Thu, Sep 16, 2010 at 5:36 PM, Jeongbae Seo <jeongbae.seo@samsung.com>
>> wrote:
>> > From: Hyuk Lee <hyuk1.lee@samsung.com>
>> >
>> > This patch adds to support HSMMC for S5PV310 and S5PC210 and setup for
>> > HSMMC host controller and also related GPIO.
>> > At most 4 channel can be used at the same time.
>> > A user can configure SDHCI data bus as 8bit or 4bit.
>> >
>> > Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
>> > Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
>> > ---
>> >  arch/arm/mach-s5pv310/Kconfig            |   39 ++++++++
>> >  arch/arm/mach-s5pv310/Makefile           |    2 +
>> >  arch/arm/mach-s5pv310/setup-sdhci-gpio.c |  156
>> ++++++++++++++++++++++++++++++
>> >  arch/arm/mach-s5pv310/setup-sdhci.c      |   69 +++++++++++++
>> >  4 files changed, 266 insertions(+), 0 deletions(-)
>> >  create mode 100644 arch/arm/mach-s5pv310/setup-sdhci-gpio.c
>> >  create mode 100644 arch/arm/mach-s5pv310/setup-sdhci.c
>> >
>> > diff --git a/arch/arm/mach-s5pv310/Kconfig
> b/arch/arm/mach-s5pv310/Kconfig
>> > index 9ac29fe..6a07968 100644
>> > --- a/arch/arm/mach-s5pv310/Kconfig
>> > +++ b/arch/arm/mach-s5pv310/Kconfig
>> > @@ -25,6 +25,17 @@ config S5PV310_SETUP_I2C2
>> >        help
>> >          Common setup code for i2c bus 2.
>> >
>> > +config S5PV310_SETUP_SDHCI
>> > +       bool
>> > +       select S5PV310_SETUP_SDHCI_GPIO
>> > +       help
>> > +         Internal helper functions for S5PV310 based SDHCI systems.
>> > +
>> > +config S5PV310_SETUP_SDHCI_GPIO
>> > +       bool
>> > +       help
>> > +         Common setup code for SDHCI gpio.
>> > +
>> >  # machine support
>> >
>> >  menu "S5PC210 Machines"
>> > @@ -33,6 +44,11 @@ config MACH_SMDKC210
>> >        bool "SMDKC210"
>> >        select CPU_S5PV310
>> >        select ARCH_SPARSEMEM_ENABLE
>> > +       select S3C_DEV_HSMMC
>> > +       select S3C_DEV_HSMMC1
>> > +       select S3C_DEV_HSMMC2
>> > +       select S3C_DEV_HSMMC3
>> > +       select S5PV310_SETUP_SDHCI
>> >        help
>> >          Machine support for Samsung SMDKC210
>> >          S5PC210(MCP) is one of package option of S5PV310
>> > @@ -53,9 +69,32 @@ config MACH_SMDKV310
>> >        bool "SMDKV310"
>> >        select CPU_S5PV310
>> >        select ARCH_SPARSEMEM_ENABLE
>> > +       select S3C_DEV_HSMMC
>> > +       select S3C_DEV_HSMMC1
>> > +       select S3C_DEV_HSMMC2
>> > +       select S3C_DEV_HSMMC3
>> > +       select S5PV310_SETUP_SDHCI
>> >        help
>> >          Machine support for Samsung SMDKV310
>> >
>> >  endmenu
>> >
>> > +comment "Configuration for HSMMC bus width"
>> > +
>> > +menu "Use 8-bit bus width"
>> > +
>> > +config S5PV310_SDHCI_CH0_8BIT
>> > +       bool "Channel 0 with 8-bit bus"
>> > +       help
>> > +         Support HSMMC Channel 0 8-bit bus.
>> > +         If selected, Channel 1 is disabled.
>> > +
>> > +config S5PV310_SDHCI_CH2_8BIT
>> > +       bool "Channel 2 with 8-bit bus"
>> > +       help
>> > +         Support HSMMC Channel 2 8-bit bus.
>> > +         If selected, Channel 3 is disabled.
>>
>> I think it's not needed since most boards have fixed bandwith and
>> cfg_gpio handle it regardless these configuration.
>> Also we use the select state if needed, e.g., when MMC0 uses
>> 8-buswidth then don't select MMC1.
>> I saw the SMDK board use this configuration. but just choose bandwidth 4
> or 8.
>>
> Hmm...
>
> If it can be supported 4bit or 8bit option on each board, should be added
> above configuration for selecting it.
why? each sdhci_cfg_gpio can configure the pins properly. each board
has each own SD/MMC. In your test board maybe can switch 4 or 8 but
most board has fixed one.

If SMDK has 8-bit just assign 8 and unselect MMC1 or 3. that's all. No
configurations.

>
> Mr. Seo, it would be nice if you could add un-selection CH1 or CH3 when
> selecting 8bit support later.
>
> Anyway, looks ok to me now..will apply.
>
>> Others good.
>>
> (snip)
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>