gpio: expose pull-up/pull-down line flags to userspace

PATCH Superseded Kent Gibson 3 participants 10 messages Oct 11, 2019 – Oct 16, 2019

Subsystems (2)

GPIO GPIO mockup driver

lore

Share & tools

lore

Kent Gibson

author

2019-10-11 15:47 UTC #0 lore raw

cover gpio: expose pull-up/pull-down line flags to userspace

This series adds gross control of pull-up/pull-down to the GPIO uAPI. Gross control means enabling and disabling of bias functionality, not finer grained control such as setting biasing impedances.

The support allows both input and output lines to have any one of the following biases applied as part of the line handle or event request:
0. As Is - bias is left alone. This is the default for ABI compatibility.
1. Pull Up - pull-up bias is enabled.
2. Pull Down - pull-down bias is enabled.
3. Pull None - bias is explicitly disabled.

The biases are encoded in two flags, PULL_UP and PULL_DOWN, where setting both is interpreted as Pull None. So the flags effectively form a two bit field encoding the values above.

The setting of biases on output lines may seem odd, but is to allow for utilisation of internal pull-up/pull-down on open drain and open source outputs, where supported in hardware.

Patches are against:
github.com/brgl/linux/commit/82fc38f6ab599ee03f7a8ed078de8abb41e6e611
which contains the initial patch from Drew Fustini, with Bartosz Golaszewski, that adds support for pull-up/down flags in line handle requests.

Patch 1 adds support to line event requests.
Patch 2 adds pull-up/down support to the gpio-mockup for uAPI testing. Patch 3 rejects biasing changes to lines requested as-is. Patch 4 adds support for disabling bias (pull none).
Patch 5 adds support for setting bias on output lines.

Kent Gibson (5):
gpiolib: add support for pull up/down to lineevent_create
gpio: mockup: add set_config to support pull up/down
gpiolib: pull requires explicit input mode
gpiolib: disable bias on inputs when pull up/down are both set
gpiolib: allow pull up/down on outputs

 drivers/gpio/gpio-mockup.c | 94 ++++++++++++++++++++++++--------------
 drivers/gpio/gpiolib.c     | 55 ++++++++++++++++------
 2 files changed, 100 insertions(+), 49 deletions(-)

Signature · 1 line

2.23.0

Kent Gibson

author

2019-10-11 15:48 UTC #1 lore raw

PATCH 1/5 gpiolib: add support for pull up/down to lineevent_create

This patch adds support for pull up/down to lineevent_create. Use cases include receiving asynchronous presses from a push button without an external pull up/down.

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpiolib.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

drivers/gpio/gpiolib.c +18 -2

index 994e5d71375d..0912a00b2960 100644
@@ -920,8 +920,20 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 	/* This is just wrong: we don't look for events on output lines */
 	if ((lflags & GPIOHANDLE_REQUEST_OUTPUT) ||
 	    (lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN) ||
-	    (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE) ||
-	    (lflags & GPIOHANDLE_REQUEST_PULL_UP) ||
+	    (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE))
+		return -EINVAL;
+
+	/* PULL_UP and PULL_DOWN flags only make sense for input mode. */
+	if (!(lflags & GPIOHANDLE_REQUEST_INPUT) &&
+	    ((lflags & GPIOHANDLE_REQUEST_PULL_UP) ||
+	     (lflags & GPIOHANDLE_REQUEST_PULL_DOWN)))
+		return -EINVAL;
+
+	/*
+	 * Do not allow both pull-up and pull-down flags to be set as they
+	 *  are contradictory.
+	 */
+	if ((lflags & GPIOHANDLE_REQUEST_PULL_UP) &&
 	    (lflags & GPIOHANDLE_REQUEST_PULL_DOWN))
 		return -EINVAL;

@@ -951,6 +963,10 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 
 	if (lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW)
 		set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+	if (lflags & GPIOHANDLE_REQUEST_PULL_DOWN)
