| Message ID | 20240909-kselftest-gpio-set-get-config-v1-1-16a065afc3c1@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Verify bias functionality for pinctrl_paris driver through new gpio test | expand |
Il 09/09/24 20:37, Nícolas F. R. A. Prado ha scritto: > Currently the set_config callback in the gpio_chip registered by the > pinctrl_paris driver only supports PIN_CONFIG_INPUT_DEBOUNCE, despite [...] only supports operations configuring the input debounce parameter of the EINT controller and denies configuring params on the other AP GPIOs [...] (reword as needed) > many other configurations already being implemented and available > through the pinctrl API for configuration of pins by the Devicetree and > other drivers. > > Expose all configurations currently implemented through the GPIO API so > they can also be set from userspace, which is particularly useful to > allow testing them from userspace. > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > --- > drivers/pinctrl/mediatek/pinctrl-paris.c | 20 ++++++++++---------- You can do the same for pinctrl-moore too, it's trivial. Other than that, I agree about performing this change, as this may be useful for more than just testing. Cheers, Angelo > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c > index e12316c42698..668f8055a544 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-paris.c > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c > @@ -255,10 +255,9 @@ static int mtk_pinconf_get(struct pinctrl_dev *pctldev, > return err; > } > > -static int mtk_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, > +static int mtk_pinconf_set(struct mtk_pinctrl *hw, unsigned int pin, > enum pin_config_param param, u32 arg) > { > - struct mtk_pinctrl *hw = pinctrl_dev_get_drvdata(pctldev); > const struct mtk_pin_desc *desc; > int err = -ENOTSUPP; > u32 reg; > @@ -795,7 +794,7 @@ static int mtk_pconf_group_set(struct pinctrl_dev *pctldev, unsigned group, > int i, ret; > > for (i = 0; i < num_configs; i++) { > - ret = mtk_pinconf_set(pctldev, grp->pin, > + ret = mtk_pinconf_set(hw, grp->pin, > pinconf_to_config_param(configs[i]), > pinconf_to_config_argument(configs[i])); > if (ret < 0) > @@ -937,18 +936,19 @@ static int mtk_gpio_set_config(struct gpio_chip *chip, unsigned int offset, > { > struct mtk_pinctrl *hw = gpiochip_get_data(chip); > const struct mtk_pin_desc *desc; > - u32 debounce; > + enum pin_config_param param = pinconf_to_config_param(config); > + u32 arg = pinconf_to_config_argument(config); > > desc = (const struct mtk_pin_desc *)&hw->soc->pins[offset]; > > - if (!hw->eint || > - pinconf_to_config_param(config) != PIN_CONFIG_INPUT_DEBOUNCE || > - desc->eint.eint_n == EINT_NA) > - return -ENOTSUPP; > + if (param == PIN_CONFIG_INPUT_DEBOUNCE) { > + if (!hw->eint || desc->eint.eint_n == EINT_NA) > + return -ENOTSUPP; > > - debounce = pinconf_to_config_argument(config); > + return mtk_eint_set_debounce(hw->eint, desc->eint.eint_n, arg); > + } > > - return mtk_eint_set_debounce(hw->eint, desc->eint.eint_n, debounce); > + return mtk_pinconf_set(hw, offset, param, arg); > } > > static int mtk_build_gpiochip(struct mtk_pinctrl *hw) >
Il 11/09/24 12:10, AngeloGioacchino Del Regno ha scritto: > Il 09/09/24 20:37, Nícolas F. R. A. Prado ha scritto: >> Currently the set_config callback in the gpio_chip registered by the >> pinctrl_paris driver only supports PIN_CONFIG_INPUT_DEBOUNCE, despite > > [...] only supports operations configuring the input debounce parameter > of the EINT controller and denies configuring params on the other AP GPIOs [...] > > (reword as needed) > >> many other configurations already being implemented and available >> through the pinctrl API for configuration of pins by the Devicetree and >> other drivers. >> >> Expose all configurations currently implemented through the GPIO API so >> they can also be set from userspace, which is particularly useful to >> allow testing them from userspace. >> >> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> >> --- >> drivers/pinctrl/mediatek/pinctrl-paris.c | 20 ++++++++++---------- > > You can do the same for pinctrl-moore too, it's trivial. > > Other than that, I agree about performing this change, as this may be useful > for more than just testing. > Nicolas, please don't forget to respin this patch. Thanks, Angelo >> 1 file changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/ >> pinctrl-paris.c >> index e12316c42698..668f8055a544 100644 >> --- a/drivers/pinctrl/mediatek/pinctrl-paris.c >> +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c >> @@ -255,10 +255,9 @@ static int mtk_pinconf_get(struct pinctrl_dev *pctldev, >> return err; >> } >> -static int mtk_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, >> +static int mtk_pinconf_set(struct mtk_pinctrl *hw, unsigned int pin, >> enum pin_config_param param, u32 arg) >> { >> - struct mtk_pinctrl *hw = pinctrl_dev_get_drvdata(pctldev); >> const struct mtk_pin_desc *desc; >> int err = -ENOTSUPP; >> u32 reg; >> @@ -795,7 +794,7 @@ static int mtk_pconf_group_set(struct pinctrl_dev *pctldev, >> unsigned group, >> int i, ret; >> for (i = 0; i < num_configs; i++) { >> - ret = mtk_pinconf_set(pctldev, grp->pin, >> + ret = mtk_pinconf_set(hw, grp->pin, >> pinconf_to_config_param(configs[i]), >> pinconf_to_config_argument(configs[i])); >> if (ret < 0) >> @@ -937,18 +936,19 @@ static int mtk_gpio_set_config(struct gpio_chip *chip, >> unsigned int offset, >> { >> struct mtk_pinctrl *hw = gpiochip_get_data(chip); >> const struct mtk_pin_desc *desc; >> - u32 debounce; >> + enum pin_config_param param = pinconf_to_config_param(config); >> + u32 arg = pinconf_to_config_argument(config); >> desc = (const struct mtk_pin_desc *)&hw->soc->pins[offset]; >> - if (!hw->eint || >> - pinconf_to_config_param(config) != PIN_CONFIG_INPUT_DEBOUNCE || >> - desc->eint.eint_n == EINT_NA) >> - return -ENOTSUPP; >> + if (param == PIN_CONFIG_INPUT_DEBOUNCE) { >> + if (!hw->eint || desc->eint.eint_n == EINT_NA) >> + return -ENOTSUPP; >> - debounce = pinconf_to_config_argument(config); >> + return mtk_eint_set_debounce(hw->eint, desc->eint.eint_n, arg); >> + } >> - return mtk_eint_set_debounce(hw->eint, desc->eint.eint_n, debounce); >> + return mtk_pinconf_set(hw, offset, param, arg); >> } >> static int mtk_build_gpiochip(struct mtk_pinctrl *hw) >> > >
On Thu, Oct 24, 2024 at 05:17:05PM +0200, AngeloGioacchino Del Regno wrote: > Il 11/09/24 12:10, AngeloGioacchino Del Regno ha scritto: > > Il 09/09/24 20:37, Nícolas F. R. A. Prado ha scritto: > > > Currently the set_config callback in the gpio_chip registered by the > > > pinctrl_paris driver only supports PIN_CONFIG_INPUT_DEBOUNCE, despite > > > > [...] only supports operations configuring the input debounce parameter > > of the EINT controller and denies configuring params on the other AP GPIOs [...] > > > > (reword as needed) > > > > > many other configurations already being implemented and available > > > through the pinctrl API for configuration of pins by the Devicetree and > > > other drivers. > > > > > > Expose all configurations currently implemented through the GPIO API so > > > they can also be set from userspace, which is particularly useful to > > > allow testing them from userspace. > > > > > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > --- > > > drivers/pinctrl/mediatek/pinctrl-paris.c | 20 ++++++++++---------- > > > > You can do the same for pinctrl-moore too, it's trivial. > > > > Other than that, I agree about performing this change, as this may be useful > > for more than just testing. > > > > Nicolas, please don't forget to respin this patch. I was hoping to get some feedback on the test itself as well, particularly from Linus as the pinctrl maintainer, but it's also been a while so I'll send a v2 with the feedback here addressed. Thanks, Nícolas
diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c index e12316c42698..668f8055a544 100644 --- a/drivers/pinctrl/mediatek/pinctrl-paris.c +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c @@ -255,10 +255,9 @@ static int mtk_pinconf_get(struct pinctrl_dev *pctldev, return err; } -static int mtk_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, +static int mtk_pinconf_set(struct mtk_pinctrl *hw, unsigned int pin, enum pin_config_param param, u32 arg) { - struct mtk_pinctrl *hw = pinctrl_dev_get_drvdata(pctldev); const struct mtk_pin_desc *desc; int err = -ENOTSUPP; u32 reg; @@ -795,7 +794,7 @@ static int mtk_pconf_group_set(struct pinctrl_dev *pctldev, unsigned group, int i, ret; for (i = 0; i < num_configs; i++) { - ret = mtk_pinconf_set(pctldev, grp->pin, + ret = mtk_pinconf_set(hw, grp->pin, pinconf_to_config_param(configs[i]), pinconf_to_config_argument(configs[i])); if (ret < 0) @@ -937,18 +936,19 @@ static int mtk_gpio_set_config(struct gpio_chip *chip, unsigned int offset, { struct mtk_pinctrl *hw = gpiochip_get_data(chip); const struct mtk_pin_desc *desc; - u32 debounce; + enum pin_config_param param = pinconf_to_config_param(config); + u32 arg = pinconf_to_config_argument(config); desc = (const struct mtk_pin_desc *)&hw->soc->pins[offset]; - if (!hw->eint || - pinconf_to_config_param(config) != PIN_CONFIG_INPUT_DEBOUNCE || - desc->eint.eint_n == EINT_NA) - return -ENOTSUPP; + if (param == PIN_CONFIG_INPUT_DEBOUNCE) { + if (!hw->eint || desc->eint.eint_n == EINT_NA) + return -ENOTSUPP; - debounce = pinconf_to_config_argument(config); + return mtk_eint_set_debounce(hw->eint, desc->eint.eint_n, arg); + } - return mtk_eint_set_debounce(hw->eint, desc->eint.eint_n, debounce); + return mtk_pinconf_set(hw, offset, param, arg); } static int mtk_build_gpiochip(struct mtk_pinctrl *hw)
Currently the set_config callback in the gpio_chip registered by the pinctrl_paris driver only supports PIN_CONFIG_INPUT_DEBOUNCE, despite many other configurations already being implemented and available through the pinctrl API for configuration of pins by the Devicetree and other drivers. Expose all configurations currently implemented through the GPIO API so they can also be set from userspace, which is particularly useful to allow testing them from userspace. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- drivers/pinctrl/mediatek/pinctrl-paris.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)