[PATCH v1 01/17] pinctrl: cy8c95x0: make irq_chip immutable

[PATCH v1 01/17] pinctrl: cy8c95x0: make irq_chip immutable

[Andy Shevchenko]

Since recently, the kernel is nagging about mutable irq_chips:

   "not an immutable chip, please consider fixing it!"

Drop the unneeded copy, flag it as IRQCHIP_IMMUTABLE, add the new
helper functions and call the appropriate gpiolib functions.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index a29df0920f4f..8bb01f852c54 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -90,7 +90,6 @@ MODULE_DEVICE_TABLE(of, cy8c95x0_dt_ids);
  * @irq_trig_high:  I/O bits affected by a high voltage level
  * @push_pull:      I/O bits configured as push pull driver
  * @shiftmask:      Mask used to compensate for Gport2 width
- * @irq_chip:       IRQ chip configuration
  * @nport:          Number of Gports in this chip
  * @gpio_chip:      gpiolib chip
  * @driver_data:    private driver data
@@ -112,7 +111,6 @@ struct cy8c95x0_pinctrl {
 	DECLARE_BITMAP(irq_trig_high, MAX_LINE);
 	DECLARE_BITMAP(push_pull, MAX_LINE);
 	DECLARE_BITMAP(shiftmask, MAX_LINE);
-	struct irq_chip irq_chip;
 	int nport;
 	struct gpio_chip gpio_chip;
 	unsigned long driver_data;
@@ -844,16 +842,20 @@ static void cy8c95x0_irq_mask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct cy8c95x0_pinctrl *chip = gpiochip_get_data(gc);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 
-	set_bit(irqd_to_hwirq(d), chip->irq_mask);
+	set_bit(hwirq, chip->irq_mask);
+	gpiochip_disable_irq(gc, hwirq);
 }
 
 static void cy8c95x0_irq_unmask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct cy8c95x0_pinctrl *chip = gpiochip_get_data(gc);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 
-	clear_bit(irqd_to_hwirq(d), chip->irq_mask);
+	gpiochip_enable_irq(gc, hwirq);
+	clear_bit(hwirq, chip->irq_mask);
 }
 
 static void cy8c95x0_irq_bus_lock(struct irq_data *d)
@@ -931,6 +933,18 @@ static void cy8c95x0_irq_shutdown(struct irq_data *d)
 	clear_bit(hwirq, chip->irq_trig_high);
 }
 