+		set_bit(FLAG_PULL_DOWN, &desc->flags);
+	if (lflags & GPIOHANDLE_REQUEST_PULL_UP)
+		set_bit(FLAG_PULL_UP, &desc->flags);
 
 	ret = gpiod_direction_input(desc);
 	if (ret)

Signature · 1 line

2.23.0

Kent Gibson

author

2019-10-11 15:48 UTC #2 lore raw

PATCH 2/5 gpio: mockup: add set_config to support pull up/down

In reply to Kent Gibson

This patch adds support for the pull up/down state set via gpiolib line requests to be reflected in the state of the mockup.
Use case is for testing of the GPIO uAPI, specifically the pull up/down flags.

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 94 ++++++++++++++++++++++++--------------
 1 file changed, 60 insertions(+), 34 deletions(-)

drivers/gpio/gpio-mockup.c +60 ~~-34~~

index 213aedc97dc2..493077229677 100644
@@ -146,6 +146,61 @@ static void gpio_mockup_set_multiple(struct gpio_chip *gc,
 	mutex_unlock(&chip->lock);
 }
 
+static int gpio_mockup_apply_pull(struct gpio_mockup_chip *chip,
+			unsigned int offset, int value)
+{
+	struct gpio_desc *desc;
+	struct gpio_chip *gc;
+	struct irq_sim *sim;
+	int curr, irq, irq_type;
+
+	gc = &chip->gc;
+	desc = &gc->gpiodev->descs[offset];
+	sim = &chip->irqsim;
+
+	mutex_lock(&chip->lock);
+
+	if (test_bit(FLAG_REQUESTED, &desc->flags) &&
+		!test_bit(FLAG_IS_OUT, &desc->flags)) {
+		curr = __gpio_mockup_get(chip, offset);
+		if (curr == value)
+			goto out;
+
+		irq = irq_sim_irqnum(sim, offset);
+		irq_type = irq_get_trigger_type(irq);
+
+		if ((value == 1 && (irq_type & IRQ_TYPE_EDGE_RISING)) ||
+			(value == 0 && (irq_type & IRQ_TYPE_EDGE_FALLING)))
+			irq_sim_fire(sim, offset);
+	}
+
+	/* Change the value unless we're actively driving the line. */
+	if (!test_bit(FLAG_REQUESTED, &desc->flags) ||
+		!test_bit(FLAG_IS_OUT, &desc->flags))
+		__gpio_mockup_set(chip, offset, value);
+
+out:
+	chip->lines[offset].pull = value;
+	mutex_unlock(&chip->lock);
+	return 0;
+}
+
+static int gpio_mockup_set_config(struct gpio_chip *gc,
+				   unsigned int offset, unsigned long config)
+{
+	struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
+
+	switch (pinconf_to_config_param(config)) {
+	case PIN_CONFIG_BIAS_PULL_UP:
+		return gpio_mockup_apply_pull(chip, offset, 1);
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		return gpio_mockup_apply_pull(chip, offset, 0);
+	default:
+		break;
+	}
+	return -ENOTSUPP;
+}
+
 static int gpio_mockup_dirout(struct gpio_chip *gc,
 			      unsigned int offset, int value)
 {

@@ -226,12 +281,8 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file,
 					 size_t size, loff_t *ppos)
 {
 	struct gpio_mockup_dbgfs_private *priv;
-	int rv, val, curr, irq, irq_type;
-	struct gpio_mockup_chip *chip;
+	int rv, val;
 	struct seq_file *sfile;
-	struct gpio_desc *desc;
-	struct gpio_chip *gc;
-	struct irq_sim *sim;
 
 	if (*ppos != 0)
 		return -EINVAL;

@@ -244,35 +295,9 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file,
 
 	sfile = file->private_data;
 	priv = sfile->private;
-	chip = priv->chip;
-	gc = &chip->gc;
-	desc = &gc->gpiodev->descs[priv->offset];
-	sim = &chip->irqsim;
-
-	mutex_lock(&chip->lock);
-
-	if (test_bit(FLAG_REQUESTED, &desc->flags) &&
-	    !test_bit(FLAG_IS_OUT, &desc->flags)) {
-		curr = __gpio_mockup_get(chip, priv->offset);
-		if (curr == val)
-			goto out;
-
-		irq = irq_sim_irqnum(sim, priv->offset);
-		irq_type = irq_get_trigger_type(irq);
-
-		if ((val == 1 && (irq_type & IRQ_TYPE_EDGE_RISING)) ||
-		    (val == 0 && (irq_type & IRQ_TYPE_EDGE_FALLING)))
-			irq_sim_fire(sim, priv->offset);
-	}
-
-	/* Change the value unless we're actively driving the line. */
-	if (!test_bit(FLAG_REQUESTED, &desc->flags) ||
-	    !test_bit(FLAG_IS_OUT, &desc->flags))
-		__gpio_mockup_set(chip, priv->offset, val);
-
-out:
-	chip->lines[priv->offset].pull = val;
-	mutex_unlock(&chip->lock);
+	rv = gpio_mockup_apply_pull(priv->chip, priv->offset, val);
+	if (rv)
+		return rv;
 
 	return size;
 }

