2024-03-13 23:54:43

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 00/11] pinctrl: aw9523: number of cleanups

Just noticed that the newly introduced driver has some leftovers or unneeded
customisation. Hence this series. It may (or may not :-) be sent as an update
after v6.9-rc1 for v6.9 cycle, but TBH there is nothing functional critical.

Andy Shevchenko (11):
pinctrl: aw9523: Destroy mutex on ->remove()
pinctrl: aw9523: Use correct error code for not supported
functionality
pinctrl: aw9523: Always try both ports in aw9523_gpio_set_multiple()
pinctrl: aw9523: Make use of struct pinfunction and
PINCTRL_PINFUNCTION()
pinctrl: aw9523: Use temporary variable for HW IRQ number
pinctrl: aw9523: Get rid of redundant ' & U8_MAX' pieces
pinctrl: aw9523: Remove unused irqchip field in struct aw9523_irq
pinctrl: aw9523: Make use of dev_err_probe()
pinctrl: aw9523: Sort headers and group pinctrl/*
pinctrl: aw9523: Fix indentation in a few places
pinctrl: aw9523: Remove redundant dependency to OF

drivers/pinctrl/Kconfig | 2 +-
drivers/pinctrl/pinctrl-aw9523.c | 132 +++++++++++--------------------
2 files changed, 48 insertions(+), 86 deletions(-)

--
2.44.0



2024-03-14 07:04:36

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v1 00/11] pinctrl: aw9523: number of cleanups

On Thu, Mar 14, 2024 at 01:52:03AM +0200, Andy Shevchenko wrote:
> Just noticed that the newly introduced driver has some leftovers or unneeded
> customisation. Hence this series. It may (or may not :-) be sent as an update
> after v6.9-rc1 for v6.9 cycle, but TBH there is nothing functional critical.
>

Reviewed-by: Dan Carpenter <[email protected]>

regards,
dan carpenter


2024-03-13 23:56:24

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 08/11] pinctrl: aw9523: Make use of dev_err_probe()

From: Andy Shevchenko <[email protected]>

Simplify the error handling in probe function by switching from
dev_err() to dev_err_probe().

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 44b798c39e26b..17c359f9c45c0 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -816,10 +816,8 @@ static int aw9523_init_irq(struct aw9523 *awi, int irq)

ret = devm_request_threaded_irq(dev, irq, NULL, aw9523_irq_thread_func,
IRQF_ONESHOT, dev_name(dev), awi);
- if (ret) {
- dev_err(dev, "Failed to request irq %d\n", irq);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to request irq %d\n", irq);

girq = &awi->gpio.irq;
gpio_irq_chip_set_chip(girq, &aw9523_irq_chip);
@@ -1016,8 +1014,7 @@ static int aw9523_probe(struct i2c_client *client)

awi->pctl = devm_pinctrl_register(dev, pdesc, awi);
if (IS_ERR(awi->pctl)) {
- ret = PTR_ERR(awi->pctl);
- dev_err(dev, "Cannot register pinctrl: %d", ret);
+ ret = dev_err_probe(dev, PTR_ERR(awi->pctl), "Cannot register pinctrl");
goto err_disable_vregs;
}

--
2.44.0


2024-03-13 23:56:28

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 11/11] pinctrl: aw9523: Remove redundant dependency to OF

Driver does not dependent on OF, remove it.
While here, add missing mod_devicetable.h.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/Kconfig | 2 +-
drivers/pinctrl/pinctrl-aw9523.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index d45657aa986ae..c413109b1173b 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -129,7 +129,7 @@ config PINCTRL_AXP209

config PINCTRL_AW9523
tristate "Awinic AW9523/AW9523B I2C GPIO expander pinctrl driver"
- depends on OF && I2C
+ depends on I2C
select PINMUX
select PINCONF
select GENERIC_PINCONF
diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 7299b5bb6d52f..43285e6d0e5b7 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -12,6 +12,7 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/property.h>
--
2.44.0


2024-03-13 23:57:52

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 06/11] pinctrl: aw9523: Get rid of redundant ' & U8_MAX' pieces

When the variable is declared as u8, no need to perform ' & U8_MAX'
as it's implied anyway.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 118896373844a..9e8e658f81404 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -603,7 +603,7 @@ static int aw9523_gpio_get_multiple(struct gpio_chip *chip,
mutex_lock(&awi->i2c_lock);

/* Port 0 (gpio 0-7) */
- m = *mask & U8_MAX;
+ m = *mask;
if (m) {
ret = _aw9523_gpio_get_multiple(awi, 0, &state, m);
if (ret)
@@ -612,7 +612,7 @@ static int aw9523_gpio_get_multiple(struct gpio_chip *chip,
*bits = state;

/* Port 1 (gpio 8-15) */
- m = (*mask >> 8) & U8_MAX;
+ m = *mask >> 8;
if (m) {
ret = _aw9523_gpio_get_multiple(awi, AW9523_PINS_PER_PORT,
&state, m);
@@ -635,20 +635,20 @@ static void aw9523_gpio_set_multiple(struct gpio_chip *chip,
unsigned int reg;
int ret;

- mask_lo = *mask & U8_MAX;
- mask_hi = (*mask >> 8) & U8_MAX;
+ mask_lo = *mask;
+ mask_hi = *mask >> 8;
+ bits_lo = *bits;
+ bits_hi = *bits >> 8;
+
mutex_lock(&awi->i2c_lock);
if (mask_hi) {
reg = AW9523_REG_OUT_STATE(AW9523_PINS_PER_PORT);
- bits_hi = (*bits >> 8) & U8_MAX;
-
ret = regmap_write_bits(awi->regmap, reg, mask_hi, bits_hi);
if (ret)
dev_warn(awi->dev, "Cannot write port1 out level\n");
}
if (mask_lo) {
reg = AW9523_REG_OUT_STATE(0);
- bits_lo = *bits & U8_MAX;
ret = regmap_write_bits(awi->regmap, reg, mask_lo, bits_lo);
if (ret)
dev_warn(awi->dev, "Cannot write port0 out level\n");
--
2.44.0


2024-03-13 23:56:54

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 04/11] pinctrl: aw9523: Make use of struct pinfunction and PINCTRL_PINFUNCTION()

From: Andy Shevchenko <[email protected]>

Since pin control provides a generic data type and a macro for
the pin function definition, use them in the driver.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index d93640a02d1d3..79916e6bf6f4e 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -66,18 +66,6 @@ struct aw9523_irq {
u16 cached_gpio;
};

-/*
- * struct aw9523_pinmux - Pin mux params
- * @name: Name of the mux
- * @grps: Groups of the mux
- * @num_grps: Number of groups (sizeof array grps)
- */
-struct aw9523_pinmux {
- const char *name;
- const char * const *grps;
- const u8 num_grps;
-};
-
/*
* struct aw9523 - Main driver structure
* @dev: device handle
@@ -158,17 +146,9 @@ static const char * const gpio_pwm_groups[] = {
};

/* Warning: Do NOT reorder this array */
-static const struct aw9523_pinmux aw9523_pmx[] = {
- {
- .name = "pwm",
- .grps = gpio_pwm_groups,
- .num_grps = ARRAY_SIZE(gpio_pwm_groups),
- },
- {
- .name = "gpio",
- .grps = gpio_pwm_groups,
- .num_grps = ARRAY_SIZE(gpio_pwm_groups),
- },
+static const struct pinfunction aw9523_pmx[] = {
+ PINCTRL_PINFUNCTION("pwm", gpio_pwm_groups, ARRAY_SIZE(gpio_pwm_groups)),
+ PINCTRL_PINFUNCTION("gpio", gpio_pwm_groups, ARRAY_SIZE(gpio_pwm_groups)),
};

static int aw9523_pmx_get_funcs_count(struct pinctrl_dev *pctl)
@@ -184,10 +164,10 @@ static const char *aw9523_pmx_get_fname(struct pinctrl_dev *pctl,

static int aw9523_pmx_get_groups(struct pinctrl_dev *pctl, unsigned int sel,
const char * const **groups,
- unsigned int * const num_groups)
+ unsigned int * const ngroups)
{
- *groups = aw9523_pmx[sel].grps;
- *num_groups = aw9523_pmx[sel].num_grps;
+ *groups = aw9523_pmx[sel].groups;
+ *ngroups = aw9523_pmx[sel].ngrpoups;
return 0;
}

--
2.44.0


2024-03-13 23:55:44

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 09/11] pinctrl: aw9523: Sort headers and group pinctrl/*

One header was misplaced and group pinctrl/* ones to show the relation
with the pin control subsystem.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 17c359f9c45c0..50e2a94f59892 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -13,17 +13,18 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
-#include <linux/mutex.h>
#include <linux/module.h>
-#include <linux/pinctrl/pinconf.h>
-#include <linux/pinctrl/pinctrl.h>
-#include <linux/pinctrl/pinmux.h>
-#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/mutex.h>
#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>

+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
#define AW9523_MAX_FUNCS 2
#define AW9523_NUM_PORTS 2
#define AW9523_PINS_PER_PORT 8
--
2.44.0


2024-03-13 23:57:14

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 05/11] pinctrl: aw9523: Use temporary variable for HW IRQ number

There are two different ways on how to get HW IRQ number in some functions.
Unify that by using temporary variable and irqd_to_hwirq() call.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 79916e6bf6f4e..118896373844a 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -428,12 +428,12 @@ static int aw9523_gpio_irq_type(struct irq_data *d, unsigned int type)
static void aw9523_irq_mask(struct irq_data *d)
{
struct aw9523 *awi = gpiochip_get_data(irq_data_get_irq_chip_data(d));
- unsigned int n = d->hwirq % AW9523_PINS_PER_PORT;
+ irq_hw_number_t hwirq = irqd_to_hwirq(d);
+ unsigned int n = hwirq % AW9523_PINS_PER_PORT;

- regmap_update_bits(awi->regmap,
- AW9523_REG_INTR_DIS(d->hwirq),
+ regmap_update_bits(awi->regmap, AW9523_REG_INTR_DIS(hwirq),
BIT(n), BIT(n));
- gpiochip_disable_irq(&awi->gpio, irqd_to_hwirq(d));
+ gpiochip_disable_irq(&awi->gpio, hwirq);
}

/*
@@ -446,11 +446,11 @@ static void aw9523_irq_mask(struct irq_data *d)
static void aw9523_irq_unmask(struct irq_data *d)
{
struct aw9523 *awi = gpiochip_get_data(irq_data_get_irq_chip_data(d));
- unsigned int n = d->hwirq % AW9523_PINS_PER_PORT;
+ irq_hw_number_t hwirq = irqd_to_hwirq(d);
+ unsigned int n = hwirq % AW9523_PINS_PER_PORT;

- gpiochip_enable_irq(&awi->gpio, irqd_to_hwirq(d));
- regmap_update_bits(awi->regmap,
- AW9523_REG_INTR_DIS(d->hwirq),
+ gpiochip_enable_irq(&awi->gpio, hwirq);
+ regmap_update_bits(awi->regmap, AW9523_REG_INTR_DIS(hwirq),
BIT(n), 0);
}

--
2.44.0


2024-03-13 23:56:15

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 10/11] pinctrl: aw9523: Fix indentation in a few places

In the comment, function prototype, and array of strings indentation
is kinda broken. Reindent that.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 50e2a94f59892..7299b5bb6d52f 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -1,8 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Awinic AW9523B i2c pin controller driver
- * Copyright (c) 2020, AngeloGioacchino Del Regno
- * <[email protected]>
+ * Copyright (c) 2020, AngeloGioacchino Del Regno <[email protected]>
*/

#include <linux/bitfield.h>
@@ -139,9 +138,10 @@ static const struct pinctrl_ops aw9523_pinctrl_ops = {
};

static const char * const gpio_pwm_groups[] = {
- "gpio0", "gpio1", "gpio2", "gpio3", "gpio4", "gpio5",
- "gpio6", "gpio7", "gpio8", "gpio9", "gpio10", "gpio11",
- "gpio12", "gpio13", "gpio14", "gpio15"
+ "gpio0", "gpio1", "gpio2", "gpio3", /* 0-3 */
+ "gpio4", "gpio5", "gpio6", "gpio7", /* 4-7 */
+ "gpio8", "gpio9", "gpio10", "gpio11", /* 8-11 */
+ "gpio12", "gpio13", "gpio14", "gpio15", /* 11-15 */
};

/* Warning: Do NOT reorder this array */
@@ -388,8 +388,8 @@ static int aw9523_get_pin_direction(struct regmap *regmap, u8 pin, u8 n)
*
* Return: Zero for success or negative number for error
*/
-static int aw9523_get_port_state(struct regmap *regmap, u8 pin,
- u8 regbit, unsigned int *state)
+static int aw9523_get_port_state(struct regmap *regmap, u8 pin, u8 regbit,
+ unsigned int *state)
{
u8 reg;
int dir;
@@ -984,8 +984,7 @@ static int aw9523_probe(struct i2c_client *client)
}

mutex_init(&awi->i2c_lock);
- lockdep_set_subclass(&awi->i2c_lock,
- i2c_adapter_depth(client->adapter));
+ lockdep_set_subclass(&awi->i2c_lock, i2c_adapter_depth(client->adapter));

pdesc = devm_kzalloc(dev, sizeof(*pdesc), GFP_KERNEL);
if (!pdesc)
--
2.44.0


2024-03-13 23:57:01

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 03/11] pinctrl: aw9523: Always try both ports in aw9523_gpio_set_multiple()

The ports are equivalent from the user's point of view. Don't limit
trying them both if writing to one fails.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 65d523697b731..d93640a02d1d3 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -653,7 +653,7 @@ static void aw9523_gpio_set_multiple(struct gpio_chip *chip,
struct aw9523 *awi = gpiochip_get_data(chip);
u8 mask_lo, mask_hi, bits_lo, bits_hi;
unsigned int reg;
- int ret = 0;
+ int ret;

mask_lo = *mask & U8_MAX;
mask_hi = (*mask >> 8) & U8_MAX;
@@ -663,10 +663,8 @@ static void aw9523_gpio_set_multiple(struct gpio_chip *chip,
bits_hi = (*bits >> 8) & U8_MAX;

ret = regmap_write_bits(awi->regmap, reg, mask_hi, bits_hi);
- if (ret) {
+ if (ret)
dev_warn(awi->dev, "Cannot write port1 out level\n");
- goto out;
- }
}
if (mask_lo) {
reg = AW9523_REG_OUT_STATE(0);
@@ -675,7 +673,6 @@ static void aw9523_gpio_set_multiple(struct gpio_chip *chip,
if (ret)
dev_warn(awi->dev, "Cannot write port0 out level\n");
}
-out:
mutex_unlock(&awi->i2c_lock);
}

--
2.44.0


2024-03-13 23:57:04

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 02/11] pinctrl: aw9523: Use correct error code for not supported functionality

The pin control subsystem internally uses ENOTSUPP for the not supported
functionality. The checkpatch is false positive about this error code.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 66629af0b88b4..65d523697b731 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -6,6 +6,7 @@
*/

#include <linux/bitfield.h>
+#include <linux/errno.h>
#include <linux/gpio/consumer.h>
#include <linux/gpio/driver.h>
#include <linux/i2c.h>
@@ -239,7 +240,7 @@ static int aw9523_pcfg_param_to_reg(enum pin_config_param pcp, int pin, u8 *r)
reg = AW9523_REG_OUT_STATE(pin);
break;
default:
- return -EOPNOTSUPP;
+ return -ENOTSUPP;
}
*r = reg;

@@ -290,7 +291,7 @@ static int aw9523_pconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
val = FIELD_GET(AW9523_GCR_GPOMD_MASK, val);
break;
default:
- return -EOPNOTSUPP;
+ return -ENOTSUPP;
}
if (val < 1)
return -EINVAL;
@@ -344,7 +345,7 @@ static int aw9523_pconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
case PIN_CONFIG_DRIVE_OPEN_DRAIN:
/* Open-Drain is supported only on port 0 */
if (pin >= AW9523_PINS_PER_PORT) {
- rc = -EOPNOTSUPP;
+ rc = -ENOTSUPP;
goto end;
}
mask = AW9523_GCR_GPOMD_MASK;
@@ -361,7 +362,7 @@ static int aw9523_pconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
val = AW9523_GCR_GPOMD_MASK;
break;
default:
- rc = -EOPNOTSUPP;
+ rc = -ENOTSUPP;
goto end;
}

