diff mbox series

[v2,1/2] pinctrl: mcp23s08: Rename and change function that wraps regmap_update_bits()

Message ID 20230324164957.485924-2-u.kleine-koenig@pengutronix.de
State Accepted
Commit d490be6dd8aa3194d8b5d509846275b4b2b2ac38
Headers show
Series pinctrl: mcp23s08: Implement gpio bulk functions | expand

Commit Message

Uwe Kleine-König March 24, 2023, 4:49 p.m. UTC
The semantic of mcp_set_mask() was surprising to me when I first read
that driver. So it was unexpected that in the call

	mcp_set_mask(mcp, MCP_OLAT, mask, value);

value was a bool. Make the function a thinner wrapper around
regmap_update_bits() and rename it to also have a similar name.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Linus Walleij March 29, 2023, 2:31 p.m. UTC | #1
On Fri, Mar 24, 2023 at 5:50 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> The semantic of mcp_set_mask() was surprising to me when I first read
> that driver. So it was unexpected that in the call
>
>         mcp_set_mask(mcp, MCP_OLAT, mask, value);
>
> value was a bool. Make the function a thinner wrapper around
> regmap_update_bits() and rename it to also have a similar name.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Both patches applied, thanks!

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 5f356edfd0fd..79a41d418482 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -144,10 +144,9 @@  static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val)
 	return regmap_write(mcp->regmap, reg << mcp->reg_shift, val);
 }
 
-static int mcp_set_mask(struct mcp23s08 *mcp, unsigned int reg,
-		       unsigned int mask, bool enabled)
+static int mcp_update_bits(struct mcp23s08 *mcp, unsigned int reg,
+			   unsigned int mask, unsigned int val)
 {
-	u16 val  = enabled ? 0xffff : 0x0000;
 	return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift,
 				  mask, val);
 }
@@ -156,7 +155,7 @@  static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg,
 		       unsigned int pin, bool enabled)
 {
 	u16 mask = BIT(pin);
-	return mcp_set_mask(mcp, reg, mask, enabled);
+	return mcp_update_bits(mcp, reg, mask, enabled ? mask : 0);
 }
 
 static const struct pinctrl_pin_desc mcp23x08_pins[] = {
@@ -310,7 +309,7 @@  static int mcp23s08_get(struct gpio_chip *chip, unsigned offset)
 
 static int __mcp23s08_set(struct mcp23s08 *mcp, unsigned mask, bool value)
 {
-	return mcp_set_mask(mcp, MCP_OLAT, mask, value);
+	return mcp_update_bits(mcp, MCP_OLAT, mask, value ? mask : 0);
 }
 
 static void mcp23s08_set(struct gpio_chip *chip, unsigned offset, int value)
@@ -333,7 +332,7 @@  mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value)
 	mutex_lock(&mcp->lock);
 	status = __mcp23s08_set(mcp, mask, value);
 	if (status == 0) {
-		status = mcp_set_mask(mcp, MCP_IODIR, mask, false);
+		status = mcp_update_bits(mcp, MCP_IODIR, mask, 0);
 	}
 	mutex_unlock(&mcp->lock);
 	return status;