@@ -418,6 +443,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	gc->direction_output = gpio_mockup_dirout;
 	gc->direction_input = gpio_mockup_dirin;
 	gc->get_direction = gpio_mockup_get_direction;
+	gc->set_config = gpio_mockup_set_config;
 	gc->to_irq = gpio_mockup_to_irq;
 	gc->free = gpio_mockup_free;

Signature · 1 line

2.23.0

Kent Gibson

author

2019-10-11 15:48 UTC #3 lore raw

PATCH 3/5 gpiolib: pull requires explicit input mode

In reply to Kent Gibson

This patch prevents pull up/down flags being applied to as-is line requests, which should be left as-is, and for output mode for which setting pulls is not currently supported.

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpiolib.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/gpio/gpiolib.c +6 -0

index 0912a00b2960..a634c340920b 100644
@@ -559,6 +559,12 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	     (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE)))
 		return -EINVAL;
 
+	/* PULL_UP and PULL_DOWN flags only make sense for input mode. */
+	if (!(lflags & GPIOHANDLE_REQUEST_INPUT) &&
+	    ((lflags & GPIOHANDLE_REQUEST_PULL_UP) ||
+	     (lflags & GPIOHANDLE_REQUEST_PULL_DOWN)))
+		return -EINVAL;
+
 	lh = kzalloc(sizeof(*lh), GFP_KERNEL);
 	if (!lh)
 		return -ENOMEM;

Signature · 1 line

2.23.0

Kent Gibson

author

2019-10-11 15:48 UTC #4 lore raw

PATCH 4/5 gpiolib: disable bias on inputs when pull up/down are both set

In reply to Kent Gibson

This patch allows pull up/down bias to be disabled, allowing the line to float or to be biased only by external circuitry. Use case is for where the bias has been applied previously, either by default or by the user, but that setting may conflict with the current use of the line.

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpiolib.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

drivers/gpio/gpiolib.c +7 ~~-15~~

index a634c340920b..f0665ea396cd 100644
@@ -539,11 +539,6 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	    (lflags & GPIOHANDLE_REQUEST_OUTPUT))
 		return -EINVAL;
 
-	/* Same with pull-up and pull-down. */
-	if ((lflags & GPIOHANDLE_REQUEST_PULL_UP) &&
-	    (lflags & GPIOHANDLE_REQUEST_PULL_DOWN))
-		return -EINVAL;
-
 	/*
 	 * Do not allow OPEN_SOURCE & OPEN_DRAIN flags in a single request. If
 	 * the hardware actually supports enabling both at the same time the

@@ -935,14 +930,6 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 	     (lflags & GPIOHANDLE_REQUEST_PULL_DOWN)))
 		return -EINVAL;
 
-	/*
-	 * Do not allow both pull-up and pull-down flags to be set as they
-	 *  are contradictory.
-	 */
-	if ((lflags & GPIOHANDLE_REQUEST_PULL_UP) &&
-	    (lflags & GPIOHANDLE_REQUEST_PULL_DOWN))
-		return -EINVAL;
-
 	le = kzalloc(sizeof(*le), GFP_KERNEL);
 	if (!le)
 		return -ENOMEM;

