[1/6,V5] EXYNOS5: Add pinmux support for SPI

From: Rajeshwari Shinde <rajeshwari.s@samsung.com>

This patch adds pinmux support for SPI channels

Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
Signed-off-by: Hatim Ali <hatim.rv@samsung.com>
---
Changes since v4:
	Fixed minor nits suggested by Simon Glass

 arch/arm/cpu/armv7/exynos/pinmux.c        |   51 +++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-exynos/periph.h |    5 +++
 2 files changed, 56 insertions(+), 0 deletions(-)

Hi Hatim,

On Tue, Oct 16, 2012 at 3:58 AM, Hatim Ali <hatim.rv@samsung.com> wrote:
> From: Rajeshwari Shinde <rajeshwari.s@samsung.com>
>
> This patch adds pinmux support for SPI channels
>
> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
> Signed-off-by: Hatim Ali <hatim.rv@samsung.com>

Acked-by: Simon Glass <sjg@chromium.org>

> ---
> Changes since v4:
>         Fixed minor nits suggested by Simon Glass
>
>  arch/arm/cpu/armv7/exynos/pinmux.c        |   51 +++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-exynos/periph.h |    5 +++
>  2 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/exynos/pinmux.c b/arch/arm/cpu/armv7/exynos/pinmux.c
> index 5796d56..e01bef4 100644
> --- a/arch/arm/cpu/armv7/exynos/pinmux.c
> +++ b/arch/arm/cpu/armv7/exynos/pinmux.c
> @@ -112,6 +112,7 @@ static int exynos5_mmc_config(int peripheral, int flags)
>                 s5p_gpio_set_pull(bank, i, GPIO_PULL_UP);
>                 s5p_gpio_set_drv(bank, i, GPIO_DRV_4X);
>         }
> +
>         return 0;
>  }
>
> @@ -230,6 +231,49 @@ static void exynos5_i2c_config(int peripheral, int flags)
>         }
>  }
>
> +void exynos5_spi_config(int peripheral)
> +{
> +       int cfg = 0, pin = 0, i;
> +       struct s5p_gpio_bank *bank = NULL;
> +       struct exynos5_gpio_part1 *gpio1 =
> +               (struct exynos5_gpio_part1 *) samsung_get_base_gpio_part1();
> +       struct exynos5_gpio_part2 *gpio2 =
> +               (struct exynos5_gpio_part2 *) samsung_get_base_gpio_part2();
> +
> +       switch (peripheral) {
> +       case PERIPH_ID_SPI0:
> +               bank = &gpio1->a2;
> +               cfg = GPIO_FUNC(0x2);
> +               pin = 0;
> +               break;
> +       case PERIPH_ID_SPI1:
> +               bank = &gpio1->a2;
> +               cfg = GPIO_FUNC(0x2);
> +               pin = 4;
> +               break;
> +       case PERIPH_ID_SPI2:
> +               bank = &gpio1->b1;
> +               cfg = GPIO_FUNC(0x5);
> +               pin = 1;
> +               break;
> +       case PERIPH_ID_SPI3:
> +               bank = &gpio2->f1;
> +               cfg = GPIO_FUNC(0x2);
> +               pin = 0;
> +               break;
> +       case PERIPH_ID_SPI4:
> +               for (i = 2; i < 4; i++)
> +                       s5p_gpio_cfg_pin(&gpio2->f0, i, GPIO_FUNC(0x4));
> +               for (i = 4; i < 6; i++)
> +                       s5p_gpio_cfg_pin(&gpio2->e0, i, GPIO_FUNC(0x4));
> +               break;

Sorry I did't notice this earlier. I find it confusing that this code
uses f0 and e0, with the loop starting at something other than 0, but
the loop below for SPI4 starts at the pin.

Can I suggest here:

> +               for (i = 0; i < 2; i++) {
> +                       s5p_gpio_cfg_pin(&gpio2->f2, i, GPIO_FUNC(0x4));
> +                       s5p_gpio_cfg_pin(&gpio2->e4, i, GPIO_FUNC(0x4));
>+                }

Note: this is just a suggestion, it is up to you, and I have acked
this patch anyway.

Regards,
Simon

> +       }
> +       if (peripheral != PERIPH_ID_SPI4) {
> +               for (i = pin; i < pin + 4; i++)
> +                       s5p_gpio_cfg_pin(bank, i, cfg);
> +       }
> +}
> +
>  static int exynos5_pinmux_config(int peripheral, int flags)
>  {
>         switch (peripheral) {
> @@ -257,6 +301,13 @@ static int exynos5_pinmux_config(int peripheral, int flags)
>         case PERIPH_ID_I2C7:
>                 exynos5_i2c_config(peripheral, flags);
>                 break;
> +       case PERIPH_ID_SPI0:
> +       case PERIPH_ID_SPI1:
> +       case PERIPH_ID_SPI2:
> +       case PERIPH_ID_SPI3:
> +       case PERIPH_ID_SPI4:
> +               exynos5_spi_config(peripheral);
> +               break;
>         default:
>                 debug("%s: invalid peripheral %d", __func__, peripheral);
>                 return -1;
> diff --git a/arch/arm/include/asm/arch-exynos/periph.h b/arch/arm/include/asm/arch-exynos/periph.h
> index 082611c..4054fb6 100644
> --- a/arch/arm/include/asm/arch-exynos/periph.h
> +++ b/arch/arm/include/asm/arch-exynos/periph.h
> @@ -44,6 +44,11 @@ enum periph_id {
>         PERIPH_ID_SDMMC3,
>         PERIPH_ID_SDMMC4,
>         PERIPH_ID_SROMC,
> +       PERIPH_ID_SPI0,
> +       PERIPH_ID_SPI1,
> +       PERIPH_ID_SPI2,
> +       PERIPH_ID_SPI3,
> +       PERIPH_ID_SPI4,
>         PERIPH_ID_UART0,
>         PERIPH_ID_UART1,
>         PERIPH_ID_UART2,
> --
> 1.7.2.3
>

On 18 October 2012 08:43, Simon Glass <sjg@chromium.org> wrote:
> Hi Hatim,
>
> On Tue, Oct 16, 2012 at 3:58 AM, Hatim Ali <hatim.rv@samsung.com> wrote:
>> From: Rajeshwari Shinde <rajeshwari.s@samsung.com>
>>
>> This patch adds pinmux support for SPI channels
>>
>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
>> Signed-off-by: Hatim Ali <hatim.rv@samsung.com>
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
>> ---
>> Changes since v4:
>>         Fixed minor nits suggested by Simon Glass
>>
>>  arch/arm/cpu/armv7/exynos/pinmux.c        |   51 +++++++++++++++++++++++++++++
>>  arch/arm/include/asm/arch-exynos/periph.h |    5 +++
>>  2 files changed, 56 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/exynos/pinmux.c b/arch/arm/cpu/armv7/exynos/pinmux.c
>> index 5796d56..e01bef4 100644
>> --- a/arch/arm/cpu/armv7/exynos/pinmux.c
>> +++ b/arch/arm/cpu/armv7/exynos/pinmux.c
>> @@ -112,6 +112,7 @@ static int exynos5_mmc_config(int peripheral, int flags)
>>                 s5p_gpio_set_pull(bank, i, GPIO_PULL_UP);
>>                 s5p_gpio_set_drv(bank, i, GPIO_DRV_4X);
>>         }
>> +
>>         return 0;
>>  }
>>
>> @@ -230,6 +231,49 @@ static void exynos5_i2c_config(int peripheral, int flags)
>>         }
>>  }
>>
>> +void exynos5_spi_config(int peripheral)
>> +{
>> +       int cfg = 0, pin = 0, i;
>> +       struct s5p_gpio_bank *bank = NULL;
>> +       struct exynos5_gpio_part1 *gpio1 =
>> +               (struct exynos5_gpio_part1 *) samsung_get_base_gpio_part1();
>> +       struct exynos5_gpio_part2 *gpio2 =
>> +               (struct exynos5_gpio_part2 *) samsung_get_base_gpio_part2();
>> +
>> +       switch (peripheral) {
>> +       case PERIPH_ID_SPI0:
>> +               bank = &gpio1->a2;
>> +               cfg = GPIO_FUNC(0x2);
>> +               pin = 0;
>> +               break;
>> +       case PERIPH_ID_SPI1:
>> +               bank = &gpio1->a2;
>> +               cfg = GPIO_FUNC(0x2);
>> +               pin = 4;
>> +               break;
>> +       case PERIPH_ID_SPI2:
>> +               bank = &gpio1->b1;
>> +               cfg = GPIO_FUNC(0x5);
>> +               pin = 1;
>> +               break;
>> +       case PERIPH_ID_SPI3:
>> +               bank = &gpio2->f1;
>> +               cfg = GPIO_FUNC(0x2);
>> +               pin = 0;
>> +               break;
>> +       case PERIPH_ID_SPI4:
>> +               for (i = 2; i < 4; i++)
>> +                       s5p_gpio_cfg_pin(&gpio2->f0, i, GPIO_FUNC(0x4));
>> +               for (i = 4; i < 6; i++)
>> +                       s5p_gpio_cfg_pin(&gpio2->e0, i, GPIO_FUNC(0x4));
>> +               break;
>
> Sorry I did't notice this earlier. I find it confusing that this code
> uses f0 and e0, with the loop starting at something other than 0, but
> the loop below for SPI4 starts at the pin.
>
> Can I suggest here:
>
>> +               for (i = 0; i < 2; i++) {
>> +                       s5p_gpio_cfg_pin(&gpio2->f2, i, GPIO_FUNC(0x4));
>> +                       s5p_gpio_cfg_pin(&gpio2->e4, i, GPIO_FUNC(0x4));
>>+                }

Hm... no.
It can be possible.

+               for (i = 0; i < 2; i++) {
+                       s5p_gpio_cfg_pin(&gpio2->f0, i + 2, GPIO_FUNC(0x4));
+                       s5p_gpio_cfg_pin(&gpio2->e0, i + 4, GPIO_FUNC(0x4));
+                }

>
> Note: this is just a suggestion, it is up to you, and I have acked
> this patch anyway.

Yes, It's up to him. :)

Thanks.
Minkyu Kang.