gpio: mockup: updates for v4.16

PATCH In review Bartosz Golaszewski 2 participants 20 messages Nov 27, 2017 – Dec 1, 2017

Subsystems (2)

GPIO GPIO mockup driver

lore

Share & tools

lore

Bartosz Golaszewski

author

2017-11-27 10:49 UTC #0 lore raw

cover gpio: mockup: updates for v4.16

The following series contains a number of updates for the gpio-mockup module.

The most significant changes are:
- rework the way we probe the devices: use a separate platform device for every mockup chip, pass the chip configuration over platform data to probe()
- parse the module params in the init function, not in probe()
- extend the debugfs layout for event generation
- improve the module params sanitization
- implement the set_multiple callback

Others are mostly less significant fixes and minor coding style tweaks.

Bartosz Golaszewski (18):
gpio: mockup: add missing prefixes
gpio: mockup: parse the module params in init, not probe
gpio: mockup: verify the number of GPIO chips requested
gpio: mockup: rework device probing
gpio: mockup: implement gpio_mockup_err()
gpio: mockup: remove a stray tab
gpio: mockup: merge gpio_mockup_add() into gpio_mockup_probe()
gpio: mockup: pass the named_lines parameter over platform_data
gpio: mockup: extend the debugfs layout
gpio: mockup: change the type of value field in line state struct
gpio: mockup: group code by logic
gpio: mockup: fix debugfs handling
gpio: mockup: verify that ngpio > 0
gpio: mockup: tweak line breaks
gpio: mockup: implement gpio_mockup_set_multiple()
gpio: mockup: modify the return value check for devm_irq_sim_init()
gpio: mockup: rename gpio_mockup_params_nr to gpio_mockup_num_ranges
gpio: mockup: add helpers for accessing the gpio ranges

 drivers/gpio/gpio-mockup.c | 288 ++++++++++++++++++++++++++-------------------
 1 file changed, 166 insertions(+), 122 deletions(-)

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #1 lore raw

PATCH 1/18 gpio: mockup: add missing prefixes

Add the gpio_mockup_ prefix to the remaining symbols that still don't have it, so that the entire driver code is consistent.

Signed-off-by Bartosz Golaszewski <[email protected]>

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

drivers/gpio/gpio-mockup.c +12 ~~-11~~

index 9532d86a82f7..d21c6d651287 100644
@@ -327,8 +327,9 @@ static struct platform_driver gpio_mockup_driver = {
 	.probe = gpio_mockup_probe,
 };
 
-static struct platform_device *pdev;
-static int __init mock_device_init(void)
+static struct platform_device *gpio_mockup_pdev;
+
+static int __init gpio_mockup_init(void)
 {
 	int err;

@@ -337,34 +338,34 @@ static int __init mock_device_init(void)
 		pr_err("%s: error creating debugfs directory\n",
 		       GPIO_MOCKUP_NAME);
 
-	pdev = platform_device_alloc(GPIO_MOCKUP_NAME, -1);
-	if (!pdev)
+	gpio_mockup_pdev = platform_device_alloc(GPIO_MOCKUP_NAME, -1);
+	if (!gpio_mockup_pdev)
 		return -ENOMEM;
 
-	err = platform_device_add(pdev);
+	err = platform_device_add(gpio_mockup_pdev);
 	if (err) {
-		platform_device_put(pdev);
+		platform_device_put(gpio_mockup_pdev);
 		return err;
 	}
 
 	err = platform_driver_register(&gpio_mockup_driver);
 	if (err) {
-		platform_device_unregister(pdev);
+		platform_device_unregister(gpio_mockup_pdev);
 		return err;
 	}
 
 	return 0;
 }
 
-static void __exit mock_device_exit(void)
+static void __exit gpio_mockup_exit(void)
 {
 	debugfs_remove_recursive(gpio_mockup_dbg_dir);
 	platform_driver_unregister(&gpio_mockup_driver);
-	platform_device_unregister(pdev);
+	platform_device_unregister(gpio_mockup_pdev);
 }
 
-module_init(mock_device_init);
-module_exit(mock_device_exit);
+module_init(gpio_mockup_init);
+module_exit(gpio_mockup_exit);
 
 MODULE_AUTHOR("Kamlakant Patel <[email protected]>");
 MODULE_AUTHOR("Bamvor Jian Zhang <[email protected]>");

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #2 lore raw

PATCH 2/18 gpio: mockup: parse the module params in init, not probe

In reply to Bartosz Golaszewski

If the module parameters are invalid, we should bail out from the init function instead of detecting it during the device probe. That way we don't even allow the user to load the module if we don't accept the arguments.