@@ -2931,6 +2918,7 @@ static int gpio_set_config(struct gpio_chip *gc, unsigned offset,
 	unsigned arg;
 
 	switch (mode) {
+	case PIN_CONFIG_BIAS_DISABLE:
 	case PIN_CONFIG_BIAS_PULL_DOWN:
 	case PIN_CONFIG_BIAS_PULL_UP:
 		arg = 1;

@@ -2991,7 +2979,11 @@ int gpiod_direction_input(struct gpio_desc *desc)
 	if (ret == 0)
 		clear_bit(FLAG_IS_OUT, &desc->flags);
 
-	if (test_bit(FLAG_PULL_UP, &desc->flags))
+	if (test_bit(FLAG_PULL_UP, &desc->flags) &&
+		test_bit(FLAG_PULL_DOWN, &desc->flags))
+		gpio_set_config(chip, gpio_chip_hwgpio(desc),
+				PIN_CONFIG_BIAS_DISABLE);
+	else if (test_bit(FLAG_PULL_UP, &desc->flags))
 		gpio_set_config(chip, gpio_chip_hwgpio(desc),
 				PIN_CONFIG_BIAS_PULL_UP);
 	else if (test_bit(FLAG_PULL_DOWN, &desc->flags))

@@ -4462,7 +4454,7 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 
 	if (lflags & GPIO_PULL_UP)
 		set_bit(FLAG_PULL_UP, &desc->flags);
-	else if (lflags & GPIO_PULL_DOWN)
+	if (lflags & GPIO_PULL_DOWN)
 		set_bit(FLAG_PULL_DOWN, &desc->flags);
 
 	ret = gpiod_set_transitory(desc, (lflags & GPIO_TRANSITORY));

Signature · 1 line

2.23.0

Kent Gibson

author

2019-10-11 15:48 UTC #5 lore raw

PATCH 5/5 gpiolib: allow pull up/down on outputs

In reply to Kent Gibson

This patch allows pull up/down bias to be set on outputs. Use case is for open source or open drain applications where internal pull up/down may conflict with external biasing.

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpiolib.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

drivers/gpio/gpiolib.c +23 ~~-12~~

index f0665ea396cd..38ead7a87d44 100644
@@ -554,8 +554,9 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	     (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE)))
 		return -EINVAL;
 