--
2.44.0


2024-03-13 23:55:12

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 07/11] pinctrl: aw9523: Remove unused irqchip field in struct aw9523_irq

The irqchip field is allocated, assigned but never used. Remove it.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 8 --------
1 file changed, 8 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 9e8e658f81404..44b798c39e26b 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -57,12 +57,10 @@
/*
* struct aw9523_irq - Interrupt controller structure
* @lock: mutex locking for the irq bus
- * @irqchip: structure holding irqchip params
* @cached_gpio: stores the previous gpio status for bit comparison
*/
struct aw9523_irq {
struct mutex lock;
- struct irq_chip *irqchip;
u16 cached_gpio;
};

@@ -805,21 +803,15 @@ static int aw9523_init_irq(struct aw9523 *awi, int irq)
{
struct device *dev = awi->dev;
struct gpio_irq_chip *girq;
- struct irq_chip *irqchip;
int ret;

if (!device_property_read_bool(dev, "interrupt-controller"))
return 0;

- irqchip = devm_kzalloc(dev, sizeof(*irqchip), GFP_KERNEL);
- if (!irqchip)
- return -ENOMEM;
-
awi->irq = devm_kzalloc(dev, sizeof(*awi->irq), GFP_KERNEL);
if (!awi->irq)
return -ENOMEM;

- awi->irq->irqchip = irqchip;
mutex_init(&awi->irq->lock);

ret = devm_request_threaded_irq(dev, irq, NULL, aw9523_irq_thread_func,
--
2.44.0


2024-03-13 23:54:55

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 01/11] pinctrl: aw9523: Destroy mutex on ->remove()

If aw9523_hw_init() fails on ->remove() the mutex left alive.
Destroy it in that case as well. While at it, remove never
true check at the beginning of the function.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/pinctrl-aw9523.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-aw9523.c b/drivers/pinctrl/pinctrl-aw9523.c
index 4edd371c469fb..66629af0b88b4 100644
--- a/drivers/pinctrl/pinctrl-aw9523.c
+++ b/drivers/pinctrl/pinctrl-aw9523.c
@@ -1067,10 +1067,6 @@ static int aw9523_probe(struct i2c_client *client)
static void aw9523_remove(struct i2c_client *client)
{
struct aw9523 *awi = i2c_get_clientdata(client);
- int ret;
-
- if (!awi)
- return;

/*
* If the chip VIO is connected to a regulator that we can turn
@@ -1082,10 +1078,8 @@ static void aw9523_remove(struct i2c_client *client)
regulator_disable(awi->vio_vreg);
} else {
mutex_lock(&awi->i2c_lock);
- ret = aw9523_hw_init(awi);
+ aw9523_hw_init(awi);
mutex_unlock(&awi->i2c_lock);
- if (ret)
- return;
}

mutex_destroy(&awi->i2c_lock);
--
2.44.0


2024-03-15 11:14:09

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 04/11] pinctrl: aw9523: Make use of struct pinfunction and PINCTRL_PINFUNCTION()

Hi Andy,

kernel test robot noticed the following build errors:

[auto build test ERROR on linusw-pinctrl/devel]
[also build test ERROR on linusw-pinctrl/for-next linus/master next-20240314]
[cannot apply to v6.8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/pinctrl-aw9523-Destroy-mutex-on-remove/20240314-075853
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link: https://lore.kernel.org/r/20240313235422.180075-5-andy.shevchenko%40gmail.com
patch subject: [PATCH v1 04/11] pinctrl: aw9523: Make use of struct pinfunction and PINCTRL_PINFUNCTION()
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240315/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 503c55e17037436dcd45ac69dea8967e67e3f5e8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240315/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from drivers/pinctrl/pinctrl-aw9523.c:11:
In file included from include/linux/gpio/driver.h:8:
In file included from include/linux/irqchip/chained_irq.h:10:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/pinctrl/pinctrl-aw9523.c:11:
In file included from include/linux/gpio/driver.h:8:
In file included from include/linux/irqchip/chained_irq.h:10:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/pinctrl/pinctrl-aw9523.c:11:
In file included from include/linux/gpio/driver.h:8:
In file included from include/linux/irqchip/chained_irq.h:10:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/pinctrl/pinctrl-aw9523.c:12:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:20:
In file included from include/linux/mm.h:2188:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/pinctrl/pinctrl-aw9523.c:170:29: error: no member named 'ngrpoups' in 'struct pinfunction'; did you mean 'ngroups'?
170 | *ngroups = aw9523_pmx[sel].ngrpoups;
| ^~~~~~~~
| ngroups
include/linux/pinctrl/pinctrl.h:218:9: note: 'ngroups' declared here
218 | size_t ngroups;
| ^
7 warnings and 1 error generated.


vim +170 drivers/pinctrl/pinctrl-aw9523.c

164
165 static int aw9523_pmx_get_groups(struct pinctrl_dev *pctl, unsigned int sel,
166 const char * const **groups,
167 unsigned int * const ngroups)
168 {
169 *groups = aw9523_pmx[sel].groups;
> 170 *ngroups = aw9523_pmx[sel].ngrpoups;
171 return 0;
172 }
173

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-03-28 09:25:10

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v1 00/11] pinctrl: aw9523: number of cleanups

On Thu, Mar 14, 2024 at 12:54 AM Andy Shevchenko
<[email protected]> wrote:

> Just noticed that the newly introduced driver has some leftovers or unneeded
> customisation. Hence this series. It may (or may not :-) be sent as an update
> after v6.9-rc1 for v6.9 cycle, but TBH there is nothing functional critical.

There is some speling mistake making patch 4 fail according to kernelbot,
if you rebase on v6.9-rc1 and fix patch 4 I think I can just apply the lot.

Thanks!
Linus Walleij

2024-03-28 14:15:40

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 00/11] pinctrl: aw9523: number of cleanups

On Thu, Mar 28, 2024 at 11:22 AM Linus Walleij <[email protected]> wrote:
>
> On Thu, Mar 14, 2024 at 12:54 AM Andy Shevchenko
> <[email protected]> wrote:
>
> > Just noticed that the newly introduced driver has some leftovers or unneeded
> > customisation. Hence this series. It may (or may not :-) be sent as an update
> > after v6.9-rc1 for v6.9 cycle, but TBH there is nothing functional critical.
>
> There is some speling mistake making patch 4 fail according to kernelbot,
> if you rebase on v6.9-rc1 and fix patch 4 I think I can just apply the lot.

Yeah, I will do it soon.

--
With Best Regards,
Andy Shevchenko