[PATCH v2 0/2] pinctrl: sx150x: set multiple pins at once

[PATCH v2 0/2] pinctrl: sx150x: set multiple pins at once

[Peter Rosin]

Hi!

v1 -> v2 changes

- removed remains of (broken) support for sx150[789], as it was
  overengineered.
- add comment as to why sx150[789] is not supported.
- add ack from Neil for patch 1/2

I have only tested this on an 8-bit sx1502. I'm also fairly certain
that there needs to be locking for this to work as intended for the
bigger chips with an oscio pin, so those are not supported by this
patch.

Cheers,
Peter

Peter Rosin (2):
  pinctrl: sx150x: various spelling fixes and some white-space cleanup
  pinctrl: sx150x: support setting multiple pins at once

 drivers/pinctrl/pinctrl-sx150x.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

-- 
2.1.4

[PATCH v2 0/2] pinctrl: sx150x: set multiple pins at once

[Peter Rosin]

Hi!

v1 -> v2 changes

- removed remains of (broken) support for sx150[789], as it was
  overengineered.
- add comment as to why sx150[789] is not supported.
- add ack from Neil for patch 1/2

I have only tested this on an 8-bit sx1502. I'm also fairly certain
that there needs to be locking for this to work as intended for the
bigger chips with an oscio pin, so those are not supported by this
patch.

Cheers,
Peter

Peter Rosin (2):
  pinctrl: sx150x: various spelling fixes and some white-space cleanup
  pinctrl: sx150x: support setting multiple pins at once

 drivers/pinctrl/pinctrl-sx150x.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

-- 
2.1.4

[PATCH v2 1/2] pinctrl: sx150x: various spelling fixes and some white-space cleanup

