2022-10-07 15:41:31

by Ryan.Wanner

[permalink] [raw]
Subject: [PATCH 0/2] pinctrl:at91-pio4:add support for pullup/down

From: Ryan Wanner <[email protected]>

This patch set adds support for pull up/down pinctl configuration.
The implementation is based off of other pinctl drivers that have
implemented line bias configurations.

The second patch addes a case for PIN_CONFIG_PERSIST_STATE
this shows up becuse the gpiod api passes this into the new config_set
function that was just implemented. Looking at other drivers like TI
driver, added the ENOTSUPP to the switch case for that state flag.

How this was tested was by using a gpio program that I created to test
configuration from userspace. This program was run in the
background using & then using gpioinfo function checked if the change
has been detected by the gpiod api. Then using devmem reading the
regester making sure that the correct bit was set. The registers where
checked before and during the program is being run, making sure the
change happens.

In the program Pin 127 would be passed into the test program. Before
the program was ran devmem for pin 127 config register. After
the progam is running in the background devmem for the same status
register is called, the result is showing the change in pinconfig.
The device used to test was the SAMA5D27_som1_ek.

Ryan Wanner (2):
pinctrl: at91-pio4: Add configuration to userspace
pinctrl: at91-pio4: Add persist state case in config

drivers/pinctrl/pinctrl-at91-pio4.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

--
2.34.1


2022-10-07 15:52:27

by Ryan.Wanner

[permalink] [raw]
Subject: [PATCH 2/2] pinctrl: at91-pio4: Add persist state case in config

From: Ryan Wanner <[email protected]>

Adding persist state case to atmel_conf_pin_config_group_set() function.
After adding configuration support for userspace gpiod api, there was an
extra flag PIN_CONFIG_PERSIST_STATE that was not passed in before.

Based on other drivers like TI drivers, added a switch case and return
ENOTSUPP in that case.