-	/* PULL_UP and PULL_DOWN flags only make sense for input mode. */
-	if (!(lflags & GPIOHANDLE_REQUEST_INPUT) &&
+	/* PULL_UP and PULL_DOWN flags only allowed for input or output mode. */
+	if (!((lflags & GPIOHANDLE_REQUEST_INPUT) ||
+	      (lflags & GPIOHANDLE_REQUEST_OUTPUT)) &&
 	    ((lflags & GPIOHANDLE_REQUEST_PULL_UP) ||
 	     (lflags & GPIOHANDLE_REQUEST_PULL_DOWN)))
 		return -EINVAL;

@@ -2932,6 +2933,24 @@ static int gpio_set_config(struct gpio_chip *gc, unsigned offset,
 	return gc->set_config ? gc->set_config(gc, offset, config) : -ENOTSUPP;
 }
 
+static int gpio_set_bias(struct gpio_chip *chip, struct gpio_desc *desc)
+{
+	int bias = 0;
+
+	if (test_bit(FLAG_PULL_UP, &desc->flags) &&
+	    test_bit(FLAG_PULL_DOWN, &desc->flags))
+		bias = PIN_CONFIG_BIAS_DISABLE;
+	else if (test_bit(FLAG_PULL_UP, &desc->flags))
+		bias = PIN_CONFIG_BIAS_PULL_UP;
+	else if (test_bit(FLAG_PULL_DOWN, &desc->flags))
+		bias = PIN_CONFIG_BIAS_PULL_DOWN;
+
+	if (bias)
+		return gpio_set_config(chip, gpio_chip_hwgpio(desc), bias);
+
+	return 0;
+}
+
 /**
  * gpiod_direction_input - set the GPIO direction to input
  * @desc:	GPIO to set to input

@@ -2979,16 +2998,7 @@ int gpiod_direction_input(struct gpio_desc *desc)
 	if (ret == 0)
 		clear_bit(FLAG_IS_OUT, &desc->flags);
 
-	if (test_bit(FLAG_PULL_UP, &desc->flags) &&
-		test_bit(FLAG_PULL_DOWN, &desc->flags))
-		gpio_set_config(chip, gpio_chip_hwgpio(desc),
-				PIN_CONFIG_BIAS_DISABLE);
-	else if (test_bit(FLAG_PULL_UP, &desc->flags))
-		gpio_set_config(chip, gpio_chip_hwgpio(desc),
-				PIN_CONFIG_BIAS_PULL_UP);
-	else if (test_bit(FLAG_PULL_DOWN, &desc->flags))
-		gpio_set_config(chip, gpio_chip_hwgpio(desc),
-				PIN_CONFIG_BIAS_PULL_DOWN);
+	gpio_set_bias(chip, desc);
 
 	trace_gpio_direction(desc_to_gpio(desc), 1, ret);

@@ -3114,6 +3124,7 @@ int gpiod_direction_output(struct gpio_desc *desc, int value)
 	}
 
 set_output_value:
+	gpio_set_bias(gc, desc);
 	return gpiod_direction_output_raw_commit(desc, value);
 }
 EXPORT_SYMBOL_GPL(gpiod_direction_output);

Signature · 1 line

2.23.0

Bartosz Golaszewski

2019-10-11 17:51 UTC #6 lore raw

In reply to Kent Gibson

pt., 11 paź 2019 o 17:47 Kent Gibson <[email protected]> napisał(a):

Quoted 38 lines

This series adds gross control of pull-up/pull-down to the GPIO uAPI. Gross control means enabling and disabling of bias functionality, not finer grained control such as setting biasing impedances.
The support allows both input and output lines to have any one of the following biases applied as part of the line handle or event request:
0. As Is - bias is left alone. This is the default for ABI compatibility.
1. Pull Up - pull-up bias is enabled.
2. Pull Down - pull-down bias is enabled.
3. Pull None - bias is explicitly disabled.
The biases are encoded in two flags, PULL_UP and PULL_DOWN, where setting both is interpreted as Pull None. So the flags effectively form a two bit field encoding the values above.
The setting of biases on output lines may seem odd, but is to allow for utilisation of internal pull-up/pull-down on open drain and open source outputs, where supported in hardware.
Patches are against:
github.com/brgl/linux/commit/82fc38f6ab599ee03f7a8ed078de8abb41e6e611
which contains the initial patch from Drew Fustini, with Bartosz Golaszewski, that adds support for pull-up/down flags in line handle requests.
Patch 1 adds support to line event requests.
Patch 2 adds pull-up/down support to the gpio-mockup for uAPI testing. Patch 3 rejects biasing changes to lines requested as-is. Patch 4 adds support for disabling bias (pull none).
Patch 5 adds support for setting bias on output lines.
Kent Gibson (5):
gpiolib: add support for pull up/down to lineevent_create
gpio: mockup: add set_config to support pull up/down
gpiolib: pull requires explicit input mode
gpiolib: disable bias on inputs when pull up/down are both set
gpiolib: allow pull up/down on outputs

Quoted 7 lines

 drivers/gpio/gpio-mockup.c | 94 ++++++++++++++++++++++++--------------
 drivers/gpio/gpiolib.c     | 55 ++++++++++++++++------
 2 files changed, 100 insertions(+), 49 deletions(-)

--
2.23.0

Hi Kent,

thanks for doing that, but please make it easier to review. The cover letter shouldn't be sent in response to this thread but be part of the patch series. Please don't rebase the patches on top of my development/experimental branch - every patch needs to spend some time on the mailing list. Rebase the series on top of the latest mainline release candidate. Pull in Drew's changes and just squash my code into your patches - it was not finished anyway. You can send patches from other developers or make them part of your series - there's no problem with that as long as you keep the authorship.

That'll make it much easier to review and understand.

Thanks in advance!
Bartosz

Kent Gibson

author

2019-10-12 00:53 UTC #7 lore raw

Quoted 47 lines · Bartosz Golaszewski

pt., 11 paź 2019 o 17:47 Kent Gibson <[email protected]> napisał(a):
Quoted 38 lines
This series adds gross control of pull-up/pull-down to the GPIO uAPI. Gross control means enabling and disabling of bias functionality, not finer grained control such as setting biasing impedances.
The support allows both input and output lines to have any one of the following biases applied as part of the line handle or event request:
0. As Is - bias is left alone. This is the default for ABI compatibility.
1. Pull Up - pull-up bias is enabled.
2. Pull Down - pull-down bias is enabled.
3. Pull None - bias is explicitly disabled.
The biases are encoded in two flags, PULL_UP and PULL_DOWN, where setting both is interpreted as Pull None. So the flags effectively form a two bit field encoding the values above.
The setting of biases on output lines may seem odd, but is to allow for utilisation of internal pull-up/pull-down on open drain and open source outputs, where supported in hardware.
Patches are against:
github.com/brgl/linux/commit/82fc38f6ab599ee03f7a8ed078de8abb41e6e611
which contains the initial patch from Drew Fustini, with Bartosz Golaszewski, that adds support for pull-up/down flags in line handle requests.
Patch 1 adds support to line event requests.
Patch 2 adds pull-up/down support to the gpio-mockup for uAPI testing. Patch 3 rejects biasing changes to lines requested as-is. Patch 4 adds support for disabling bias (pull none).
Patch 5 adds support for setting bias on output lines.
Kent Gibson (5):
gpiolib: add support for pull up/down to lineevent_create
gpio: mockup: add set_config to support pull up/down
gpiolib: pull requires explicit input mode
gpiolib: disable bias on inputs when pull up/down are both set
gpiolib: allow pull up/down on outputs
Quoted 7 lines
 drivers/gpio/gpio-mockup.c | 94 ++++++++++++++++++++++++--------------
 drivers/gpio/gpiolib.c     | 55 ++++++++++++++++------
 2 files changed, 100 insertions(+), 49 deletions(-)
--
2.23.0

Quoted 5 lines

Hi Kent,
thanks for doing that, but please make it easier to review. The cover letter shouldn't be sent in response to this thread but be part of the patch series.

Not sure what you mean - this is a new thread.
Should the updated series be a reply to this email (yours), or a new v2 thread?

Quoted 10 lines

Please don't rebase the patches on top of my development/experimental branch - every patch needs to spend some time on the mailing list. Rebase the series on top of the latest mainline release candidate. Pull in Drew's changes and just squash my code into your patches - it was not finished anyway. You can send patches from other developers or make them part of your series - there's no problem with that as long as you keep the authorship.
That'll make it much easier to review and understand.

Fair enough. I was unsure about including others' patches in the series, so kept it minimal. Will update the series as soon as I get the chance (and know where to send it).

Cheers,
Kent.

Bartosz Golaszewski

2019-10-12 05:52 UTC #8 lore raw

sob., 12 paź 2019 o 02:53 Kent Gibson <[email protected]> napisał(a):

Quoted 49 lines · Bartosz Golaszewski

Quoted 47 lines
pt., 11 paź 2019 o 17:47 Kent Gibson <[email protected]> napisał(a):
Quoted 45 lines
This series adds gross control of pull-up/pull-down to the GPIO uAPI. Gross control means enabling and disabling of bias functionality, not finer grained control such as setting biasing impedances.
The support allows both input and output lines to have any one of the following biases applied as part of the line handle or event request:
0. As Is - bias is left alone. This is the default for ABI compatibility.
1. Pull Up - pull-up bias is enabled.
2. Pull Down - pull-down bias is enabled.
3. Pull None - bias is explicitly disabled.
The biases are encoded in two flags, PULL_UP and PULL_DOWN, where setting both is interpreted as Pull None. So the flags effectively form a two bit field encoding the values above.
The setting of biases on output lines may seem odd, but is to allow for utilisation of internal pull-up/pull-down on open drain and open source outputs, where supported in hardware.
Patches are against:
github.com/brgl/linux/commit/82fc38f6ab599ee03f7a8ed078de8abb41e6e611
which contains the initial patch from Drew Fustini, with Bartosz Golaszewski, that adds support for pull-up/down flags in line handle requests.
Patch 1 adds support to line event requests.
Patch 2 adds pull-up/down support to the gpio-mockup for uAPI testing. Patch 3 rejects biasing changes to lines requested as-is. Patch 4 adds support for disabling bias (pull none).
Patch 5 adds support for setting bias on output lines.
Kent Gibson (5):
gpiolib: add support for pull up/down to lineevent_create
gpio: mockup: add set_config to support pull up/down
gpiolib: pull requires explicit input mode
gpiolib: disable bias on inputs when pull up/down are both set
gpiolib: allow pull up/down on outputs
 drivers/gpio/gpio-mockup.c | 94 ++++++++++++++++++++++++--------------
 drivers/gpio/gpiolib.c     | 55 ++++++++++++++++------
 2 files changed, 100 insertions(+), 49 deletions(-)
--
2.23.0

Quoted 7 lines

Quoted 5 lines
Hi Kent,
thanks for doing that, but please make it easier to review. The cover letter shouldn't be sent in response to this thread but be part of the patch series.
Not sure what you mean - this is a new thread.

Sorry for that, I just looked at the gmail client and it somehow interpreted it as a reply. In my defense: I'm travelling and changing timezones ATM thus the confusion. :)

Quoted 3 lines

Should the updated series be a reply to this email (yours), or a new v2 thread?

In general every new version of a series should start a new thread.

Quoted 8 lines

Quoted 8 lines
Please don't rebase the patches on top of my development/experimental branch - every patch needs to spend some time on the mailing list. Rebase the series on top of the latest mainline release candidate. Pull in Drew's changes and just squash my code into your patches - it was not finished anyway. You can send patches from other developers or make them part of your series - there's no problem with that as long as you keep the authorship.

Quoted 6 lines

Quoted 2 lines
That'll make it much easier to review and understand.
Fair enough. I was unsure about including others' patches in the series, so kept it minimal. Will update the series as soon as I get the chance (and know where to send it).

OK I've got your v2 - looks like something I can apply on top of v5.4-rc2, so I'll try to review it tomorrow morning.

Bart

Quoted 2 lines

Cheers,
Kent.

Linus Walleij

2019-10-16 11:52 UTC #9 lore raw

Quoted 6 lines · Bartosz Golaszewski

sob., 12 paź 2019 o 02:53 Kent Gibson <[email protected]> napisał(a):
Quoted 2 lines
Should the updated series be a reply to this email (yours), or a new v2 thread?
In general every new version of a series should start a new thread.

It's actually a very tricky problem and we discussed it at the lates maintainers summit and decided that some elaborate tooling to track patch series is needed going forward.

See:
https://lwn.net/Articles/797613/

Quoted 2 lines

OK I've got your v2 - looks like something I can apply on top of v5.4-rc2, so I'll try to review it tomorrow morning.

I give Bart some slack to look at the patches but I feel we can apply them when everyone is aligned.

Yours,
Linus Walleij