[PATCH v1 1/3] pinctrl: cy8c95x0: Lock register accesses in cy8c95x0_set_mux()

[PATCH v1 1/3] pinctrl: cy8c95x0: Lock register accesses in cy8c95x0_set_mux()

[Andy Shevchenko]

It seems that cy8c95x0_set_mux() missed serialization of IO access.
And its implementation looks half-baked. Add locking to the function.

Fixes: e6cbbe42944d ("pinctrl: Add Cypress cy8c95x0 support")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-cy8c95x0.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 79f73d364f3f..75be06d29dc1 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1152,8 +1152,13 @@ static int cy8c95x0_set_mux(struct pinctrl_dev *pctldev, unsigned int selector,
 			    unsigned int group)
 {
 	struct cy8c95x0_pinctrl *chip = pinctrl_dev_get_drvdata(pctldev);
+	int ret;
 
-	return cy8c95x0_pinmux_cfg(chip, selector, group);
+	mutex_lock(&chip->i2c_lock);
+	ret = cy8c95x0_pinmux_cfg(chip, selector, group);
+	mutex_unlock(&chip->i2c_lock);
+
+	return ret;
 }
 
 static const struct pinmux_ops cy8c95x0_pmxops = {
-- 
2.35.1

[PATCH v1 2/3] pinctrl: cy8c95x0: Drop atomicity on operations on push_pull

[Andy Shevchenko]

The push_pull member is always accessed under the mutex, hence
no need to use atomic operations on it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-cy8c95x0.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 75be06d29dc1..367a9386dfb7 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -573,7 +573,8 @@ static int cy8c95x0_gpio_direction_input(struct gpio_chip *gc, unsigned int off)
 		ret = regmap_write_bits(chip->regmap, CY8C95X0_DRV_HIZ, bit, bit);
 		if (ret)
 			goto out;
-		clear_bit(off, chip->push_pull);
+
+		__clear_bit(off, chip->push_pull);
 	}
 
 out:
@@ -775,27 +776,27 @@ static int cy8c95x0_gpio_set_pincfg(struct cy8c95x0_pinctrl *chip,
 
 	switch (param) {
 	case PIN_CONFIG_BIAS_PULL_UP:
-		clear_bit(off, chip->push_pull);
+		__clear_bit(off, chip->push_pull);
 		reg = CY8C95X0_DRV_PU;
 		break;
 	case PIN_CONFIG_BIAS_PULL_DOWN:
-		clear_bit(off, chip->push_pull);
+		__clear_bit(off, chip->push_pull);
 		reg = CY8C95X0_DRV_PD;
 		break;
 	case PIN_CONFIG_BIAS_DISABLE:
-		clear_bit(off, chip->push_pull);
+		__clear_bit(off, chip->push_pull);
 		reg = CY8C95X0_DRV_HIZ;
 		break;
 	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
-		clear_bit(off, chip->push_pull);
+		__clear_bit(off, chip->push_pull);
 		reg = CY8C95X0_DRV_ODL;
 		break;
 	case PIN_CONFIG_DRIVE_OPEN_SOURCE:
-		clear_bit(off, chip->push_pull);
+		__clear_bit(off, chip->push_pull);
 		reg = CY8C95X0_DRV_ODH;
 		break;
 	case PIN_CONFIG_DRIVE_PUSH_PULL:
-		set_bit(off, chip->push_pull);
+		__set_bit(off, chip->push_pull);
 		reg = CY8C95X0_DRV_PP_FAST;
 		break;
 	case PIN_CONFIG_MODE_PWM:
-- 
2.35.1

[PATCH v1 3/3] pinctrl: cy8c95x0: Align function names in cy8c95x0_pmxops

[Andy Shevchenko]

Align the function names in the cy8c95x0_pmxops() to follow
the struct pinmux_ops members naming schema.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-cy8c95x0.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 367a9386dfb7..68509a2301b8 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1103,7 +1103,7 @@ static const struct pinctrl_ops cy8c95x0_pinctrl_ops = {
 	.pin_dbg_show = cy8c95x0_pin_dbg_show,
 };
 
-static const char *cy8c95x0_get_functions_name(struct pinctrl_dev *pctldev, unsigned int selector)
+static const char *cy8c95x0_get_function_name(struct pinctrl_dev *pctldev, unsigned int selector)
 {
 	return cy8c95x0_get_fname(selector);
 }
@@ -1113,9 +1113,9 @@ static int cy8c95x0_get_functions_count(struct pinctrl_dev *pctldev)
 	return 2;
 }
 
-static int cy8c95x0_get_groups(struct pinctrl_dev *pctldev, unsigned int selector,
-			       const char * const **groups,
-			       unsigned int * const num_groups)
+static int cy8c95x0_get_function_groups(struct pinctrl_dev *pctldev, unsigned int selector,
+					const char * const **groups,
+					unsigned int * const num_groups)
 {
 	struct cy8c95x0_pinctrl *chip = pinctrl_dev_get_drvdata(pctldev);
 
@@ -1164,8 +1164,8 @@ static int cy8c95x0_set_mux(struct pinctrl_dev *pctldev, unsigned int selector,
 
 static const struct pinmux_ops cy8c95x0_pmxops = {
 	.get_functions_count = cy8c95x0_get_functions_count,
-	.get_function_name = cy8c95x0_get_functions_name,
-	.get_function_groups = cy8c95x0_get_groups,
+	.get_function_name = cy8c95x0_get_function_name,
+	.get_function_groups = cy8c95x0_get_function_groups,
 	.set_mux = cy8c95x0_set_mux,
 	.strict = true,
 };
-- 
2.35.1

Re: [PATCH v1 1/3] pinctrl: cy8c95x0: Lock register accesses in cy8c95x0_set_mux()

[Linus Walleij]

On Fri, Sep 16, 2022 at 10:54 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> It seems that cy8c95x0_set_mux() missed serialization of IO access.
> And its implementation looks half-baked. Add locking to the function.
>
> Fixes: e6cbbe42944d ("pinctrl: Add Cypress cy8c95x0 support")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

All three patches are straight-forward and elegant so I just applied
them.

Yours,
Linus Walleij