Signed-off-by: Ryan Wanner <[email protected]>
---
drivers/pinctrl/pinctrl-at91-pio4.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
index 13b77f1eb6e2..40f1b9397767 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -780,6 +780,8 @@ static int atmel_conf_pin_config_group_get(struct pinctrl_dev *pctldev,
return -EINVAL;
arg = (res & ATMEL_PIO_DRVSTR_MASK) >> ATMEL_PIO_DRVSTR_OFFSET;
break;
+ case PIN_CONFIG_PERSIST_STATE:
+ return -ENOTSUPP;
default:
return -ENOTSUPP;
}
@@ -888,6 +890,8 @@ static int atmel_conf_pin_config_group_set(struct pinctrl_dev *pctldev,
dev_warn(pctldev->dev, "drive strength not updated (incorrect value)\n");
}
break;
+ case PIN_CONFIG_PERSIST_STATE:
+ return -ENOTSUPP;
default:
dev_warn(pctldev->dev,
"unsupported configuration parameter: %u\n",
--
2.34.1

2022-10-07 16:08:37

by Ryan.Wanner

[permalink] [raw]
Subject: [PATCH 1/2] pinctrl: at91-pio4: Add configuration to userspace

From: Ryan Wanner <[email protected]>

Adding support for line bias flags that have been implented in gpio API.
There are functions in the gpiod library that can control line bias from
userspace this adds that functionality to this driver.

Adding .pin_config_set allows the driver's pin configuration to be
accessed from userspace. The general idea for this as been taken from
stm32, intel, and rockchip drivers that have userspace access for bias
flags.

Signed-off-by: Ryan Wanner <[email protected]>
---
drivers/pinctrl/pinctrl-at91-pio4.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
index 517f2a6330ad..13b77f1eb6e2 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -902,6 +902,25 @@ static int atmel_conf_pin_config_group_set(struct pinctrl_dev *pctldev,
return 0;
}

+static int atmel_conf_pin_config_set(struct pinctrl_dev *pctldev,
+ unsigned pin,
+ unsigned long *configs,
+ unsigned num_configs)
+{
+ struct atmel_group *grp = atmel_pctl_find_group_by_pin(pctldev, pin);
+
+ return atmel_conf_pin_config_group_set(pctldev, grp->pin, configs, num_configs);
+}
+
+static int atmel_conf_pin_config_get(struct pinctrl_dev *pctldev,
+ unsigned pin,
+ unsigned long *configs)
+{
+ struct atmel_group *grp = atmel_pctl_find_group_by_pin(pctldev, pin);
+
+ return atmel_conf_pin_config_group_get(pctldev, grp->pin, configs);
+}
+
static void atmel_conf_pin_config_dbg_show(struct pinctrl_dev *pctldev,
struct seq_file *s,
unsigned int pin_id)
@@ -949,6 +968,8 @@ static const struct pinconf_ops atmel_confops = {
.pin_config_group_get = atmel_conf_pin_config_group_get,
.pin_config_group_set = atmel_conf_pin_config_group_set,
.pin_config_dbg_show = atmel_conf_pin_config_dbg_show,
+ .pin_config_set = atmel_conf_pin_config_set,
+ .pin_config_get = atmel_conf_pin_config_get,
};

static struct pinctrl_desc atmel_pinctrl_desc = {
@@ -1139,6 +1160,7 @@ static int atmel_pinctrl_probe(struct platform_device *pdev)
atmel_pioctrl->gpio_chip->label = dev_name(dev);
atmel_pioctrl->gpio_chip->parent = dev;
atmel_pioctrl->gpio_chip->names = atmel_pioctrl->group_names;
+ atmel_pioctrl->gpio_chip->set_config = gpiochip_generic_config;

atmel_pioctrl->pm_wakeup_sources = devm_kcalloc(dev,
atmel_pioctrl->nbanks,
--
2.34.1

2022-10-10 09:25:03

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH 2/2] pinctrl: at91-pio4: Add persist state case in config

On 07/10/2022 at 17:16, [email protected] wrote:
> From: Ryan Wanner <[email protected]>
>
> Adding persist state case to atmel_conf_pin_config_group_set() function.
> After adding configuration support for userspace gpiod api, there was an
> extra flag PIN_CONFIG_PERSIST_STATE that was not passed in before.
>
> Based on other drivers like TI drivers, added a switch case and return
> ENOTSUPP in that case.
>
> Signed-off-by: Ryan Wanner <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

Thanks, best regards,
Nicolas

> ---
> drivers/pinctrl/pinctrl-at91-pio4.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
> index 13b77f1eb6e2..40f1b9397767 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -780,6 +780,8 @@ static int atmel_conf_pin_config_group_get(struct pinctrl_dev *pctldev,
> return -EINVAL;
> arg = (res & ATMEL_PIO_DRVSTR_MASK) >> ATMEL_PIO_DRVSTR_OFFSET;
> break;
> + case PIN_CONFIG_PERSIST_STATE:
> + return -ENOTSUPP;
> default:
> return -ENOTSUPP;
> }
> @@ -888,6 +890,8 @@ static int atmel_conf_pin_config_group_set(struct pinctrl_dev *pctldev,
> dev_warn(pctldev->dev, "drive strength not updated (incorrect value)\n");
> }
> break;
> + case PIN_CONFIG_PERSIST_STATE:
> + return -ENOTSUPP;
> default:
> dev_warn(pctldev->dev,
> "unsupported configuration parameter: %u\n",


--
Nicolas Ferre

2022-10-10 09:52:15

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH 1/2] pinctrl: at91-pio4: Add configuration to userspace

On 07/10/2022 at 17:16, [email protected] wrote:
> From: Ryan Wanner <[email protected]>
>
> Adding support for line bias flags that have been implented in gpio API.

Typo: implemented

> There are functions in the gpiod library that can control line bias from
> userspace this adds that functionality to this driver.
>
> Adding .pin_config_set allows the driver's pin configuration to be
> accessed from userspace. The general idea for this as been taken from
> stm32, intel, and rockchip drivers that have userspace access for bias
> flags.
>
> Signed-off-by: Ryan Wanner <[email protected]>

Tested-by: Nicolas Ferre <[email protected]> # on sama5d27 som1 ek
Acked-by: Nicolas Ferre <[email protected]>

Thanks, best regards,
Nicolas

> ---
> drivers/pinctrl/pinctrl-at91-pio4.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
> index 517f2a6330ad..13b77f1eb6e2 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -902,6 +902,25 @@ static int atmel_conf_pin_config_group_set(struct pinctrl_dev *pctldev,
> return 0;
> }
>
> +static int atmel_conf_pin_config_set(struct pinctrl_dev *pctldev,
> + unsigned pin,
> + unsigned long *configs,
> + unsigned num_configs)
> +{
> + struct atmel_group *grp = atmel_pctl_find_group_by_pin(pctldev, pin);
> +
> + return atmel_conf_pin_config_group_set(pctldev, grp->pin, configs, num_configs);
> +}
> +
> +static int atmel_conf_pin_config_get(struct pinctrl_dev *pctldev,
> + unsigned pin,
> + unsigned long *configs)
> +{
> + struct atmel_group *grp = atmel_pctl_find_group_by_pin(pctldev, pin);
> +
> + return atmel_conf_pin_config_group_get(pctldev, grp->pin, configs);
> +}
> +
> static void atmel_conf_pin_config_dbg_show(struct pinctrl_dev *pctldev,
> struct seq_file *s,
> unsigned int pin_id)
> @@ -949,6 +968,8 @@ static const struct pinconf_ops atmel_confops = {
> .pin_config_group_get = atmel_conf_pin_config_group_get,
> .pin_config_group_set = atmel_conf_pin_config_group_set,
> .pin_config_dbg_show = atmel_conf_pin_config_dbg_show,
> + .pin_config_set = atmel_conf_pin_config_set,
> + .pin_config_get = atmel_conf_pin_config_get,
> };
>
> static struct pinctrl_desc atmel_pinctrl_desc = {
> @@ -1139,6 +1160,7 @@ static int atmel_pinctrl_probe(struct platform_device *pdev)
> atmel_pioctrl->gpio_chip->label = dev_name(dev);
> atmel_pioctrl->gpio_chip->parent = dev;
> atmel_pioctrl->gpio_chip->names = atmel_pioctrl->group_names;
> + atmel_pioctrl->gpio_chip->set_config = gpiochip_generic_config;
>
> atmel_pioctrl->pm_wakeup_sources = devm_kcalloc(dev,
> atmel_pioctrl->nbanks,


--
Nicolas Ferre

2022-10-10 09:52:20

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH 0/2] pinctrl:at91-pio4:add support for pullup/down

On 07/10/2022 at 17:16, [email protected] wrote:
> From: Ryan Wanner <[email protected]>
>
> This patch set adds support for pull up/down pinctl configuration.
> The implementation is based off of other pinctl drivers that have
> implemented line bias configurations.
>
> The second patch addes a case for PIN_CONFIG_PERSIST_STATE
> this shows up becuse the gpiod api passes this into the new config_set
> function that was just implemented. Looking at other drivers like TI
> driver, added the ENOTSUPP to the switch case for that state flag.
>
> How this was tested was by using a gpio program that I created to test
> configuration from userspace. This program was run in the

Tested using the ioctl GPIO_V2_GET_LINE_IOCTL and with the configs flags:
GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN, GPIO_V2_LINE_FLAG_BIAS_PULL_UP and
GPIO_V2_LINE_FLAG_BIAS_DISABLED.

> background using & then using gpioinfo function checked if the change
> has been detected by the gpiod api. Then using devmem reading the
> regester making sure that the correct bit was set. The registers where
> checked before and during the program is being run, making sure the
> change happens.
>
> In the program Pin 127 would be passed into the test program. Before
> the program was ran devmem for pin 127 config register. After
> the progam is running in the background devmem for the same status
> register is called, the result is showing the change in pinconfig.
> The device used to test was the SAMA5D27_som1_ek.

Same here, on sama5d27 som1 ek board:
Tested-by: Nicolas Ferre <[email protected]>

Thanks, best regards,
Nicolas

>
> Ryan Wanner (2):
> pinctrl: at91-pio4: Add configuration to userspace
> pinctrl: at91-pio4: Add persist state case in config
>
> drivers/pinctrl/pinctrl-at91-pio4.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>


--
Nicolas Ferre

2022-10-17 10:08:01

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 0/2] pinctrl:at91-pio4:add support for pullup/down

On Fri, Oct 7, 2022 at 5:16 PM <[email protected]> wrote:

> From: Ryan Wanner <[email protected]>
>
> This patch set adds support for pull up/down pinctl configuration.
> The implementation is based off of other pinctl drivers that have
> implemented line bias configurations.
>
> The second patch addes a case for PIN_CONFIG_PERSIST_STATE
> this shows up becuse the gpiod api passes this into the new config_set
> function that was just implemented. Looking at other drivers like TI
> driver, added the ENOTSUPP to the switch case for that state flag.
>
> How this was tested was by using a gpio program that I created to test
> configuration from userspace. This program was run in the
> background using & then using gpioinfo function checked if the change
> has been detected by the gpiod api. Then using devmem reading the
> regester making sure that the correct bit was set. The registers where
> checked before and during the program is being run, making sure the
> change happens.
>
> In the program Pin 127 would be passed into the test program. Before
> the program was ran devmem for pin 127 config register. After
> the progam is running in the background devmem for the same status
> register is called, the result is showing the change in pinconfig.
> The device used to test was the SAMA5D27_som1_ek.
>
> Ryan Wanner (2):
> pinctrl: at91-pio4: Add configuration to userspace
> pinctrl: at91-pio4: Add persist state case in config

Patches applied!

Yours,
Linus Walleij