[PATCH v1 0/8] pinctrl: cy8c95x0: Yet another cleanup series and a fix

[PATCH v1 0/8] pinctrl: cy8c95x0: Yet another cleanup series and a fix

[Andy Shevchenko]

While playing on with Intel Galileo board again, I found that I messed up a bit
with the driver. Here is a fix followed by a set of ad-hoc cleanups. Obviously
tested on the above mentioned board.

Andy Shevchenko (8):
  pinctrl: cy8c95x0: Don't miss reading the last bank registers
  pinctrl: cy8c95x0: Use devm_mutex_init() for mutex initialization
  pinctrl: cy8c95x0: remove duplicate error message
  pinctrl: cy8c95x0: Unify messages with help of dev_err_probe()
  pinctrl: cy8c95x0: Move driver data to the local variable in ->probe()
  pinctrl: cy8c95x0: Drop unused 'name' in struct cy8c95x0_pinctrl
  pinctrl: cy8c95x0: Eliminate fragile use of I²C ID table
  pinctrl: cy8c95x0: Gather ID tables in one place

 drivers/pinctrl/pinctrl-cy8c95x0.c | 97 ++++++++++++------------------
 1 file changed, 40 insertions(+), 57 deletions(-)

-- 
2.50.1

[PATCH v1 1/8] pinctrl: cy8c95x0: Don't miss reading the last bank registers

[Andy Shevchenko]

When code had been changed to use for_each_set_clump8(), it mistakenly
switched from chip->nport to chip->tpin since the cy8c9540 and cy8c9560
have a 4-pin gap. This, in particular, led to the missed read of
the last bank interrupt status register and hence missing interrupts
on those pins. Restore the upper limit in for_each_set_clump8() to take
into consideration that gap.

Fixes: 83e29a7a1fdf ("pinctrl: cy8c95x0; Switch to use for_each_set_clump8()")
Cc: stable@vger.kernel.org
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-cy8c95x0.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index a4b04bf6d081..5c055d344ac9 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -627,7 +627,7 @@ static int cy8c95x0_write_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
 	bitmap_scatter(tmask, mask, chip->map, MAX_LINE);
 	bitmap_scatter(tval, val, chip->map, MAX_LINE);
 