+static const struct irq_chip cy8c95x0_irqchip = {
+	.name = "cy8c95x0-irq",
+	.irq_mask = cy8c95x0_irq_mask,
+	.irq_unmask = cy8c95x0_irq_unmask,
+	.irq_bus_lock = cy8c95x0_irq_bus_lock,
+	.irq_bus_sync_unlock = cy8c95x0_irq_bus_sync_unlock,
+	.irq_set_type = cy8c95x0_irq_set_type,
+	.irq_shutdown = cy8c95x0_irq_shutdown,
+	.flags = IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
 static bool cy8c95x0_irq_pending(struct cy8c95x0_pinctrl *chip, unsigned long *pending)
 {
 	DECLARE_BITMAP(ones, MAX_LINE);
@@ -1136,7 +1150,6 @@ static const struct pinconf_ops cy8c95x0_pinconf_ops = {
 
 static int cy8c95x0_irq_setup(struct cy8c95x0_pinctrl *chip, int irq)
 {
-	struct irq_chip *irq_chip = &chip->irq_chip;
 	struct gpio_irq_chip *girq = &chip->gpio_chip.irq;
 	DECLARE_BITMAP(pending_irqs, MAX_LINE);
 	int ret;
@@ -1155,15 +1168,8 @@ static int cy8c95x0_irq_setup(struct cy8c95x0_pinctrl *chip, int irq)
 	/* Mask all interrupts */
 	bitmap_fill(chip->irq_mask, MAX_LINE);
 
-	irq_chip->name = devm_kasprintf(chip->dev, GFP_KERNEL, "%s-irq", chip->name);
-	irq_chip->irq_mask = cy8c95x0_irq_mask;
-	irq_chip->irq_unmask = cy8c95x0_irq_unmask;
-	irq_chip->irq_bus_lock = cy8c95x0_irq_bus_lock;
-	irq_chip->irq_bus_sync_unlock = cy8c95x0_irq_bus_sync_unlock;
-	irq_chip->irq_set_type = cy8c95x0_irq_set_type;
-	irq_chip->irq_shutdown = cy8c95x0_irq_shutdown;
+	gpio_irq_chip_set_chip(girq, &cy8c95x0_irqchip);
 
-	girq->chip = irq_chip;
 	/* This will let us handle the parent IRQ in the driver */
 	girq->parent_handler = NULL;
 	girq->num_parents = 0;
-- 
2.35.1

[PATCH v1 02/17] pinctrl: cy8c95x0: Allow IRQ chip core to handle numbering

[Andy Shevchenko]

No need to assign first line number for IRQ chip.
Let IRQ core to decide.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 8bb01f852c54..521acfdeef38 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1177,7 +1177,6 @@ static int cy8c95x0_irq_setup(struct cy8c95x0_pinctrl *chip, int irq)
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_simple_irq;
 	girq->threaded = true;
-	girq->first = 0;
 
 	ret = devm_request_threaded_irq(chip->dev, irq,
 					NULL, cy8c95x0_irq_handler,
-- 
2.35.1

[PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Andy Shevchenko]

It's unclear why many of static registers were marked as volatile.
They are pretty much bidirectional and static in a sense that
written value is kept there until a new write or chip reset.
Drop those registers from the list to allow them to be cached.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 521acfdeef38..5ef772d29a36 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -303,17 +303,6 @@ static bool cy8c95x0_volatile_register(struct device *dev, unsigned int reg)
 	switch (reg) {
 	case CY8C95X0_INPUT_(0) ... CY8C95X0_INPUT_(7):
 	case CY8C95X0_INTSTATUS_(0) ... CY8C95X0_INTSTATUS_(7):
-	case CY8C95X0_INTMASK:
-	case CY8C95X0_INVERT:
-	case CY8C95X0_PWMSEL:
-	case CY8C95X0_DIRECTION:
-	case CY8C95X0_DRV_PU:
-	case CY8C95X0_DRV_PD:
-	case CY8C95X0_DRV_ODH:
-	case CY8C95X0_DRV_ODL:
-	case CY8C95X0_DRV_PP_FAST:
-	case CY8C95X0_DRV_PP_SLOW:
-	case CY8C95X0_DRV_HIZ:
 		return true;
 	}
 
-- 
2.35.1

[PATCH v1 04/17] pinctrl: cy8c95x0: Fix return value in cy8c95x0_detect()

[Andy Shevchenko]

It's an obvious typo in never tested piece of code that
successful detection shouldn't fail. Fix that.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 5ef772d29a36..f016c283af57 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1255,7 +1255,7 @@ static int cy8c95x0_detect(struct i2c_client *client,
 	dev_info(&client->dev, "Found a %s chip at 0x%02x.\n", name, client->addr);
 	strscpy(info->type, name, I2C_NAME_SIZE);
 
-	return -ENODEV;
+	return 0;
 }
 
 static int cy8c95x0_probe(struct i2c_client *client)
-- 
2.35.1

[PATCH v1 05/17] pinctrl: cy8c95x0: Fix pin control name to enable more than one

[Andy Shevchenko]

The Cypress GPIO expander is an I²C discrete component. Hence
the platform may contain more than one of a such. Currently
this has limitations in the driver due to same name used for
all chips of a type. Replace this with device instance specific
name.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index f016c283af57..a05fbf818bf2 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1188,8 +1188,7 @@ static int cy8c95x0_setup_pinctrl(struct cy8c95x0_pinctrl *chip)
 	pd->confops = &cy8c95x0_pinconf_ops;
 	pd->pmxops = &cy8c95x0_pmxops;
 	pd->npins = chip->gpio_chip.ngpio;
-	pd->name = devm_kasprintf(chip->dev, GFP_KERNEL, "pinctrl-%s",
-				  chip->name);
+	pd->name = dev_name(chip->dev);
 	pd->pins = cy8c9560_pins;
 	pd->npins = chip->tpin;
 	pd->owner = THIS_MODULE;
-- 
2.35.1

[PATCH v1 06/17] pinctrl: cy8c95x0: Drop unneeded npins assignment

[Andy Shevchenko]

The npins field is assigned twice. Remove the first occurrence.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index a05fbf818bf2..eac8b073e19f 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1187,7 +1187,6 @@ static int cy8c95x0_setup_pinctrl(struct cy8c95x0_pinctrl *chip)
 	pd->pctlops = &cy8c95x0_pinctrl_ops;
 	pd->confops = &cy8c95x0_pinconf_ops;
 	pd->pmxops = &cy8c95x0_pmxops;
-	pd->npins = chip->gpio_chip.ngpio;
 	pd->name = dev_name(chip->dev);
 	pd->pins = cy8c9560_pins;
 	pd->npins = chip->tpin;
-- 
2.35.1

[PATCH v1 07/17] pinctrl: cy8c95x0: Enable GPIO range

[Andy Shevchenko]

Since it's a pin control, GPIO counterpart needs to know the mapping
between pin numbering and GPIO numbering. Enable this by calling
gpiochip_add_pin_range() at the chip addition time.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index eac8b073e19f..f09311d2d3fa 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -801,7 +801,20 @@ static void cy8c95x0_gpio_set_multiple(struct gpio_chip *gc,
 	cy8c95x0_write_regs_mask(chip, CY8C95X0_OUTPUT, bits, mask);
 }
 
-static int cy8c95x0_setup_gpiochip(struct cy8c95x0_pinctrl *chip, int ngpio)
+static int cy8c95x0_add_pin_ranges(struct gpio_chip *gc)
+{
+	struct cy8c95x0_pinctrl *chip = gpiochip_get_data(gc);
+	struct device *dev = chip->dev;
+	int ret;
+
+	ret = gpiochip_add_pin_range(gc, dev_name(dev), 0, 0, chip->tpin);
+	if (ret)
+		dev_err(dev, "failed to add GPIO pin range\n");
+
+	return ret;
+}
+
+static int cy8c95x0_setup_gpiochip(struct cy8c95x0_pinctrl *chip)
 {
 	struct gpio_chip *gc = &chip->gpio_chip;
 
@@ -814,9 +827,10 @@ static int cy8c95x0_setup_gpiochip(struct cy8c95x0_pinctrl *chip, int ngpio)
 	gc->set_multiple = cy8c95x0_gpio_set_multiple;
 	gc->set_config = cy8c95x0_gpio_set_config;
 	gc->can_sleep = true;
+	gc->add_pin_ranges = cy8c95x0_add_pin_ranges;
 
 	gc->base = -1;
-	gc->ngpio = ngpio;
+	gc->ngpio = chip->tpin;
 
 	gc->parent = chip->dev;
 	gc->owner = THIS_MODULE;
@@ -1328,11 +1342,11 @@ static int cy8c95x0_probe(struct i2c_client *client)
 			goto err_exit;
 	}
 
-	ret = cy8c95x0_setup_gpiochip(chip, chip->tpin);
+	ret = cy8c95x0_setup_pinctrl(chip);
 	if (ret)
 		goto err_exit;
 
-	ret = cy8c95x0_setup_pinctrl(chip);
+	ret = cy8c95x0_setup_gpiochip(chip);
 	if (ret)
 		goto err_exit;
 
-- 
2.35.1

[PATCH v1 08/17] pinctrl: cy8c95x0: Remove device initialization

[Andy Shevchenko]

The Cypress CY8C95x0 chips have an internal EEPROM that defines
initial configuration. It might be that bootloader or other
entity wrote the platform related setup into it. Don't override
it in the driver.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index f09311d2d3fa..b09f9485e57d 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1213,30 +1213,6 @@ static int cy8c95x0_setup_pinctrl(struct cy8c95x0_pinctrl *chip)
 	return 0;
 }
 
-static int device_cy8c95x0_init(struct cy8c95x0_pinctrl *chip)
-{
-	DECLARE_BITMAP(ones, MAX_LINE);
-	DECLARE_BITMAP(zeros, MAX_LINE);
-	int ret;
-
-	/* Set all pins to input. This is the POR default. */
-	bitmap_fill(ones, MAX_LINE);
-	ret = cy8c95x0_write_regs_mask(chip, CY8C95X0_DIRECTION, ones, ones);
-	if (ret) {
-		dev_err(chip->dev, "Failed to set pins to input\n");
-		return ret;
-	}
-
-	bitmap_zero(zeros, MAX_LINE);
-	ret = cy8c95x0_write_regs_mask(chip, CY8C95X0_INVERT, zeros, ones);
-	if (ret) {
-		dev_err(chip->dev, "Failed to set polarity inversion\n");
-		return ret;
-	}
-
-	return 0;
-}
-
 static int cy8c95x0_detect(struct i2c_client *client,
 			   struct i2c_board_info *info)
 {
@@ -1332,10 +1308,6 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	bitmap_set(chip->shiftmask, 0, 20);
 	mutex_init(&chip->i2c_lock);
 
-	ret = device_cy8c95x0_init(chip);
-	if (ret)
-		goto err_exit;
-
 	if (client->irq) {
 		ret = cy8c95x0_irq_setup(chip, client->irq);
 		if (ret)
-- 
2.35.1

[PATCH v1 09/17] pinctrl: cy8c95x0: Remove useless conditionals

[Andy Shevchenko]

The pin control framework checks pin boundaries before calling
the respective driver's callbacks. Hence no need to check for
pin boundaries, the respective conditionals won't be ever true.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index b09f9485e57d..97da22016cce 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1029,14 +1029,6 @@ static int cy8c95x0_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
 					   const unsigned int **pins,
 					   unsigned int *num_pins)
 {
-	struct cy8c95x0_pinctrl *chip = pinctrl_dev_get_drvdata(pctldev);
-
-	if (group >= chip->tpin) {
-		*pins = NULL;
-		*num_pins = 0;
-		return 0;
-	}
-
 	*pins = &cy8c9560_pins[group].number;
 	*num_pins = 1;
 	return 0;
@@ -1104,9 +1096,6 @@ static int cy8c95x0_set_mux(struct pinctrl_dev *pctldev, unsigned int selector,
 {
 	struct cy8c95x0_pinctrl *chip = pinctrl_dev_get_drvdata(pctldev);
 
-	if (group >= chip->tpin)
-		return -EINVAL;
-
 	return cy8c95x0_pinmux_cfg(chip, selector, group);
 }
 
@@ -1133,9 +1122,6 @@ static int cy8c95x0_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
 	int ret = 0;
 	int i;
 
-	if (WARN_ON(pin >= chip->tpin))
-		return -EINVAL;
-
 	for (i = 0; i < num_configs; i++) {
 		ret = cy8c95x0_gpio_set_pincfg(chip, pin, configs[i]);
 		if (ret)
-- 
2.35.1

[PATCH v1 10/17] pinctrl: cy8c95x0: Remove custom ->set_config()

[Andy Shevchenko]

Since we have pin configuration getter and setter provided,
there is no need to duplicate that in the custom ->set_config().
Instead, switch to gpiochip_generic_config().

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 97da22016cce..55bd48c9d020 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -761,30 +761,6 @@ static int cy8c95x0_gpio_set_pincfg(struct cy8c95x0_pinctrl *chip,
 	return ret;
 }
 
-static int cy8c95x0_gpio_set_config(struct gpio_chip *gc, unsigned int offset,
-				    unsigned long config)
-{
-	struct cy8c95x0_pinctrl *chip = gpiochip_get_data(gc);
-	unsigned long arg = pinconf_to_config_argument(config);
-
-	switch (pinconf_to_config_param(config)) {
-	case PIN_CONFIG_INPUT_ENABLE:
-		return cy8c95x0_gpio_direction_input(gc, offset);
-	case PIN_CONFIG_OUTPUT:
-		return cy8c95x0_gpio_direction_output(gc, offset, arg);
-	case PIN_CONFIG_MODE_PWM:
-	case PIN_CONFIG_BIAS_PULL_UP:
-	case PIN_CONFIG_BIAS_PULL_DOWN:
-	case PIN_CONFIG_BIAS_DISABLE:
-	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
-	case PIN_CONFIG_DRIVE_OPEN_SOURCE:
-	case PIN_CONFIG_DRIVE_PUSH_PULL:
-		return cy8c95x0_gpio_set_pincfg(chip, offset, config);
-	default:
-		return -ENOTSUPP;
-	}
-}
-
 static int cy8c95x0_gpio_get_multiple(struct gpio_chip *gc,
 				      unsigned long *mask, unsigned long *bits)
 {
@@ -825,7 +801,7 @@ static int cy8c95x0_setup_gpiochip(struct cy8c95x0_pinctrl *chip)
 	gc->get_direction = cy8c95x0_gpio_get_direction;
 	gc->get_multiple = cy8c95x0_gpio_get_multiple;
 	gc->set_multiple = cy8c95x0_gpio_set_multiple;
-	gc->set_config = cy8c95x0_gpio_set_config;
+	gc->set_config = gpiochip_generic_config,
 	gc->can_sleep = true;
 	gc->add_pin_ranges = cy8c95x0_add_pin_ranges;
 
-- 
2.35.1

[PATCH v1 11/17] pinctrl: cy8c95x0: Use 'default' in all switch-cases

[Andy Shevchenko]

Move the default values to the 'default' case in the switches.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 55bd48c9d020..6fe442b44cab 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -279,9 +279,9 @@ static bool cy8c95x0_readable_register(struct device *dev, unsigned int reg)
 	switch (reg) {
 	case 0x24 ... 0x27:
 		return false;
+	default:
+		return true;
 	}
-
-	return true;
 }
 
 static bool cy8c95x0_writeable_register(struct device *dev, unsigned int reg)
@@ -293,9 +293,9 @@ static bool cy8c95x0_writeable_register(struct device *dev, unsigned int reg)
 		return false;
 	case 0x24 ... 0x27:
 		return false;
+	default:
+		return true;
 	}
-
-	return true;
 }
 
 static bool cy8c95x0_volatile_register(struct device *dev, unsigned int reg)
@@ -304,9 +304,9 @@ static bool cy8c95x0_volatile_register(struct device *dev, unsigned int reg)
 	case CY8C95X0_INPUT_(0) ... CY8C95X0_INPUT_(7):
 	case CY8C95X0_INTSTATUS_(0) ... CY8C95X0_INTSTATUS_(7):
 		return true;
+	default:
+		return false;
 	}
-
-	return false;
 }
 
 static bool cy8c95x0_precious_register(struct device *dev, unsigned int reg)
@@ -314,9 +314,9 @@ static bool cy8c95x0_precious_register(struct device *dev, unsigned int reg)
 	switch (reg) {
 	case CY8C95X0_INTSTATUS_(0) ... CY8C95X0_INTSTATUS_(7):
 		return true;
+	default:
+		return false;
 	}
-
-	return false;
 }
 
 static const struct reg_default cy8c95x0_reg_defaults[] = {
@@ -1244,6 +1244,8 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	case 60:
 		strscpy(chip->name, cy8c95x0_id[2].name, I2C_NAME_SIZE);
 		break;
+	default:
+		return -ENODEV;
 	}
 
 	reg = devm_regulator_get(&client->dev, "vdd");
-- 
2.35.1

[PATCH v1 12/17] pinctrl: cy8c95x0: Implement ->pin_dbg_show()

[Andy Shevchenko]

The introduced callback ->pin_dbg_show() is useful for debugging.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 6fe442b44cab..ea81b5ae27a6 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1010,25 +1010,49 @@ static int cy8c95x0_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
+static const char *cy8c95x0_get_fname(unsigned int selector)
+{
+	if (selector == 0)
+		return "gpio";
+	else
+		return "pwm";
+}
+
+static void cy8c95x0_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+				  unsigned int pin)
+{
+	struct cy8c95x0_pinctrl *chip = pinctrl_dev_get_drvdata(pctldev);
+	DECLARE_BITMAP(mask, MAX_LINE);
+	DECLARE_BITMAP(pwm, MAX_LINE);
+
+	bitmap_zero(mask, MAX_LINE);
+	__set_bit(pin, mask);
+
+	if (cy8c95x0_read_regs_mask(chip, CY8C95X0_PWMSEL, pwm, mask)) {
+		seq_puts(s, "not available");
+		return;
+	}
+
+	seq_printf(s, "MODE:%s", cy8c95x0_get_fname(test_bit(pin, pwm)));
+}
+
 static const struct pinctrl_ops cy8c95x0_pinctrl_ops = {
 	.get_groups_count = cy8c95x0_pinctrl_get_groups_count,
 	.get_group_name = cy8c95x0_pinctrl_get_group_name,
 	.get_group_pins = cy8c95x0_pinctrl_get_group_pins,
 	.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
 	.dt_free_map = pinconf_generic_dt_free_map,
+	.pin_dbg_show = cy8c95x0_pin_dbg_show,
 };
 
-static int cy8c95x0_get_functions_count(struct pinctrl_dev *pctldev)
+static const char *cy8c95x0_get_functions_name(struct pinctrl_dev *pctldev, unsigned int selector)
 {
-	return 2;
+	return cy8c95x0_get_fname(selector);
 }
 
-static const char *cy8c95x0_get_fname(struct pinctrl_dev *pctldev, unsigned int selector)
+static int cy8c95x0_get_functions_count(struct pinctrl_dev *pctldev)
 {
-	if (selector == 0)
-		return "gpio";
-	else
-		return "pwm";
+	return 2;
 }
 
 static int cy8c95x0_get_groups(struct pinctrl_dev *pctldev, unsigned int selector,
@@ -1077,7 +1101,7 @@ 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_fname,
+	.get_function_name = cy8c95x0_get_functions_name,
 	.get_function_groups = cy8c95x0_get_groups,
 	.set_mux = cy8c95x0_set_mux,
 	.strict = true,
-- 
2.35.1

[PATCH v1 13/17] pinctrl: cy8c95x0: Make use of device properties

[Andy Shevchenko]

Convert the module to be property provider agnostic and allow
it to be used on non-OF platforms.

Add mod_devicetable.h include.

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

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index fc0e529e633f..c09562fbb1b7 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -137,7 +137,7 @@ config PINCTRL_BM1880
 
 config PINCTRL_CY8C95X0
 	tristate "Cypress CY8C95X0 I2C pinctrl and GPIO driver"
-	depends on I2C && OF
+	depends on I2C
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
 	select PINMUX
diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index ea81b5ae27a6..699da63d9c39 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -13,15 +13,16 @@
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_platform.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinmux.h>
-#include <linux/regmap.h>
-#include <linux/regulator/consumer.h>
 
 /* Fast access registers */
 #define CY8C95X0_INPUT		0x00
@@ -1040,8 +1041,10 @@ static const struct pinctrl_ops cy8c95x0_pinctrl_ops = {
 	.get_groups_count = cy8c95x0_pinctrl_get_groups_count,
 	.get_group_name = cy8c95x0_pinctrl_get_group_name,
 	.get_group_pins = cy8c95x0_pinctrl_get_group_pins,
+#ifdef CONFIG_OF
 	.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
 	.dt_free_map = pinconf_generic_dt_free_map,
+#endif
 	.pin_dbg_show = cy8c95x0_pin_dbg_show,
 };
 
@@ -1245,9 +1248,8 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	chip->dev = &client->dev;
 
 	/* Set the device type */
-	if (client->dev.of_node)
-		chip->driver_data = (unsigned long)of_device_get_match_data(&client->dev);
-	else
+	chip->driver_data = (unsigned long)device_get_match_data(&client->dev);
+	if (!chip->driver_data)
 		chip->driver_data = i2c_match_id(cy8c95x0_id, client)->driver_data;
 
 	if (!chip->driver_data)
-- 
2.35.1

[PATCH v1 14/17] pinctrl: cy8c95x0: support ACPI device found on Galileo Gen1

[Andy Shevchenko]

Add support of the expander found on Intel Galileo Gen1 board.
The platform information comes from ACPI.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 699da63d9c39..5d8a7d236e62 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -1330,10 +1330,17 @@ static int cy8c95x0_remove(struct i2c_client *client)
 	return 0;
 }
 
+static const struct acpi_device_id cy8c95x0_acpi_ids[] = {
+	{ "INT3490", 40, },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, cy8c95x0_acpi_ids);
+
 static struct i2c_driver cy8c95x0_driver = {
 	.driver = {
 		.name	= "cy8c95x0-pinctrl",
 		.of_match_table = cy8c95x0_dt_ids,
+		.acpi_match_table = cy8c95x0_acpi_ids,
 	},
 	.probe_new	= cy8c95x0_probe,
 	.remove		= cy8c95x0_remove,
-- 
2.35.1

[PATCH v1 15/17] pinctrl: cy8c95x0: Override IRQ for one of the expanders on Galileo Gen 1

[Andy Shevchenko]

ACPI table on Intel Galileo Gen 1 has wrong pin number for IRQ resource
of the I²C GPIO expander. Since we know what that number is and luckily
have GPIO bases fixed for SoC's controllers, we may use a simple DMI quirk
to match the platform and retrieve GpioInt() pin on it for the expander in
question.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 5d8a7d236e62..f38e7dc794e9 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -7,7 +7,9 @@
  * Author: Naresh Solanki <Naresh.Solanki@9elements.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/bitmap.h>
+#include <linux/dmi.h>
 #include <linux/gpio/driver.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
@@ -73,6 +75,46 @@ static const struct of_device_id cy8c95x0_dt_ids[] = {
 
 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[] = {
+	{ "irq-gpios", &cy8c95x0_irq_gpios, 1, ACPI_GPIO_QUIRK_ABSOLUTE_NUMBER },
+	{ }
+};
+
+static int cy8c95x0_acpi_get_irq(struct device *dev)
+{
+	int ret;
+
+	ret = devm_acpi_dev_add_driver_gpios(dev, cy8c95x0_acpi_irq_gpios);
+	if (ret)
+		dev_warn(dev, "can't add GPIO ACPI mapping\n");
+
+	ret = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(dev), "irq-gpios", 0);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev, "ACPI interrupt quirk (IRQ %d)\n", ret);
+	return ret;
+}
+
+static const struct dmi_system_id cy8c95x0_dmi_acpi_irq_info[] = {
+	{
+		/*
+		 * On Intel Galileo Gen 1 board the IRQ pin is provided
+		 * as an absolute number instead of being relative.
+		 * Since first controller (gpio-sch.c) and second
+		 * (gpio-dwapb.c) are at the fixed bases, we may safely
+		 * refer to the number in the global space to get an IRQ
+		 * out of it.
+		 */
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "Galileo"),
+		},
+	},
+	{}
+};
+
 #define MAX_BANK 8
 #define BANK_SZ 8
 #define MAX_LINE	(MAX_BANK * BANK_SZ)
