[PATCH RFC 0/3] Verify bias functionality for pinctrl_paris driver through new gpio test

[PATCH RFC 0/3] Verify bias functionality for pinctrl_paris driver through new gpio test

[Nícolas F. R. A. Prado]

This series was motivated by the regression fixed by 166bf8af9122
("pinctrl: mediatek: common-v2: Fix broken bias-disable for
PULL_PU_PD_RSEL_TYPE"). A bug was introduced in the pinctrl_paris driver
which prevented certain pins from having their bias configured.

Running this test on the mt8195-tomato platform with the test plan
included below[1] shows the test passing with the fix applied, but failing
without the fix:

With fix:
  $ ./gpio-setget-config.py
  TAP version 13
  # Using test plan file: ./google,tomato.yaml
  1..3
  ok 1 pinctrl_paris.34.pull-up
  ok 2 pinctrl_paris.34.pull-down
  ok 3 pinctrl_paris.34.disabled
  # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0

Without fix:
  $ ./gpio-setget-config.py
  TAP version 13
  # Using test plan file: ./google,tomato.yaml
  1..3
  # Bias doesn't match: Expected pull-up, read pull-down.
  not ok 1 pinctrl_paris.34.pull-up
  ok 2 pinctrl_paris.34.pull-down
  # Bias doesn't match: Expected disabled, read pull-down.
  not ok 3 pinctrl_paris.34.disabled
  # Totals: pass:1 fail:2 xfail:0 xpass:0 skip:0 error:0

In order to achieve this, the first patch exposes bias configuration
through the GPIO API in the pinctrl_paris driver, patch 2 extends the
gpio-mockup-cdev utility for use by patch 3, and patch 3 introduces a
new GPIO kselftest that takes a test plan in YAML, which can be tailored
per-platform to specify the configurations to test, and sets and gets
back each pin configuration to verify that they match and thus that the
driver is behaving as expected.

Since the GPIO uAPI only allows setting the pin configuration, getting
it back is done through pinconf-pins in the pinctrl debugfs folder.

The test currently only verifies bias but it would be easy to extend to
verify other pin configurations.

The test plan YAML file can be customized for each use-case and is
platform-dependant. For that reason, only an example is included in
patch 3 and the user is supposed to provide their test plan. That said,
the aim is to collect test plans for ease of use at [2].

[1] This is the test plan used for mt8195-tomato:

- label: "pinctrl_paris"
  tests:
  # Pin 34 has type MTK_PULL_PU_PD_RSEL_TYPE and is unused.
  # Setting bias to MTK_PULL_PU_PD_RSEL_TYPE pins was fixed by
  # 166bf8af9122 ("pinctrl: mediatek: common-v2: Fix broken bias-disable for PULL_PU_PD_RSEL_TYPE")
  - pin: 34
    bias: "pull-up"
  - pin: 34
    bias: "pull-down"
  - pin: 34
    bias: "disabled"

[2] https://github.com/kernelci/platform-test-parameters

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
Nícolas F. R. A. Prado (3):
      pinctrl: mediatek: paris: Expose more configurations to GPIO set_config
      selftest: gpio: Add wait flag to gpio-mockup-cdev
      selftest: gpio: Add a new set-get config test

 drivers/pinctrl/mediatek/pinctrl-paris.c           |  20 +--
 tools/testing/selftests/gpio/Makefile              |   2 +-
 tools/testing/selftests/gpio/gpio-mockup-cdev.c    |  14 +-
 .../gpio-set-get-config-example-test-plan.yaml     |  15 ++
 .../testing/selftests/gpio/gpio-set-get-config.py  | 183 +++++++++++++++++++++
 5 files changed, 220 insertions(+), 14 deletions(-)
---
base-commit: 6a7917c89f219f09b1d88d09f376000914a52763
change-id: 20240906-kselftest-gpio-set-get-config-6e5bb670c1a5

Best regards,
-- 
Nícolas F. R. A. Prado <nfraprado@collabora.com>

[PATCH RFC 1/3] pinctrl: mediatek: paris: Expose more configurations to GPIO set_config

[Nícolas F. R. A. Prado]

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(-)

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)