Signed-off-by Bartosz Golaszewski <[email protected]>

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

drivers/gpio/gpio-mockup.c +3 -3

index d21c6d651287..b70f3b0c30b1 100644
@@ -275,9 +275,6 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	struct gpio_mockup_chip *chips;
 	char *chip_name;
 
-	if (gpio_mockup_params_nr < 2 || (gpio_mockup_params_nr % 2))
-		return -EINVAL;
-
 	/* Each chip is described by two values. */
 	num_chips = gpio_mockup_params_nr / 2;

@@ -333,6 +330,9 @@ static int __init gpio_mockup_init(void)
 {
 	int err;
 
+	if (gpio_mockup_params_nr < 2 || (gpio_mockup_params_nr % 2))
+		return -EINVAL;
+
 	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
 	if (!gpio_mockup_dbg_dir)
 		pr_err("%s: error creating debugfs directory\n",

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #3 lore raw

PATCH 3/18 gpio: mockup: verify the number of GPIO chips requested

In reply to Bartosz Golaszewski

The number of supported mockup chips is limited. Check this limit when parsing the module parameters.

Also: make sure that each chip is described with a <base - ngpio> pair.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/gpio/gpio-mockup.c +3 -1

index b70f3b0c30b1..0a269cbe197c 100644
@@ -330,7 +330,9 @@ static int __init gpio_mockup_init(void)
 {
 	int err;
 
-	if (gpio_mockup_params_nr < 2 || (gpio_mockup_params_nr % 2))
+	if ((gpio_mockup_params_nr < 2) ||
+	    (gpio_mockup_params_nr % 2) ||
+	    (gpio_mockup_params_nr > GPIO_MOCKUP_MAX_RANGES))
 		return -EINVAL;
 
 	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #4 lore raw

PATCH 4/18 gpio: mockup: rework device probing

In reply to Bartosz Golaszewski

We currently create a single platform device in init and then parse the configuration passed to us via module parameters in probe() before creating GPIO chips and registering them with the gpiolib framework.

The relation between platform devices and mockup chips should be 1:1.

Create a separate platform device for each mockup chip using convenient helpers (platform_device_register_resndata()). Pass a platform data structure to probe() in which the configuration (GPIO base, number of lines, chip index) extracted from the module params is stored. Make probe() create a single mockup chip for every platform device.

This approach has several advantages:
- we only parse the module parameters in init() and can bail out before attaching any device if the input is invalid (currently we would have to examine kernel logs),
- we'll get notified by the device framework about errors in probe() for specific chips,
- probe() gets simplified and only does what it's supposed to.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 122 ++++++++++++++++++++++++++-------------------
 1 file changed, 71 insertions(+), 51 deletions(-)

drivers/gpio/gpio-mockup.c +71 ~~-51~~

index 0a269cbe197c..5ba8f2089de8 100644
@@ -62,6 +62,12 @@ struct gpio_mockup_dbgfs_private {
 	int offset;
 };
 
+struct gpio_mockup_platform_data {
+	int base;
+	int ngpio;
+	int index;
+};
+
 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);

@@ -70,7 +76,6 @@ static bool gpio_mockup_named_lines;
 module_param_named(gpio_mockup_named_lines,
 		   gpio_mockup_named_lines, bool, 0400);
 
-static const char gpio_mockup_name_start = 'A';
 static struct dentry *gpio_mockup_dbg_dir;
 
 static int gpio_mockup_get(struct gpio_chip *gc, unsigned int offset)

@@ -270,48 +275,32 @@ static int gpio_mockup_add(struct device *dev,
 
 static int gpio_mockup_probe(struct platform_device *pdev)
 {
-	int ret, i, base, ngpio, num_chips;
-	struct device *dev = &pdev->dev;
-	struct gpio_mockup_chip *chips;
-	char *chip_name;
+	struct gpio_mockup_platform_data *pdata;
+	struct gpio_mockup_chip *chip;
+	int rv, base, ngpio;
+	struct device *dev;
+	char *name;
 
-	/* Each chip is described by two values. */
-	num_chips = gpio_mockup_params_nr / 2;
+	dev = &pdev->dev;
+	pdata = dev_get_platdata(dev);
+	base = pdata->base;
+	ngpio = pdata->ngpio;
 
-	chips = devm_kcalloc(dev, num_chips, sizeof(*chips), GFP_KERNEL);
-	if (!chips)
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
 		return -ENOMEM;
 
-	platform_set_drvdata(pdev, chips);
-
-	for (i = 0; i < num_chips; i++) {
-		base = gpio_mockup_ranges[i * 2];
-
-		if (base == -1)
-			ngpio = gpio_mockup_ranges[i * 2 + 1];
-		else
-			ngpio = gpio_mockup_ranges[i * 2 + 1] - base;
-
-		if (ngpio >= 0) {
-			chip_name = devm_kasprintf(dev, GFP_KERNEL,
-						   "%s-%c", GPIO_MOCKUP_NAME,
-						   gpio_mockup_name_start + i);
-			if (!chip_name)
-				return -ENOMEM;
-
-			ret = gpio_mockup_add(dev, &chips[i],
-					      chip_name, base, ngpio);
-		} else {
-			ret = -EINVAL;
-		}
-
-		if (ret) {
-			dev_err(dev,
-				"adding gpiochip failed: %d (base: %d, ngpio: %d)\n",
-				ret, base, base < 0 ? ngpio : base + ngpio);
+	name = devm_kasprintf(dev, GFP_KERNEL, "%s-%c",
+			      pdev->name, pdata->index);
+	if (!name)
+		return -ENOMEM;
 
-			return ret;
-		}
+	rv = gpio_mockup_add(dev, chip, name, base, ngpio);
+	if (rv) {
+		dev_err(dev,
+			"adding gpiochip failed (base: %d, ngpio: %d)\n",
+			base, base < 0 ? ngpio : base + ngpio);
+		return rv;
 	}
 
 	return 0;

@@ -324,36 +313,67 @@ static struct platform_driver gpio_mockup_driver = {
 	.probe = gpio_mockup_probe,
 };
 
-static struct platform_device *gpio_mockup_pdev;
+static struct platform_device *gpio_mockup_pdevs[GPIO_MOCKUP_MAX_GC];
+
+static void gpio_mockup_unregister_pdevs(void)
+{
+	struct platform_device *pdev;
+	int i;
+
+	for (i = 0; i < GPIO_MOCKUP_MAX_GC; i++) {
+		pdev = gpio_mockup_pdevs[i];
+
+		if (pdev)
+			platform_device_unregister(pdev);
+	}
+}
 
 static int __init gpio_mockup_init(void)
 {
-	int err;
+	int i, num_chips, err = 0, index = 'A';
+	struct gpio_mockup_platform_data pdata;
+	struct platform_device *pdev;
 
 	if ((gpio_mockup_params_nr < 2) ||
 	    (gpio_mockup_params_nr % 2) ||
 	    (gpio_mockup_params_nr > GPIO_MOCKUP_MAX_RANGES))
 		return -EINVAL;
 
+	/* Each chip is described by two values. */
+	num_chips = gpio_mockup_params_nr / 2;
+
 	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
 	if (!gpio_mockup_dbg_dir)
 		pr_err("%s: error creating debugfs directory\n",
 		       GPIO_MOCKUP_NAME);
 
-	gpio_mockup_pdev = platform_device_alloc(GPIO_MOCKUP_NAME, -1);
-	if (!gpio_mockup_pdev)
-		return -ENOMEM;
-
-	err = platform_device_add(gpio_mockup_pdev);
+	err = platform_driver_register(&gpio_mockup_driver);
 	if (err) {
-		platform_device_put(gpio_mockup_pdev);
+		pr_err("%s: error registering platform driver\n",
+		       GPIO_MOCKUP_NAME);
 		return err;
 	}
 
-	err = platform_driver_register(&gpio_mockup_driver);
-	if (err) {
-		platform_device_unregister(gpio_mockup_pdev);
-		return err;
+	for (i = 0; i < num_chips; i++) {
+		pdata.index = index++;
+		pdata.base = gpio_mockup_ranges[i * 2];
+		pdata.ngpio = pdata.base < 0
+				? gpio_mockup_ranges[i * 2 + 1]
+				: gpio_mockup_ranges[i * 2 + 1] - pdata.base;
+
+		pdev = platform_device_register_resndata(NULL,
+							 GPIO_MOCKUP_NAME,
+							 i, NULL, 0, &pdata,
+							 sizeof(pdata));
+		if (!pdev) {
+			pr_err("%s: error registering device",
+			       GPIO_MOCKUP_NAME);
+			platform_driver_unregister(&gpio_mockup_driver);
+			gpio_mockup_unregister_pdevs();
+			return -ENOMEM;
+		}
+
+		gpio_mockup_pdevs[i] = pdev;
 	}
 
 	return 0;

@@ -363,7 +383,7 @@ static void __exit gpio_mockup_exit(void)
 {
 	debugfs_remove_recursive(gpio_mockup_dbg_dir);
 	platform_driver_unregister(&gpio_mockup_driver);
-	platform_device_unregister(gpio_mockup_pdev);
+	gpio_mockup_unregister_pdevs();
 }
 
 module_init(gpio_mockup_init);

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:49 UTC #5 lore raw

PATCH 5/18 gpio: mockup: implement gpio_mockup_err()

In reply to Bartosz Golaszewski

Visually shrink the pr_err() calls by encapsulating adding the module name prefix to the message in a macro.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

drivers/gpio/gpio-mockup.c +5 -6

index 5ba8f2089de8..0aa68242f33b 100644
@@ -34,6 +34,8 @@
  */
 #define GPIO_MOCKUP_MAX_RANGES	(GPIO_MOCKUP_MAX_GC * 2)
 
+#define gpio_mockup_err(...)	pr_err(GPIO_MOCKUP_NAME ": " __VA_ARGS__)
+
 enum {
 	GPIO_MOCKUP_DIR_OUT = 0,
 	GPIO_MOCKUP_DIR_IN = 1,

@@ -344,13 +346,11 @@ static int __init gpio_mockup_init(void)
 
 	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
 	if (!gpio_mockup_dbg_dir)
-		pr_err("%s: error creating debugfs directory\n",
-		       GPIO_MOCKUP_NAME);
+		gpio_mockup_err("error creating debugfs directory\n");
 
 	err = platform_driver_register(&gpio_mockup_driver);
 	if (err) {
-		pr_err("%s: error registering platform driver\n",
-		       GPIO_MOCKUP_NAME);
+		gpio_mockup_err("error registering platform driver\n");
 		return err;
 	}

@@ -366,8 +366,7 @@ static int __init gpio_mockup_init(void)
 							 i, NULL, 0, &pdata,
 							 sizeof(pdata));
 		if (!pdev) {
-			pr_err("%s: error registering device",
-			       GPIO_MOCKUP_NAME);
+			gpio_mockup_err("error registering device");
 			platform_driver_unregister(&gpio_mockup_driver);
 			gpio_mockup_unregister_pdevs();
 			return -ENOMEM;

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #6 lore raw

PATCH 6/18 gpio: mockup: remove a stray tab

In reply to Bartosz Golaszewski

Minor coding style fix.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpio-mockup.c +1 -1

index 0aa68242f33b..9b2823a33538 100644
@@ -27,7 +27,7 @@
 #include "gpiolib.h"
 
 #define GPIO_MOCKUP_NAME	"gpio-mockup"
-#define	GPIO_MOCKUP_MAX_GC	10
+#define GPIO_MOCKUP_MAX_GC	10
 /*
  * We're storing two values per chip: the GPIO base and the number
  * of GPIO lines.

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #7 lore raw

PATCH 7/18 gpio: mockup: merge gpio_mockup_add() into gpio_mockup_probe()

In reply to Bartosz Golaszewski

Now that the probe() function only does what is should, there's no need to split the chip adding logic into a separate routine. Merge gpio_mockup_add() into gpio_mockup_probe().

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 78 ++++++++++++++++++----------------------------
 1 file changed, 31 insertions(+), 47 deletions(-)

drivers/gpio/gpio-mockup.c +31 ~~-47~~

index 9b2823a33538..2dbaeb69d2d2 100644
@@ -231,13 +231,30 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 	dev_err(dev, "error creating debugfs directory\n");
 }
 
-static int gpio_mockup_add(struct device *dev,
-			   struct gpio_mockup_chip *chip,
-			   const char *name, int base, int ngpio)
+static int gpio_mockup_probe(struct platform_device *pdev)
 {
-	struct gpio_chip *gc = &chip->gc;
-	int ret;
+	struct gpio_mockup_platform_data *pdata;
+	struct gpio_mockup_chip *chip;
+	struct gpio_chip *gc;
+	int rv, base, ngpio;
+	struct device *dev;
+	char *name;
+
+	dev = &pdev->dev;
+	pdata = dev_get_platdata(dev);
+	base = pdata->base;
+	ngpio = pdata->ngpio;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "%s-%c",
+			      pdev->name, pdata->index);
+	if (!name)
+		return -ENOMEM;
 
+	gc = &chip->gc;
 	gc->base = base;
 	gc->ngpio = ngpio;
 	gc->label = name;

@@ -256,18 +273,18 @@ static int gpio_mockup_add(struct device *dev,
 		return -ENOMEM;
 
 	if (gpio_mockup_named_lines) {
-		ret = gpio_mockup_name_lines(dev, chip);
-		if (ret)
-			return ret;
+		rv = gpio_mockup_name_lines(dev, chip);
+		if (rv)
+			return rv;
 	}
 
-	ret = devm_irq_sim_init(dev, &chip->irqsim, gc->ngpio);
-	if (ret)
-		return ret;
+	rv = devm_irq_sim_init(dev, &chip->irqsim, gc->ngpio);
+	if (rv)
+		return rv;
 
-	ret = devm_gpiochip_add_data(dev, &chip->gc, chip);
-	if (ret)
-		return ret;
+	rv = devm_gpiochip_add_data(dev, &chip->gc, chip);
+	if (rv)
+		return rv;
 
 	if (gpio_mockup_dbg_dir)
 		gpio_mockup_debugfs_setup(dev, chip);

@@ -275,39 +292,6 @@ static int gpio_mockup_add(struct device *dev,
 	return 0;
 }
 
-static int gpio_mockup_probe(struct platform_device *pdev)
-{
-	struct gpio_mockup_platform_data *pdata;
-	struct gpio_mockup_chip *chip;
-	int rv, base, ngpio;
-	struct device *dev;
-	char *name;
-
-	dev = &pdev->dev;
-	pdata = dev_get_platdata(dev);
-	base = pdata->base;
-	ngpio = pdata->ngpio;
-
-	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
-	if (!chip)
-		return -ENOMEM;
-
-	name = devm_kasprintf(dev, GFP_KERNEL, "%s-%c",
-			      pdev->name, pdata->index);
-	if (!name)
-		return -ENOMEM;
-
-	rv = gpio_mockup_add(dev, chip, name, base, ngpio);
-	if (rv) {
-		dev_err(dev,
-			"adding gpiochip failed (base: %d, ngpio: %d)\n",
-			base, base < 0 ? ngpio : base + ngpio);
-		return rv;
-	}
-
-	return 0;
-}
-
 static struct platform_driver gpio_mockup_driver = {
 	.driver = {
 		.name = GPIO_MOCKUP_NAME,

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #8 lore raw

PATCH 8/18 gpio: mockup: pass the named_lines parameter over platform_data

In reply to Bartosz Golaszewski

Move the last bits of code dealing with module parameters to the init function. Add a new variable to platform data, which indicates to the probe function if it should name the GPIO lines. If we ever want to make the line naming more fine-grained (e.g. per chip switch) it will be easier this way.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/gpio/gpio-mockup.c +3 -1

index 2dbaeb69d2d2..9da90ad87030 100644
@@ -68,6 +68,7 @@ struct gpio_mockup_platform_data {
 	int base;
 	int ngpio;
 	int index;
+	bool named_lines;
 };
 
 static int gpio_mockup_ranges[GPIO_MOCKUP_MAX_RANGES];

@@ -272,7 +273,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	if (!chip->lines)
 		return -ENOMEM;
 
-	if (gpio_mockup_named_lines) {
+	if (pdata->named_lines) {
 		rv = gpio_mockup_name_lines(dev, chip);
 		if (rv)
 			return rv;

@@ -344,6 +345,7 @@ static int __init gpio_mockup_init(void)
 		pdata.ngpio = pdata.base < 0
 				? gpio_mockup_ranges[i * 2 + 1]
 				: gpio_mockup_ranges[i * 2 + 1] - pdata.base;
+		pdata.named_lines = gpio_mockup_named_lines;
 
 		pdev = platform_device_register_resndata(NULL,
 							 GPIO_MOCKUP_NAME,

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #9 lore raw

PATCH 9/18 gpio: mockup: extend the debugfs layout

In reply to Bartosz Golaszewski

Currently each chip has a dedicated directory in debugfs for event triggers. We use the chip's label for the directory name, but the user can't really associate these directories with chip names without parsing the relevant sysfs entries.

Use chip names for directory names. For backward compatibility: create links pointing to the actual directories named using the chip labels.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

drivers/gpio/gpio-mockup.c +8 -2

index 9da90ad87030..9a880e15ea0b 100644
@@ -196,17 +196,23 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 				      struct gpio_mockup_chip *chip)
 {
 	struct gpio_mockup_dbgfs_private *priv;
-	struct dentry *evfile;
+	struct dentry *evfile, *link;
 	struct gpio_chip *gc;
+	const char *devname;
 	char *name;
 	int i;
 
 	gc = &chip->gc;
+	devname = dev_name(&gc->gpiodev->dev);
 
-	chip->dbg_dir = debugfs_create_dir(gc->label, gpio_mockup_dbg_dir);
+	chip->dbg_dir = debugfs_create_dir(devname, gpio_mockup_dbg_dir);
 	if (!chip->dbg_dir)
 		goto err;
 
+	link = debugfs_create_symlink(gc->label, gpio_mockup_dbg_dir, devname);
+	if (!link)
+		goto err;
+
 	for (i = 0; i < gc->ngpio; i++) {
 		name = devm_kasprintf(dev, GFP_KERNEL, "%d", i);
 		if (!name)

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:49 UTC #10 lore raw

PATCH 10/18 gpio: mockup: change the type of value field in line state struct

In reply to Bartosz Golaszewski

GPIO values are universally represented as integers. Change the type of the variable storing the current line value to int for consistency.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpio-mockup.c +1 -1

index 9a880e15ea0b..8a61328a255d 100644
@@ -48,7 +48,7 @@ enum {
  */
 struct gpio_mockup_line_status {
 	int dir;
-	bool value;
+	int value;
 };
 
 struct gpio_mockup_chip {

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #11 lore raw

PATCH 11/18 gpio: mockup: group code by logic

In reply to Bartosz Golaszewski

Keep GPIO chip callbacks, event trigger callbacks and mockup chip setup code visibly separated. We're mostly good - just need to move the line naming routine below.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

drivers/gpio/gpio-mockup.c +23 ~~-23~~

index 8a61328a255d..dba5e79e3278 100644
@@ -123,29 +123,6 @@ static int gpio_mockup_get_direction(struct gpio_chip *gc, unsigned int offset)
 	return chip->lines[offset].dir;
 }
 
-static int gpio_mockup_name_lines(struct device *dev,
-				  struct gpio_mockup_chip *chip)
-{
-	struct gpio_chip *gc = &chip->gc;
-	char **names;
-	int i;
-
-	names = devm_kcalloc(dev, gc->ngpio, sizeof(char *), GFP_KERNEL);
-	if (!names)
-		return -ENOMEM;
-
-	for (i = 0; i < gc->ngpio; i++) {
-		names[i] = devm_kasprintf(dev, GFP_KERNEL,
-					  "%s-%d", gc->label, i);
-		if (!names[i])
-			return -ENOMEM;
-	}
-
-	gc->names = (const char *const *)names;
-
-	return 0;
-}
-
 static int gpio_mockup_to_irq(struct gpio_chip *gc, unsigned int offset)
 {
 	struct gpio_mockup_chip *chip = gpiochip_get_data(gc);

@@ -238,6 +215,29 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 	dev_err(dev, "error creating debugfs directory\n");
 }
 
+static int gpio_mockup_name_lines(struct device *dev,
+				  struct gpio_mockup_chip *chip)
+{
+	struct gpio_chip *gc = &chip->gc;
+	char **names;
+	int i;
+
+	names = devm_kcalloc(dev, gc->ngpio, sizeof(char *), GFP_KERNEL);
+	if (!names)
+		return -ENOMEM;
+
+	for (i = 0; i < gc->ngpio; i++) {
+		names[i] = devm_kasprintf(dev, GFP_KERNEL,
+					  "%s-%d", gc->label, i);
+		if (!names[i])
+			return -ENOMEM;
+	}
+
+	gc->names = (const char *const *)names;
+
+	return 0;
+}
+
 static int gpio_mockup_probe(struct platform_device *pdev)
 {
 	struct gpio_mockup_platform_data *pdata;

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #12 lore raw

PATCH 12/18 gpio: mockup: fix debugfs handling

In reply to Bartosz Golaszewski

The debugfs routines returning pointers can return NULL or error codes embedded with ERR_PTR(). Check the return values with IS_ERR_OR_NULL().

While we're at it: make the error message more specific so it's not confused with the one emitted when the top-level gpio-mockup debugfs directory creation fails.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/gpio/gpio-mockup.c +5 -5

index dba5e79e3278..68a0c1e06a16 100644
@@ -183,11 +183,11 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 	devname = dev_name(&gc->gpiodev->dev);
 
 	chip->dbg_dir = debugfs_create_dir(devname, gpio_mockup_dbg_dir);
-	if (!chip->dbg_dir)
+	if (IS_ERR_OR_NULL(chip->dbg_dir))
 		goto err;
 
 	link = debugfs_create_symlink(gc->label, gpio_mockup_dbg_dir, devname);
-	if (!link)
+	if (IS_ERR_OR_NULL(link))
 		goto err;
 
 	for (i = 0; i < gc->ngpio; i++) {

@@ -205,14 +205,14 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 
 		evfile = debugfs_create_file(name, 0200, chip->dbg_dir, priv,
 					     &gpio_mockup_event_ops);
-		if (!evfile)
+		if (IS_ERR_OR_NULL(evfile))
 			goto err;
 	}
 
 	return;
 
 err:
-	dev_err(dev, "error creating debugfs directory\n");
+	dev_err(dev, "error creating debugfs event files\n");
 }
 
 static int gpio_mockup_name_lines(struct device *dev,

@@ -336,7 +336,7 @@ static int __init gpio_mockup_init(void)
 	num_chips = gpio_mockup_params_nr / 2;
 
 	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
-	if (!gpio_mockup_dbg_dir)
+	if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
 		gpio_mockup_err("error creating debugfs directory\n");
 
 	err = platform_driver_register(&gpio_mockup_driver);

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:49 UTC #13 lore raw

PATCH 13/18 gpio: mockup: verify that ngpio > 0

In reply to Bartosz Golaszewski

Improve the module params sanitization: bail out from init if the user tries to pass a non-positive number of GPIO lines for any mockup chip.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 9 +++++++++
 1 file changed, 9 insertions(+)

drivers/gpio/gpio-mockup.c +9 -0

index 68a0c1e06a16..532f5807390b 100644
@@ -335,6 +335,15 @@ static int __init gpio_mockup_init(void)
 	/* Each chip is described by two values. */
 	num_chips = gpio_mockup_params_nr / 2;
 
+	/*
+	 * The second value in the <base GPIO - number of GPIOS> pair must
+	 * always be greater than 0.
+	 */
+	for (i = 0; i < num_chips; i++) {
+		if (gpio_mockup_ranges[i * 2 + 1] < 0)
+			return -EINVAL;
+	}
+
 	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
 	if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
 		gpio_mockup_err("error creating debugfs directory\n");

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #14 lore raw

PATCH 14/18 gpio: mockup: tweak line breaks

In reply to Bartosz Golaszewski

Minor readability tweak: prefer breaking the lines in a way where the second part is longer than the first.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/gpio/gpio-mockup.c +4 -4

index 532f5807390b..dfb9ee03a2f0 100644
@@ -88,16 +88,16 @@ static int gpio_mockup_get(struct gpio_chip *gc, unsigned int offset)
 	return chip->lines[offset].value;
 }
 
-static void gpio_mockup_set(struct gpio_chip *gc, unsigned int offset,
-			    int value)
+static void gpio_mockup_set(struct gpio_chip *gc,
+			    unsigned int offset, int value)
 {
 	struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
 
 	chip->lines[offset].value = !!value;
 }
 
-static int gpio_mockup_dirout(struct gpio_chip *gc, unsigned int offset,
-			      int value)
+static int gpio_mockup_dirout(struct gpio_chip *gc,
+			      unsigned int offset, int value)
 {
 	struct gpio_mockup_chip *chip = gpiochip_get_data(gc);

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #15 lore raw

PATCH 15/18 gpio: mockup: implement gpio_mockup_set_multiple()

In reply to Bartosz Golaszewski

Implement the set_multiple() callback and register it with the gpiolib framework. This is only meant to also test the internal kernel API.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/gpio/gpio-mockup.c +11 -0

index dfb9ee03a2f0..cbc823e43151 100644
@@ -96,6 +96,16 @@ static void gpio_mockup_set(struct gpio_chip *gc,
 	chip->lines[offset].value = !!value;
 }
 
+static void gpio_mockup_set_multiple(struct gpio_chip *gc,
+				     unsigned long *mask, unsigned long *bits)
+{
+	unsigned int bit;
+
+	for_each_set_bit(bit, mask, gc->ngpio)
+		gpio_mockup_set(gc, bit, test_bit(bit, bits));
+
+}
+
 static int gpio_mockup_dirout(struct gpio_chip *gc,
 			      unsigned int offset, int value)
 {

@@ -269,6 +279,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	gc->parent = dev;
 	gc->get = gpio_mockup_get;
 	gc->set = gpio_mockup_set;
+	gc->set_multiple = gpio_mockup_set_multiple;
 	gc->direction_output = gpio_mockup_dirout;
 	gc->direction_input = gpio_mockup_dirin;
 	gc->get_direction = gpio_mockup_get_direction;

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:49 UTC #16 lore raw

PATCH 16/18 gpio: mockup: modify the return value check for devm_irq_sim_init()

In reply to Bartosz Golaszewski

As discussed with Marc Zyngier: irq_sim_init() and its devres variant should return the base of the allocated interrupt range on success rather than 0. This will be modified later - first, change the way users handle the return value of these routines.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpio-mockup.c +1 -1

index cbc823e43151..0abb53038ba8 100644
@@ -297,7 +297,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	}
 
 	rv = devm_irq_sim_init(dev, &chip->irqsim, gc->ngpio);
-	if (rv)
+	if (rv < 0)
 		return rv;
 
 	rv = devm_gpiochip_add_data(dev, &chip->gc, chip);

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #17 lore raw

PATCH 17/18 gpio: mockup: rename gpio_mockup_params_nr to gpio_mockup_num_ranges

In reply to Bartosz Golaszewski

This variable holds the number of mockup GPIO ranges so rename it accordingly.

Signed-off-by Bartosz Golaszewski <[email protected]>

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

drivers/gpio/gpio-mockup.c +6 -6

index 0abb53038ba8..a6bfbe58bc63 100644
@@ -72,8 +72,8 @@ struct gpio_mockup_platform_data {
 };
 
 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);
+static int gpio_mockup_num_ranges;
+module_param_array(gpio_mockup_ranges, int, &gpio_mockup_num_ranges, 0400);
 
 static bool gpio_mockup_named_lines;
 module_param_named(gpio_mockup_named_lines,

@@ -338,13 +338,13 @@ static int __init gpio_mockup_init(void)
 	struct gpio_mockup_platform_data pdata;
 	struct platform_device *pdev;
 
-	if ((gpio_mockup_params_nr < 2) ||
-	    (gpio_mockup_params_nr % 2) ||
-	    (gpio_mockup_params_nr > GPIO_MOCKUP_MAX_RANGES))
+	if ((gpio_mockup_num_ranges < 2) ||
+	    (gpio_mockup_num_ranges % 2) ||
+	    (gpio_mockup_num_ranges > GPIO_MOCKUP_MAX_RANGES))
 		return -EINVAL;
 
 	/* Each chip is described by two values. */
-	num_chips = gpio_mockup_params_nr / 2;
+	num_chips = gpio_mockup_num_ranges / 2;
 
 	/*
 	 * The second value in the <base GPIO - number of GPIOS> pair must

Signature · 1 line

2.15.0

Bartosz Golaszewski

author

2017-11-27 10:48 UTC #18 lore raw

PATCH 18/18 gpio: mockup: add helpers for accessing the gpio ranges

In reply to Bartosz Golaszewski

In order to avoid repeating the calculations on every access - add helpers for gpio base and ngpio components of the ranges array.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

drivers/gpio/gpio-mockup.c +14 -4

index a6bfbe58bc63..ea8c730d8af1 100644
@@ -81,6 +81,16 @@ module_param_named(gpio_mockup_named_lines,
 
 static struct dentry *gpio_mockup_dbg_dir;
 
+static int gpio_mockup_range_base(unsigned int index)
+{
+	return gpio_mockup_ranges[index * 2];
+}
+
+static int gpio_mockup_range_ngpio(unsigned int index)
+{
+	return gpio_mockup_ranges[index * 2 + 1];
+}
+
 static int gpio_mockup_get(struct gpio_chip *gc, unsigned int offset)
 {
 	struct gpio_mockup_chip *chip = gpiochip_get_data(gc);

@@ -351,7 +361,7 @@ static int __init gpio_mockup_init(void)
 	 * always be greater than 0.
 	 */
 	for (i = 0; i < num_chips; i++) {
-		if (gpio_mockup_ranges[i * 2 + 1] < 0)
+		if (gpio_mockup_range_ngpio(i) < 0)
 			return -EINVAL;
 	}

@@ -367,10 +377,10 @@ static int __init gpio_mockup_init(void)
 
 	for (i = 0; i < num_chips; i++) {
 		pdata.index = index++;
-		pdata.base = gpio_mockup_ranges[i * 2];
+		pdata.base = gpio_mockup_range_base(i);
 		pdata.ngpio = pdata.base < 0
-				? gpio_mockup_ranges[i * 2 + 1]
-				: gpio_mockup_ranges[i * 2 + 1] - pdata.base;
+				? gpio_mockup_range_ngpio(i)
+				: gpio_mockup_range_ngpio(i) - pdata.base;
 		pdata.named_lines = gpio_mockup_named_lines;
 
 		pdev = platform_device_register_resndata(NULL,

Signature · 1 line

2.15.0

Linus Walleij

2017-12-01 19:38 UTC #19 lore raw

In reply to Bartosz Golaszewski

Quoted 2 lines · Bartosz Golaszewski

The following series contains a number of updates for the gpio-mockup module.

All 13 patches applied. Excellent work!

Yours,
Linus Walleij