-	for_each_set_clump8(offset, bits, tmask, chip->tpin) {
+	for_each_set_clump8(offset, bits, tmask, chip->nport * BANK_SZ) {
 		unsigned int i = offset / 8;
 
 		write_val = bitmap_get_value8(tval, offset);
@@ -655,7 +655,7 @@ static int cy8c95x0_read_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
 	bitmap_scatter(tmask, mask, chip->map, MAX_LINE);
 	bitmap_scatter(tval, val, chip->map, MAX_LINE);
 
-	for_each_set_clump8(offset, bits, tmask, chip->tpin) {
+	for_each_set_clump8(offset, bits, tmask, chip->nport * BANK_SZ) {
 		unsigned int i = offset / 8;
 
 		ret = cy8c95x0_regmap_read_bits(chip, reg, i, bits, &read_val);
-- 
2.50.1

[PATCH v1 2/8] pinctrl: cy8c95x0: Use devm_mutex_init() for mutex initialization

[Andy Shevchenko]

Use devm_mutex_init() since it brings some benefits when
CONFIG_DEBUG_MUTEXES is enabled.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 5c055d344ac9..0647c7f94162 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1312,7 +1312,9 @@ static int cy8c95x0_irq_setup(struct cy8c95x0_pinctrl *chip, int irq)
 	DECLARE_BITMAP(pending_irqs, MAX_LINE);
 	int ret;
 
-	mutex_init(&chip->irq_lock);
+	ret = devm_mutex_init(chip->dev, &chip->irq_lock);
+	if (ret)
+		return ret;
 
 	bitmap_zero(pending_irqs, MAX_LINE);
 
@@ -1474,7 +1476,9 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	bitmap_fill(chip->map, MAX_LINE);
 	bitmap_clear(chip->map, 20, 4);
 
-	mutex_init(&chip->i2c_lock);
+	ret = devm_mutex_init(dev, &chip->i2c_lock);
+	if (ret)
+		return ret;
 
 	if (dmi_first_match(cy8c95x0_dmi_acpi_irq_info)) {
 		ret = cy8c95x0_acpi_get_irq(&client->dev);
-- 
2.50.1

[PATCH v1 3/8] pinctrl: cy8c95x0: remove duplicate error message

[Andy Shevchenko]

The pin control core is covered to report any error via message.
The devm_request_threaded_irq() already prints an error message.
Remove the duplicates.

While at it, drop the info message as the same information about
an IRQ in use can be retrieved differently.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 0647c7f94162..165ccbda035f 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1310,6 +1310,7 @@ static int cy8c95x0_irq_setup(struct cy8c95x0_pinctrl *chip, int irq)
 {
 	struct gpio_irq_chip *girq = &chip->gpio_chip.irq;
 	DECLARE_BITMAP(pending_irqs, MAX_LINE);
+	struct device *dev = chip->dev;
 	int ret;
 
 	ret = devm_mutex_init(chip->dev, &chip->irq_lock);
@@ -1338,17 +1339,9 @@ static int cy8c95x0_irq_setup(struct cy8c95x0_pinctrl *chip, int irq)
 	girq->handler = handle_simple_irq;
 	girq->threaded = true;
 
-	ret = devm_request_threaded_irq(chip->dev, irq,
-					NULL, cy8c95x0_irq_handler,
-					IRQF_ONESHOT | IRQF_SHARED,
-					dev_name(chip->dev), chip);
-	if (ret) {
-		dev_err(chip->dev, "failed to request irq %d\n", irq);
-		return ret;
-	}
-	dev_info(chip->dev, "Registered threaded IRQ\n");
-
-	return 0;
+	return devm_request_threaded_irq(dev, irq, NULL, cy8c95x0_irq_handler,
+					 IRQF_ONESHOT | IRQF_SHARED,
+					 dev_name(chip->dev), chip);
 }
 
 static int cy8c95x0_setup_pinctrl(struct cy8c95x0_pinctrl *chip)
@@ -1364,11 +1357,7 @@ static int cy8c95x0_setup_pinctrl(struct cy8c95x0_pinctrl *chip)
 	pd->owner = THIS_MODULE;
 
 	chip->pctldev = devm_pinctrl_register(chip->dev, pd, chip);
-	if (IS_ERR(chip->pctldev))
-		return dev_err_probe(chip->dev, PTR_ERR(chip->pctldev),
-			"can't register controller\n");
-
-	return 0;
+	return PTR_ERR_OR_ZERO(chip->pctldev);
 }
 
 static int cy8c95x0_detect(struct i2c_client *client,
-- 
2.50.1

[PATCH v1 4/8] pinctrl: cy8c95x0: Unify messages with help of dev_err_probe()

[Andy Shevchenko]

Unify error messages that might appear during probe phase by
switching to use dev_err_probe().

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 165ccbda035f..94a599486638 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1321,10 +1321,8 @@ static int cy8c95x0_irq_setup(struct cy8c95x0_pinctrl *chip, int irq)
 
 	/* Read IRQ status register to clear all pending interrupts */
 	ret = cy8c95x0_irq_pending(chip, pending_irqs);
-	if (ret) {
-		dev_err(chip->dev, "failed to clear irq status register\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to clear irq status register\n");
 
 	/* Mask all interrupts */
 	bitmap_fill(chip->irq_mask, MAX_LINE);
-- 
2.50.1

[PATCH v1 5/8] pinctrl: cy8c95x0: Move driver data to the local variable in ->probe()

[Andy Shevchenko]

For all these years of driver existence the driver_data has been used
only as a raw material for other fields in the struct cy8c95x0_pinctrl.
Move it from the structure to be just a local variable in ->probe().
Later, if ever need arises, we may reconsider that.

While at it, drop an unneeded validation and replace uintptr_t with plain
unsigned long which is more readable and works in the same way.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 94a599486638..4f804a9358b7 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -144,7 +144,6 @@ static const struct dmi_system_id cy8c95x0_dmi_acpi_irq_info[] = {
  * @map:            Mask used to compensate for Gport2 width
  * @nport:          Number of Gports in this chip
  * @gpio_chip:      gpiolib chip
- * @driver_data:    private driver data
  * @dev:            struct device
  * @pctldev:        pin controller device
  * @pinctrl_desc:   pin controller description
@@ -165,7 +164,6 @@ struct cy8c95x0_pinctrl {
 	DECLARE_BITMAP(map, MAX_LINE);
 	unsigned int nport;
 	struct gpio_chip gpio_chip;
-	unsigned long driver_data;
 	struct device *dev;
 	struct pinctrl_dev *pctldev;
 	struct pinctrl_desc pinctrl_desc;
@@ -1397,6 +1395,7 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	struct cy8c95x0_pinctrl *chip;
 	struct regmap_config regmap_conf;
 	struct regmap_range_cfg regmap_range_conf;
+	unsigned long driver_data;
 	int ret;
 
 	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
@@ -1406,11 +1405,9 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	chip->dev = dev;
 
 	/* Set the device type */
-	chip->driver_data = (uintptr_t)i2c_get_match_data(client);
-	if (!chip->driver_data)
-		return -ENODEV;
+	driver_data = (unsigned long)i2c_get_match_data(client);
 
-	chip->tpin = chip->driver_data & CY8C95X0_GPIO_MASK;
+	chip->tpin = driver_data & CY8C95X0_GPIO_MASK;
 	chip->nport = DIV_ROUND_UP(CY8C95X0_PIN_TO_OFFSET(chip->tpin), BANK_SZ);
 
 	memcpy(&regmap_range_conf, &cy8c95x0_ranges[0], sizeof(regmap_range_conf));
-- 
2.50.1

[PATCH v1 6/8] pinctrl: cy8c95x0: Drop unused 'name' in struct cy8c95x0_pinctrl

[Andy Shevchenko]

The 'name' is only assigned and never used. Drop it for good.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 4f804a9358b7..0bcc9ac19696 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -147,7 +147,6 @@ static const struct dmi_system_id cy8c95x0_dmi_acpi_irq_info[] = {
  * @dev:            struct device
  * @pctldev:        pin controller device
  * @pinctrl_desc:   pin controller description
- * @name:           Chip controller name
  * @tpin:           Total number of pins
  * @gpio_reset:     GPIO line handler that can reset the IC
  */
@@ -167,7 +166,6 @@ struct cy8c95x0_pinctrl {
 	struct device *dev;
 	struct pinctrl_dev *pctldev;
 	struct pinctrl_desc pinctrl_desc;
-	char name[32];
 	unsigned int tpin;
 	struct gpio_desc *gpio_reset;
 };
@@ -1414,15 +1412,12 @@ static int cy8c95x0_probe(struct i2c_client *client)
 
 	switch (chip->tpin) {
 	case 20:
-		strscpy(chip->name, cy8c95x0_id[0].name);
 		regmap_range_conf.range_max = CY8C95X0_VIRTUAL + 3 * MUXED_STRIDE - 1;
 		break;
 	case 40:
-		strscpy(chip->name, cy8c95x0_id[1].name);
 		regmap_range_conf.range_max = CY8C95X0_VIRTUAL + 6 * MUXED_STRIDE - 1;
 		break;
 	case 60:
-		strscpy(chip->name, cy8c95x0_id[2].name);
 		regmap_range_conf.range_max = CY8C95X0_VIRTUAL + 8 * MUXED_STRIDE - 1;
 		break;
 	default:
-- 
2.50.1

[PATCH v1 7/8] pinctrl: cy8c95x0: Eliminate fragile use of I²C ID table

[Andy Shevchenko]

The I²C ID table is a subject to new entries that may potentially
break the order of the existing ones. Avoid this by using string
literals for the chip naming. Note, linker will deduplicate same
string literals and use only a single copy, hence it won't be the
change in size in data section.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 0bcc9ac19696..1d3e0617b235 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1369,13 +1369,13 @@ static int cy8c95x0_detect(struct i2c_client *client,
 		return ret;
 	switch (ret & GENMASK(7, 4)) {
 	case 0x20:
-		name = cy8c95x0_id[0].name;
+		name = "cy8c9520";
 		break;
 	case 0x40:
-		name = cy8c95x0_id[1].name;
+		name = "cy8c9540";
 		break;
 	case 0x60:
-		name = cy8c95x0_id[2].name;
+		name = "cy8c9560";
 		break;
 	default:
 		return -ENODEV;
-- 
2.50.1

[PATCH v1 8/8] pinctrl: cy8c95x0: Gather ID tables in one place

[Andy Shevchenko]

We have three ID tables spread over the driver code. Move all of them
closer to the end of the file where the first user appears to be. With
that done, drop unneeded trailing commas.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 1d3e0617b235..0d295ebc33d1 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -72,24 +72,6 @@
 #define CY8C95X0_MUX_REGMAP_TO_OFFSET(x, p) \
 	(CY8C95X0_VIRTUAL + (x) - CY8C95X0_PORTSEL + (p) * MUXED_STRIDE)
 
-static const struct i2c_device_id cy8c95x0_id[] = {
-	{ "cy8c9520", 20, },
-	{ "cy8c9540", 40, },
-	{ "cy8c9560", 60, },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, cy8c95x0_id);
-
-#define OF_CY8C95X(__nrgpio) ((void *)(__nrgpio))
-
-static const struct of_device_id cy8c95x0_dt_ids[] = {
-	{ .compatible = "cypress,cy8c9520", .data = OF_CY8C95X(20), },
-	{ .compatible = "cypress,cy8c9540", .data = OF_CY8C95X(40), },
-	{ .compatible = "cypress,cy8c9560", .data = OF_CY8C95X(60), },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, cy8c95x0_dt_ids);
-
 static const struct acpi_gpio_params cy8c95x0_irq_gpios = { 0, 0, true };
 
 static const struct acpi_gpio_mapping cy8c95x0_acpi_irq_gpios[] = {
@@ -1478,8 +1460,26 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	return cy8c95x0_setup_gpiochip(chip);
 }
 
+static const struct i2c_device_id cy8c95x0_id[] = {
+	{ "cy8c9520", 20 },
+	{ "cy8c9540", 40 },
+	{ "cy8c9560", 60 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, cy8c95x0_id);
+
+#define OF_CY8C95X(__nrgpio) ((void *)(__nrgpio))
+
+static const struct of_device_id cy8c95x0_dt_ids[] = {
+	{ .compatible = "cypress,cy8c9520", .data = OF_CY8C95X(20) },
+	{ .compatible = "cypress,cy8c9540", .data = OF_CY8C95X(40) },
+	{ .compatible = "cypress,cy8c9560", .data = OF_CY8C95X(60) },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cy8c95x0_dt_ids);
+
 static const struct acpi_device_id cy8c95x0_acpi_ids[] = {
-	{ "INT3490", 40, },
+	{ "INT3490", 40 },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, cy8c95x0_acpi_ids);
-- 
2.50.1

Re: [PATCH v1 0/8] pinctrl: cy8c95x0: Yet another cleanup series and a fix

[Linus Walleij]

On Mon, Feb 23, 2026 at 7:09 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> While playing on with Intel Galileo board again, I found that I messed up a bit
> with the driver. Here is a fix followed by a set of ad-hoc cleanups. Obviously
> tested on the above mentioned board.
>
> Andy Shevchenko (8):
>   pinctrl: cy8c95x0: Don't miss reading the last bank registers

This patch applied for fixes!

> pinctrl: cy8c95x0: Use devm_mutex_init() for mutex initialization
> pinctrl: cy8c95x0: remove duplicate error message
> pinctrl: cy8c95x0: Unify messages with help of dev_err_probe()
> pinctrl: cy8c95x0: Move driver data to the local variable in ->probe()
> pinctrl: cy8c95x0: Drop unused 'name' in struct cy8c95x0_pinctrl
> pinctrl: cy8c95x0: Eliminate fragile use of I²C ID table
> pinctrl: cy8c95x0: Gather ID tables in one place

The rest 7 for next.

Yours,
Linus Walleij