-- 
2.46.0

[PATCH RFC 2/3] selftest: gpio: Add wait flag to gpio-mockup-cdev

[Nícolas F. R. A. Prado]

Add a -w flag to the gpio-mockup-cdev utility that causes the program to
wait until a signal is received before exiting, even when its behavior
is to retrieve the GPIO value of the line. This allows using this
utility to keep a GPIO line configured even when in input mode, which
will be relied on in other tests.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 tools/testing/selftests/gpio/gpio-mockup-cdev.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/gpio/gpio-mockup-cdev.c b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
index d1640f44f8ac..f674dcafa60a 100644
--- a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
+++ b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
@@ -15,6 +15,7 @@
 #include <unistd.h>
 #include <sys/ioctl.h>
 #include <linux/gpio.h>
+#include <stdbool.h>
 
 #define CONSUMER	"gpio-mockup-cdev"
 
@@ -95,6 +96,7 @@ static void usage(char *prog)
 	printf("               (default is to leave bias unchanged):\n");
 	printf("        -l: set line active low (default is active high)\n");
 	printf("        -s: set line value (default is to get line value)\n");
+	printf("        -w: wait even in get mode\n");
 	printf("        -u: uAPI version to use (default is 2)\n");
 	exit(-1);
 }
@@ -120,13 +122,14 @@ int main(int argc, char *argv[])
 	unsigned int offset, val = 0, abiv;
 	uint32_t flags_v1;
 	uint64_t flags_v2;
+	bool wait = false;
 
 	abiv = 2;
 	ret = 0;
 	flags_v1 = GPIOHANDLE_REQUEST_INPUT;
 	flags_v2 = GPIO_V2_LINE_FLAG_INPUT;
 
