* [PATCH 0/6] gpiolib refactorings after device addition
@ 2016-02-11 10:26 Linus Walleij
2016-02-11 10:26 ` [PATCH 1/6] gpio: remember to finally free gpio_device Linus Walleij
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Linus Walleij @ 2016-02-11 10:26 UTC (permalink / raw)
To: linux-gpio, Alexandre Courbot, Johan Hovold, Michael Welling,
Markus Pargmann
Cc: Bamvor Jian Zhang, Grant Likely, Linus Walleij
This is a series of refactorings which is basically internalizing
some of the stuff from the gpio_chip managed and owned by each
subdriver over into the gpio_device which is managed and owned by
the gpiolib core.
I will merge them soon-ish unless there are major comments, as it
IMO completes a step in the transition to the new device and
chardev-backed gpiolib.
Linus Walleij (6):
gpio: remember to finally free gpio_device
gpio: move sysfs mock device to the gpio_device
gpio: move descriptors into gpio_device
gpio: reflect base and ngpio into gpio_device
gpio: reference count the gpio device for each desc
gpio: move the pin ranges into gpio_device
drivers/gpio/gpiolib-sysfs.c | 35 ++---
drivers/gpio/gpiolib.c | 317 ++++++++++++++++++++++---------------------
drivers/gpio/gpiolib.h | 23 +++-
include/linux/gpio/driver.h | 4 -
4 files changed, 198 insertions(+), 181 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/6] gpio: remember to finally free gpio_device
2016-02-11 10:26 [PATCH 0/6] gpiolib refactorings after device addition Linus Walleij
@ 2016-02-11 10:26 ` Linus Walleij
2016-02-11 10:26 ` [PATCH 2/6] gpio: move sysfs mock device to the gpio_device Linus Walleij
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2016-02-11 10:26 UTC (permalink / raw)
To: linux-gpio, Alexandre Courbot, Johan Hovold, Michael Welling,
Markus Pargmann
Cc: Bamvor Jian Zhang, Grant Likely, Linus Walleij
When the device core reference count for the device goes to
0 and it calls .release() we free resources and so can also
finally free up the GPIO state container, struct gpio_device.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 70e0fff0a8a7..36f8be3f910b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -405,6 +405,7 @@ static void gpiodevice_release(struct device *dev)
cdev_del(&gdev->chrdev);
list_del(&gdev->list);
ida_simple_remove(&gpio_ida, gdev->id);
+ kfree(gdev);
}
/**
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/6] gpio: move sysfs mock device to the gpio_device
2016-02-11 10:26 [PATCH 0/6] gpiolib refactorings after device addition Linus Walleij
2016-02-11 10:26 ` [PATCH 1/6] gpio: remember to finally free gpio_device Linus Walleij
@ 2016-02-11 10:26 ` Linus Walleij
2016-02-11 10:26 ` [PATCH 3/6] gpio: move descriptors into gpio_device Linus Walleij
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2016-02-11 10:26 UTC (permalink / raw)
To: linux-gpio, Alexandre Courbot, Johan Hovold, Michael Welling,
Markus Pargmann
Cc: Bamvor Jian Zhang, Grant Likely, Linus Walleij
Since gpio_device is the struct that survives if the backing
gpio_chip is removed, move the sysfs mock device to this state
container so it becomes part of the dangling state of the
GPIO device on removal.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib-sysfs.c | 23 ++++++++++++-----------
drivers/gpio/gpiolib.c | 4 ++--
drivers/gpio/gpiolib.h | 11 +++++++----
include/linux/gpio/driver.h | 2 --
4 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 28d3bf2328aa..94ba4bb8b4f8 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -572,7 +572,7 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
mutex_lock(&sysfs_lock);
/* check if chip is being removed */
- if (!chip || !chip->cdev) {
+ if (!chip || !gdev->mockdev) {
status = -ENODEV;
goto err_unlock;
}
@@ -718,9 +718,10 @@ err_unlock:
}
EXPORT_SYMBOL_GPL(gpiod_unexport);
-int gpiochip_sysfs_register(struct gpio_chip *chip)
+int gpiochip_sysfs_register(struct gpio_device *gdev)
{
struct device *dev;
+ struct gpio_chip *chip = gdev->chip;
/*
* Many systems add gpio chips for SOC support very early,
@@ -732,7 +733,7 @@ int gpiochip_sysfs_register(struct gpio_chip *chip)
return 0;
/* use chip->base for the ID; it's already known to be unique */
- dev = device_create_with_groups(&gpio_class, chip->parent,
+ dev = device_create_with_groups(&gpio_class, &gdev->dev,
MKDEV(0, 0),
chip, gpiochip_groups,
"gpiochip%d", chip->base);
@@ -740,25 +741,26 @@ int gpiochip_sysfs_register(struct gpio_chip *chip)
return PTR_ERR(dev);
mutex_lock(&sysfs_lock);
- chip->cdev = dev;
+ gdev->mockdev = dev;
mutex_unlock(&sysfs_lock);
return 0;
}
-void gpiochip_sysfs_unregister(struct gpio_chip *chip)
+void gpiochip_sysfs_unregister(struct gpio_device *gdev)
{
struct gpio_desc *desc;
+ struct gpio_chip *chip = gdev->chip;
unsigned int i;
- if (!chip->cdev)
+ if (!gdev->mockdev)
return;
- device_unregister(chip->cdev);
+ device_unregister(gdev->mockdev);
/* prevent further gpiod exports */
mutex_lock(&sysfs_lock);
- chip->cdev = NULL;
+ gdev->mockdev = NULL;
mutex_unlock(&sysfs_lock);
/* unregister gpiod class devices owned by sysfs */
@@ -787,7 +789,7 @@ static int __init gpiolib_sysfs_init(void)
*/
spin_lock_irqsave(&gpio_lock, flags);
list_for_each_entry(gdev, &gpio_devices, list) {
- if (gdev->chip->cdev)
+ if (gdev->mockdev)
continue;
/*
@@ -800,12 +802,11 @@ static int __init gpiolib_sysfs_init(void)
* gpio_lock prevents us from doing this.
*/
spin_unlock_irqrestore(&gpio_lock, flags);
- status = gpiochip_sysfs_register(gdev->chip);
+ status = gpiochip_sysfs_register(gdev);
spin_lock_irqsave(&gpio_lock, flags);
}
spin_unlock_irqrestore(&gpio_lock, flags);
-
return status;
}
postcore_initcall(gpiolib_sysfs_init);
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 36f8be3f910b..5763290f777c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -558,7 +558,7 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
if (status)
goto err_remove_chardev;
- status = gpiochip_sysfs_register(chip);
+ status = gpiochip_sysfs_register(gdev);
if (status)
goto err_remove_device;
@@ -615,7 +615,7 @@ void gpiochip_remove(struct gpio_chip *chip)
gdev->chip = NULL;
/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
- gpiochip_sysfs_unregister(chip);
+ gpiochip_sysfs_unregister(gdev);
gpiochip_irqchip_remove(chip);
acpi_gpiochip_remove(chip);
gpiochip_remove_pin_ranges(chip);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 1524ba0ca99d..c5a5b57463c7 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -27,6 +27,8 @@ struct acpi_device;
* @id: numerical ID number for the GPIO chip
* @dev: the GPIO device struct
* @chrdev: character device for the GPIO device
+ * @mockdev: class device used by the deprecated sysfs interface (may be
+ * NULL)
* @owner: helps prevent removal of modules exporting active GPIOs
* @chip: pointer to the corresponding gpiochip, holding static
* data for this device
@@ -41,6 +43,7 @@ struct gpio_device {
int id;
struct device dev;
struct cdev chrdev;
+ struct device *mockdev;
struct module *owner;
struct gpio_chip *chip;
struct list_head list;
@@ -190,17 +193,17 @@ static int __maybe_unused gpio_chip_hwgpio(const struct gpio_desc *desc)
#ifdef CONFIG_GPIO_SYSFS
-int gpiochip_sysfs_register(struct gpio_chip *chip);
-void gpiochip_sysfs_unregister(struct gpio_chip *chip);
+int gpiochip_sysfs_register(struct gpio_device *gdev);
+void gpiochip_sysfs_unregister(struct gpio_device *gdev);
#else
-static inline int gpiochip_sysfs_register(struct gpio_chip *chip)
+static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
{
return 0;
}
-static inline void gpiochip_sysfs_unregister(struct gpio_chip *chip)
+static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
{
}
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index f3f1dbd43c9b..4db64ab534ef 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -24,7 +24,6 @@ struct gpio_device;
* @label: for diagnostics
* @gpiodev: the internal state holder, opaque struct
* @parent: optional parent device providing the GPIOs
- * @cdev: class device used by sysfs interface (may be NULL)
* @owner: helps prevent removal of modules exporting active GPIOs
* @data: per-instance data assigned by the driver
* @request: optional hook for chip-specific activation, such as
@@ -110,7 +109,6 @@ struct gpio_chip {
const char *label;
struct gpio_device *gpiodev;
struct device *parent;
- struct device *cdev;
struct module *owner;
void *data;
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/6] gpio: move descriptors into gpio_device
2016-02-11 10:26 [PATCH 0/6] gpiolib refactorings after device addition Linus Walleij
2016-02-11 10:26 ` [PATCH 1/6] gpio: remember to finally free gpio_device Linus Walleij
2016-02-11 10:26 ` [PATCH 2/6] gpio: move sysfs mock device to the gpio_device Linus Walleij
@ 2016-02-11 10:26 ` Linus Walleij
2016-02-11 10:26 ` [PATCH 4/6] gpio: reflect base and ngpio " Linus Walleij
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2016-02-11 10:26 UTC (permalink / raw)
To: linux-gpio, Alexandre Courbot, Johan Hovold, Michael Welling,
Markus Pargmann
Cc: Bamvor Jian Zhang, Grant Likely, Linus Walleij
We need gpio_device to hold the descriptors so that they can
be lifecycled with the struct gpio_device held from userspace.
Move the descriptor array into gpio_device. Also rename it from
"desc" (singularis) to "descs" (pluralis) to reflect the fact
that it is an array.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib-sysfs.c | 2 +-
drivers/gpio/gpiolib.c | 53 ++++++++++++++++++--------------------------
drivers/gpio/gpiolib.h | 4 +++-
include/linux/gpio/driver.h | 2 --
4 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 94ba4bb8b4f8..de65633471af 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -765,7 +765,7 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
/* unregister gpiod class devices owned by sysfs */
for (i = 0; i < chip->ngpio; i++) {
- desc = &chip->desc[i];
+ desc = &chip->gpiodev->descs[i];
if (test_and_clear_bit(FLAG_SYSFS, &desc->flags))
gpiod_free(desc);
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5763290f777c..f3fcd415a77b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -88,7 +88,7 @@ struct gpio_desc *gpio_to_desc(unsigned gpio)
if (gdev->chip->base <= gpio &&
gdev->chip->base + gdev->chip->ngpio > gpio) {
spin_unlock_irqrestore(&gpio_lock, flags);
- return &gdev->chip->desc[gpio - gdev->chip->base];
+ return &gdev->descs[gpio - gdev->chip->base];
}
}
@@ -110,7 +110,7 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip,
if (hwnum >= chip->ngpio)
return ERR_PTR(-EINVAL);
- return &chip->desc[hwnum];
+ return &chip->gpiodev->descs[hwnum];
}
/**
@@ -120,7 +120,7 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip,
*/
int desc_to_gpio(const struct gpio_desc *desc)
{
- return desc->chip->base + (desc - &desc->chip->desc[0]);
+ return desc->chip->base + (desc - &desc->chip->gpiodev->descs[0]);
}
EXPORT_SYMBOL_GPL(desc_to_gpio);
@@ -277,7 +277,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
int i;
for (i = 0; i != gdev->chip->ngpio; ++i) {
- struct gpio_desc *gpio = &gdev->chip->desc[i];
+ struct gpio_desc *gpio = &gdev->descs[i];
if (!gpio->name || !name)
continue;
@@ -320,7 +320,7 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
/* Then add all names to the GPIO descriptors */
for (i = 0; i != gc->ngpio; ++i)
- gc->desc[i].name = gc->names[i];
+ gc->gpiodev->descs[i].name = gc->names[i];
return 0;
}
@@ -431,7 +431,6 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
int status = 0;
unsigned i;
int base = chip->base;
- struct gpio_desc *descs;
struct gpio_device *gdev;
/*
@@ -470,9 +469,9 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
else
gdev->owner = THIS_MODULE;
- /* FIXME: devm_kcalloc() these and move to gpio_device */
- descs = kcalloc(chip->ngpio, sizeof(descs[0]), GFP_KERNEL);
- if (!descs) {
+ gdev->descs = devm_kcalloc(&gdev->dev, chip->ngpio,
+ sizeof(gdev->descs[0]), GFP_KERNEL);
+ if (!gdev->descs) {
status = -ENOMEM;
goto err_free_gdev;
}
@@ -483,7 +482,7 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
if (chip->ngpio == 0) {
chip_err(chip, "tried to insert a GPIO chip with zero lines\n");
status = -EINVAL;
- goto err_free_descs;
+ goto err_free_gdev;
}
spin_lock_irqsave(&gpio_lock, flags);
@@ -493,7 +492,7 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
if (base < 0) {
status = base;
spin_unlock_irqrestore(&gpio_lock, flags);
- goto err_free_descs;
+ goto err_free_gdev;
}
chip->base = base;
}
@@ -501,11 +500,11 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
status = gpiodev_add_to_list(gdev);
if (status) {
spin_unlock_irqrestore(&gpio_lock, flags);
- goto err_free_descs;
+ goto err_free_gdev;
}
for (i = 0; i < chip->ngpio; i++) {
- struct gpio_desc *desc = &descs[i];
+ struct gpio_desc *desc = &gdev->descs[i];
/* REVISIT: maybe a pointer to gpio_device is better */
desc->chip = chip;
@@ -518,7 +517,6 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
*/
desc->flags = !chip->direction_input ? (1 << FLAG_IS_OUT) : 0;
}
- chip->desc = descs;
spin_unlock_irqrestore(&gpio_lock, flags);
@@ -583,9 +581,6 @@ err_remove_from_list:
spin_lock_irqsave(&gpio_lock, flags);
list_del(&gdev->list);
spin_unlock_irqrestore(&gpio_lock, flags);
- chip->desc = NULL;
-err_free_descs:
- kfree(descs);
err_free_gdev:
ida_simple_remove(&gpio_ida, gdev->id);
kfree(gdev);
@@ -608,7 +603,7 @@ void gpiochip_remove(struct gpio_chip *chip)
struct gpio_device *gdev = chip->gpiodev;
struct gpio_desc *desc;
unsigned long flags;
- unsigned id;
+ unsigned i;
bool requested = false;
/* Numb the device, cancelling all outstanding operations */
@@ -623,8 +618,8 @@ void gpiochip_remove(struct gpio_chip *chip)
of_gpiochip_remove(chip);
spin_lock_irqsave(&gpio_lock, flags);
- for (id = 0; id < chip->ngpio; id++) {
- desc = &chip->desc[id];
+ for (i = 0; i < chip->ngpio; i++) {
+ desc = &gdev->descs[i];
desc->chip = NULL;
if (test_bit(FLAG_REQUESTED, &desc->flags))
requested = true;
@@ -635,10 +630,6 @@ void gpiochip_remove(struct gpio_chip *chip)
dev_crit(&chip->gpiodev->dev,
"REMOVING GPIOCHIP WITH GPIOS STILL REQUESTED\n");
- /* FIXME: need to be moved to gpio_device and held there */
- kfree(chip->desc);
- chip->desc = NULL;
-
/*
* The gpiochip side puts its use of the device to rest here:
* if there are no userspace clients, the chardev and device will
@@ -1250,7 +1241,7 @@ const char *gpiochip_is_requested(struct gpio_chip *chip, unsigned offset)
if (offset >= chip->ngpio)
return NULL;
- desc = &chip->desc[offset];
+ desc = &chip->gpiodev->descs[offset];
if (test_bit(FLAG_REQUESTED, &desc->flags) == 0)
return NULL;
@@ -1837,14 +1828,14 @@ int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset)
if (offset >= chip->ngpio)
return -EINVAL;
- if (test_bit(FLAG_IS_OUT, &chip->desc[offset].flags)) {
+ if (test_bit(FLAG_IS_OUT, &chip->gpiodev->descs[offset].flags)) {
chip_err(chip,
"%s: tried to flag a GPIO set as output for IRQ\n",
__func__);
return -EIO;
}
- set_bit(FLAG_USED_AS_IRQ, &chip->desc[offset].flags);
+ set_bit(FLAG_USED_AS_IRQ, &chip->gpiodev->descs[offset].flags);
return 0;
}
EXPORT_SYMBOL_GPL(gpiochip_lock_as_irq);
@@ -1862,7 +1853,7 @@ void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset)
if (offset >= chip->ngpio)
return;
- clear_bit(FLAG_USED_AS_IRQ, &chip->desc[offset].flags);
+ clear_bit(FLAG_USED_AS_IRQ, &chip->gpiodev->descs[offset].flags);
}
EXPORT_SYMBOL_GPL(gpiochip_unlock_as_irq);
@@ -2549,8 +2540,8 @@ static void gpiochip_free_hogs(struct gpio_chip *chip)
int id;
for (id = 0; id < chip->ngpio; id++) {
- if (test_bit(FLAG_IS_HOGGED, &chip->desc[id].flags))
- gpiochip_free_own_desc(&chip->desc[id]);
+ if (test_bit(FLAG_IS_HOGGED, &chip->gpiodev->descs[id].flags))
+ gpiochip_free_own_desc(&chip->gpiodev->descs[id]);
}
}
@@ -2673,7 +2664,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
unsigned i;
unsigned gpio = chip->base;
- struct gpio_desc *gdesc = &chip->desc[0];
+ struct gpio_desc *gdesc = &chip->gpiodev->descs[0];
int is_out;
int is_irq;
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index c5a5b57463c7..39b8301c98b6 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -32,6 +32,7 @@ struct acpi_device;
* @owner: helps prevent removal of modules exporting active GPIOs
* @chip: pointer to the corresponding gpiochip, holding static
* data for this device
+ * @descs: array of ngpio descriptors.
* @list: links gpio_device:s together for traversal
*
* This state container holds most of the runtime variable data
@@ -46,6 +47,7 @@ struct gpio_device {
struct device *mockdev;
struct module *owner;
struct gpio_chip *chip;
+ struct gpio_desc *descs;
struct list_head list;
};
@@ -152,7 +154,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
*/
static int __maybe_unused gpio_chip_hwgpio(const struct gpio_desc *desc)
{
- return desc - &desc->chip->desc[0];
+ return desc - &desc->chip->gpiodev->descs[0];
}
/* With descriptor prefix */
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 4db64ab534ef..bfc842c2fc57 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -52,7 +52,6 @@ struct gpio_device;
* get rid of the static GPIO number space in the long run.
* @ngpio: the number of GPIOs handled by this controller; the last GPIO
* handled is (base + ngpio - 1).
- * @desc: array of ngpio descriptors. Private.
* @names: if set, must be an array of strings to use as alternative
* names for the GPIOs in this chip. Any entry in the array
* may be NULL if there is no alias for the GPIO, however the
@@ -140,7 +139,6 @@ struct gpio_chip {
struct gpio_chip *chip);
int base;
u16 ngpio;
- struct gpio_desc *desc;
const char *const *names;
bool can_sleep;
bool irq_not_threaded;
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/6] gpio: reflect base and ngpio into gpio_device
2016-02-11 10:26 [PATCH 0/6] gpiolib refactorings after device addition Linus Walleij
` (2 preceding siblings ...)
2016-02-11 10:26 ` [PATCH 3/6] gpio: move descriptors into gpio_device Linus Walleij
@ 2016-02-11 10:26 ` Linus Walleij
2016-02-11 10:26 ` [PATCH 5/6] gpio: reference count the gpio device for each desc Linus Walleij
2016-02-11 10:26 ` [PATCH 6/6] gpio: move the pin ranges into gpio_device Linus Walleij
5 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2016-02-11 10:26 UTC (permalink / raw)
To: linux-gpio, Alexandre Courbot, Johan Hovold, Michael Welling,
Markus Pargmann
Cc: Bamvor Jian Zhang, Grant Likely, Linus Walleij
Some information about the GPIO chip need to stay around also
after the gpio_chip has been removed and only the gpio_device
persist. The base and ngpio are such things, for example we
don't want a new chip arriving to overlap the number space
of a dangling gpio_device, and the chardev may still query
the device for the number of lines etc.
Note that the code that assigns base and insert gpio_device
into the global list no longer check for a missing gpio_chip:
we respect the number space allocated by any other gpio_device.
As a consequence of the gdev being referenced directly from
the gpio_desc, we need to verify it differently from all
in-kernel API calls that fall through to direct queries to
the gpio_chip vtable: we first check that desc is !NULL, then
that desc->gdev is !NULL, then, if desc->gdev->chip is NULL,
we *BAIL OUT* without any error, so as to manage the case
where operations are requested on a device that is gone.
These checks were non-uniform and partly missing in the past:
so to simplify: create the macros VALIDATE_DESC() that will
return -EINVAL if the desc or desc->gdev is missing and just
0 if the chip is gone, and conversely VALIDATE_DESC_VOID()
for the case where the function does not return an error.
By using these macros, we get warning messages about missing
gdev with reference to the right function in the kernel log.
Despite the macro business this simplifies the code and make
it more readable than if we copy/paste the same descriptor
checking code into all code ABI call sites (IMHO).
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib-sysfs.c | 12 +-
drivers/gpio/gpiolib.c | 255 +++++++++++++++++++++++--------------------
drivers/gpio/gpiolib.h | 10 +-
3 files changed, 148 insertions(+), 129 deletions(-)
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index de65633471af..c56309491e8b 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -180,7 +180,7 @@ static int gpio_sysfs_request_irq(struct device *dev, unsigned char flags)
* Remove this redundant call (along with the corresponding
* unlock) when those drivers have been fixed.
*/
- ret = gpiochip_lock_as_irq(desc->chip, gpio_chip_hwgpio(desc));
+ ret = gpiochip_lock_as_irq(desc->gdev->chip, gpio_chip_hwgpio(desc));
if (ret < 0)
goto err_put_kn;
@@ -194,7 +194,7 @@ static int gpio_sysfs_request_irq(struct device *dev, unsigned char flags)
return 0;
err_unlock:
- gpiochip_unlock_as_irq(desc->chip, gpio_chip_hwgpio(desc));
+ gpiochip_unlock_as_irq(desc->gdev->chip, gpio_chip_hwgpio(desc));
err_put_kn:
sysfs_put(data->value_kn);
@@ -212,7 +212,7 @@ static void gpio_sysfs_free_irq(struct device *dev)
data->irq_flags = 0;
free_irq(data->irq, data);
- gpiochip_unlock_as_irq(desc->chip, gpio_chip_hwgpio(desc));
+ gpiochip_unlock_as_irq(desc->gdev->chip, gpio_chip_hwgpio(desc));
sysfs_put(data->value_kn);
}
@@ -566,8 +566,8 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
return -EINVAL;
}
- chip = desc->chip;
- gdev = chip->gpiodev;
+ gdev = desc->gdev;
+ chip = gdev->chip;
mutex_lock(&sysfs_lock);
@@ -765,7 +765,7 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
/* unregister gpiod class devices owned by sysfs */
for (i = 0; i < chip->ngpio; i++) {
- desc = &chip->gpiodev->descs[i];
+ desc = &gdev->descs[i];
if (test_and_clear_bit(FLAG_SYSFS, &desc->flags))
gpiod_free(desc);
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f3fcd415a77b..cbc9c764a7fb 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -85,10 +85,10 @@ struct gpio_desc *gpio_to_desc(unsigned gpio)
spin_lock_irqsave(&gpio_lock, flags);
list_for_each_entry(gdev, &gpio_devices, list) {
- if (gdev->chip->base <= gpio &&
- gdev->chip->base + gdev->chip->ngpio > gpio) {
+ if (gdev->base <= gpio &&
+ gdev->base + gdev->ngpio > gpio) {
spin_unlock_irqrestore(&gpio_lock, flags);
- return &gdev->descs[gpio - gdev->chip->base];
+ return &gdev->descs[gpio - gdev->base];
}
}
@@ -107,10 +107,12 @@ EXPORT_SYMBOL_GPL(gpio_to_desc);
struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip,
u16 hwnum)
{
- if (hwnum >= chip->ngpio)
+ struct gpio_device *gdev = chip->gpiodev;
+
+ if (hwnum >= gdev->ngpio)
return ERR_PTR(-EINVAL);
- return &chip->gpiodev->descs[hwnum];
+ return &gdev->descs[hwnum];
}
/**
@@ -120,7 +122,7 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip,
*/
int desc_to_gpio(const struct gpio_desc *desc)
{
- return desc->chip->base + (desc - &desc->chip->gpiodev->descs[0]);
+ return desc->gdev->base + (desc - &desc->gdev->descs[0]);
}
EXPORT_SYMBOL_GPL(desc_to_gpio);
@@ -131,7 +133,9 @@ EXPORT_SYMBOL_GPL(desc_to_gpio);
*/
struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
{
- return desc ? desc->chip : NULL;
+ if (!desc || !desc->gdev || !desc->gdev->chip)
+ return NULL;
+ return desc->gdev->chip;
}
EXPORT_SYMBOL_GPL(gpiod_to_chip);
@@ -143,11 +147,11 @@ static int gpiochip_find_base(int ngpio)
list_for_each_entry_reverse(gdev, &gpio_devices, list) {
/* found a free space? */
- if (gdev->chip->base + gdev->chip->ngpio <= base)
+ if (gdev->base + gdev->ngpio <= base)
break;
else
/* nope, check the space right before the chip */
- base = gdev->chip->base - ngpio;
+ base = gdev->base - ngpio;
}
if (gpio_is_valid(base)) {
@@ -214,14 +218,7 @@ static int gpiodev_add_to_list(struct gpio_device *gdev)
}
list_for_each_entry(iterator, &gpio_devices, list) {
- /*
- * The list may contain dangling GPIO devices with no
- * live chip assigned.
- */
- if (!iterator->chip)
- continue;
- if (iterator->chip->base >=
- gdev->chip->base + gdev->chip->ngpio) {
+ if (iterator->base >= gdev->base + gdev->ngpio) {
/*
* Iterator is the first GPIO chip so there is no
* previous one
@@ -234,8 +231,8 @@ static int gpiodev_add_to_list(struct gpio_device *gdev)
* [base, base + ngpio - 1]) between previous
* and iterator chip.
*/
- if (previous->chip->base + previous->chip->ngpio
- <= gdev->chip->base)
+ if (previous->base + previous->ngpio
+ <= gdev->base)
goto found;
}
}
@@ -249,7 +246,7 @@ static int gpiodev_add_to_list(struct gpio_device *gdev)
*/
iterator = list_last_entry(&gpio_devices, struct gpio_device, list);
- if (iterator->chip->base + iterator->chip->ngpio <= gdev->chip->base) {
+ if (iterator->base + iterator->ngpio <= gdev->base) {
list_add(&gdev->list, &iterator->list);
return 0;
}
@@ -276,15 +273,15 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
list_for_each_entry(gdev, &gpio_devices, list) {
int i;
- for (i = 0; i != gdev->chip->ngpio; ++i) {
- struct gpio_desc *gpio = &gdev->descs[i];
+ for (i = 0; i != gdev->ngpio; ++i) {
+ struct gpio_desc *desc = &gdev->descs[i];
- if (!gpio->name || !name)
+ if (!desc->name || !name)
continue;
- if (!strcmp(gpio->name, name)) {
+ if (!strcmp(desc->name, name)) {
spin_unlock_irqrestore(&gpio_lock, flags);
- return gpio;
+ return desc;
}
}
}
@@ -302,6 +299,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
*/
static int gpiochip_set_desc_names(struct gpio_chip *gc)
{
+ struct gpio_device *gdev = gc->gpiodev;
int i;
if (!gc->names)
@@ -313,14 +311,14 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
gpio = gpio_name_to_desc(gc->names[i]);
if (gpio)
- dev_warn(&gc->gpiodev->dev,
+ dev_warn(&gdev->dev,
"Detected name collision for GPIO name '%s'\n",
gc->names[i]);
}
/* Then add all names to the GPIO descriptors */
for (i = 0; i != gc->ngpio; ++i)
- gc->gpiodev->descs[i].name = gc->names[i];
+ gdev->descs[i].name = gc->names[i];
return 0;
}
@@ -344,7 +342,7 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
strncpy(chipinfo.name, dev_name(&gdev->dev),
sizeof(chipinfo.name));
chipinfo.name[sizeof(chipinfo.name)-1] = '\0';
- chipinfo.lines = chip->ngpio;
+ chipinfo.lines = gdev->ngpio;
if (copy_to_user(ip, &chipinfo, sizeof(chipinfo)))
return -EFAULT;
return 0;
@@ -476,17 +474,24 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
goto err_free_gdev;
}
- /* FIXME: move driver data into gpio_device dev_set_drvdata() */
- chip->data = data;
-
if (chip->ngpio == 0) {
chip_err(chip, "tried to insert a GPIO chip with zero lines\n");
status = -EINVAL;
goto err_free_gdev;
}
+ gdev->ngpio = chip->ngpio;
+ /* FIXME: move driver data into gpio_device dev_set_drvdata() */
+ chip->data = data;
spin_lock_irqsave(&gpio_lock, flags);
+ /*
+ * TODO: this allocates a Linux GPIO number base in the global
+ * GPIO numberspace for this chip. In the long run we want to
+ * get *rid* of this numberspace and use only descriptors, but
+ * it may be a pipe dream. It will not happen before we get rid
+ * of the sysfs interface anyways.
+ */
if (base < 0) {
base = gpiochip_find_base(chip->ngpio);
if (base < 0) {
@@ -494,8 +499,15 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
spin_unlock_irqrestore(&gpio_lock, flags);
goto err_free_gdev;
}
+ /*
+ * TODO: it should not be necessary to reflect the assigned
+ * base outside of the GPIO subsystem. Go over drivers and
+ * see if anyone makes use of this, else drop this and assign
+ * a poison instead.
+ */
chip->base = base;
}
+ gdev->base = base;
status = gpiodev_add_to_list(gdev);
if (status) {
@@ -506,8 +518,7 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
for (i = 0; i < chip->ngpio; i++) {
struct gpio_desc *desc = &gdev->descs[i];
- /* REVISIT: maybe a pointer to gpio_device is better */
- desc->chip = chip;
+ desc->gdev = gdev;
/* REVISIT: most hardware initializes GPIOs as inputs (often
* with pullups enabled) so power usage is minimized. Linux
@@ -563,9 +574,9 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
/* From this point, the .release() function cleans up gpio_device */
gdev->dev.release = gpiodevice_release;
get_device(&gdev->dev);
- pr_debug("%s: registered GPIOs %d to %d on device: %s\n", __func__,
- chip->base, chip->base + chip->ngpio - 1,
- chip->label ? : "generic");
+ pr_debug("%s: registered GPIOs %d to %d on device: %s (%s)\n",
+ __func__, gdev->base, gdev->base + gdev->ngpio - 1,
+ dev_name(&gdev->dev), chip->label ? : "generic");
return 0;
@@ -586,8 +597,8 @@ err_free_gdev:
kfree(gdev);
/* failures here can mean systems won't boot... */
pr_err("%s: GPIOs %d..%d (%s) failed to register\n", __func__,
- chip->base, chip->base + chip->ngpio - 1,
- chip->label ? : "generic");
+ gdev->base, gdev->base + gdev->ngpio - 1,
+ chip->label ? : "generic");
return status;
}
EXPORT_SYMBOL_GPL(gpiochip_add_data);
@@ -618,16 +629,15 @@ void gpiochip_remove(struct gpio_chip *chip)
of_gpiochip_remove(chip);
spin_lock_irqsave(&gpio_lock, flags);
- for (i = 0; i < chip->ngpio; i++) {
+ for (i = 0; i < gdev->ngpio; i++) {
desc = &gdev->descs[i];
- desc->chip = NULL;
if (test_bit(FLAG_REQUESTED, &desc->flags))
requested = true;
}
spin_unlock_irqrestore(&gpio_lock, flags);
if (requested)
- dev_crit(&chip->gpiodev->dev,
+ dev_crit(&gdev->dev,
"REMOVING GPIOCHIP WITH GPIOS STILL REQUESTED\n");
/*
@@ -967,7 +977,7 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip) {}
*/
int gpiochip_generic_request(struct gpio_chip *chip, unsigned offset)
{
- return pinctrl_request_gpio(chip->base + offset);
+ return pinctrl_request_gpio(chip->gpiodev->base + offset);
}
EXPORT_SYMBOL_GPL(gpiochip_generic_request);
@@ -978,7 +988,7 @@ EXPORT_SYMBOL_GPL(gpiochip_generic_request);
*/
void gpiochip_generic_free(struct gpio_chip *chip, unsigned offset)
{
- pinctrl_free_gpio(chip->base + offset);
+ pinctrl_free_gpio(chip->gpiodev->base + offset);
}
EXPORT_SYMBOL_GPL(gpiochip_generic_free);
@@ -996,6 +1006,7 @@ int gpiochip_add_pingroup_range(struct gpio_chip *chip,
unsigned int gpio_offset, const char *pin_group)
{
struct gpio_pin_range *pin_range;
+ struct gpio_device *gdev = chip->gpiodev;
int ret;
pin_range = kzalloc(sizeof(*pin_range), GFP_KERNEL);
@@ -1008,7 +1019,7 @@ int gpiochip_add_pingroup_range(struct gpio_chip *chip,
pin_range->range.id = gpio_offset;
pin_range->range.gc = chip;
pin_range->range.name = chip->label;
- pin_range->range.base = chip->base + gpio_offset;
+ pin_range->range.base = gdev->base + gpio_offset;
pin_range->pctldev = pctldev;
ret = pinctrl_get_group_pins(pctldev, pin_group,
@@ -1045,6 +1056,7 @@ int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
unsigned int npins)
{
struct gpio_pin_range *pin_range;
+ struct gpio_device *gdev = chip->gpiodev;
int ret;
pin_range = kzalloc(sizeof(*pin_range), GFP_KERNEL);
@@ -1057,7 +1069,7 @@ int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
pin_range->range.id = gpio_offset;
pin_range->range.gc = chip;
pin_range->range.name = chip->label;
- pin_range->range.base = chip->base + gpio_offset;
+ pin_range->range.base = gdev->base + gpio_offset;
pin_range->range.pin_base = pin_offset;
pin_range->range.npins = npins;
pin_range->pctldev = pinctrl_find_and_add_gpio_range(pinctl_name,
@@ -1104,7 +1116,7 @@ EXPORT_SYMBOL_GPL(gpiochip_remove_pin_ranges);
*/
static int __gpiod_request(struct gpio_desc *desc, const char *label)
{
- struct gpio_chip *chip = desc->chip;
+ struct gpio_chip *chip = desc->gdev->chip;
int status;
unsigned long flags;
@@ -1153,27 +1165,48 @@ done:
return status;
}
+/*
+ * This descriptor validation needs to be inserted verbatim into each
+ * function taking a descriptor, so we need to use a preprocessor
+ * macro to avoid endless duplication.
+ */
+#define VALIDATE_DESC(desc) do { \
+ if (!desc || !desc->gdev) { \
+ pr_warn("%s: invalid GPIO\n", __func__); \
+ return -EINVAL; \
+ } \
+ if ( !desc->gdev->chip ) { \
+ dev_warn(&desc->gdev->dev, \
+ "%s: backing chip is gone\n", __func__); \
+ return 0; \
+ } } while (0)
+
+#define VALIDATE_DESC_VOID(desc) do { \
+ if (!desc || !desc->gdev) { \
+ pr_warn("%s: invalid GPIO\n", __func__); \
+ return; \
+ } \
+ if (!desc->gdev->chip) { \
+ dev_warn(&desc->gdev->dev, \
+ "%s: backing chip is gone\n", __func__); \
+ return; \
+ } } while (0)
+
+
int gpiod_request(struct gpio_desc *desc, const char *label)
{
int status = -EPROBE_DEFER;
- struct gpio_chip *chip;
-
- if (!desc) {
- pr_warn("%s: invalid GPIO\n", __func__);
- return -EINVAL;
- }
+ struct gpio_device *gdev;
- chip = desc->chip;
- if (!chip)
- goto done;
+ VALIDATE_DESC(desc);
+ gdev = desc->gdev;
- if (try_module_get(chip->gpiodev->owner)) {
+ if (try_module_get(gdev->owner)) {
status = __gpiod_request(desc, label);
if (status < 0)
- module_put(chip->gpiodev->owner);
+ module_put(gdev->owner);
}
-done:
if (status)
gpiod_dbg(desc, "%s: status %d\n", __func__, status);
@@ -1192,7 +1225,7 @@ static bool __gpiod_free(struct gpio_desc *desc)
spin_lock_irqsave(&gpio_lock, flags);
- chip = desc->chip;
+ chip = desc->gdev->chip;
if (chip && test_bit(FLAG_REQUESTED, &desc->flags)) {
if (chip->free) {
spin_unlock_irqrestore(&gpio_lock, flags);
@@ -1216,7 +1249,7 @@ static bool __gpiod_free(struct gpio_desc *desc)
void gpiod_free(struct gpio_desc *desc)
{
if (desc && __gpiod_free(desc))
- module_put(desc->chip->gpiodev->owner);
+ module_put(desc->gdev->owner);
else
WARN_ON(extra_checks);
}
@@ -1293,7 +1326,8 @@ void gpiochip_free_own_desc(struct gpio_desc *desc)
}
EXPORT_SYMBOL_GPL(gpiochip_free_own_desc);
-/* Drivers MUST set GPIO direction before making get/set calls. In
+/*
+ * Drivers MUST set GPIO direction before making get/set calls. In
* some cases this is done in early boot, before IRQs are enabled.
*
* As a rule these aren't called more than once (except for drivers
@@ -1316,12 +1350,9 @@ int gpiod_direction_input(struct gpio_desc *desc)
struct gpio_chip *chip;
int status = -EINVAL;
- if (!desc || !desc->chip) {
- pr_warn("%s: invalid GPIO\n", __func__);
- return -EINVAL;
- }
+ VALIDATE_DESC(desc);
+ chip = desc->gdev->chip;
- chip = desc->chip;
if (!chip->get || !chip->direction_input) {
gpiod_warn(desc,
"%s: missing get() or direction_input() operations\n",
@@ -1360,7 +1391,7 @@ static int _gpiod_direction_output_raw(struct gpio_desc *desc, int value)
if (!value && test_bit(FLAG_OPEN_SOURCE, &desc->flags))
return gpiod_direction_input(desc);
- chip = desc->chip;
+ chip = desc->gdev->chip;
if (!chip->set || !chip->direction_output) {
gpiod_warn(desc,
"%s: missing set() or direction_output() operations\n",
@@ -1389,10 +1420,7 @@ static int _gpiod_direction_output_raw(struct gpio_desc *desc, int value)
*/
int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
{
- if (!desc || !desc->chip) {
- pr_warn("%s: invalid GPIO\n", __func__);
- return -EINVAL;
- }
+ VALIDATE_DESC(desc);
return _gpiod_direction_output_raw(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_direction_output_raw);
@@ -1411,10 +1439,7 @@ EXPORT_SYMBOL_GPL(gpiod_direction_output_raw);
*/
int gpiod_direction_output(struct gpio_desc *desc, int value)
{
- if (!desc || !desc->chip) {
- pr_warn("%s: invalid GPIO\n", __func__);
- return -EINVAL;
- }
+ VALIDATE_DESC(desc);
if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
return _gpiod_direction_output_raw(desc, value);
@@ -1433,12 +1458,8 @@ int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
{
struct gpio_chip *chip;
- if (!desc || !desc->chip) {
- pr_warn("%s: invalid GPIO\n", __func__);
- return -EINVAL;
- }
-
- chip = desc->chip;
+ VALIDATE_DESC(desc);
+ chip = desc->gdev->chip;
if (!chip->set || !chip->set_debounce) {
gpiod_dbg(desc,
"%s: missing set() or set_debounce() operations\n",
@@ -1458,6 +1479,7 @@ EXPORT_SYMBOL_GPL(gpiod_set_debounce);
*/
int gpiod_is_active_low(const struct gpio_desc *desc)
{
+ VALIDATE_DESC(desc);
return test_bit(FLAG_ACTIVE_LOW, &desc->flags);
}
EXPORT_SYMBOL_GPL(gpiod_is_active_low);
@@ -1490,7 +1512,7 @@ static int _gpiod_get_raw_value(const struct gpio_desc *desc)
int offset;
int value;
- chip = desc->chip;
+ chip = desc->gdev->chip;
offset = gpio_chip_hwgpio(desc);
value = chip->get ? chip->get(chip, offset) : -EIO;
value = value < 0 ? value : !!value;
@@ -1510,10 +1532,9 @@ static int _gpiod_get_raw_value(const struct gpio_desc *desc)
*/
int gpiod_get_raw_value(const struct gpio_desc *desc)
{
- if (!desc)
- return 0;
+ VALIDATE_DESC(desc);
/* Should be using gpio_get_value_cansleep() */
- WARN_ON(desc->chip->can_sleep);
+ WARN_ON(desc->gdev->chip->can_sleep);
return _gpiod_get_raw_value(desc);
}
EXPORT_SYMBOL_GPL(gpiod_get_raw_value);
@@ -1531,10 +1552,10 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_value);
int gpiod_get_value(const struct gpio_desc *desc)
{
int value;
- if (!desc)
- return 0;
+
+ VALIDATE_DESC(desc);
/* Should be using gpio_get_value_cansleep() */
- WARN_ON(desc->chip->can_sleep);
+ WARN_ON(desc->gdev->chip->can_sleep);
value = _gpiod_get_raw_value(desc);
if (value < 0)
@@ -1555,7 +1576,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
static void _gpio_set_open_drain_value(struct gpio_desc *desc, bool value)
{
int err = 0;
- struct gpio_chip *chip = desc->chip;
+ struct gpio_chip *chip = desc->gdev->chip;
int offset = gpio_chip_hwgpio(desc);
if (value) {
@@ -1582,7 +1603,7 @@ static void _gpio_set_open_drain_value(struct gpio_desc *desc, bool value)
static void _gpio_set_open_source_value(struct gpio_desc *desc, bool value)
{
int err = 0;
- struct gpio_chip *chip = desc->chip;
+ struct gpio_chip *chip = desc->gdev->chip;
int offset = gpio_chip_hwgpio(desc);
if (value) {
@@ -1605,7 +1626,7 @@ static void _gpiod_set_raw_value(struct gpio_desc *desc, bool value)
{
struct gpio_chip *chip;
- chip = desc->chip;
+ chip = desc->gdev->chip;
trace_gpio_value(desc_to_gpio(desc), 0, value);
if (test_bit(FLAG_OPEN_DRAIN, &desc->flags))
_gpio_set_open_drain_value(desc, value);
@@ -1653,7 +1674,7 @@ static void gpiod_set_array_value_priv(bool raw, bool can_sleep,
int i = 0;
while (i < array_size) {
- struct gpio_chip *chip = desc_array[i]->chip;
+ struct gpio_chip *chip = desc_array[i]->gdev->chip;
unsigned long mask[BITS_TO_LONGS(chip->ngpio)];
unsigned long bits[BITS_TO_LONGS(chip->ngpio)];
int count = 0;
@@ -1687,7 +1708,8 @@ static void gpiod_set_array_value_priv(bool raw, bool can_sleep,
count++;
}
i++;
- } while ((i < array_size) && (desc_array[i]->chip == chip));
+ } while ((i < array_size) &&
+ (desc_array[i]->gdev->chip == chip));
/* push collected bits to outputs */
if (count != 0)
gpio_chip_set_multiple(chip, mask, bits);
@@ -1707,10 +1729,9 @@ static void gpiod_set_array_value_priv(bool raw, bool can_sleep,
*/
void gpiod_set_raw_value(struct gpio_desc *desc, int value)
{
- if (!desc)
- return;
+ VALIDATE_DESC_VOID(desc);
/* Should be using gpio_set_value_cansleep() */
- WARN_ON(desc->chip->can_sleep);
+ WARN_ON(desc->gdev->chip->can_sleep);
_gpiod_set_raw_value(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
@@ -1728,10 +1749,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
*/
void gpiod_set_value(struct gpio_desc *desc, int value)
{
- if (!desc)
- return;
+ VALIDATE_DESC_VOID(desc);
/* Should be using gpio_set_value_cansleep() */
- WARN_ON(desc->chip->can_sleep);
+ WARN_ON(desc->gdev->chip->can_sleep);
if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
_gpiod_set_raw_value(desc, value);
@@ -1789,9 +1809,8 @@ EXPORT_SYMBOL_GPL(gpiod_set_array_value);
*/
int gpiod_cansleep(const struct gpio_desc *desc)
{
- if (!desc)
- return 0;
- return desc->chip->can_sleep;
+ VALIDATE_DESC(desc);
+ return desc->gdev->chip->can_sleep;
}
EXPORT_SYMBOL_GPL(gpiod_cansleep);
@@ -1807,9 +1826,8 @@ int gpiod_to_irq(const struct gpio_desc *desc)
struct gpio_chip *chip;
int offset;
- if (!desc)
- return -EINVAL;
- chip = desc->chip;
+ VALIDATE_DESC(desc);
+ chip = desc->gdev->chip;
offset = gpio_chip_hwgpio(desc);
return chip->to_irq ? chip->to_irq(chip, offset) : -ENXIO;
}
@@ -1869,8 +1887,7 @@ EXPORT_SYMBOL_GPL(gpiochip_unlock_as_irq);
int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc)
{
might_sleep_if(extra_checks);
- if (!desc)
- return 0;
+ VALIDATE_DESC(desc);
return _gpiod_get_raw_value(desc);
}
EXPORT_SYMBOL_GPL(gpiod_get_raw_value_cansleep);
@@ -1889,9 +1906,7 @@ int gpiod_get_value_cansleep(const struct gpio_desc *desc)
int value;
might_sleep_if(extra_checks);
- if (!desc)
- return 0;
-
+ VALIDATE_DESC(desc);
value = _gpiod_get_raw_value(desc);
if (value < 0)
return value;
@@ -1916,8 +1931,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value)
{
might_sleep_if(extra_checks);
- if (!desc)
- return;
+ VALIDATE_DESC_VOID(desc);
_gpiod_set_raw_value(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_value_cansleep);
@@ -1935,9 +1949,7 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_value_cansleep);
void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
{
might_sleep_if(extra_checks);
- if (!desc)
- return;
-
+ VALIDATE_DESC_VOID(desc);
if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
_gpiod_set_raw_value(desc, value);
@@ -2660,15 +2672,16 @@ core_initcall(gpiolib_dev_init);
#ifdef CONFIG_DEBUG_FS
-static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
{
unsigned i;
- unsigned gpio = chip->base;
- struct gpio_desc *gdesc = &chip->gpiodev->descs[0];
+ struct gpio_chip *chip = gdev->chip;
+ unsigned gpio = gdev->base;
+ struct gpio_desc *gdesc = &gdev->descs[0];
int is_out;
int is_irq;
- for (i = 0; i < chip->ngpio; i++, gpio++, gdesc++) {
+ for (i = 0; i < gdev->ngpio; i++, gpio++, gdesc++) {
if (!test_bit(FLAG_REQUESTED, &gdesc->flags)) {
if (gdesc->name) {
seq_printf(s, " gpio-%-3d (%-20.20s)\n",
@@ -2747,7 +2760,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
seq_printf(s, "%s%s: GPIOs %d-%d", (char *)s->private,
dev_name(&gdev->dev),
- chip->base, chip->base + chip->ngpio - 1);
+ gdev->base, gdev->base + gdev->ngpio - 1);
parent = chip->parent;
if (parent)
seq_printf(s, ", parent: %s/%s",
@@ -2762,7 +2775,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
if (chip->dbg_show)
chip->dbg_show(s, chip);
else
- gpiolib_dbg_show(s, chip);
+ gpiolib_dbg_show(s, gdev);
return 0;
}
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 39b8301c98b6..d154984c71d9 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -33,6 +33,10 @@ struct acpi_device;
* @chip: pointer to the corresponding gpiochip, holding static
* data for this device
* @descs: array of ngpio descriptors.
+ * @ngpio: the number of GPIO lines on this GPIO device, equal to the size
+ * of the @descs array.
+ * @base: GPIO base in the DEPRECATED global Linux GPIO numberspace, assigned
+ * at device creation time.
* @list: links gpio_device:s together for traversal
*
* This state container holds most of the runtime variable data
@@ -48,6 +52,8 @@ struct gpio_device {
struct module *owner;
struct gpio_chip *chip;
struct gpio_desc *descs;
+ int base;
+ u16 ngpio;
struct list_head list;
};
@@ -125,7 +131,7 @@ extern struct spinlock gpio_lock;
extern struct list_head gpio_devices;
struct gpio_desc {
- struct gpio_chip *chip;
+ struct gpio_device *gdev;
unsigned long flags;
/* flag symbols are bit numbers */
#define FLAG_REQUESTED 0
@@ -154,7 +160,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
*/
static int __maybe_unused gpio_chip_hwgpio(const struct gpio_desc *desc)
{
- return desc - &desc->chip->gpiodev->descs[0];
+ return desc - &desc->gdev->descs[0];
}
/* With descriptor prefix */
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/6] gpio: reference count the gpio device for each desc
2016-02-11 10:26 [PATCH 0/6] gpiolib refactorings after device addition Linus Walleij
` (3 preceding siblings ...)
2016-02-11 10:26 ` [PATCH 4/6] gpio: reflect base and ngpio " Linus Walleij
@ 2016-02-11 10:26 ` Linus Walleij
2016-02-11 10:26 ` [PATCH 6/6] gpio: move the pin ranges into gpio_device Linus Walleij
5 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2016-02-11 10:26 UTC (permalink / raw)
To: linux-gpio, Alexandre Courbot, Johan Hovold, Michael Welling,
Markus Pargmann
Cc: Bamvor Jian Zhang, Grant Likely, Linus Walleij
Every time a descriptor is retrieved from the gpiolib, we issue
module_get() to reference count the module supplying the GPIOs.
We also need to call device_get() and device_put() as we also
reference the backing gpio_device when doing this.
Since the sysfs GPIO interface is using gpiod_get() this will
also reference count the sysfs requests until all GPIOs are
unexported.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cbc9c764a7fb..157ab40d19b1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1205,6 +1205,8 @@ int gpiod_request(struct gpio_desc *desc, const char *label)
status = __gpiod_request(desc, label);
if (status < 0)
module_put(gdev->owner);
+ else
+ get_device(&gdev->dev);
}
if (status)
@@ -1248,10 +1250,12 @@ static bool __gpiod_free(struct gpio_desc *desc)
void gpiod_free(struct gpio_desc *desc)
{
- if (desc && __gpiod_free(desc))
+ if (desc && desc->gdev && __gpiod_free(desc)) {
module_put(desc->gdev->owner);
- else
+ put_device(&desc->gdev->dev);
+ } else {
WARN_ON(extra_checks);
+ }
}
/**
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 6/6] gpio: move the pin ranges into gpio_device
2016-02-11 10:26 [PATCH 0/6] gpiolib refactorings after device addition Linus Walleij
` (4 preceding siblings ...)
2016-02-11 10:26 ` [PATCH 5/6] gpio: reference count the gpio device for each desc Linus Walleij
@ 2016-02-11 10:26 ` Linus Walleij
5 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2016-02-11 10:26 UTC (permalink / raw)
To: linux-gpio, Alexandre Courbot, Johan Hovold, Michael Welling,
Markus Pargmann
Cc: Bamvor Jian Zhang, Grant Likely, Linus Walleij
Instead of keeping this reference to the pin ranges in the
client driver-supplied gpio_chip, move it to the internal
gpio_device as the drivers have no need to inspect this.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 157ab40d19b1..08e93857a7d5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -532,8 +532,7 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
spin_unlock_irqrestore(&gpio_lock, flags);
#ifdef CONFIG_PINCTRL
- /* FIXME: move pin ranges to gpio_device */
- INIT_LIST_HEAD(&chip->pin_ranges);
+ INIT_LIST_HEAD(&gdev->pin_ranges);
#endif
status = gpiochip_set_desc_names(chip);
@@ -1036,7 +1035,7 @@ int gpiochip_add_pingroup_range(struct gpio_chip *chip,
gpio_offset, gpio_offset + pin_range->range.npins - 1,
pinctrl_dev_get_devname(pctldev), pin_group);
- list_add_tail(&pin_range->node, &chip->pin_ranges);
+ list_add_tail(&pin_range->node, &gdev->pin_ranges);
return 0;
}
@@ -1085,7 +1084,7 @@ int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
pinctl_name,
pin_offset, pin_offset + npins - 1);
- list_add_tail(&pin_range->node, &chip->pin_ranges);
+ list_add_tail(&pin_range->node, &gdev->pin_ranges);
return 0;
}
@@ -1098,8 +1097,9 @@ EXPORT_SYMBOL_GPL(gpiochip_add_pin_range);
void gpiochip_remove_pin_ranges(struct gpio_chip *chip)
{
struct gpio_pin_range *pin_range, *tmp;
+ struct gpio_device *gdev = chip->gpiodev;
- list_for_each_entry_safe(pin_range, tmp, &chip->pin_ranges, node) {
+ list_for_each_entry_safe(pin_range, tmp, &gdev->pin_ranges, node) {
list_del(&pin_range->node);
pinctrl_remove_gpio_range(pin_range->pctldev,
&pin_range->range);
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-02-11 10:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-11 10:26 [PATCH 0/6] gpiolib refactorings after device addition Linus Walleij
2016-02-11 10:26 ` [PATCH 1/6] gpio: remember to finally free gpio_device Linus Walleij
2016-02-11 10:26 ` [PATCH 2/6] gpio: move sysfs mock device to the gpio_device Linus Walleij
2016-02-11 10:26 ` [PATCH 3/6] gpio: move descriptors into gpio_device Linus Walleij
2016-02-11 10:26 ` [PATCH 4/6] gpio: reflect base and ngpio " Linus Walleij
2016-02-11 10:26 ` [PATCH 5/6] gpio: reference count the gpio device for each desc Linus Walleij
2016-02-11 10:26 ` [PATCH 6/6] gpio: move the pin ranges into gpio_device Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).