[U-Boot] [PATCH 2/4] S5P: GPIO: Add GPIO pin numbering to driver

Rajeshwari Birje rajeshwari.birje at gmail.com
Tue Dec 11 13:17:07 CET 2012


Hi Minkyu Kang,

Thank you for comments.

On Wed, Dec 5, 2012 at 4:51 PM, Minkyu Kang <mk7.kang at samsung.com> wrote:
> Dear Rajeshwari,
>
> On 05/12/12 19:46, Minkyu Kang wrote:
>> API's for GPIO pin numbering support are added to the generic S5P
>> gpio driver
>>
>> Signed-off-by: Leela Krishna Amudala <l.krishna at samsung.com>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> Signed-off-by: Rajeshawari Shinde <rajeshwari.s at samsung.com>
>> ---
>>  drivers/gpio/s5p_gpio.c |  158
>> +++++++++++++++++++++++++++++++++++++++++++++--
>>  1 files changed, 152 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpio/s5p_gpio.c b/drivers/gpio/s5p_gpio.c
>> index 47f3213..5c051d4 100644
>> --- a/drivers/gpio/s5p_gpio.c
>> +++ b/drivers/gpio/s5p_gpio.c
>> @@ -142,20 +142,165 @@ void s5p_gpio_set_rate(struct s5p_gpio_bank *bank,
>> int gpio, int mode)
>>       writel(value, &bank->drv);
>>  }
>>
>> -struct s5p_gpio_bank *s5p_gpio_get_bank(unsigned gpio)
>> +
>> +int s5p_gpio_get_pin(unsigned gpio)
>>  {
>> -     int bank = gpio / GPIO_PER_BANK;
>> -     bank *= sizeof(struct s5p_gpio_bank);
>> +     return gpio % GPIO_PER_BANK;
>> +}
>>
>> -     return (struct s5p_gpio_bank *) (s5p_gpio_base(gpio) + bank);
>> +#ifdef HAVE_GENERIC_GPIO
>
> Is it generic naming?
-- As of now it has been defined only for EXYNOS5.
>
> Also.. why we support two types of GPIO functions?
> If you want to support generic GPIO, you should be replaced new gpio functions completely.
- We can do it, we can have only 1 set of gpio functions.
>
>> +static struct s5p_gpio_bank *gpio_get_bank(unsigned int gpio)
>> +{
>> +     int bank_offset;
>> +
>> +     if (gpio < GPIO_MAX_PORT_PART_1) {
>> +             bank_offset = gpio / GPIO_PER_BANK;
>> +             return (struct s5p_gpio_bank *) (EXYNOS5_GPIO_PART1_BASE +
>
> Is it exynos5 specific?
Yes it is exynos5 specific as in EXYNOS5 the gpio banks are not
continuous and the number of pins are also not uniform across the gpio
banks hence the exsistting funcion gives us wrong values. Hence this
calculation will change for Exynos5 and we need to a dedicated
function, do suggest me if you have any other idea.
>
>> (bank_offset *
>> +                                             sizeof(struct
>> s5p_gpio_bank)));
>> +     } else if (gpio < GPIO_MAX_PORT_PART_2) {
>> +             bank_offset = (gpio - GPIO_MAX_PORT_PART_1) / GPIO_PER_BANK;
>> +             return (struct s5p_gpio_bank *) (EXYNOS5_GPIO_PART2_BASE +
>> (bank_offset *
>> +                                             sizeof(struct
>> s5p_gpio_bank)));
>> +     } else if (gpio < GPIO_MAX_PORT_PART_3) {
>> +             bank_offset = (gpio - GPIO_MAX_PORT_PART_2) / GPIO_PER_BANK;
>> +             return (struct s5p_gpio_bank *) (EXYNOS5_GPIO_PART3_BASE +
>> (bank_offset *
>> +                                             sizeof(struct
>> s5p_gpio_bank)));
>> +     }
>> +     else
>
> should be moved to upper line.
> And need the brace at this else state.
- Will do this
>
>> +             return (struct s5p_gpio_bank *) EXYNOS5_GPIO_PART4_BASE;
>> +
>> +     return NULL;
>>  }
>>
>> -int s5p_gpio_get_pin(unsigned gpio)
>> +void gpio_cfg_pin(int gpio, int cfg)
>>  {
>> -     return gpio % GPIO_PER_BANK;
>> +     unsigned int value;
>> +     struct s5p_gpio_bank *bank = gpio_get_bank(gpio);
>> +
>> +     value = readl(&bank->con);
>> +     value &= ~CON_MASK(GPIO_BIT(gpio));
>> +     value |= CON_SFR(GPIO_BIT(gpio), cfg);
>> +     writel(value, &bank->con);
>> +}
>> +
>> +void gpio_set_pull(int gpio, int mode)
>> +{
>> +     unsigned int value;
>> +     struct s5p_gpio_bank *bank = gpio_get_bank(gpio);
>> +
>> +     value = readl(&bank->pull);
>> +     value &= ~PULL_MASK(GPIO_BIT(gpio));
>> +
>> +     switch (mode) {
>> +     case GPIO_PULL_DOWN:
>> +     case GPIO_PULL_UP:
>> +             value |= PULL_MODE(GPIO_BIT(gpio), mode);
>> +             break;
>> +     default:
>> +             break;
>> +     }
>> +
>> +     writel(value, &bank->pull);
>> +}
>> +
>> +void gpio_set_drv(int gpio, int mode)
>> +{
>> +     unsigned int value;
>> +     struct s5p_gpio_bank *bank = gpio_get_bank(gpio);
>> +
>> +     value = readl(&bank->drv);
>> +     value &= ~DRV_MASK(GPIO_BIT(gpio));
>> +
>> +     switch (mode) {
>> +     case GPIO_DRV_1X:
>> +     case GPIO_DRV_2X:
>> +     case GPIO_DRV_3X:
>> +     case GPIO_DRV_4X:
>> +             value |= DRV_SET(GPIO_BIT(gpio), mode);
>> +             break;
>> +     default:
>> +             return;
>> +     }
>> +
>> +     writel(value, &bank->drv);
>>  }
>>
>> +void gpio_set_rate(int gpio, int mode)
>> +{
>> +     unsigned int value;
>> +     struct s5p_gpio_bank *bank = gpio_get_bank(gpio);
>> +
>> +     value = readl(&bank->drv);
>> +     value &= ~RATE_MASK(GPIO_BIT(gpio));
>> +
>> +     switch (mode) {
>> +     case GPIO_DRV_FAST:
>> +     case GPIO_DRV_SLOW:
>> +             value |= RATE_SET(GPIO_BIT(gpio));
>> +             break;
>> +     default:
>> +             return;
>> +     }
>> +
>> +     writel(value, &bank->drv);
>> +}
>> +
>> +int gpio_direction_input(unsigned gpio)
>> +{
>> +     gpio_cfg_pin(gpio, GPIO_INPUT);
>> +
>> +     return 0;
>> +}
>> +
>> +int gpio_direction_output(unsigned gpio, int value)
>> +{
>> +     unsigned int val;
>> +     struct s5p_gpio_bank *bank = gpio_get_bank(gpio);
>> +
>> +     gpio_cfg_pin(gpio, GPIO_OUTPUT);
>> +
>> +     val = readl(&bank->dat);
>> +     val &= ~DAT_MASK(GPIO_BIT(gpio));
>> +     if (value)
>> +             val |= DAT_SET(GPIO_BIT(gpio));
>> +     writel(val, &bank->dat);
>> +
>> +     return 0;
>> +}
>> +
>> +int gpio_get_value(unsigned gpio)
>> +{
>> +     unsigned int value;
>> +     struct s5p_gpio_bank *bank = gpio_get_bank(gpio);
>> +
>> +     value = readl(&bank->dat);
>> +     return !!(value & DAT_MASK(GPIO_BIT(gpio)));
>> +}
>> +
>> +
>> +int gpio_set_value(unsigned gpio, int value)
>> +{
>> +     unsigned int val;
>> +     struct s5p_gpio_bank *bank = gpio_get_bank(gpio);
>> +
>> +     val = readl(&bank->dat);
>> +     val &= ~DAT_MASK(GPIO_BIT(gpio));
>> +     if (value)
>> +             val |= DAT_SET(GPIO_BIT(gpio));
>> +     writel(val, &bank->dat);
>> +
>> +     return 0;
>> +}
>> +
>> +#else
>>  /* Common GPIO API */
>> +struct s5p_gpio_bank *s5p_gpio_get_bank(unsigned gpio)
>> +{
>> +     int bank = gpio / GPIO_PER_BANK;
>> +     bank *= sizeof(struct s5p_gpio_bank);
>> +
>> +     return (struct s5p_gpio_bank *) (s5p_gpio_base(gpio) + bank);
>> +}
>>
>>  int gpio_request(unsigned gpio, const char *label)
>>  {
>> @@ -194,3 +339,4 @@ int gpio_set_value(unsigned gpio, int value)
>>
>>       return 0;
>>  }
>> +#endif /* HAVE_GENERIC_GPIO */
>>
>
> Thanks.
> Minkyu Kang.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Regards,
Rajeshwari Shinde.


More information about the U-Boot mailing list