@@ -1298,6 +1340,12 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	bitmap_set(chip->shiftmask, 0, 20);
 	mutex_init(&chip->i2c_lock);
 
+	if (dmi_first_match(cy8c95x0_dmi_acpi_irq_info)) {
+		ret = cy8c95x0_acpi_get_irq(&client->dev);
+		if (ret > 0)
+			client->irq = ret;
+	}
+
 	if (client->irq) {
 		ret = cy8c95x0_irq_setup(chip, client->irq);
 		if (ret)
-- 
2.35.1

[PATCH v1 16/17] pinctrl: cy8c95x0: use bits.h macros for all masks

[Andy Shevchenko]

Make use of the GENMASK() (far less error-prone, far more concise).

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index f38e7dc794e9..57302cb0186f 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -363,14 +363,14 @@ static bool cy8c95x0_precious_register(struct device *dev, unsigned int reg)
 }
 
 static const struct reg_default cy8c95x0_reg_defaults[] = {
-	{ CY8C95X0_OUTPUT_(0), 0xff },
-	{ CY8C95X0_OUTPUT_(1), 0xff },
-	{ CY8C95X0_OUTPUT_(2), 0xff },
-	{ CY8C95X0_OUTPUT_(3), 0xff },
-	{ CY8C95X0_OUTPUT_(4), 0xff },
-	{ CY8C95X0_OUTPUT_(5), 0xff },
-	{ CY8C95X0_OUTPUT_(6), 0xff },
-	{ CY8C95X0_OUTPUT_(7), 0xff },
+	{ CY8C95X0_OUTPUT_(0), GENMASK(7, 0) },
+	{ CY8C95X0_OUTPUT_(1), GENMASK(7, 0) },
+	{ CY8C95X0_OUTPUT_(2), GENMASK(7, 0) },
+	{ CY8C95X0_OUTPUT_(3), GENMASK(7, 0) },
+	{ CY8C95X0_OUTPUT_(4), GENMASK(7, 0) },
+	{ CY8C95X0_OUTPUT_(5), GENMASK(7, 0) },
+	{ CY8C95X0_OUTPUT_(6), GENMASK(7, 0) },
+	{ CY8C95X0_OUTPUT_(7), GENMASK(7, 0) },
 	{ CY8C95X0_PORTSEL, 0 },
 	{ CY8C95X0_PWMSEL, 0 },
 };