[Peter Rosin]

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/pinctrl/pinctrl-sx150x.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c
index 63778058eec7..ef4ef88e0ee9 100644
--- a/drivers/pinctrl/pinctrl-sx150x.c
+++ b/drivers/pinctrl/pinctrl-sx150x.c
@@ -384,7 +384,7 @@ static int sx150x_gpio_oscio_set(struct sx150x_pinctrl *pctl,
 }
 
 static void sx150x_gpio_set(struct gpio_chip *chip, unsigned int offset,
-			       int value)
+			    int value)
 {
 	struct sx150x_pinctrl *pctl = gpiochip_get_data(chip);
 
@@ -396,7 +396,7 @@ static void sx150x_gpio_set(struct gpio_chip *chip, unsigned int offset,
 }
 
 static int sx150x_gpio_direction_input(struct gpio_chip *chip,
-				      unsigned int offset)
+				       unsigned int offset)
 {
 	struct sx150x_pinctrl *pctl = gpiochip_get_data(chip);
 
@@ -409,7 +409,7 @@ static int sx150x_gpio_direction_input(struct gpio_chip *chip,
 }
 
 static int sx150x_gpio_direction_output(struct gpio_chip *chip,
-				       unsigned int offset, int value)
+					unsigned int offset, int value)
 {
 	struct sx150x_pinctrl *pctl = gpiochip_get_data(chip);
 	int ret;
@@ -880,7 +880,7 @@ static unsigned int sx150x_maybe_swizzle(struct sx150x_pinctrl *pctl,
 	 *	reg + 3 [ 3 3 2 2 1 1 0 0 ]
 	 *
 	 * SX1503 and SX1506 deviate from that data layout, instead storing
-	 * thier contents as follows:
+	 * their contents as follows:
 	 *
 	 *	reg     [ f f e e d d c c ]
 	 *	reg + 1 [ 7 7 6 6 5 5 4 4 ]
@@ -915,9 +915,8 @@ static unsigned int sx150x_maybe_swizzle(struct sx150x_pinctrl *pctl,
  *
  * This way the rest of the driver code, interfacing with the chip via
  * regmap API, can work assuming that each GPIO pin is represented by
- * a group of bits at an offset proportioan to GPIO number within a
+ * a group of bits at an offset proportional to GPIO number within a
  * given register.
- *
  */
 static int sx150x_regmap_reg_read(void *context, unsigned int reg,
 				  unsigned int *result)
@@ -929,7 +928,7 @@ static int sx150x_regmap_reg_read(void *context, unsigned int reg,
 	unsigned int idx, val;
 
 	/*
-	 * There are four potential cases coverd by this function:
+	 * There are four potential cases covered by this function:
 	 *
 	 * 1) 8-pin chip, single configuration bit register
 	 *
-- 
2.1.4

[PATCH v2 1/2] pinctrl: sx150x: various spelling fixes and some white-space cleanup

[Peter Rosin]

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/pinctrl/pinctrl-sx150x.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c
index 63778058eec7..ef4ef88e0ee9 100644
--- a/drivers/pinctrl/pinctrl-sx150x.c
+++ b/drivers/pinctrl/pinctrl-sx150x.c
@@ -384,7 +384,7 @@ static int sx150x_gpio_oscio_set(struct sx150x_pinctrl *pctl,
 }
 
 static void sx150x_gpio_set(struct gpio_chip *chip, unsigned int offset,
-			       int value)
+			    int value)
 {
 	struct sx150x_pinctrl *pctl = gpiochip_get_data(chip);
 
@@ -396,7 +396,7 @@ static void sx150x_gpio_set(struct gpio_chip *chip, unsigned int offset,
 }
 
 static int sx150x_gpio_direction_input(struct gpio_chip *chip,
-				      unsigned int offset)
+				       unsigned int offset)
 {
 	struct sx150x_pinctrl *pctl = gpiochip_get_data(chip);
 
@@ -409,7 +409,7 @@ static int sx150x_gpio_direction_input(struct gpio_chip *chip,
 }
 
 static int sx150x_gpio_direction_output(struct gpio_chip *chip,
-				       unsigned int offset, int value)
+					unsigned int offset, int value)
 {
 	struct sx150x_pinctrl *pctl = gpiochip_get_data(chip);
 	int ret;
@@ -880,7 +880,7 @@ static unsigned int sx150x_maybe_swizzle(struct sx150x_pinctrl *pctl,
 	 *	reg + 3 [ 3 3 2 2 1 1 0 0 ]
 	 *
 	 * SX1503 and SX1506 deviate from that data layout, instead storing
-	 * thier contents as follows:
+	 * their contents as follows:
 	 *
 	 *	reg     [ f f e e d d c c ]
 	 *	reg + 1 [ 7 7 6 6 5 5 4 4 ]
@@ -915,9 +915,8 @@ static unsigned int sx150x_maybe_swizzle(struct sx150x_pinctrl *pctl,
  *
  * This way the rest of the driver code, interfacing with the chip via
  * regmap API, can work assuming that each GPIO pin is represented by
- * a group of bits at an offset proportioan to GPIO number within a
+ * a group of bits at an offset proportional to GPIO number within a
  * given register.
- *
  */
 static int sx150x_regmap_reg_read(void *context, unsigned int reg,
 				  unsigned int *result)
@@ -929,7 +928,7 @@ static int sx150x_regmap_reg_read(void *context, unsigned int reg,
 	unsigned int idx, val;
 
 	/*
-	 * There are four potential cases coverd by this function:
+	 * There are four potential cases covered by this function:
 	 *
 	 * 1) 8-pin chip, single configuration bit register
 	 *
-- 
2.1.4

[PATCH v2 2/2] pinctrl: sx150x: support setting multiple pins at once

[Peter Rosin]

If the chip does not have an oscio pin, all pins are configured in
the same regmap register making it trivial to update all pins at
once, so do that. If an oscio pin is present, there needs to be
more locking in place to handle all cases correctly, so this is
skipped.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/pinctrl/pinctrl-sx150x.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c
index ef4ef88e0ee9..f9e559e22537 100644
--- a/drivers/pinctrl/pinctrl-sx150x.c
+++ b/drivers/pinctrl/pinctrl-sx150x.c
@@ -395,6 +395,15 @@ static void sx150x_gpio_set(struct gpio_chip *chip, unsigned int offset,
 
 }
 
+static void sx150x_gpio_set_multiple(struct gpio_chip *chip,
+				     unsigned long *mask,
+				     unsigned long *bits)
+{
+	struct sx150x_pinctrl *pctl = gpiochip_get_data(chip);
+
+	regmap_write_bits(pctl->regmap, pctl->data->reg_data, *mask, *bits);
+}
+
 static int sx150x_gpio_direction_input(struct gpio_chip *chip,
 				       unsigned int offset)
 {
@@ -1075,6 +1084,14 @@ static int sx150x_probe(struct i2c_client *client,
 	pctl->gpio.of_node = dev->of_node;
 #endif
 	pctl->gpio.can_sleep = true;
+	/*
+	 * Setting multiple pins is not safe when all pins are not
+	 * handled by the same regmap register. The oscio pin (present
+	 * on the SX150X_789 chips) lives in its own register, so
+	 * would require locking that is not in place at this time.
+	 */
+	if (pctl->data->model != SX150X_789)
+		pctl->gpio.set_multiple = sx150x_gpio_set_multiple;
 
 	ret = devm_gpiochip_add_data(dev, &pctl->gpio, pctl);
 	if (ret)
-- 
2.1.4

[PATCH v2 2/2] pinctrl: sx150x: support setting multiple pins at once

[Peter Rosin]

If the chip does not have an oscio pin, all pins are configured in
the same regmap register making it trivial to update all pins at
once, so do that. If an oscio pin is present, there needs to be
more locking in place to handle all cases correctly, so this is
skipped.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/pinctrl/pinctrl-sx150x.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c
index ef4ef88e0ee9..f9e559e22537 100644
--- a/drivers/pinctrl/pinctrl-sx150x.c
+++ b/drivers/pinctrl/pinctrl-sx150x.c
@@ -395,6 +395,15 @@ static void sx150x_gpio_set(struct gpio_chip *chip, unsigned int offset,
 
 }
 
+static void sx150x_gpio_set_multiple(struct gpio_chip *chip,
+				     unsigned long *mask,
+				     unsigned long *bits)
+{
+	struct sx150x_pinctrl *pctl = gpiochip_get_data(chip);
+
+	regmap_write_bits(pctl->regmap, pctl->data->reg_data, *mask, *bits);
+}
+
 static int sx150x_gpio_direction_input(struct gpio_chip *chip,
 				       unsigned int offset)
 {
@@ -1075,6 +1084,14 @@ static int sx150x_probe(struct i2c_client *client,
 	pctl->gpio.of_node = dev->of_node;
 #endif
 	pctl->gpio.can_sleep = true;
+	/*
+	 * Setting multiple pins is not safe when all pins are not
+	 * handled by the same regmap register. The oscio pin (present
+	 * on the SX150X_789 chips) lives in its own register, so
+	 * would require locking that is not in place at this time.
+	 */
+	if (pctl->data->model != SX150X_789)
+		pctl->gpio.set_multiple = sx150x_gpio_set_multiple;
 
 	ret = devm_gpiochip_add_data(dev, &pctl->gpio, pctl);
 	if (ret)
-- 
2.1.4

Re: [PATCH v2 1/2] pinctrl: sx150x: various spelling fixes and some white-space cleanup

[Linus Walleij]

On Wed, Nov 23, 2016 at 11:18 AM, Peter Rosin <peda@axentia.se> wrote:

> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH v2 2/2] pinctrl: sx150x: support setting multiple pins at once

[Linus Walleij]

On Wed, Nov 23, 2016 at 11:18 AM, Peter Rosin <peda@axentia.se> wrote:

> If the chip does not have an oscio pin, all pins are configured in
> the same regmap register making it trivial to update all pins at
> once, so do that. If an oscio pin is present, there needs to be
> more locking in place to handle all cases correctly, so this is
> skipped.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Tired of waiting for ACKs. Looks good. Patch applied.

Yours,
Linus Walleij