* [PATCH v3] pinctrl: samsung: Add support for pull-up and pull-down [not found] <CGME20240704043358epcas5p282acd174113c7baf3f7a3472ba4c39ff@epcas5p2.samsung.com> @ 2024-07-04 4:26 ` Vishnu Reddy 2024-07-05 9:38 ` Krzysztof Kozlowski 0 siblings, 1 reply; 3+ messages in thread From: Vishnu Reddy @ 2024-07-04 4:26 UTC (permalink / raw) To: krzysztof.kozlowski, s.nawrocki, alim.akhtar, linus.walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, pankaj.dubey, ravi.patel, gost.dev gpiolib framework has the implementation of setting up the PUD configuration for GPIO pins but there is no driver support. Add support to handle the PUD configuration request from the userspace in samsung pinctrl driver. Signed-off-by: Vishnu Reddy <vishnu.reddy@samsung.com> --- drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 15 ++++ drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 15 ++++ drivers/pinctrl/samsung/pinctrl-samsung.c | 80 ++++++++++++++++++++ drivers/pinctrl/samsung/pinctrl-samsung.h | 24 ++++++ 4 files changed, 134 insertions(+) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c index 85ddf49a5188..426d1daacef2 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c @@ -40,6 +40,20 @@ static const struct samsung_pin_bank_type bank_type_alive = { #define S5P_OTHERS_RET_MMC (1 << 29) #define S5P_OTHERS_RET_UART (1 << 28) +/* + * s5pv210_pud_value_init: initialize the drvdata pud_val with s5pv210 pud values + * s5pv210_pull_disable: 0 + * s5pv210_pull_down_enable: 1 + * s5pv210_pull_up_enable: 2 + */ +static void s5pv210_pud_value_init(struct samsung_pinctrl_drv_data *drvdata) +{ + unsigned int i, *pud_val = drvdata->pud_val; + + for (i = 0; i < PUD_MAX; i++) + pud_val[i] = i; +} + static void s5pv210_retention_disable(struct samsung_pinctrl_drv_data *drvdata) { void __iomem *clk_base = (void __iomem *)drvdata->retention_ctrl->priv; @@ -133,6 +147,7 @@ static const struct samsung_pin_ctrl s5pv210_pin_ctrl[] __initconst = { .nr_banks = ARRAY_SIZE(s5pv210_pin_bank), .eint_gpio_init = exynos_eint_gpio_init, .eint_wkup_init = exynos_eint_wkup_init, + .pud_value_init = s5pv210_pud_value_init, .suspend = exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, .retention_data = &s5pv210_retention_data, diff --git a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c index c5d92db4fdb1..cf57b0f16999 100644 --- a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c +++ b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c @@ -255,6 +255,20 @@ static int s3c64xx_irq_get_trigger(unsigned int type) return trigger; } +/* + * s3c64xx_pud_value_init: initialize the drvdata pud_val with s3c64xx pud values + * s3c64xx_pull_disable: 0 + * s3c64xx_pull_down_enable: 1 + * s3c64xx_pull_up_enable: 2 + */ +static void s3c64xx_pud_value_init(struct samsung_pinctrl_drv_data *drvdata) +{ + unsigned int i, *pud_val = drvdata->pud_val; + + for (i = 0; i < PUD_MAX; i++) + pud_val[i] = i; +} + static void s3c64xx_irq_set_handler(struct irq_data *d, unsigned int type) { /* Edge- and level-triggered interrupts need different handlers */ @@ -797,6 +811,7 @@ static const struct samsung_pin_ctrl s3c64xx_pin_ctrl[] __initconst = { .nr_banks = ARRAY_SIZE(s3c64xx_pin_banks0), .eint_gpio_init = s3c64xx_eint_gpio_init, .eint_wkup_init = s3c64xx_eint_eint0_init, + .pud_value_init = s3c64xx_pud_value_init, }, }; diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c index 623df65a5d6f..7d7e924c1fdb 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.c +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c @@ -997,6 +997,80 @@ static int samsung_pinctrl_unregister(struct platform_device *pdev, return 0; } +/* + * samsung_pud_value_init: initialize the drvdata pud_val + */ +static void samsung_pud_value_init(struct samsung_pinctrl_drv_data *drvdata) +{ + unsigned int *pud_val = drvdata->pud_val; + + pud_val[PUD_PULL_DISABLE] = PIN_PUD_PULL_UP_DOWN_DISABLE; + pud_val[PUD_PULL_DOWN] = PIN_PUD_PULL_DOWN_ENABLE; + pud_val[PUD_PULL_UP] = PIN_PUD_PULL_UP_ENABLE; +} + +/* + * samsung_gpio_set_pud will enable or disable the pull-down and + * pull-up for the gpio pins in the PUD register. + */ +static void samsung_gpio_set_pud(struct gpio_chip *gc, unsigned int offset, + unsigned int value) +{ + struct samsung_pin_bank *bank = gpiochip_get_data(gc); + const struct samsung_pin_bank_type *type = bank->type; + void __iomem *reg; + unsigned int data, mask; + + reg = bank->pctl_base + bank->pctl_offset; + data = readl(reg + type->reg_offset[PINCFG_TYPE_PUD]); + mask = (1 << type->fld_width[PINCFG_TYPE_PUD]) - 1; + data &= ~(mask << (offset * type->fld_width[PINCFG_TYPE_PUD])); + data |= value << (offset * type->fld_width[PINCFG_TYPE_PUD]); + writel(data, reg + type->reg_offset[PINCFG_TYPE_PUD]); +} + +/* + * samsung_gpio_set_config will identify the type of PUD config based + * on the gpiolib request to enable or disable the PUD configuration. + */ +static int samsung_gpio_set_config(struct gpio_chip *gc, unsigned int offset, + unsigned long config) +{ + struct samsung_pin_bank *bank = gpiochip_get_data(gc); + struct samsung_pinctrl_drv_data *drvdata = bank->drvdata; + unsigned int value; + int ret = 0; + unsigned long flags; + + switch (pinconf_to_config_param(config)) { + case PIN_CONFIG_BIAS_DISABLE: + value = drvdata->pud_val[PUD_PULL_DISABLE]; + break; + case PIN_CONFIG_BIAS_PULL_DOWN: + value = drvdata->pud_val[PUD_PULL_DOWN]; + break; + case PIN_CONFIG_BIAS_PULL_UP: + value = drvdata->pud_val[PUD_PULL_UP]; + break; + default: + return -ENOTSUPP; + } + + ret = clk_enable(drvdata->pclk); + if (ret) { + dev_err(drvdata->dev, "failed to enable clock\n"); + return ret; + } + + raw_spin_lock_irqsave(&bank->slock, flags); + samsung_gpio_set_pud(gc, offset, value); + raw_spin_unlock_irqrestore(&bank->slock, flags); + + clk_disable(drvdata->pclk); + + return ret; +} + static const struct gpio_chip samsung_gpiolib_chip = { .request = gpiochip_generic_request, .free = gpiochip_generic_free, @@ -1006,6 +1080,7 @@ static const struct gpio_chip samsung_gpiolib_chip = { .direction_output = samsung_gpio_direction_output, .to_irq = samsung_gpio_to_irq, .add_pin_ranges = samsung_add_pin_ranges, + .set_config = samsung_gpio_set_config, .owner = THIS_MODULE, }; @@ -1237,6 +1312,11 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) if (ctrl->eint_wkup_init) ctrl->eint_wkup_init(drvdata); + if (ctrl->pud_value_init) + ctrl->pud_value_init(drvdata); + else + samsung_pud_value_init(drvdata); + ret = samsung_gpiolib_register(pdev, drvdata); if (ret) goto err_unregister; diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h index d50ba6f07d5d..61384096b6d7 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.h +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h @@ -61,6 +61,28 @@ enum pincfg_type { #define PIN_CON_FUNC_INPUT 0x0 #define PIN_CON_FUNC_OUTPUT 0x1 +/* + * Values for the pin PUD register. + */ +#define PIN_PUD_PULL_UP_DOWN_DISABLE 0x0 +#define PIN_PUD_PULL_DOWN_ENABLE 0x1 +#define PIN_PUD_PULL_UP_ENABLE 0x3 + +/* + * enum pud_index: Index values to access the pud_val array in + * struct samsung_pinctrl_drv_data. + * @PUD_PULL_DISABLE: Index for value of pud disable + * @PUD_PULL_DOWN: Index for the value of pull down enable + * @PUD_PULL_UP: Index for the value of pull up enable + * @PUD_MAX: Maximun value of the index + */ +enum pud_index { + PUD_PULL_DISABLE, + PUD_PULL_DOWN, + PUD_PULL_UP, + PUD_MAX, +}; + /** * enum eint_type - possible external interrupt types. * @EINT_TYPE_NONE: bank does not support external interrupts @@ -261,6 +283,7 @@ struct samsung_pin_ctrl { int (*eint_gpio_init)(struct samsung_pinctrl_drv_data *); int (*eint_wkup_init)(struct samsung_pinctrl_drv_data *); + void (*pud_value_init)(struct samsung_pinctrl_drv_data *drvdata); void (*suspend)(struct samsung_pinctrl_drv_data *); void (*resume)(struct samsung_pinctrl_drv_data *); }; @@ -307,6 +330,7 @@ struct samsung_pinctrl_drv_data { struct samsung_pin_bank *pin_banks; unsigned int nr_banks; unsigned int nr_pins; + unsigned int pud_val[PUD_MAX]; struct samsung_retention_ctrl *retention_ctrl; -- 2.17.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] pinctrl: samsung: Add support for pull-up and pull-down 2024-07-04 4:26 ` [PATCH v3] pinctrl: samsung: Add support for pull-up and pull-down Vishnu Reddy @ 2024-07-05 9:38 ` Krzysztof Kozlowski 2024-07-05 11:59 ` Vishnu Reddy 0 siblings, 1 reply; 3+ messages in thread From: Krzysztof Kozlowski @ 2024-07-05 9:38 UTC (permalink / raw) To: Vishnu Reddy, s.nawrocki, alim.akhtar, linus.walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, pankaj.dubey, ravi.patel, gost.dev On 04/07/2024 06:26, Vishnu Reddy wrote: > gpiolib framework has the implementation of setting up the > PUD configuration for GPIO pins but there is no driver support. > > Add support to handle the PUD configuration request from the > userspace in samsung pinctrl driver. > > Signed-off-by: Vishnu Reddy <vishnu.reddy@samsung.com> > --- > drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 15 ++++ > drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 15 ++++ > drivers/pinctrl/samsung/pinctrl-samsung.c | 80 ++++++++++++++++++++ > drivers/pinctrl/samsung/pinctrl-samsung.h | 24 ++++++ > 4 files changed, 134 insertions(+) > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c > index 85ddf49a5188..426d1daacef2 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c > @@ -40,6 +40,20 @@ static const struct samsung_pin_bank_type bank_type_alive = { > #define S5P_OTHERS_RET_MMC (1 << 29) > #define S5P_OTHERS_RET_UART (1 << 28) > > +/* > + * s5pv210_pud_value_init: initialize the drvdata pud_val with s5pv210 pud values > + * s5pv210_pull_disable: 0 > + * s5pv210_pull_down_enable: 1 > + * s5pv210_pull_up_enable: 2 Please use proper defines, e.g. S5P_PIN_PUD_PULL_DISABLE > + */ > +static void s5pv210_pud_value_init(struct samsung_pinctrl_drv_data *drvdata) > +{ > + unsigned int i, *pud_val = drvdata->pud_val; > + > + for (i = 0; i < PUD_MAX; i++) > + pud_val[i] = i; pud_val[PUD_PULL_DISABLE] = PROPER_DEFINE > +} > + > static void s5pv210_retention_disable(struct samsung_pinctrl_drv_data *drvdata) > { > void __iomem *clk_base = (void __iomem *)drvdata->retention_ctrl->priv; > @@ -133,6 +147,7 @@ static const struct samsung_pin_ctrl s5pv210_pin_ctrl[] __initconst = { > .nr_banks = ARRAY_SIZE(s5pv210_pin_bank), > .eint_gpio_init = exynos_eint_gpio_init, > .eint_wkup_init = exynos_eint_wkup_init, > + .pud_value_init = s5pv210_pud_value_init, > .suspend = exynos_pinctrl_suspend, > .resume = exynos_pinctrl_resume, > .retention_data = &s5pv210_retention_data, > diff --git a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c > index c5d92db4fdb1..cf57b0f16999 100644 > --- a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c > +++ b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c > @@ -255,6 +255,20 @@ static int s3c64xx_irq_get_trigger(unsigned int type) > return trigger; > } > > +/* > + * s3c64xx_pud_value_init: initialize the drvdata pud_val with s3c64xx pud values > + * s3c64xx_pull_disable: 0 > + * s3c64xx_pull_down_enable: 1 > + * s3c64xx_pull_up_enable: 2 Use proper defines. Comments are not the place to define magic values. > + */ > +static void s3c64xx_pud_value_init(struct samsung_pinctrl_drv_data *drvdata) > +{ > + unsigned int i, *pud_val = drvdata->pud_val; > + > + for (i = 0; i < PUD_MAX; i++) > + pud_val[i] = i; > +} > + > static void s3c64xx_irq_set_handler(struct irq_data *d, unsigned int type) > { > /* Edge- and level-triggered interrupts need different handlers */ > @@ -797,6 +811,7 @@ static const struct samsung_pin_ctrl s3c64xx_pin_ctrl[] __initconst = { > .nr_banks = ARRAY_SIZE(s3c64xx_pin_banks0), > .eint_gpio_init = s3c64xx_eint_gpio_init, > .eint_wkup_init = s3c64xx_eint_eint0_init, > + .pud_value_init = s3c64xx_pud_value_init, > }, > }; > > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c > index 623df65a5d6f..7d7e924c1fdb 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c > @@ -997,6 +997,80 @@ static int samsung_pinctrl_unregister(struct platform_device *pdev, > return 0; > } > > +/* > + *samsung_pud_value_init: initialize the drvdata pud_val You use some weird style of comments. It's not kerneldoc, it's not a proper neither useful comment. Either use proper kerneldoc (and test it!) or drop duplicated function names. They are not helping. This applies everywhere. > + */ > +static void samsung_pud_value_init(struct samsung_pinctrl_drv_data *drvdata) > +{ > + unsigned int *pud_val = drvdata->pud_val; > + > + pud_val[PUD_PULL_DISABLE] = PIN_PUD_PULL_UP_DOWN_DISABLE; > + pud_val[PUD_PULL_DOWN] = PIN_PUD_PULL_DOWN_ENABLE; > + pud_val[PUD_PULL_UP] = PIN_PUD_PULL_UP_ENABLE; > +} > + > +/* > + * samsung_gpio_set_pud will enable or disable the pull-down and > + * pull-up for the gpio pins in the PUD register. > + */ > +static void samsung_gpio_set_pud(struct gpio_chip *gc, unsigned int offset, > + unsigned int value) > +{ > + struct samsung_pin_bank *bank = gpiochip_get_data(gc); > + const struct samsung_pin_bank_type *type = bank->type; > + void __iomem *reg; > + unsigned int data, mask; > + > + reg = bank->pctl_base + bank->pctl_offset; > + data = readl(reg + type->reg_offset[PINCFG_TYPE_PUD]); > + mask = (1 << type->fld_width[PINCFG_TYPE_PUD]) - 1; > + data &= ~(mask << (offset * type->fld_width[PINCFG_TYPE_PUD])); > + data |= value << (offset * type->fld_width[PINCFG_TYPE_PUD]); > + writel(data, reg + type->reg_offset[PINCFG_TYPE_PUD]); > +} > + > +/* > + * samsung_gpio_set_config will identify the type of PUD config based > + * on the gpiolib request to enable or disable the PUD configuration. > + */ > +static int samsung_gpio_set_config(struct gpio_chip *gc, unsigned int offset, > + unsigned long config) > +{ > + struct samsung_pin_bank *bank = gpiochip_get_data(gc); > + struct samsung_pinctrl_drv_data *drvdata = bank->drvdata; > + unsigned int value; > + int ret = 0; > + unsigned long flags; > + > + switch (pinconf_to_config_param(config)) { > + case PIN_CONFIG_BIAS_DISABLE: > + value = drvdata->pud_val[PUD_PULL_DISABLE]; > + break; > + case PIN_CONFIG_BIAS_PULL_DOWN: > + value = drvdata->pud_val[PUD_PULL_DOWN]; > + break; > + case PIN_CONFIG_BIAS_PULL_UP: > + value = drvdata->pud_val[PUD_PULL_UP]; > + break; > + default: > + return -ENOTSUPP; > + } > + > + ret = clk_enable(drvdata->pclk); > + if (ret) { > + dev_err(drvdata->dev, "failed to enable clock\n"); > + return ret; > + } > + > + raw_spin_lock_irqsave(&bank->slock, flags); > + samsung_gpio_set_pud(gc, offset, value); > + raw_spin_unlock_irqrestore(&bank->slock, flags); > + > + clk_disable(drvdata->pclk); > + > + return ret; > +} > + > static const struct gpio_chip samsung_gpiolib_chip = { > .request = gpiochip_generic_request, > .free = gpiochip_generic_free, > @@ -1006,6 +1080,7 @@ static const struct gpio_chip samsung_gpiolib_chip = { > .direction_output = samsung_gpio_direction_output, > .to_irq = samsung_gpio_to_irq, > .add_pin_ranges = samsung_add_pin_ranges, > + .set_config = samsung_gpio_set_config, > .owner = THIS_MODULE, > }; > > @@ -1237,6 +1312,11 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) > if (ctrl->eint_wkup_init) > ctrl->eint_wkup_init(drvdata); > > + if (ctrl->pud_value_init) > + ctrl->pud_value_init(drvdata); > + else > + samsung_pud_value_init(drvdata); > + > ret = samsung_gpiolib_register(pdev, drvdata); > if (ret) > goto err_unregister; > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h > index d50ba6f07d5d..61384096b6d7 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.h > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h > @@ -61,6 +61,28 @@ enum pincfg_type { > #define PIN_CON_FUNC_INPUT 0x0 > #define PIN_CON_FUNC_OUTPUT 0x1 > > +/* > + * Values for the pin PUD register. > + */ > +#define PIN_PUD_PULL_UP_DOWN_DISABLE 0x0 EXYNOS_PIN_PUD_PULL_DISABLE > +#define PIN_PUD_PULL_DOWN_ENABLE 0x1 EXYNOS_PIN_PID_PULL_DOWN > +#define PIN_PUD_PULL_UP_ENABLE 0x3 EXYNOS_PIN_PID_PULL_UP > + > +/* > + * enum pud_index: Index values to access the pud_val array in > + * struct samsung_pinctrl_drv_data. > + * @PUD_PULL_DISABLE: Index for value of pud disable > + * @PUD_PULL_DOWN: Index for the value of pull down enable > + * @PUD_PULL_UP: Index for the value of pull up enable > + * @PUD_MAX: Maximun value of the index Typo: Maximum > + */ > +enum pud_index { > + PUD_PULL_DISABLE, > + PUD_PULL_DOWN, > + PUD_PULL_UP, > + PUD_MAX, > +}; Best regards, Krzysztof ^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH v3] pinctrl: samsung: Add support for pull-up and pull-down 2024-07-05 9:38 ` Krzysztof Kozlowski @ 2024-07-05 11:59 ` Vishnu Reddy 0 siblings, 0 replies; 3+ messages in thread From: Vishnu Reddy @ 2024-07-05 11:59 UTC (permalink / raw) To: 'Krzysztof Kozlowski', s.nawrocki, alim.akhtar, linus.walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, pankaj.dubey, ravi.patel, gost.dev > -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 05 July 2024 15:09 > To: Vishnu Reddy <vishnu.reddy@samsung.com>; > s.nawrocki@samsung.com; alim.akhtar@samsung.com; > linus.walleij@linaro.org > Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- > soc@vger.kernel.org; linux-gpio@vger.kernel.org; linux- > kernel@vger.kernel.org; pankaj.dubey@samsung.com; > ravi.patel@samsung.com; gost.dev@samsung.com > Subject: Re: [PATCH v3] pinctrl: samsung: Add support for pull-up and pull- > down > > On 04/07/2024 06:26, Vishnu Reddy wrote: > > gpiolib framework has the implementation of setting up the PUD > > configuration for GPIO pins but there is no driver support. > > > > Add support to handle the PUD configuration request from the userspace > > in samsung pinctrl driver. > > > > Signed-off-by: Vishnu Reddy <vishnu.reddy@samsung.com> > > --- > > drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 15 ++++ > > drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 15 ++++ > > drivers/pinctrl/samsung/pinctrl-samsung.c | 80 > ++++++++++++++++++++ > > drivers/pinctrl/samsung/pinctrl-samsung.h | 24 ++++++ > > 4 files changed, 134 insertions(+) > > > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c > > b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c > > index 85ddf49a5188..426d1daacef2 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm.c > > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm.c > > @@ -40,6 +40,20 @@ static const struct samsung_pin_bank_type > bank_type_alive = { > > #define S5P_OTHERS_RET_MMC (1 << 29) > > #define S5P_OTHERS_RET_UART (1 << 28) > > > > +/* > > + * s5pv210_pud_value_init: initialize the drvdata pud_val with s5pv210 > pud values > > + * s5pv210_pull_disable: 0 > > + * s5pv210_pull_down_enable: 1 > > + * s5pv210_pull_up_enable: 2 > > Please use proper defines, e.g. S5P_PIN_PUD_PULL_DISABLE Ack, Will update. > > > > + */ > > +static void s5pv210_pud_value_init(struct samsung_pinctrl_drv_data > > +*drvdata) { > > + unsigned int i, *pud_val = drvdata->pud_val; > > + > > + for (i = 0; i < PUD_MAX; i++) > > + pud_val[i] = i; > > pud_val[PUD_PULL_DISABLE] = PROPER_DEFINE > > > +} > > + > > static void s5pv210_retention_disable(struct samsung_pinctrl_drv_data > > *drvdata) { > > void __iomem *clk_base = (void __iomem > > *)drvdata->retention_ctrl->priv; @@ -133,6 +147,7 @@ static const struct > samsung_pin_ctrl s5pv210_pin_ctrl[] __initconst = { > > .nr_banks = ARRAY_SIZE(s5pv210_pin_bank), > > .eint_gpio_init = exynos_eint_gpio_init, > > .eint_wkup_init = exynos_eint_wkup_init, > > + .pud_value_init = s5pv210_pud_value_init, > > .suspend = exynos_pinctrl_suspend, > > .resume = exynos_pinctrl_resume, > > .retention_data = &s5pv210_retention_data, > > diff --git a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c > > b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c > > index c5d92db4fdb1..cf57b0f16999 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c > > +++ b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c > > @@ -255,6 +255,20 @@ static int s3c64xx_irq_get_trigger(unsigned int > type) > > return trigger; > > } > > > > +/* > > + * s3c64xx_pud_value_init: initialize the drvdata pud_val with s3c64xx pud > values > > + * s3c64xx_pull_disable: 0 > > + * s3c64xx_pull_down_enable: 1 > > + * s3c64xx_pull_up_enable: 2 > > Use proper defines. Comments are not the place to define magic values. Ack, Will update. > > > > + */ > > +static void s3c64xx_pud_value_init(struct samsung_pinctrl_drv_data > > +*drvdata) { > > + unsigned int i, *pud_val = drvdata->pud_val; > > + > > + for (i = 0; i < PUD_MAX; i++) > > + pud_val[i] = i; > > +} > > + > > static void s3c64xx_irq_set_handler(struct irq_data *d, unsigned int > > type) { > > /* Edge- and level-triggered interrupts need different handlers */ > > @@ -797,6 +811,7 @@ static const struct samsung_pin_ctrl > s3c64xx_pin_ctrl[] __initconst = { > > .nr_banks = ARRAY_SIZE(s3c64xx_pin_banks0), > > .eint_gpio_init = s3c64xx_eint_gpio_init, > > .eint_wkup_init = s3c64xx_eint_eint0_init, > > + .pud_value_init = s3c64xx_pud_value_init, > > }, > > }; > > > > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c > > b/drivers/pinctrl/samsung/pinctrl-samsung.c > > index 623df65a5d6f..7d7e924c1fdb 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c > > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c > > @@ -997,6 +997,80 @@ static int samsung_pinctrl_unregister(struct > platform_device *pdev, > > return 0; > > } > > > > +/* > > + *samsung_pud_value_init: initialize the drvdata pud_val > > You use some weird style of comments. It's not kerneldoc, it's not a proper > neither useful comment. Either use proper kerneldoc (and test > it!) or drop duplicated function names. They are not helping. This applies > everywhere. Ack, Will update. > > > > + */ > > +static void samsung_pud_value_init(struct samsung_pinctrl_drv_data > > +*drvdata) { > > + unsigned int *pud_val = drvdata->pud_val; > > + > > + pud_val[PUD_PULL_DISABLE] = > PIN_PUD_PULL_UP_DOWN_DISABLE; > > + pud_val[PUD_PULL_DOWN] = PIN_PUD_PULL_DOWN_ENABLE; > > + pud_val[PUD_PULL_UP] = PIN_PUD_PULL_UP_ENABLE; } > > + > > +/* > > + * samsung_gpio_set_pud will enable or disable the pull-down and > > + * pull-up for the gpio pins in the PUD register. > > + */ > > +static void samsung_gpio_set_pud(struct gpio_chip *gc, unsigned int > offset, > > + unsigned int value) > > +{ > > + struct samsung_pin_bank *bank = gpiochip_get_data(gc); > > + const struct samsung_pin_bank_type *type = bank->type; > > + void __iomem *reg; > > + unsigned int data, mask; > > + > > + reg = bank->pctl_base + bank->pctl_offset; > > + data = readl(reg + type->reg_offset[PINCFG_TYPE_PUD]); > > + mask = (1 << type->fld_width[PINCFG_TYPE_PUD]) - 1; > > + data &= ~(mask << (offset * type->fld_width[PINCFG_TYPE_PUD])); > > + data |= value << (offset * type->fld_width[PINCFG_TYPE_PUD]); > > + writel(data, reg + type->reg_offset[PINCFG_TYPE_PUD]); > > +} > > + > > +/* > > + * samsung_gpio_set_config will identify the type of PUD config based > > + * on the gpiolib request to enable or disable the PUD configuration. > > + */ > > +static int samsung_gpio_set_config(struct gpio_chip *gc, unsigned int > offset, > > + unsigned long config) > > +{ > > + struct samsung_pin_bank *bank = gpiochip_get_data(gc); > > + struct samsung_pinctrl_drv_data *drvdata = bank->drvdata; > > + unsigned int value; > > + int ret = 0; > > + unsigned long flags; > > + > > + switch (pinconf_to_config_param(config)) { > > + case PIN_CONFIG_BIAS_DISABLE: > > + value = drvdata->pud_val[PUD_PULL_DISABLE]; > > + break; > > + case PIN_CONFIG_BIAS_PULL_DOWN: > > + value = drvdata->pud_val[PUD_PULL_DOWN]; > > + break; > > + case PIN_CONFIG_BIAS_PULL_UP: > > + value = drvdata->pud_val[PUD_PULL_UP]; > > + break; > > + default: > > + return -ENOTSUPP; > > + } > > + > > + ret = clk_enable(drvdata->pclk); > > + if (ret) { > > + dev_err(drvdata->dev, "failed to enable clock\n"); > > + return ret; > > + } > > + > > + raw_spin_lock_irqsave(&bank->slock, flags); > > + samsung_gpio_set_pud(gc, offset, value); > > + raw_spin_unlock_irqrestore(&bank->slock, flags); > > + > > + clk_disable(drvdata->pclk); > > + > > + return ret; > > +} > > + > > static const struct gpio_chip samsung_gpiolib_chip = { > > .request = gpiochip_generic_request, > > .free = gpiochip_generic_free, > > @@ -1006,6 +1080,7 @@ static const struct gpio_chip > samsung_gpiolib_chip = { > > .direction_output = samsung_gpio_direction_output, > > .to_irq = samsung_gpio_to_irq, > > .add_pin_ranges = samsung_add_pin_ranges, > > + .set_config = samsung_gpio_set_config, > > .owner = THIS_MODULE, > > }; > > > > @@ -1237,6 +1312,11 @@ static int samsung_pinctrl_probe(struct > platform_device *pdev) > > if (ctrl->eint_wkup_init) > > ctrl->eint_wkup_init(drvdata); > > > > + if (ctrl->pud_value_init) > > + ctrl->pud_value_init(drvdata); > > + else > > + samsung_pud_value_init(drvdata); > > + > > ret = samsung_gpiolib_register(pdev, drvdata); > > if (ret) > > goto err_unregister; > > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h > > b/drivers/pinctrl/samsung/pinctrl-samsung.h > > index d50ba6f07d5d..61384096b6d7 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-samsung.h > > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h > > @@ -61,6 +61,28 @@ enum pincfg_type { > > #define PIN_CON_FUNC_INPUT 0x0 > > #define PIN_CON_FUNC_OUTPUT 0x1 > > > > +/* > > + * Values for the pin PUD register. > > + */ > > +#define PIN_PUD_PULL_UP_DOWN_DISABLE 0x0 > > EXYNOS_PIN_PUD_PULL_DISABLE > > > +#define PIN_PUD_PULL_DOWN_ENABLE 0x1 > > EXYNOS_PIN_PID_PULL_DOWN > > > +#define PIN_PUD_PULL_UP_ENABLE 0x3 > > EXYNOS_PIN_PID_PULL_UP Ack, Will update. > > > + > > +/* > > + * enum pud_index: Index values to access the pud_val array in > > + * struct samsung_pinctrl_drv_data. > > + * @PUD_PULL_DISABLE: Index for value of pud disable > > + * @PUD_PULL_DOWN: Index for the value of pull down enable > > + * @PUD_PULL_UP: Index for the value of pull up enable > > + * @PUD_MAX: Maximun value of the index > > Typo: Maximum Ack, Will update. > > > + */ > > +enum pud_index { > > + PUD_PULL_DISABLE, > > + PUD_PULL_DOWN, > > + PUD_PULL_UP, > > + PUD_MAX, > > +}; > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-07-05 12:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20240704043358epcas5p282acd174113c7baf3f7a3472ba4c39ff@epcas5p2.samsung.com>
2024-07-04 4:26 ` [PATCH v3] pinctrl: samsung: Add support for pull-up and pull-down Vishnu Reddy
2024-07-05 9:38 ` Krzysztof Kozlowski
2024-07-05 11:59 ` Vishnu Reddy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).