@@ -1257,7 +1257,7 @@ static int cy8c95x0_detect(struct i2c_client *client,
 	ret = i2c_smbus_read_byte_data(client, CY8C95X0_DEVID);
 	if (ret < 0)
 		return ret;
-	switch (ret & 0xf0) {
+	switch (ret & GENMASK(7, 4)) {
 	case 0x20:
 		name = cy8c95x0_id[0].name;
 		break;
-- 
2.35.1

[PATCH v1 17/17] pinctrl: cy8c95x0: Correct comment style

[Andy Shevchenko]

In a few comments the style is not aligned with the rest.
Correct them.

While at it, drop unneeded blank lines and deduplicate 'Author'.

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

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 57302cb0186f..fc2c54164669 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -3,8 +3,8 @@
  * CY8C95X0 20/40/60 pin I2C GPIO port expander with interrupt support
  *
  * Copyright (C) 2022 9elements GmbH
- * Author: Patrick Rudolph <patrick.rudolph@9elements.com>
- * Author: Naresh Solanki <Naresh.Solanki@9elements.com>
+ * Authors: Patrick Rudolph <patrick.rudolph@9elements.com>
+ *	    Naresh Solanki <Naresh.Solanki@9elements.com>
  */
 
 #include <linux/acpi.h>
@@ -37,7 +37,7 @@
 
 /* Port Select configures the port */
 #define CY8C95X0_PORTSEL	0x18
-/* port settings, write PORTSEL first */
+/* Port settings, write PORTSEL first */
 #define CY8C95X0_INTMASK	0x19
 #define CY8C95X0_PWMSEL		0x1A
 #define CY8C95X0_INVERT		0x1B
@@ -72,7 +72,6 @@ static const struct of_device_id cy8c95x0_dt_ids[] = {
 	{ .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 };
@@ -418,7 +417,7 @@ static int cy8c95x0_write_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
 			continue;
 
 		switch (reg) {
-		/* muxed registers */
+		/* Muxed registers */
 		case CY8C95X0_INTMASK:
 		case CY8C95X0_PWMSEL:
 		case CY8C95X0_INVERT:
@@ -435,7 +434,7 @@ static int cy8c95x0_write_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
 				goto out;
 			off = reg;
 			break;
-		/* direct access registers */
+		/* Direct access registers */
 		case CY8C95X0_INPUT:
 		case CY8C95X0_OUTPUT:
 		case CY8C95X0_INTSTATUS:
@@ -489,7 +488,7 @@ static int cy8c95x0_read_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
 			continue;
 
 		switch (reg) {
-		/* muxed registers */
+		/* Muxed registers */
 		case CY8C95X0_INTMASK:
 		case CY8C95X0_PWMSEL:
 		case CY8C95X0_INVERT:
@@ -506,7 +505,7 @@ static int cy8c95x0_read_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
 				goto out;
 			off = reg;
 			break;
-		/* direct access registers */
+		/* Direct access registers */
 		case CY8C95X0_INPUT:
 		case CY8C95X0_OUTPUT:
 		case CY8C95X0_INTSTATUS:
@@ -581,18 +580,18 @@ static int cy8c95x0_gpio_direction_output(struct gpio_chip *gc,
 	u8 bit = cypress_get_pin_mask(chip, off);
 	int ret;
 
-	/* set output level */
+	/* Set output level */
 	ret = regmap_write_bits(chip->regmap, outreg, bit, val ? bit : 0);
 	if (ret)
 		return ret;
 
 	mutex_lock(&chip->i2c_lock);
-	/* select port */
+	/* Select port... */
 	ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
 	if (ret)
 		goto out;
 
-	/* then direction */
+	/* ...then direction */
 	ret = regmap_write_bits(chip->regmap, CY8C95X0_DIRECTION, bit, 0);
 
 out:
@@ -613,7 +612,7 @@ static int cy8c95x0_gpio_get_value(struct gpio_chip *gc, unsigned int off)
 	if (ret < 0) {
 		/*
 		 * NOTE:
-		 * diagnostic already emitted; that's all we should
+		 * Diagnostic already emitted; that's all we should
 		 * do unless gpio_*_value_cansleep() calls become different
 		 * from their nonsleeping siblings (and report faults).
 		 */
@@ -676,7 +675,7 @@ static int cy8c95x0_gpio_get_pincfg(struct cy8c95x0_pinctrl *chip,
 
 	mutex_lock(&chip->i2c_lock);
 
-	/* select port */
+	/* Select port */
 	ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
 	if (ret < 0)
 		goto out;
@@ -731,7 +730,8 @@ static int cy8c95x0_gpio_get_pincfg(struct cy8c95x0_pinctrl *chip,
 		ret = -ENOTSUPP;
 		goto out;
 	}
-	/* Writing 1 to one of the drive mode registers will automatically
+	/*
+	 * Writing 1 to one of the drive mode registers will automatically
 	 * clear conflicting set bits in the other drive mode registers.
 	 */
 	ret = regmap_read(chip->regmap, reg, &reg_val);
@@ -757,7 +757,7 @@ static int cy8c95x0_gpio_set_pincfg(struct cy8c95x0_pinctrl *chip,
 
 	mutex_lock(&chip->i2c_lock);
 
-	/* select port */
+	/* Select port */
 	ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
 	if (ret < 0)
 		goto out;
@@ -794,7 +794,8 @@ static int cy8c95x0_gpio_set_pincfg(struct cy8c95x0_pinctrl *chip,
 		ret = -ENOTSUPP;
 		goto out;
 	}
-	/* Writing 1 to one of the drive mode registers will automatically
+	/*
+	 * Writing 1 to one of the drive mode registers will automatically
 	 * clear conflicting set bits in the other drive mode registers.
 	 */
 	ret = regmap_write_bits(chip->regmap, reg, bit, bit);
@@ -1119,7 +1120,7 @@ static int cy8c95x0_pinmux_cfg(struct cy8c95x0_pinctrl *chip,
 	u8 bit = cypress_get_pin_mask(chip, off);
 	int ret;
 
-	/* select port */
+	/* Select port */
 	ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
 	if (ret < 0)
 		return ret;
@@ -1236,11 +1237,12 @@ static int cy8c95x0_setup_pinctrl(struct cy8c95x0_pinctrl *chip)
 	pd->pins = cy8c9560_pins;
 	pd->npins = chip->tpin;
 	pd->owner = THIS_MODULE;
-	chip->pctldev = devm_pinctrl_register(chip->dev, pd, chip);
 
+	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;
 }
 
@@ -1293,7 +1295,6 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	chip->driver_data = (unsigned long)device_get_match_data(&client->dev);
 	if (!chip->driver_data)
 		chip->driver_data = i2c_match_id(cy8c95x0_id, client)->driver_data;
-
 	if (!chip->driver_data)
 		return -ENODEV;
 
@@ -1395,7 +1396,6 @@ static struct i2c_driver cy8c95x0_driver = {
 	.id_table	= cy8c95x0_id,
 	.detect		= cy8c95x0_detect,
 };
-
 module_i2c_driver(cy8c95x0_driver);
 
 MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
-- 
2.35.1

Re: [PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Andy Shevchenko]

On Fri, Sep 2, 2022 at 9:36 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> It's unclear why many of static registers were marked as volatile.

the static (yeah, forgot it)

> They are pretty much bidirectional and static in a sense that
> written value is kept there until a new write or chip reset.
> Drop those registers from the list to allow them to be cached.

This patch is not correct due to indexing access. It's sneaked since I
forgot I added it into my main repo. The proper approach should be to
create virtual registers and decode them before use. This allows to
cache all ports and as a benefit to debug print all port actual
statuses.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Andy Shevchenko]

On Fri, Sep 02, 2022 at 09:42:00PM +0300, Andy Shevchenko wrote:
> On Fri, Sep 2, 2022 at 9:36 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > It's unclear why many of static registers were marked as volatile.
> 
> the static (yeah, forgot it)
> 
> > They are pretty much bidirectional and static in a sense that
> > written value is kept there until a new write or chip reset.
> > Drop those registers from the list to allow them to be cached.
> 
> This patch is not correct due to indexing access. It's sneaked since I
> forgot I added it into my main repo. The proper approach should be to
> create virtual registers and decode them before use. This allows to
> cache all ports and as a benefit to debug print all port actual
> statuses.

To be clear: With this one removed from the bunch the rest can be applied w.o.
any change.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Linus Walleij]

On Mon, Sep 5, 2022 at 2:57 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Fri, Sep 02, 2022 at 09:42:00PM +0300, Andy Shevchenko wrote:
> > On Fri, Sep 2, 2022 at 9:36 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > It's unclear why many of static registers were marked as volatile.
> >
> > the static (yeah, forgot it)
> >
> > > They are pretty much bidirectional and static in a sense that
> > > written value is kept there until a new write or chip reset.
> > > Drop those registers from the list to allow them to be cached.
> >
> > This patch is not correct due to indexing access. It's sneaked since I
> > forgot I added it into my main repo. The proper approach should be to
> > create virtual registers and decode them before use. This allows to
> > cache all ports and as a benefit to debug print all port actual
> > statuses.
>
> To be clear: With this one removed from the bunch the rest can be applied w.o.
> any change.

I'll give Patrick a day or two to test/review and then I'll just apply
them all except this one, they are all pretty self-evident except ACPI
things which have obviously been tested on hardware so from my
point of view it's good to merge.

Yours,
Linus Walleij

Re: [PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Andy Shevchenko]

On Mon, Sep 5, 2022 at 4:34 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Sep 5, 2022 at 2:57 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Sep 02, 2022 at 09:42:00PM +0300, Andy Shevchenko wrote:
> > > On Fri, Sep 2, 2022 at 9:36 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > It's unclear why many of static registers were marked as volatile.
> > >
> > > the static (yeah, forgot it)
> > >
> > > > They are pretty much bidirectional and static in a sense that
> > > > written value is kept there until a new write or chip reset.
> > > > Drop those registers from the list to allow them to be cached.
> > >
> > > This patch is not correct due to indexing access. It's sneaked since I
> > > forgot I added it into my main repo. The proper approach should be to
> > > create virtual registers and decode them before use. This allows to
> > > cache all ports and as a benefit to debug print all port actual
> > > statuses.
> >
> > To be clear: With this one removed from the bunch the rest can be applied w.o.
> > any change.
>
> I'll give Patrick a day or two to test/review and then I'll just apply
> them all except this one, they are all pretty self-evident

Sure!

> except ACPI
> things which have obviously been tested on hardware

Yes, I have a Galileo Gen 1 board which has been used for testing.

>  so from my
> point of view it's good to merge.

Thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Patrick Rudolph]

Hi,
I've tested the patch series on my OpenBMC platform and it works fine.

I don't think it's worth the effort to implement virtual registers for
the muxed pin configuration,
but I won't stop you.

Regards,
Patrick

On Mon, Sep 5, 2022 at 3:37 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Sep 5, 2022 at 4:34 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Mon, Sep 5, 2022 at 2:57 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Fri, Sep 02, 2022 at 09:42:00PM +0300, Andy Shevchenko wrote:
> > > > On Fri, Sep 2, 2022 at 9:36 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > >
> > > > > It's unclear why many of static registers were marked as volatile.
> > > >
> > > > the static (yeah, forgot it)
> > > >
> > > > > They are pretty much bidirectional and static in a sense that
> > > > > written value is kept there until a new write or chip reset.
> > > > > Drop those registers from the list to allow them to be cached.
> > > >
> > > > This patch is not correct due to indexing access. It's sneaked since I
> > > > forgot I added it into my main repo. The proper approach should be to
> > > > create virtual registers and decode them before use. This allows to
> > > > cache all ports and as a benefit to debug print all port actual
> > > > statuses.
> > >
> > > To be clear: With this one removed from the bunch the rest can be applied w.o.
> > > any change.
> >
> > I'll give Patrick a day or two to test/review and then I'll just apply
> > them all except this one, they are all pretty self-evident
>
> Sure!
>
> > except ACPI
> > things which have obviously been tested on hardware
>
> Yes, I have a Galileo Gen 1 board which has been used for testing.
>
> >  so from my
> > point of view it's good to merge.
>
> Thanks!
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Andy Shevchenko]

On Tue, Sep 6, 2022 at 11:36 AM Patrick Rudolph
<patrick.rudolph@9elements.com> wrote:
>
> Hi,
> I've tested the patch series on my OpenBMC platform and it works fine.

Thank you!
I guess we may consider this as an equivalent to the formal Tested-by tag?

> I don't think it's worth the effort to implement virtual registers for
> the muxed pin configuration,
> but I won't stop you.

It's not a high priority to me anyway, but it is a good feature to
have since regmap allows us to dump registers. Moreover the listing
files in debugfs currently take a lot of time, that's how I come up
with this.

> On Mon, Sep 5, 2022 at 3:37 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Mon, Sep 5, 2022 at 4:34 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Mon, Sep 5, 2022 at 2:57 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Fri, Sep 02, 2022 at 09:42:00PM +0300, Andy Shevchenko wrote:
> > > > > On Fri, Sep 2, 2022 at 9:36 PM Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > >
> > > > > > It's unclear why many of static registers were marked as volatile.
> > > > >
> > > > > the static (yeah, forgot it)
> > > > >
> > > > > > They are pretty much bidirectional and static in a sense that
> > > > > > written value is kept there until a new write or chip reset.
> > > > > > Drop those registers from the list to allow them to be cached.
> > > > >
> > > > > This patch is not correct due to indexing access. It's sneaked since I
> > > > > forgot I added it into my main repo. The proper approach should be to
> > > > > create virtual registers and decode them before use. This allows to
> > > > > cache all ports and as a benefit to debug print all port actual
> > > > > statuses.
> > > >
> > > > To be clear: With this one removed from the bunch the rest can be applied w.o.
> > > > any change.
> > >
> > > I'll give Patrick a day or two to test/review and then I'll just apply
> > > them all except this one, they are all pretty self-evident
> >
> > Sure!
> >
> > > except ACPI
> > > things which have obviously been tested on hardware
> >
> > Yes, I have a Galileo Gen 1 board which has been used for testing.
> >
> > >  so from my
> > > point of view it's good to merge.
> >
> > Thanks!


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 17/17] pinctrl: cy8c95x0: Correct comment style

[Patrick Rudolph]

Patch series is

Tested-by: Patrick Rudolph <patrick.rudolph@9elements.com>

On Fri, Sep 2, 2022 at 8:32 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> In a few comments the style is not aligned with the rest.
> Correct them.
>
> While at it, drop unneeded blank lines and deduplicate 'Author'.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/pinctrl-cy8c95x0.c | 40 +++++++++++++++---------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
> index 57302cb0186f..fc2c54164669 100644
> --- a/drivers/pinctrl/pinctrl-cy8c95x0.c
> +++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
> @@ -3,8 +3,8 @@
>   * CY8C95X0 20/40/60 pin I2C GPIO port expander with interrupt support
>   *
>   * Copyright (C) 2022 9elements GmbH
> - * Author: Patrick Rudolph <patrick.rudolph@9elements.com>
> - * Author: Naresh Solanki <Naresh.Solanki@9elements.com>
> + * Authors: Patrick Rudolph <patrick.rudolph@9elements.com>
> + *         Naresh Solanki <Naresh.Solanki@9elements.com>
>   */
>
>  #include <linux/acpi.h>
> @@ -37,7 +37,7 @@
>
>  /* Port Select configures the port */
>  #define CY8C95X0_PORTSEL       0x18
> -/* port settings, write PORTSEL first */
> +/* Port settings, write PORTSEL first */
>  #define CY8C95X0_INTMASK       0x19
>  #define CY8C95X0_PWMSEL                0x1A
>  #define CY8C95X0_INVERT                0x1B
> @@ -72,7 +72,6 @@ static const struct of_device_id cy8c95x0_dt_ids[] = {
>         { .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 };
> @@ -418,7 +417,7 @@ static int cy8c95x0_write_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
>                         continue;
>
>                 switch (reg) {
> -               /* muxed registers */
> +               /* Muxed registers */
>                 case CY8C95X0_INTMASK:
>                 case CY8C95X0_PWMSEL:
>                 case CY8C95X0_INVERT:
> @@ -435,7 +434,7 @@ static int cy8c95x0_write_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
>                                 goto out;
>                         off = reg;
>                         break;
> -               /* direct access registers */
> +               /* Direct access registers */
>                 case CY8C95X0_INPUT:
>                 case CY8C95X0_OUTPUT:
>                 case CY8C95X0_INTSTATUS:
> @@ -489,7 +488,7 @@ static int cy8c95x0_read_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
>                         continue;
>
>                 switch (reg) {
> -               /* muxed registers */
> +               /* Muxed registers */
>                 case CY8C95X0_INTMASK:
>                 case CY8C95X0_PWMSEL:
>                 case CY8C95X0_INVERT:
> @@ -506,7 +505,7 @@ static int cy8c95x0_read_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
>                                 goto out;
>                         off = reg;
>                         break;
> -               /* direct access registers */
> +               /* Direct access registers */
>                 case CY8C95X0_INPUT:
>                 case CY8C95X0_OUTPUT:
>                 case CY8C95X0_INTSTATUS:
> @@ -581,18 +580,18 @@ static int cy8c95x0_gpio_direction_output(struct gpio_chip *gc,
>         u8 bit = cypress_get_pin_mask(chip, off);
>         int ret;
>
> -       /* set output level */
> +       /* Set output level */
>         ret = regmap_write_bits(chip->regmap, outreg, bit, val ? bit : 0);
>         if (ret)
>                 return ret;
>
>         mutex_lock(&chip->i2c_lock);
> -       /* select port */
> +       /* Select port... */
>         ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
>         if (ret)
>                 goto out;
>
> -       /* then direction */
> +       /* ...then direction */
>         ret = regmap_write_bits(chip->regmap, CY8C95X0_DIRECTION, bit, 0);
>
>  out:
> @@ -613,7 +612,7 @@ static int cy8c95x0_gpio_get_value(struct gpio_chip *gc, unsigned int off)
>         if (ret < 0) {
>                 /*
>                  * NOTE:
> -                * diagnostic already emitted; that's all we should
> +                * Diagnostic already emitted; that's all we should
>                  * do unless gpio_*_value_cansleep() calls become different
>                  * from their nonsleeping siblings (and report faults).
>                  */
> @@ -676,7 +675,7 @@ static int cy8c95x0_gpio_get_pincfg(struct cy8c95x0_pinctrl *chip,
>
>         mutex_lock(&chip->i2c_lock);
>
> -       /* select port */
> +       /* Select port */
>         ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
>         if (ret < 0)
>                 goto out;
> @@ -731,7 +730,8 @@ static int cy8c95x0_gpio_get_pincfg(struct cy8c95x0_pinctrl *chip,
>                 ret = -ENOTSUPP;
>                 goto out;
>         }
> -       /* Writing 1 to one of the drive mode registers will automatically
> +       /*
> +        * Writing 1 to one of the drive mode registers will automatically
>          * clear conflicting set bits in the other drive mode registers.
>          */
>         ret = regmap_read(chip->regmap, reg, &reg_val);
> @@ -757,7 +757,7 @@ static int cy8c95x0_gpio_set_pincfg(struct cy8c95x0_pinctrl *chip,
>
>         mutex_lock(&chip->i2c_lock);
>
> -       /* select port */
> +       /* Select port */
>         ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
>         if (ret < 0)
>                 goto out;
> @@ -794,7 +794,8 @@ static int cy8c95x0_gpio_set_pincfg(struct cy8c95x0_pinctrl *chip,
>                 ret = -ENOTSUPP;
>                 goto out;
>         }
> -       /* Writing 1 to one of the drive mode registers will automatically
> +       /*
> +        * Writing 1 to one of the drive mode registers will automatically
>          * clear conflicting set bits in the other drive mode registers.
>          */
>         ret = regmap_write_bits(chip->regmap, reg, bit, bit);
> @@ -1119,7 +1120,7 @@ static int cy8c95x0_pinmux_cfg(struct cy8c95x0_pinctrl *chip,
>         u8 bit = cypress_get_pin_mask(chip, off);
>         int ret;
>
> -       /* select port */
> +       /* Select port */
>         ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
>         if (ret < 0)
>                 return ret;
> @@ -1236,11 +1237,12 @@ static int cy8c95x0_setup_pinctrl(struct cy8c95x0_pinctrl *chip)
>         pd->pins = cy8c9560_pins;
>         pd->npins = chip->tpin;
>         pd->owner = THIS_MODULE;
> -       chip->pctldev = devm_pinctrl_register(chip->dev, pd, chip);
>
> +       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;
>  }
>
> @@ -1293,7 +1295,6 @@ static int cy8c95x0_probe(struct i2c_client *client)
>         chip->driver_data = (unsigned long)device_get_match_data(&client->dev);
>         if (!chip->driver_data)
>                 chip->driver_data = i2c_match_id(cy8c95x0_id, client)->driver_data;
> -
>         if (!chip->driver_data)
>                 return -ENODEV;
>
> @@ -1395,7 +1396,6 @@ static struct i2c_driver cy8c95x0_driver = {
>         .id_table       = cy8c95x0_id,
>         .detect         = cy8c95x0_detect,
>  };
> -
>  module_i2c_driver(cy8c95x0_driver);
>
>  MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
> --
> 2.35.1
>

Re: [PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Linus Walleij]

On Mon, Sep 5, 2022 at 3:30 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Sep 5, 2022 at 2:57 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Sep 02, 2022 at 09:42:00PM +0300, Andy Shevchenko wrote:
> > > On Fri, Sep 2, 2022 at 9:36 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > It's unclear why many of static registers were marked as volatile.
> > >
> > > the static (yeah, forgot it)
> > >
> > > > They are pretty much bidirectional and static in a sense that
> > > > written value is kept there until a new write or chip reset.
> > > > Drop those registers from the list to allow them to be cached.
> > >
> > > This patch is not correct due to indexing access. It's sneaked since I
> > > forgot I added it into my main repo. The proper approach should be to
> > > create virtual registers and decode them before use. This allows to
> > > cache all ports and as a benefit to debug print all port actual
> > > statuses.
> >
> > To be clear: With this one removed from the bunch the rest can be applied w.o.
> > any change.
>
> I'll give Patrick a day or two to test/review and then I'll just apply
> them all except this one, they are all pretty self-evident except ACPI
> things which have obviously been tested on hardware so from my
> point of view it's good to merge.

I applied all patches now except this one (3/17), some patches needed
a bit of fuzzing because other stuff in my tree, so please check the
result once it lands in linux-next.

Yours,
Linus Walleij

Re: [PATCH v1 03/17] pinctrl: cy8c95x0: Allow most of the registers to be cached

[Andy Shevchenko]

On Thu, Sep 08, 2022 at 10:03:38AM +0200, Linus Walleij wrote:
> On Mon, Sep 5, 2022 at 3:30 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Mon, Sep 5, 2022 at 2:57 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Fri, Sep 02, 2022 at 09:42:00PM +0300, Andy Shevchenko wrote:
> > > > On Fri, Sep 2, 2022 at 9:36 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > >
> > > > > It's unclear why many of static registers were marked as volatile.
> > > >
> > > > the static (yeah, forgot it)
> > > >
> > > > > They are pretty much bidirectional and static in a sense that
> > > > > written value is kept there until a new write or chip reset.
> > > > > Drop those registers from the list to allow them to be cached.
> > > >
> > > > This patch is not correct due to indexing access. It's sneaked since I
> > > > forgot I added it into my main repo. The proper approach should be to
> > > > create virtual registers and decode them before use. This allows to
> > > > cache all ports and as a benefit to debug print all port actual
> > > > statuses.
> > >
> > > To be clear: With this one removed from the bunch the rest can be applied w.o.
> > > any change.
> >
> > I'll give Patrick a day or two to test/review and then I'll just apply
> > them all except this one, they are all pretty self-evident except ACPI
> > things which have obviously been tested on hardware so from my
> > point of view it's good to merge.
> 
> I applied all patches now except this one (3/17), some patches needed
> a bit of fuzzing because other stuff in my tree, so please check the
> result once it lands in linux-next.

Looks perfect, thanks!

-- 
With Best Regards,
Andy Shevchenko