-	while ((opt = getopt(argc, argv, "lb:s:u:")) != -1) {
+	while ((opt = getopt(argc, argv, "lb:s:u:w")) != -1) {
 		switch (opt) {
 		case 'l':
 			flags_v1 |= GPIOHANDLE_REQUEST_ACTIVE_LOW;
@@ -150,10 +153,14 @@ int main(int argc, char *argv[])
 			flags_v1 |= GPIOHANDLE_REQUEST_OUTPUT;
 			flags_v2 &= ~GPIO_V2_LINE_FLAG_INPUT;
 			flags_v2 |= GPIO_V2_LINE_FLAG_OUTPUT;
+			wait = true;
 			break;
 		case 'u':
 			abiv = atoi(optarg);
 			break;
+		case 'w':
+			wait = true;
+			break;
 		default:
 			usage(argv[0]);
 		}
@@ -183,9 +190,10 @@ int main(int argc, char *argv[])
 		return lfd;
 	}
 
-	if (flags_v2 & GPIO_V2_LINE_FLAG_OUTPUT) {
+	if (wait)
 		wait_signal();
-	} else {
+
+	if (flags_v2 & GPIO_V2_LINE_FLAG_INPUT) {
 		if (abiv == 1)
 			ret = get_value_v1(lfd);
 		else

-- 
2.46.0

[PATCH RFC 3/3] selftest: gpio: Add a new set-get config test

[Nícolas F. R. A. Prado]

Add a new kselftest that sets a configuration to a GPIO line and then
gets it back to verify that it was correctly carried out by the driver.

Setting a configuration is done through the GPIO uAPI, but retrieving it
is done through the debugfs interface since that is the only place where
it can be retrieved from userspace.

The test reads the test plan from a YAML file, which includes the chips
and pin settings to set and validate.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 tools/testing/selftests/gpio/Makefile              |   2 +-
 .../gpio-set-get-config-example-test-plan.yaml     |  15 ++
 .../testing/selftests/gpio/gpio-set-get-config.py  | 183 +++++++++++++++++++++
 3 files changed, 199 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index e0884390447d..bdfeb0c9aadd 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-TEST_PROGS := gpio-mockup.sh gpio-sim.sh
+TEST_PROGS := gpio-mockup.sh gpio-sim.sh gpio-set-get-config.py
 TEST_FILES := gpio-mockup-sysfs.sh
 TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
 CFLAGS += -O2 -g -Wall $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/gpio/gpio-set-get-config-example-test-plan.yaml b/tools/testing/selftests/gpio/gpio-set-get-config-example-test-plan.yaml
new file mode 100644
index 000000000000..3b749be3c8dc
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-set-get-config-example-test-plan.yaml
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Top-level contains a list of the GPIO chips that will be tested. Each one is
+# chosen based on the GPIO chip's info label.
+- label: "gpiochip_device_label"
+  # For each GPIO chip, multiple pin configurations can be tested, which are
+  # listed under 'tests'
+  tests:
+  # pin indicates the pin number to test
+  - pin: 34
+    # bias can be 'pull-up', 'pull-down', 'disabled'
+    bias: "pull-up"
+  - pin: 34
+    bias: "pull-down"
+  - pin: 34
+    bias: "disabled"
diff --git a/tools/testing/selftests/gpio/gpio-set-get-config.py b/tools/testing/selftests/gpio/gpio-set-get-config.py
new file mode 100755
index 000000000000..6f1444c8d46b
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-set-get-config.py
@@ -0,0 +1,183 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Collabora Ltd
+
+#
+# This test validates GPIO pin configuration. It takes a test plan in YAML (see
+# gpio-set-get-config-example-test-plan.yaml) and sets and gets back each pin
+# configuration described in the plan and checks that they match in order to
+# validate that they are being applied correctly.
+#
+# When the file name for the test plan is not provided through --test-plan, it
+# will be guessed based on the platform ID (DT compatible or DMI).
+#
+
+import time
+import os
+import sys
+import argparse
+import re
+import subprocess
+import glob
+import signal
+
+import yaml
+
+# Allow ksft module to be imported from different directory
+this_dir = os.path.dirname(os.path.realpath(__file__))
+sys.path.append(os.path.join(this_dir, "../kselftest/"))
+
+import ksft
+
+
+def config_pin(chip_dev, pin_config):
+    flags = []
+    if pin_config.get("bias"):
+        flags += f"-b {pin_config['bias']}".split()
+    flags += ["-w", chip_dev, str(pin_config["pin"])]
+    gpio_mockup_cdev_path = os.path.join(this_dir, "gpio-mockup-cdev")
+    return subprocess.Popen([gpio_mockup_cdev_path] + flags)
+
+
+def get_bias_debugfs(chip_debugfs_path, pin):
+    with open(os.path.join(chip_debugfs_path, "pinconf-pins")) as f:
+        for l in f:
+            m = re.match(rf"pin {pin}.*bias (?P<bias>(pull )?\w+)", l)
+            if m:
+                return m.group("bias")
+
+
+def check_config_pin(chip, chip_debugfs_dir, pin_config):
+    test_passed = True
+
+    if pin_config.get("bias"):
+        bias = get_bias_debugfs(chip_debugfs_dir, pin_config["pin"])
+        # Convert "pull up" / "pull down" to "pull-up" / "pull-down"
+        bias = bias.replace(" ", "-")
+        if bias != pin_config["bias"]:
+            ksft.print_msg(
+                f"Bias doesn't match: Expected {pin_config['bias']}, read {bias}."
+            )
+            test_passed = False
+
+    ksft.test_result(
+        test_passed,
+        f"{chip['label']}.{pin_config['pin']}.{pin_config['bias']}",
+    )
+
+
+def get_devfs_chip_file(chip_dict):
+    gpio_chip_info_path = os.path.join(this_dir, 'gpio-chip-info')
+    for f in glob.glob("/dev/gpiochip*"):
+        proc = subprocess.run(
+            f"{gpio_chip_info_path} {f} label".split(), capture_output=True, text=True
+        )
+        if proc.returncode:
+            ksft.print_msg(f"Error opening gpio device {f}: {proc.returncode}")
+            ksft.exit_fail()
+
+        if chip_dict["label"] in proc.stdout:
+            return f
+
+
+def get_debugfs_chip_dir(chip):
+    pinctrl_debugfs = "/sys/kernel/debug/pinctrl/"
+
+    for name in os.listdir(pinctrl_debugfs):
+        if chip["label"] in name:
+            return os.path.join(pinctrl_debugfs, name)
+
+
+def run_test(test_plan_filename):
+    ksft.print_msg(f"Using test plan file: {test_plan_filename}")
+
+    with open(test_plan_filename) as f:
+        plan = yaml.safe_load(f)
+
+    num_tests = 0
+    for chip in plan:
+        num_tests += len(chip["tests"])
+
+    ksft.set_plan(num_tests)
+
+    for chip in plan:
+        chip_dev = get_devfs_chip_file(chip)
+        if not chip_dev:
+            ksft.print_msg("Couldn't find /dev file for GPIO chip")
+            ksft.exit_fail()
+        chip_debugfs_dir = get_debugfs_chip_dir(chip)
+        if not chip_debugfs_dir:
+            ksft.print_msg("Couldn't find pinctrl folder in debugfs for GPIO chip")
+            ksft.exit_fail()
+        for pin_config in chip["tests"]:
+            proc = config_pin(chip_dev, pin_config)
+            time.sleep(0.1)  # Give driver some time to update pin
+            check_config_pin(chip, chip_debugfs_dir, pin_config)
+            proc.send_signal(signal.SIGTERM)
+            proc.wait()
+
+
+def get_possible_test_plan_filenames():
+    filenames = []
+
+    dt_board_compatible_file = "/proc/device-tree/compatible"
+    if os.path.exists(dt_board_compatible_file):
+        with open(dt_board_compatible_file) as f:
+            for line in f:
+                compatibles = [compat for compat in line.split("\0") if compat]
+                filenames.extend(compatibles)
+    else:
+        dmi_id_dir = "/sys/devices/virtual/dmi/id"
+        vendor_dmi_file = os.path.join(dmi_id_dir, "sys_vendor")
+        product_dmi_file = os.path.join(dmi_id_dir, "product_name")
+
+        with open(vendor_dmi_file) as f:
+            vendor = f.read().replace("\n", "")
+        with open(product_dmi_file) as f:
+            product = f.read().replace("\n", "")
+
+        filenames = [vendor + "," + product]
+
+    return filenames
+
+
+def get_test_plan_filename(test_plan_dir):
+    chosen_test_plan_filename = ""
+    full_test_plan_paths = [
+        os.path.join(test_plan_dir, f + ".yaml")
+        for f in get_possible_test_plan_filenames()
+    ]
+    for path in full_test_plan_paths:
+        if os.path.exists(path):
+            chosen_test_plan_filename = path
+            break
+
+    if not chosen_test_plan_filename:
+        tried_paths = ",".join(["'" + p + "'" for p in full_test_plan_paths])
+        ksft.print_msg(f"No matching test plan file found (tried {tried_paths})")
+        ksft.print_cnts()
+        sys.exit(4)
+
+    return chosen_test_plan_filename
+
+
+parser = argparse.ArgumentParser()
+parser.add_argument(
+    "--test-plan-dir", default=".", help="Directory containing the test plan files"
+)
+parser.add_argument("--test-plan", help="Test plan file to use")
+args = parser.parse_args()
+
+ksft.print_header()
+
+if args.test_plan:
+    test_plan_filename = os.path.join(args.test_plan_dir, args.test_plan)
+    if not os.path.exists(test_plan_filename):
+        ksft.print_msg(f"Test plan file not found: {test_plan_filename}")
+        ksft.exit_fail()
+else:
+    test_plan_filename = get_test_plan_filename(args.test_plan_dir)
+
+run_test(test_plan_filename)
+
+ksft.finished()

-- 
2.46.0

Re: [PATCH RFC 1/3] pinctrl: mediatek: paris: Expose more configurations to GPIO set_config

[AngeloGioacchino Del Regno]

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)
> 

Re: [PATCH RFC 1/3] pinctrl: mediatek: paris: Expose more configurations to GPIO set_config

[AngeloGioacchino Del Regno]

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)
>>
> 
> 

Re: [PATCH RFC 1/3] pinctrl: mediatek: paris: Expose more configurations to GPIO set_config

[Nícolas F. R. A. Prado]

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