Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650308 > unrolled thread

[PATCH 00/10] gpio: mockup: updates for 4.13

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/10] gpio: mockup: updates for 4.13 Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
    [PATCH 08/10] gpio: mockup: don't return magic numbers from probe() Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
    [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
      Re: [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-27 18:50 +0200
        Re: [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-29 09:00 +0200
      Re: [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-27 18:50 +0200
      Re: [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization Linus Walleij <linus.walleij@linaro.org> - 2017-05-29 13:40 +0200
    [PATCH 02/10] gpio: mockup: add prefixes to the direction enum Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
      Re: [PATCH 02/10] gpio: mockup: add prefixes to the direction enum Linus Walleij <linus.walleij@linaro.org> - 2017-05-29 13:30 +0200
    [PATCH 07/10] gpio: mockup: improve readability Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
    [PATCH 01/10] gpio: mockup: fix direction values Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
      Re: [PATCH 01/10] gpio: mockup: fix direction values Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-27 18:50 +0200
      Re: [PATCH 01/10] gpio: mockup: fix direction values Linus Walleij <linus.walleij@linaro.org> - 2017-05-29 13:30 +0200
    [PATCH 06/10] gpio: mockup: refuse to accept an odd number of GPIO ranges Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
      Re: [PATCH 06/10] gpio: mockup: refuse to accept an odd number of  GPIO ranges Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-27 19:30 +0200
        Re: [PATCH 06/10] gpio: mockup: refuse to accept an odd number of  GPIO ranges Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-27 19:40 +0200
    [PATCH 09/10] gpio: mockup: improve the error message Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
    [PATCH 03/10] gpio: mockup: be quiet unless something goes wrong Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-25 10:40 +0200
      Re: [PATCH 03/10] gpio: mockup: be quiet unless something goes wrong Linus Walleij <linus.walleij@linaro.org> - 2017-05-29 13:30 +0200

#1650308 — [PATCH 00/10] gpio: mockup: updates for 4.13

This series contains a couple bug fixes and other minor updates for
the GPIO testing module.

Bartosz Golaszewski (10):
  gpio: mockup: fix direction values
  gpio: mockup: add prefixes to the direction enum
  gpio: mockup: be quiet unless something goes wrong
  gpio: mockup: support irqmask and irqunmask
  gpio: mockup: improve the debugfs input sanitization
  gpio: mockup: refuse to accept an odd number of GPIO ranges
  gpio: mockup: improve readability
  gpio: mockup: don't return magic numbers from probe()
  gpio: mockup: improve the error message
  gpio: mockup: add myself as author

 drivers/gpio/gpio-mockup.c | 82 ++++++++++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 28 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1650309 — [PATCH 08/10] gpio: mockup: don't return magic numbers from probe()

When the requested number of GPIO lines is 0, return -EINVAL, not
-1 which is -EPERM.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index f6efa97..3ffc3ce 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -381,7 +381,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 			ret = gpio_mockup_add(dev, &chips[i],
 					      chip_name, base, ngpio);
 		} else {
-			ret = -1;
+			ret = -EINVAL;
 		}
 
 		if (ret) {
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1650310 — [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization

We're currently only checking the first character of the input to the
debugfs event files, so a string like '0sdfdsf' is valid and indicates
a falling edge event.

Be more strict and only allow '0', '1', '0\n' & '1\n'.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index ba8d62a..b197b93 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -208,8 +208,8 @@ static ssize_t gpio_mockup_event_write(struct file *file,
 	struct seq_file *sfile;
 	struct gpio_desc *desc;
 	struct gpio_chip *gc;
+	char buf[2];
 	int val;
-	char buf;
 
 	sfile = file->private_data;
 	priv = sfile->private;
@@ -220,12 +220,18 @@ static ssize_t gpio_mockup_event_write(struct file *file,
 	if (!chip->lines[priv->offset].irq_enabled)
 		return size;
 
-	if (copy_from_user(&buf, usr_buf, 1))
+	if (size > 2)
+		return -EINVAL;
+
+	if (copy_from_user(&buf, usr_buf, 2))
 		return -EFAULT;
 
-	if (buf == '0')
+	if (size == 2 && buf[1] != '\n')
+		return -EINVAL;
+
+	if (buf[0] == '0')
 		val = 0;
-	else if (buf == '1')
+	else if (buf[0] == '1')
 		val = 1;
 	else
 		return -EINVAL;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1651935 — Re: [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization

On Sat, May 27, 2017 at 7:45 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, May 25, 2017 at 11:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> We're currently only checking the first character of the input to the
>> debugfs event files, so a string like '0sdfdsf' is valid and indicates
>> a falling edge event.
>>
>> Be more strict and only allow '0', '1', '0\n' & '1\n'.
>
> Why not to be so strict and use
>
> kstrtobool_from_user();
>
> instead?

Or if you still wish to be strict,
ret = kstrtou8_from_user();
if (ret)
 return ret;

if (val > 1)
 return -ERANGE;

or alike.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1652248 — Re: [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization

2017-05-27 18:47 GMT+02:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> On Sat, May 27, 2017 at 7:45 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Thu, May 25, 2017 at 11:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>> We're currently only checking the first character of the input to the
>>> debugfs event files, so a string like '0sdfdsf' is valid and indicates
>>> a falling edge event.
>>>
>>> Be more strict and only allow '0', '1', '0\n' & '1\n'.
>>
>> Why not to be so strict and use
>>
>> kstrtobool_from_user();
>>
>> instead?
>

Because it doesn't really make sense here - we're indicating a RISING
or FALLING edge event. This doesn't really correspond well with
boolean values IMO.

> Or if you still wish to be strict,
> ret = kstrtou8_from_user();
> if (ret)
>  return ret;
>
> if (val > 1)
>  return -ERANGE;
>
> or alike.

This one looks good, I'll include it in v2.

Thanks,
Bartosz

[toc] | [prev] | [next] | [standalone]

#1651936 — Re: [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization

On Thu, May 25, 2017 at 11:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> We're currently only checking the first character of the input to the
> debugfs event files, so a string like '0sdfdsf' is valid and indicates
> a falling edge event.
>
> Be more strict and only allow '0', '1', '0\n' & '1\n'.

Why not to be so strict and use

kstrtobool_from_user();

instead?

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1652440 — Re: [PATCH 05/10] gpio: mockup: improve the debugfs input sanitization

On Thu, May 25, 2017 at 10:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> We're currently only checking the first character of the input to the
> debugfs event files, so a string like '0sdfdsf' is valid and indicates
> a falling edge event.
>
> Be more strict and only allow '0', '1', '0\n' & '1\n'.
>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>

Since I applied patches 1-4 you only need to resend from this point
when posting v2.

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1650311 — [PATCH 02/10] gpio: mockup: add prefixes to the direction enum

All internal symbols except for the direction enum follow the same
convention and use the gpio_mockup prefix. Add the prefix to the
DIR_IN and DIR_OUT definitions as well for consistency across the
file.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index c18d011..b1ee45c 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -29,8 +29,8 @@
 #define	GPIO_MOCKUP_MAX_GC	10
 
 enum {
-	DIR_OUT = 0,
-	DIR_IN = 1,
+	GPIO_MOCKUP_DIR_OUT = 0,
+	GPIO_MOCKUP_DIR_IN = 1,
 };
 
 /*
@@ -93,7 +93,7 @@ static int gpio_mockup_dirout(struct gpio_chip *gc, unsigned int offset,
 	struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
 
 	gpio_mockup_set(gc, offset, value);
-	chip->lines[offset].dir = DIR_OUT;
+	chip->lines[offset].dir = GPIO_MOCKUP_DIR_OUT;
 
 	return 0;
 }
@@ -102,7 +102,7 @@ static int gpio_mockup_dirin(struct gpio_chip *gc, unsigned int offset)
 {
 	struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
 
-	chip->lines[offset].dir = DIR_IN;
+	chip->lines[offset].dir = GPIO_MOCKUP_DIR_IN;
 
 	return 0;
 }
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1652432 — Re: [PATCH 02/10] gpio: mockup: add prefixes to the direction enum

On Thu, May 25, 2017 at 10:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> All internal symbols except for the direction enum follow the same
> convention and use the gpio_mockup prefix. Add the prefix to the
> DIR_IN and DIR_OUT definitions as well for consistency across the
> file.
>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>

Patch applied.

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1650315 — [PATCH 07/10] gpio: mockup: improve readability

We currently shift bits here and there without actually explaining
what we're doing. Add some helper variables with names indicating
their purpose to improve the code readability.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index d771112..f6efa97 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -27,6 +27,11 @@
 
 #define GPIO_MOCKUP_NAME	"gpio-mockup"
 #define	GPIO_MOCKUP_MAX_GC	10
+/*
+ * We're storing two values per chip: the GPIO base and the number
+ * of GPIO lines.
+ */
+#define GPIO_MOCKUP_MAX_RANGES	(GPIO_MOCKUP_MAX_GC * 2)
 
 enum {
 	GPIO_MOCKUP_DIR_OUT = 0,
@@ -62,7 +67,7 @@ struct gpio_mockup_dbgfs_private {
 	int offset;
 };
 
-static int gpio_mockup_ranges[GPIO_MOCKUP_MAX_GC << 1];
+static int gpio_mockup_ranges[GPIO_MOCKUP_MAX_RANGES];
 static int gpio_mockup_params_nr;
 module_param_array(gpio_mockup_ranges, int, &gpio_mockup_params_nr, 0400);
 
@@ -341,23 +346,24 @@ static int gpio_mockup_add(struct device *dev,
 
 static int gpio_mockup_probe(struct platform_device *pdev)
 {
-	struct gpio_mockup_chip *chips;
+	int ret, i, base, ngpio, num_chips;
 	struct device *dev = &pdev->dev;
-	int ret, i, base, ngpio;
+	struct gpio_mockup_chip *chips;
 	char *chip_name;
 
 	if (gpio_mockup_params_nr < 2 || (gpio_mockup_params_nr % 2))
 		return -EINVAL;
 
-	chips = devm_kzalloc(dev,
-			     sizeof(*chips) * (gpio_mockup_params_nr >> 1),
-			     GFP_KERNEL);
+	/* Each chip is described by two values. */
+	num_chips = gpio_mockup_params_nr / 2;
+
+	chips = devm_kzalloc(dev, sizeof(*chips) * num_chips, GFP_KERNEL);
 	if (!chips)
 		return -ENOMEM;
 
 	platform_set_drvdata(pdev, chips);
 
-	for (i = 0; i < gpio_mockup_params_nr >> 1; i++) {
+	for (i = 0; i < num_chips; i++) {
 		base = gpio_mockup_ranges[i * 2];
 
 		if (base == -1)
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1650316 — [PATCH 01/10] gpio: mockup: fix direction values

The comment in linux/gpio/driver.h says:

 @get_direction: returns direction for signal "offset", 0=out, 1=in

We got those switched at some point. Fix the values.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index c6dadac..c18d011 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -29,8 +29,8 @@
 #define	GPIO_MOCKUP_MAX_GC	10
 
 enum {
-	DIR_IN = 0,
-	DIR_OUT,
+	DIR_OUT = 0,
+	DIR_IN = 1,
 };
 
 /*
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1651934 — Re: [PATCH 01/10] gpio: mockup: fix direction values

On Thu, May 25, 2017 at 11:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> The comment in linux/gpio/driver.h says:
>
>  @get_direction: returns direction for signal "offset", 0=out, 1=in
>
> We got those switched at some point. Fix the values.

Shouldn't it have a Fixes tag?


>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> ---
>  drivers/gpio/gpio-mockup.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> index c6dadac..c18d011 100644
> --- a/drivers/gpio/gpio-mockup.c
> +++ b/drivers/gpio/gpio-mockup.c
> @@ -29,8 +29,8 @@
>  #define        GPIO_MOCKUP_MAX_GC      10
>
>  enum {
> -       DIR_IN = 0,
> -       DIR_OUT,
> +       DIR_OUT = 0,
> +       DIR_IN = 1,
>  };
>
>  /*
> --
> 2.9.3
>



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1652431 — Re: [PATCH 01/10] gpio: mockup: fix direction values

On Thu, May 25, 2017 at 10:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> The comment in linux/gpio/driver.h says:
>
>  @get_direction: returns direction for signal "offset", 0=out, 1=in
>
> We got those switched at some point. Fix the values.
>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>

Patch applied.

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1650319 — [PATCH 06/10] gpio: mockup: refuse to accept an odd number of GPIO ranges

Currently we ignore the last odd range value, since each chip is
described by two values. Be more strict and require the user to
pass an even number of ranges.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index b197b93..d771112 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -346,7 +346,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	int ret, i, base, ngpio;
 	char *chip_name;
 
-	if (gpio_mockup_params_nr < 2)
+	if (gpio_mockup_params_nr < 2 || (gpio_mockup_params_nr % 2))
 		return -EINVAL;
 
 	chips = devm_kzalloc(dev,
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1651939 — Re: [PATCH 06/10] gpio: mockup: refuse to accept an odd number of GPIO ranges

On Thu, May 25, 2017 at 11:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> Currently we ignore the last odd range value, since each chip is
> described by two values. Be more strict and require the user to
> pass an even number of ranges.

> -       if (gpio_mockup_params_nr < 2)
> +       if (gpio_mockup_params_nr < 2 || (gpio_mockup_params_nr % 2))

     if (((gpio_mockup_params_nr + 1) & ~3) < 3)

3 might have a suffix regarding to the type of variable.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1651942 — Re: [PATCH 06/10] gpio: mockup: refuse to accept an odd number of GPIO ranges

On Sat, May 27, 2017 at 8:29 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, May 25, 2017 at 11:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> Currently we ignore the last odd range value, since each chip is
>> described by two values. Be more strict and require the user to
>> pass an even number of ranges.
>
>> -       if (gpio_mockup_params_nr < 2)
>> +       if (gpio_mockup_params_nr < 2 || (gpio_mockup_params_nr % 2))
>
>      if (((gpio_mockup_params_nr + 1) & ~3) < 3)
>
> 3 might have a suffix regarding to the type of variable.

Oh, ignore that, sorry, for a noise.


-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1650320 — [PATCH 09/10] gpio: mockup: improve the error message

Indicate the error number and make the message a bit more elaborate.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index 3ffc3ce..96a6c6e 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -385,8 +385,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 		}
 
 		if (ret) {
-			dev_err(dev, "gpio<%d..%d> add failed\n",
-				base, base < 0 ? ngpio : base + ngpio);
+			dev_err(dev,
+				"adding gpiochip failed: %d (base: %d, ngpio: %d)\n",
+				ret, base, base < 0 ? ngpio : base + ngpio);
 
 			return ret;
 		}
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1650322 — [PATCH 03/10] gpio: mockup: be quiet unless something goes wrong

When inserting and removing the module repeatedly (e.g. when running
the libgpiod test-suite) the kernel log gets clobbered with messages
reporting successful creation of dummy gpiochips.

Remove this message and only emit logs when something bad happens.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index b1ee45c..c17578e 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -364,9 +364,6 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 
 			return ret;
 		}
-
-		dev_info(dev, "gpio<%d..%d> add successful!",
-			 base, base + ngpio);
 	}
 
 	return 0;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1652429 — Re: [PATCH 03/10] gpio: mockup: be quiet unless something goes wrong

On Thu, May 25, 2017 at 10:33 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> When inserting and removing the module repeatedly (e.g. when running
> the libgpiod test-suite) the kernel log gets clobbered with messages
> reporting successful creation of dummy gpiochips.
>
> Remove this message and only emit logs when something bad happens.
>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>

Patch applied.

Yours,
Linus Walleij

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web