gpiod_export_name is similar to gpiod_export, but lets the user
determine the name used to export a gpio pin.
Currently, the pin name is determined by the chip driver with
the 'names' array in the gpio_chip data structure, or it is set
to gpioX, where X is the pin number, if no name is provided by
the chip driver.
It is, however, desirable to be able to provide the pin name when
exporting the pin, for example from platform code. In other words,
it would be useful to move the naming decision from the pin provider
to the pin consumer. The gpio-pca953x driver provides this capability
as part of its platform data. Other drivers could be enhanced in a
similar way; however, this is not always possible or easy to accomplish.
For example, mfd client drivers such as gpio-ich already use platform
data to pass information from the mfd master driver to the client driver.
Overloading this platform data to also provide an array of gpio pin names
would be a challenge if not impossible.
The alternative to use gpiod_export_link is also not always desirable,
since it only creates a named link to a different directory, meaning
the named gpio pin is not available in /sys/class/gpio but only
in some platform specific directory and thus not as generic as possible
and/or useful.
A specific example for a use case is a gpio pin which reports AC power
loss to user space. Depending on the platform and platform variant,
the pin can be provided by various gpio chip drivers and pin numbers.
It would be very desirable to have a well defined location such as
/sys/class/gpio/ac_power_loss for this pin, so user space knows where
to find the attribute without knowledge of the underlying platform
variant or oher hardware details.
Signed-off-by: Guenter Roeck <[email protected]>
---
Applies to tip of linux-gpio/for-next.
Documentation/gpio/sysfs.txt | 12 ++++++++----
drivers/gpio/gpiolib-sysfs.c | 23 ++++++++++++++++-------
include/linux/gpio/consumer.h | 9 +++++++++
3 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
index c2c3a97..8e301b2 100644
--- a/Documentation/gpio/sysfs.txt
+++ b/Documentation/gpio/sysfs.txt
@@ -125,7 +125,11 @@ requested using gpio_request():
/* export the GPIO to userspace */
int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
- /* reverse gpio_export() */
+ /* export named GPIO to userspace */
+ int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
+ bool direction_may_change);
+
+ /* reverse gpio_export() / gpiod_export_name() */
void gpiod_unexport(struct gpio_desc *desc);
/* create a sysfs link to an exported GPIO node */
@@ -136,9 +140,9 @@ requested using gpio_request():
int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
After a kernel driver requests a GPIO, it may only be made available in
-the sysfs interface by gpiod_export(). The driver can control whether the
-signal direction may change. This helps drivers prevent userspace code
-from accidentally clobbering important system state.
+the sysfs interface by gpiod_export() or gpiod_export_name(). The driver
+can control whether the signal direction may change. This helps drivers
+prevent userspace code from accidentally clobbering important system state.
This explicit exporting can help with debugging (by making some kinds
of experiments easier), or can provide an always-there interface that's
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index be45a92..7d36230 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -523,13 +523,12 @@ static struct class gpio_class = {
*
* Returns zero on success, else an error.
*/
-int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
+int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
+ bool direction_may_change)
{
unsigned long flags;
int status;
- const char *ioname = NULL;
struct device *dev;
- int offset;
/* can't export until sysfs is available ... */
if (!gpio_class.p) {
@@ -560,10 +559,6 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
direction_may_change = false;
spin_unlock_irqrestore(&gpio_lock, flags);
- offset = gpio_chip_hwgpio(desc);
- if (desc->chip->names && desc->chip->names[offset])
- ioname = desc->chip->names[offset];
-
dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
desc, ioname ? ioname : "gpio%u",
desc_to_gpio(desc));
@@ -600,6 +595,20 @@ fail_unlock:
gpiod_dbg(desc, "%s: status %d\n", __func__, status);
return status;
}
+EXPORT_SYMBOL_GPL(gpiod_export_name);
+
+int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
+{
+ const char *ioname = NULL;
+
+ if (desc) {
+ int offset = gpio_chip_hwgpio(desc);
+
+ if (desc->chip->names && desc->chip->names[offset])
+ ioname = desc->chip->names[offset];
+ }
+ return gpiod_export_name(desc, ioname, direction_may_change);
+}
EXPORT_SYMBOL_GPL(gpiod_export);
static int match_export(struct device *dev, const void *data)
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 05e53cc..986da3e 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -260,6 +260,8 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
#if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_GPIO_SYSFS)
int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
+int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
+ bool direction_may_change);
int gpiod_export_link(struct device *dev, const char *name,
struct gpio_desc *desc);
int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
@@ -273,6 +275,13 @@ static inline int gpiod_export(struct gpio_desc *desc,
return -ENOSYS;
}
+static inline int gpiod_export_name(struct gpio_desc *desc,
+ const char *ioname,
+ bool direction_may_change)
+{
+ return -ENOSYS;
+}
+
static inline int gpiod_export_link(struct device *dev, const char *name,
struct gpio_desc *desc)
{
--
1.9.1
On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck <[email protected]> wrote:
> gpiod_export_name is similar to gpiod_export, but lets the user
> determine the name used to export a gpio pin.
>
> Currently, the pin name is determined by the chip driver with
> the 'names' array in the gpio_chip data structure, or it is set
> to gpioX, where X is the pin number, if no name is provided by
> the chip driver.
Oh, my. I did not even know about this 'names' array. This is pretty
bad - a GPIO provider should not decide what its GPIOs are used for.
Luckily for you, this creates a precedent that makes this patch more
acceptable, in that it is not making the situation worse. Even though
I consider both solutions to be bad, I actually prefer your
gpiod_export_name() function to that 'names' array in gpio_chip...
>
> It is, however, desirable to be able to provide the pin name when
> exporting the pin, for example from platform code. In other words,
> it would be useful to move the naming decision from the pin provider
> to the pin consumer. The gpio-pca953x driver provides this capability
> as part of its platform data. Other drivers could be enhanced in a
> similar way; however, this is not always possible or easy to accomplish.
> For example, mfd client drivers such as gpio-ich already use platform
> data to pass information from the mfd master driver to the client driver.
> Overloading this platform data to also provide an array of gpio pin names
> would be a challenge if not impossible.
>
> The alternative to use gpiod_export_link is also not always desirable,
> since it only creates a named link to a different directory, meaning
> the named gpio pin is not available in /sys/class/gpio but only
> in some platform specific directory and thus not as generic as possible
> and/or useful.
>
> A specific example for a use case is a gpio pin which reports AC power
> loss to user space. Depending on the platform and platform variant,
> the pin can be provided by various gpio chip drivers and pin numbers.
> It would be very desirable to have a well defined location such as
> /sys/class/gpio/ac_power_loss for this pin, so user space knows where
> to find the attribute without knowledge of the underlying platform
> variant or oher hardware details.
As I explained on the other thread, I still encourage you to have
these GPIOs under device nodes that give a hint of who is provided the
GPIO (effectively exporting the (dev, function) tuple to user-space)
instead of having them popping out under /sys/class/gpio where nobody
knows where they come from and name collisions are much more likely.
Your message sounds like you have no choice but have the named GPIO
link under the gpiochip's device node, but this is not the case -
gpio_export_link() let's you specify the device under which the link
should appear. Make that device be your "scu" device and you can have
a consistent sysfs path to access your GPIOs.
Allowing GPIOs to pop up in the same directory with an arbitrary name
is just a recipe for a mess. But that's a recipe that is already
allowed thanks to that 'names' array. So I'm really confused about
what to do with this patch. If you can do with gpio_export_link() (and
I have not seen evidence of the contrary), please go that way instead.
>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> Applies to tip of linux-gpio/for-next.
>
> Documentation/gpio/sysfs.txt | 12 ++++++++----
> drivers/gpio/gpiolib-sysfs.c | 23 ++++++++++++++++-------
> include/linux/gpio/consumer.h | 9 +++++++++
> 3 files changed, 33 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
> index c2c3a97..8e301b2 100644
> --- a/Documentation/gpio/sysfs.txt
> +++ b/Documentation/gpio/sysfs.txt
> @@ -125,7 +125,11 @@ requested using gpio_request():
> /* export the GPIO to userspace */
> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
>
> - /* reverse gpio_export() */
> + /* export named GPIO to userspace */
> + int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
> + bool direction_may_change);
> +
> + /* reverse gpio_export() / gpiod_export_name() */
> void gpiod_unexport(struct gpio_desc *desc);
>
> /* create a sysfs link to an exported GPIO node */
> @@ -136,9 +140,9 @@ requested using gpio_request():
> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
>
> After a kernel driver requests a GPIO, it may only be made available in
> -the sysfs interface by gpiod_export(). The driver can control whether the
> -signal direction may change. This helps drivers prevent userspace code
> -from accidentally clobbering important system state.
> +the sysfs interface by gpiod_export() or gpiod_export_name(). The driver
> +can control whether the signal direction may change. This helps drivers
> +prevent userspace code from accidentally clobbering important system state.
>
> This explicit exporting can help with debugging (by making some kinds
> of experiments easier), or can provide an always-there interface that's
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index be45a92..7d36230 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -523,13 +523,12 @@ static struct class gpio_class = {
> *
> * Returns zero on success, else an error.
> */
> -int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
> + bool direction_may_change)
> {
> unsigned long flags;
> int status;
> - const char *ioname = NULL;
> struct device *dev;
> - int offset;
>
> /* can't export until sysfs is available ... */
> if (!gpio_class.p) {
> @@ -560,10 +559,6 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
> direction_may_change = false;
> spin_unlock_irqrestore(&gpio_lock, flags);
>
> - offset = gpio_chip_hwgpio(desc);
> - if (desc->chip->names && desc->chip->names[offset])
> - ioname = desc->chip->names[offset];
> -
> dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
> desc, ioname ? ioname : "gpio%u",
> desc_to_gpio(desc));
> @@ -600,6 +595,20 @@ fail_unlock:
> gpiod_dbg(desc, "%s: status %d\n", __func__, status);
> return status;
> }
> +EXPORT_SYMBOL_GPL(gpiod_export_name);
> +
> +int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
> +{
> + const char *ioname = NULL;
> +
> + if (desc) {
> + int offset = gpio_chip_hwgpio(desc);
> +
> + if (desc->chip->names && desc->chip->names[offset])
> + ioname = desc->chip->names[offset];
I'd add a
else
ioname = "gpio%u";
so all the name-chosing code is grouped in the same place. Then you
can remove that same check from gpiod_export_name().
> + }
> + return gpiod_export_name(desc, ioname, direction_may_change);
> +}
> EXPORT_SYMBOL_GPL(gpiod_export);
>
> static int match_export(struct device *dev, const void *data)
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index 05e53cc..986da3e 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -260,6 +260,8 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
> #if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_GPIO_SYSFS)
>
> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
> + bool direction_may_change);
> int gpiod_export_link(struct device *dev, const char *name,
> struct gpio_desc *desc);
> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
> @@ -273,6 +275,13 @@ static inline int gpiod_export(struct gpio_desc *desc,
> return -ENOSYS;
> }
>
> +static inline int gpiod_export_name(struct gpio_desc *desc,
> + const char *ioname,
> + bool direction_may_change)
> +{
> + return -ENOSYS;
> +}
> +
> static inline int gpiod_export_link(struct device *dev, const char *name,
> struct gpio_desc *desc)
> {
> --
> 1.9.1
>
Hi,
just to append to this: is in plan some kind of exporting named GPIOs from device tree to /sys/* or /dev/*? It would be
very useful not only for me. Because I thing what GPIO is used for what is hardware design dependent same as for example
what SPI chips are used. And SPI chips are in DT.
Dne 24.7.2014 v 07:44 Alexandre Courbot napsal(a):
> On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck <[email protected]> wrote:
>> gpiod_export_name is similar to gpiod_export, but lets the user
>> determine the name used to export a gpio pin.
>>
>> Currently, the pin name is determined by the chip driver with
>> the 'names' array in the gpio_chip data structure, or it is set
>> to gpioX, where X is the pin number, if no name is provided by
>> the chip driver.
>
> Oh, my. I did not even know about this 'names' array. This is pretty
> bad - a GPIO provider should not decide what its GPIOs are used for.
>
> Luckily for you, this creates a precedent that makes this patch more
> acceptable, in that it is not making the situation worse. Even though
> I consider both solutions to be bad, I actually prefer your
> gpiod_export_name() function to that 'names' array in gpio_chip...
>
>>
>> It is, however, desirable to be able to provide the pin name when
>> exporting the pin, for example from platform code. In other words,
>> it would be useful to move the naming decision from the pin provider
>> to the pin consumer. The gpio-pca953x driver provides this capability
>> as part of its platform data. Other drivers could be enhanced in a
>> similar way; however, this is not always possible or easy to accomplish.
>> For example, mfd client drivers such as gpio-ich already use platform
>> data to pass information from the mfd master driver to the client driver.
>> Overloading this platform data to also provide an array of gpio pin names
>> would be a challenge if not impossible.
>>
>> The alternative to use gpiod_export_link is also not always desirable,
>> since it only creates a named link to a different directory, meaning
>> the named gpio pin is not available in /sys/class/gpio but only
>> in some platform specific directory and thus not as generic as possible
>> and/or useful.
>>
>> A specific example for a use case is a gpio pin which reports AC power
>> loss to user space. Depending on the platform and platform variant,
>> the pin can be provided by various gpio chip drivers and pin numbers.
>> It would be very desirable to have a well defined location such as
>> /sys/class/gpio/ac_power_loss for this pin, so user space knows where
>> to find the attribute without knowledge of the underlying platform
>> variant or oher hardware details.
>
> As I explained on the other thread, I still encourage you to have
> these GPIOs under device nodes that give a hint of who is provided the
> GPIO (effectively exporting the (dev, function) tuple to user-space)
> instead of having them popping out under /sys/class/gpio where nobody
> knows where they come from and name collisions are much more likely.
>
> Your message sounds like you have no choice but have the named GPIO
> link under the gpiochip's device node, but this is not the case -
> gpio_export_link() let's you specify the device under which the link
> should appear. Make that device be your "scu" device and you can have
> a consistent sysfs path to access your GPIOs.
>
> Allowing GPIOs to pop up in the same directory with an arbitrary name
> is just a recipe for a mess. But that's a recipe that is already
> allowed thanks to that 'names' array. So I'm really confused about
> what to do with this patch. If you can do with gpio_export_link() (and
> I have not seen evidence of the contrary), please go that way instead.
>
>>
>> Signed-off-by: Guenter Roeck <[email protected]>
>> ---
>> Applies to tip of linux-gpio/for-next.
>>
>> Documentation/gpio/sysfs.txt | 12 ++++++++----
>> drivers/gpio/gpiolib-sysfs.c | 23 ++++++++++++++++-------
>> include/linux/gpio/consumer.h | 9 +++++++++
>> 3 files changed, 33 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
>> index c2c3a97..8e301b2 100644
>> --- a/Documentation/gpio/sysfs.txt
>> +++ b/Documentation/gpio/sysfs.txt
>> @@ -125,7 +125,11 @@ requested using gpio_request():
>> /* export the GPIO to userspace */
>> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
>>
>> - /* reverse gpio_export() */
>> + /* export named GPIO to userspace */
>> + int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>> + bool direction_may_change);
>> +
>> + /* reverse gpio_export() / gpiod_export_name() */
>> void gpiod_unexport(struct gpio_desc *desc);
>>
>> /* create a sysfs link to an exported GPIO node */
>> @@ -136,9 +140,9 @@ requested using gpio_request():
>> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
>>
>> After a kernel driver requests a GPIO, it may only be made available in
>> -the sysfs interface by gpiod_export(). The driver can control whether the
>> -signal direction may change. This helps drivers prevent userspace code
>> -from accidentally clobbering important system state.
>> +the sysfs interface by gpiod_export() or gpiod_export_name(). The driver
>> +can control whether the signal direction may change. This helps drivers
>> +prevent userspace code from accidentally clobbering important system state.
>>
>> This explicit exporting can help with debugging (by making some kinds
>> of experiments easier), or can provide an always-there interface that's
>> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
>> index be45a92..7d36230 100644
>> --- a/drivers/gpio/gpiolib-sysfs.c
>> +++ b/drivers/gpio/gpiolib-sysfs.c
>> @@ -523,13 +523,12 @@ static struct class gpio_class = {
>> *
>> * Returns zero on success, else an error.
>> */
>> -int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>> + bool direction_may_change)
>> {
>> unsigned long flags;
>> int status;
>> - const char *ioname = NULL;
>> struct device *dev;
>> - int offset;
>>
>> /* can't export until sysfs is available ... */
>> if (!gpio_class.p) {
>> @@ -560,10 +559,6 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>> direction_may_change = false;
>> spin_unlock_irqrestore(&gpio_lock, flags);
>>
>> - offset = gpio_chip_hwgpio(desc);
>> - if (desc->chip->names && desc->chip->names[offset])
>> - ioname = desc->chip->names[offset];
>> -
>> dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
>> desc, ioname ? ioname : "gpio%u",
>> desc_to_gpio(desc));
>> @@ -600,6 +595,20 @@ fail_unlock:
>> gpiod_dbg(desc, "%s: status %d\n", __func__, status);
>> return status;
>> }
>> +EXPORT_SYMBOL_GPL(gpiod_export_name);
>> +
>> +int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>> +{
>> + const char *ioname = NULL;
>> +
>> + if (desc) {
>> + int offset = gpio_chip_hwgpio(desc);
>> +
>> + if (desc->chip->names && desc->chip->names[offset])
>> + ioname = desc->chip->names[offset];
>
> I'd add a
>
> else
> ioname = "gpio%u";
>
> so all the name-chosing code is grouped in the same place. Then you
> can remove that same check from gpiod_export_name().
>
>> + }
>> + return gpiod_export_name(desc, ioname, direction_may_change);
>> +}
>> EXPORT_SYMBOL_GPL(gpiod_export);
>>
>> static int match_export(struct device *dev, const void *data)
>> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>> index 05e53cc..986da3e 100644
>> --- a/include/linux/gpio/consumer.h
>> +++ b/include/linux/gpio/consumer.h
>> @@ -260,6 +260,8 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
>> #if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_GPIO_SYSFS)
>>
>> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
>> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>> + bool direction_may_change);
>> int gpiod_export_link(struct device *dev, const char *name,
>> struct gpio_desc *desc);
>> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
>> @@ -273,6 +275,13 @@ static inline int gpiod_export(struct gpio_desc *desc,
>> return -ENOSYS;
>> }
>>
>> +static inline int gpiod_export_name(struct gpio_desc *desc,
>> + const char *ioname,
>> + bool direction_may_change)
>> +{
>> + return -ENOSYS;
>> +}
>> +
>> static inline int gpiod_export_link(struct device *dev, const char *name,
>> struct gpio_desc *desc)
>> {
>> --
>> 1.9.1
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 07/23/2014 10:44 PM, Alexandre Courbot wrote:
> On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck <[email protected]> wrote:
>> gpiod_export_name is similar to gpiod_export, but lets the user
>> determine the name used to export a gpio pin.
>>
>> Currently, the pin name is determined by the chip driver with
>> the 'names' array in the gpio_chip data structure, or it is set
>> to gpioX, where X is the pin number, if no name is provided by
>> the chip driver.
>
> Oh, my. I did not even know about this 'names' array. This is pretty
> bad - a GPIO provider should not decide what its GPIOs are used for.
>
> Luckily for you, this creates a precedent that makes this patch more
> acceptable, in that it is not making the situation worse. Even though
> I consider both solutions to be bad, I actually prefer your
> gpiod_export_name() function to that 'names' array in gpio_chip...
>
>>
>> It is, however, desirable to be able to provide the pin name when
>> exporting the pin, for example from platform code. In other words,
>> it would be useful to move the naming decision from the pin provider
>> to the pin consumer. The gpio-pca953x driver provides this capability
>> as part of its platform data. Other drivers could be enhanced in a
>> similar way; however, this is not always possible or easy to accomplish.
>> For example, mfd client drivers such as gpio-ich already use platform
>> data to pass information from the mfd master driver to the client driver.
>> Overloading this platform data to also provide an array of gpio pin names
>> would be a challenge if not impossible.
>>
>> The alternative to use gpiod_export_link is also not always desirable,
>> since it only creates a named link to a different directory, meaning
>> the named gpio pin is not available in /sys/class/gpio but only
>> in some platform specific directory and thus not as generic as possible
>> and/or useful.
>>
>> A specific example for a use case is a gpio pin which reports AC power
>> loss to user space. Depending on the platform and platform variant,
>> the pin can be provided by various gpio chip drivers and pin numbers.
>> It would be very desirable to have a well defined location such as
>> /sys/class/gpio/ac_power_loss for this pin, so user space knows where
>> to find the attribute without knowledge of the underlying platform
>> variant or oher hardware details.
>
> As I explained on the other thread, I still encourage you to have
> these GPIOs under device nodes that give a hint of who is provided the
> GPIO (effectively exporting the (dev, function) tuple to user-space)
> instead of having them popping out under /sys/class/gpio where nobody
> knows where they come from and name collisions are much more likely.
>
> Your message sounds like you have no choice but have the named GPIO
> link under the gpiochip's device node, but this is not the case -
> gpio_export_link() let's you specify the device under which the link
> should appear. Make that device be your "scu" device and you can have
> a consistent sysfs path to access your GPIOs.
>
Yes, I understand, but that is not acceptable for the users - see below.
> Allowing GPIOs to pop up in the same directory with an arbitrary name
> is just a recipe for a mess. But that's a recipe that is already
> allowed thanks to that 'names' array. So I'm really confused about
> what to do with this patch. If you can do with gpio_export_link() (and
> I have not seen evidence of the contrary), please go that way instead.
>
I personally don't think it is that much of a mess. Sure, once has to be
careful when selecting names, but I don't see a problem with that.
I have two users for this. Interestingly the problem is pretty
much the same, though the applications are completely different.
One is the company using the scu.c file. They are currently using the
pca953x driver approach (using the names array), but they also have
a new version of their product which also uses gpio pins from gpio-ich.
For consistency, they want all gpio pins in the same directory, meaning
/sys/class/gpio.
The currently implemented solution is to have a weak pointer to the names
array in gpio-ch.c and override it with a pointer from scu.c.
/* SCU specific gpio pin names. Only works if module is built into kernel. */
static const char * const scu_ichx_gpiolib_names[128] = {
[0] = "switch_interrupt", /* GPI0 */
[3] = "ac_loss_detect", /* GPI3 */
[16] = "debug_out", /* GPO0 */
[20] = "switch_reset", /* GPO3 */
};
[ switch_interrupt and switch_reset will ultimately make it into the kernel,
to be used by a dsa driver, but that is besides the point. ]
const char * const (* const ichx_gpiolib_names)[] = &scu_ichx_gpiolib_names;
and ichx_gpiolib_names is declared as
/* Allow overriding default gpio pin names */
const char * const (* const __weak ichx_gpiolib_names)[];
in gpio-ich.c. Pretty hackish, but at least it works. Slim chance though
to get this accepted upstream. Since the ultimate idea is to submit all
this code upstream, I would prefer to find a solution which is
acceptable for both the user and for upstream integration.
The second user is my employer. Same thing here, though even more complex
as there are several different platforms to support with different platform
drivers. Each platform exports a number of gpio pins to user space, often with
the same functionality across platforms. The request here is to have all
those pins in the same directory, for all platforms, which again suggests
using /sys/class/gpio.
Current approach is to use the "export" file to request pin exports,
which has its own challenges such as having to search for the pin numbers.
Well defined names and pins exported from the kernel would be much cleaner
and be easier to handle.
Of course, I could try to come up with a new class named "gpios" or similar,
put everything there, and start selling that idea. Somehow that doesn't
sound like a good idea to me.
>>
>> Signed-off-by: Guenter Roeck <[email protected]>
>> ---
>> Applies to tip of linux-gpio/for-next.
>>
>> Documentation/gpio/sysfs.txt | 12 ++++++++----
>> drivers/gpio/gpiolib-sysfs.c | 23 ++++++++++++++++-------
>> include/linux/gpio/consumer.h | 9 +++++++++
>> 3 files changed, 33 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
>> index c2c3a97..8e301b2 100644
>> --- a/Documentation/gpio/sysfs.txt
>> +++ b/Documentation/gpio/sysfs.txt
>> @@ -125,7 +125,11 @@ requested using gpio_request():
>> /* export the GPIO to userspace */
>> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
>>
>> - /* reverse gpio_export() */
>> + /* export named GPIO to userspace */
>> + int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>> + bool direction_may_change);
>> +
>> + /* reverse gpio_export() / gpiod_export_name() */
>> void gpiod_unexport(struct gpio_desc *desc);
>>
>> /* create a sysfs link to an exported GPIO node */
>> @@ -136,9 +140,9 @@ requested using gpio_request():
>> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
>>
>> After a kernel driver requests a GPIO, it may only be made available in
>> -the sysfs interface by gpiod_export(). The driver can control whether the
>> -signal direction may change. This helps drivers prevent userspace code
>> -from accidentally clobbering important system state.
>> +the sysfs interface by gpiod_export() or gpiod_export_name(). The driver
>> +can control whether the signal direction may change. This helps drivers
>> +prevent userspace code from accidentally clobbering important system state.
>>
>> This explicit exporting can help with debugging (by making some kinds
>> of experiments easier), or can provide an always-there interface that's
>> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
>> index be45a92..7d36230 100644
>> --- a/drivers/gpio/gpiolib-sysfs.c
>> +++ b/drivers/gpio/gpiolib-sysfs.c
>> @@ -523,13 +523,12 @@ static struct class gpio_class = {
>> *
>> * Returns zero on success, else an error.
>> */
>> -int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>> + bool direction_may_change)
>> {
>> unsigned long flags;
>> int status;
>> - const char *ioname = NULL;
>> struct device *dev;
>> - int offset;
>>
>> /* can't export until sysfs is available ... */
>> if (!gpio_class.p) {
>> @@ -560,10 +559,6 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>> direction_may_change = false;
>> spin_unlock_irqrestore(&gpio_lock, flags);
>>
>> - offset = gpio_chip_hwgpio(desc);
>> - if (desc->chip->names && desc->chip->names[offset])
>> - ioname = desc->chip->names[offset];
>> -
>> dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
>> desc, ioname ? ioname : "gpio%u",
>> desc_to_gpio(desc));
>> @@ -600,6 +595,20 @@ fail_unlock:
>> gpiod_dbg(desc, "%s: status %d\n", __func__, status);
>> return status;
>> }
>> +EXPORT_SYMBOL_GPL(gpiod_export_name);
>> +
>> +int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>> +{
>> + const char *ioname = NULL;
>> +
>> + if (desc) {
>> + int offset = gpio_chip_hwgpio(desc);
>> +
>> + if (desc->chip->names && desc->chip->names[offset])
>> + ioname = desc->chip->names[offset];
>
> I'd add a
>
> else
> ioname = "gpio%u";
>
> so all the name-chosing code is grouped in the same place. Then you
> can remove that same check from gpiod_export_name().
>
Sure, no problem, though in that case I should probably add a check
to gpiod_export_name to ensure that the passed ioname is not NULL
and return -EINVAL if it is.
Guenter
On 07/23/2014 11:29 PM, Jiří Prchal wrote:
> Hi,
> just to append to this: is in plan some kind of exporting named GPIOs from device tree to /sys/* or /dev/*? It would be very useful not only for me. Because I thing what GPIO is used for what is hardware design dependent same as for example what SPI chips are used. And SPI chips are in DT.
>
Yes, for one of my use cases that is how I would probably configure it;
alternatively it would be configured with platform data. It is
somewhat questionable, however, if this approach would be acceptable
for the Linux dt community, as it is a corner case between system
(hardware) description and configuration.
Guenter
> Dne 24.7.2014 v 07:44 Alexandre Courbot napsal(a):
>> On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck <[email protected]> wrote:
>>> gpiod_export_name is similar to gpiod_export, but lets the user
>>> determine the name used to export a gpio pin.
>>>
>>> Currently, the pin name is determined by the chip driver with
>>> the 'names' array in the gpio_chip data structure, or it is set
>>> to gpioX, where X is the pin number, if no name is provided by
>>> the chip driver.
>>
>> Oh, my. I did not even know about this 'names' array. This is pretty
>> bad - a GPIO provider should not decide what its GPIOs are used for.
>>
>> Luckily for you, this creates a precedent that makes this patch more
>> acceptable, in that it is not making the situation worse. Even though
>> I consider both solutions to be bad, I actually prefer your
>> gpiod_export_name() function to that 'names' array in gpio_chip...
>>
>>>
>>> It is, however, desirable to be able to provide the pin name when
>>> exporting the pin, for example from platform code. In other words,
>>> it would be useful to move the naming decision from the pin provider
>>> to the pin consumer. The gpio-pca953x driver provides this capability
>>> as part of its platform data. Other drivers could be enhanced in a
>>> similar way; however, this is not always possible or easy to accomplish.
>>> For example, mfd client drivers such as gpio-ich already use platform
>>> data to pass information from the mfd master driver to the client driver.
>>> Overloading this platform data to also provide an array of gpio pin names
>>> would be a challenge if not impossible.
>>>
>>> The alternative to use gpiod_export_link is also not always desirable,
>>> since it only creates a named link to a different directory, meaning
>>> the named gpio pin is not available in /sys/class/gpio but only
>>> in some platform specific directory and thus not as generic as possible
>>> and/or useful.
>>>
>>> A specific example for a use case is a gpio pin which reports AC power
>>> loss to user space. Depending on the platform and platform variant,
>>> the pin can be provided by various gpio chip drivers and pin numbers.
>>> It would be very desirable to have a well defined location such as
>>> /sys/class/gpio/ac_power_loss for this pin, so user space knows where
>>> to find the attribute without knowledge of the underlying platform
>>> variant or oher hardware details.
>>
>> As I explained on the other thread, I still encourage you to have
>> these GPIOs under device nodes that give a hint of who is provided the
>> GPIO (effectively exporting the (dev, function) tuple to user-space)
>> instead of having them popping out under /sys/class/gpio where nobody
>> knows where they come from and name collisions are much more likely.
>>
>> Your message sounds like you have no choice but have the named GPIO
>> link under the gpiochip's device node, but this is not the case -
>> gpio_export_link() let's you specify the device under which the link
>> should appear. Make that device be your "scu" device and you can have
>> a consistent sysfs path to access your GPIOs.
>>
>> Allowing GPIOs to pop up in the same directory with an arbitrary name
>> is just a recipe for a mess. But that's a recipe that is already
>> allowed thanks to that 'names' array. So I'm really confused about
>> what to do with this patch. If you can do with gpio_export_link() (and
>> I have not seen evidence of the contrary), please go that way instead.
>>
>>>
>>> Signed-off-by: Guenter Roeck <[email protected]>
>>> ---
>>> Applies to tip of linux-gpio/for-next.
>>>
>>> Documentation/gpio/sysfs.txt | 12 ++++++++----
>>> drivers/gpio/gpiolib-sysfs.c | 23 ++++++++++++++++-------
>>> include/linux/gpio/consumer.h | 9 +++++++++
>>> 3 files changed, 33 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
>>> index c2c3a97..8e301b2 100644
>>> --- a/Documentation/gpio/sysfs.txt
>>> +++ b/Documentation/gpio/sysfs.txt
>>> @@ -125,7 +125,11 @@ requested using gpio_request():
>>> /* export the GPIO to userspace */
>>> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
>>>
>>> - /* reverse gpio_export() */
>>> + /* export named GPIO to userspace */
>>> + int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>>> + bool direction_may_change);
>>> +
>>> + /* reverse gpio_export() / gpiod_export_name() */
>>> void gpiod_unexport(struct gpio_desc *desc);
>>>
>>> /* create a sysfs link to an exported GPIO node */
>>> @@ -136,9 +140,9 @@ requested using gpio_request():
>>> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
>>>
>>> After a kernel driver requests a GPIO, it may only be made available in
>>> -the sysfs interface by gpiod_export(). The driver can control whether the
>>> -signal direction may change. This helps drivers prevent userspace code
>>> -from accidentally clobbering important system state.
>>> +the sysfs interface by gpiod_export() or gpiod_export_name(). The driver
>>> +can control whether the signal direction may change. This helps drivers
>>> +prevent userspace code from accidentally clobbering important system state.
>>>
>>> This explicit exporting can help with debugging (by making some kinds
>>> of experiments easier), or can provide an always-there interface that's
>>> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
>>> index be45a92..7d36230 100644
>>> --- a/drivers/gpio/gpiolib-sysfs.c
>>> +++ b/drivers/gpio/gpiolib-sysfs.c
>>> @@ -523,13 +523,12 @@ static struct class gpio_class = {
>>> *
>>> * Returns zero on success, else an error.
>>> */
>>> -int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>>> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>>> + bool direction_may_change)
>>> {
>>> unsigned long flags;
>>> int status;
>>> - const char *ioname = NULL;
>>> struct device *dev;
>>> - int offset;
>>>
>>> /* can't export until sysfs is available ... */
>>> if (!gpio_class.p) {
>>> @@ -560,10 +559,6 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>>> direction_may_change = false;
>>> spin_unlock_irqrestore(&gpio_lock, flags);
>>>
>>> - offset = gpio_chip_hwgpio(desc);
>>> - if (desc->chip->names && desc->chip->names[offset])
>>> - ioname = desc->chip->names[offset];
>>> -
>>> dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
>>> desc, ioname ? ioname : "gpio%u",
>>> desc_to_gpio(desc));
>>> @@ -600,6 +595,20 @@ fail_unlock:
>>> gpiod_dbg(desc, "%s: status %d\n", __func__, status);
>>> return status;
>>> }
>>> +EXPORT_SYMBOL_GPL(gpiod_export_name);
>>> +
>>> +int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>>> +{
>>> + const char *ioname = NULL;
>>> +
>>> + if (desc) {
>>> + int offset = gpio_chip_hwgpio(desc);
>>> +
>>> + if (desc->chip->names && desc->chip->names[offset])
>>> + ioname = desc->chip->names[offset];
>>
>> I'd add a
>>
>> else
>> ioname = "gpio%u";
>>
>> so all the name-chosing code is grouped in the same place. Then you
>> can remove that same check from gpiod_export_name().
>>
>>> + }
>>> + return gpiod_export_name(desc, ioname, direction_may_change);
>>> +}
>>> EXPORT_SYMBOL_GPL(gpiod_export);
>>>
>>> static int match_export(struct device *dev, const void *data)
>>> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>>> index 05e53cc..986da3e 100644
>>> --- a/include/linux/gpio/consumer.h
>>> +++ b/include/linux/gpio/consumer.h
>>> @@ -260,6 +260,8 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
>>> #if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_GPIO_SYSFS)
>>>
>>> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
>>> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>>> + bool direction_may_change);
>>> int gpiod_export_link(struct device *dev, const char *name,
>>> struct gpio_desc *desc);
>>> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
>>> @@ -273,6 +275,13 @@ static inline int gpiod_export(struct gpio_desc *desc,
>>> return -ENOSYS;
>>> }
>>>
>>> +static inline int gpiod_export_name(struct gpio_desc *desc,
>>> + const char *ioname,
>>> + bool direction_may_change)
>>> +{
>>> + return -ENOSYS;
>>> +}
>>> +
>>> static inline int gpiod_export_link(struct device *dev, const char *name,
>>> struct gpio_desc *desc)
>>> {
>>> --
>>> 1.9.1
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
What about this modification? If is defined label, use it prioritlly, at second use name in chip description.
@@ -842,7 +842,9 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
spin_unlock_irqrestore(&gpio_lock, flags);
offset = gpio_chip_hwgpio(desc);
- if (desc->chip->names && desc->chip->names[offset])
+ if (desc->label)
+ ioname = desc->label;
+ else if (desc->chip->names && desc->chip->names[offset])
ioname = desc->chip->names[offset];
dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
Dne 24.7.2014 v 08:36 Guenter Roeck napsal(a):
> On 07/23/2014 11:29 PM, Jiří Prchal wrote:
>> Hi,
>> just to append to this: is in plan some kind of exporting named GPIOs from device tree to /sys/* or /dev/*? It would
>> be very useful not only for me. Because I thing what GPIO is used for what is hardware design dependent same as for
>> example what SPI chips are used. And SPI chips are in DT.
>>
>
> Yes, for one of my use cases that is how I would probably configure it;
> alternatively it would be configured with platform data. It is
> somewhat questionable, however, if this approach would be acceptable
> for the Linux dt community, as it is a corner case between system
> (hardware) description and configuration.
>
> Guenter
>
>> Dne 24.7.2014 v 07:44 Alexandre Courbot napsal(a):
>>> On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck <[email protected]> wrote:
>>>> gpiod_export_name is similar to gpiod_export, but lets the user
>>>> determine the name used to export a gpio pin.
>>>>
>>>> Currently, the pin name is determined by the chip driver with
>>>> the 'names' array in the gpio_chip data structure, or it is set
>>>> to gpioX, where X is the pin number, if no name is provided by
>>>> the chip driver.
>>>
>>> Oh, my. I did not even know about this 'names' array. This is pretty
>>> bad - a GPIO provider should not decide what its GPIOs are used for.
>>>
>>> Luckily for you, this creates a precedent that makes this patch more
>>> acceptable, in that it is not making the situation worse. Even though
>>> I consider both solutions to be bad, I actually prefer your
>>> gpiod_export_name() function to that 'names' array in gpio_chip...
>>>
>>>>
>>>> It is, however, desirable to be able to provide the pin name when
>>>> exporting the pin, for example from platform code. In other words,
>>>> it would be useful to move the naming decision from the pin provider
>>>> to the pin consumer. The gpio-pca953x driver provides this capability
>>>> as part of its platform data. Other drivers could be enhanced in a
>>>> similar way; however, this is not always possible or easy to accomplish.
>>>> For example, mfd client drivers such as gpio-ich already use platform
>>>> data to pass information from the mfd master driver to the client driver.
>>>> Overloading this platform data to also provide an array of gpio pin names
>>>> would be a challenge if not impossible.
>>>>
>>>> The alternative to use gpiod_export_link is also not always desirable,
>>>> since it only creates a named link to a different directory, meaning
>>>> the named gpio pin is not available in /sys/class/gpio but only
>>>> in some platform specific directory and thus not as generic as possible
>>>> and/or useful.
>>>>
>>>> A specific example for a use case is a gpio pin which reports AC power
>>>> loss to user space. Depending on the platform and platform variant,
>>>> the pin can be provided by various gpio chip drivers and pin numbers.
>>>> It would be very desirable to have a well defined location such as
>>>> /sys/class/gpio/ac_power_loss for this pin, so user space knows where
>>>> to find the attribute without knowledge of the underlying platform
>>>> variant or oher hardware details.
>>>
>>> As I explained on the other thread, I still encourage you to have
>>> these GPIOs under device nodes that give a hint of who is provided the
>>> GPIO (effectively exporting the (dev, function) tuple to user-space)
>>> instead of having them popping out under /sys/class/gpio where nobody
>>> knows where they come from and name collisions are much more likely.
>>>
>>> Your message sounds like you have no choice but have the named GPIO
>>> link under the gpiochip's device node, but this is not the case -
>>> gpio_export_link() let's you specify the device under which the link
>>> should appear. Make that device be your "scu" device and you can have
>>> a consistent sysfs path to access your GPIOs.
>>>
>>> Allowing GPIOs to pop up in the same directory with an arbitrary name
>>> is just a recipe for a mess. But that's a recipe that is already
>>> allowed thanks to that 'names' array. So I'm really confused about
>>> what to do with this patch. If you can do with gpio_export_link() (and
>>> I have not seen evidence of the contrary), please go that way instead.
>>>
>>>>
>>>> Signed-off-by: Guenter Roeck <[email protected]>
>>>> ---
>>>> Applies to tip of linux-gpio/for-next.
>>>>
>>>> Documentation/gpio/sysfs.txt | 12 ++++++++----
>>>> drivers/gpio/gpiolib-sysfs.c | 23 ++++++++++++++++-------
>>>> include/linux/gpio/consumer.h | 9 +++++++++
>>>> 3 files changed, 33 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
>>>> index c2c3a97..8e301b2 100644
>>>> --- a/Documentation/gpio/sysfs.txt
>>>> +++ b/Documentation/gpio/sysfs.txt
>>>> @@ -125,7 +125,11 @@ requested using gpio_request():
>>>> /* export the GPIO to userspace */
>>>> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
>>>>
>>>> - /* reverse gpio_export() */
>>>> + /* export named GPIO to userspace */
>>>> + int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>>>> + bool direction_may_change);
>>>> +
>>>> + /* reverse gpio_export() / gpiod_export_name() */
>>>> void gpiod_unexport(struct gpio_desc *desc);
>>>>
>>>> /* create a sysfs link to an exported GPIO node */
>>>> @@ -136,9 +140,9 @@ requested using gpio_request():
>>>> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
>>>>
>>>> After a kernel driver requests a GPIO, it may only be made available in
>>>> -the sysfs interface by gpiod_export(). The driver can control whether the
>>>> -signal direction may change. This helps drivers prevent userspace code
>>>> -from accidentally clobbering important system state.
>>>> +the sysfs interface by gpiod_export() or gpiod_export_name(). The driver
>>>> +can control whether the signal direction may change. This helps drivers
>>>> +prevent userspace code from accidentally clobbering important system state.
>>>>
>>>> This explicit exporting can help with debugging (by making some kinds
>>>> of experiments easier), or can provide an always-there interface that's
>>>> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
>>>> index be45a92..7d36230 100644
>>>> --- a/drivers/gpio/gpiolib-sysfs.c
>>>> +++ b/drivers/gpio/gpiolib-sysfs.c
>>>> @@ -523,13 +523,12 @@ static struct class gpio_class = {
>>>> *
>>>> * Returns zero on success, else an error.
>>>> */
>>>> -int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>>>> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>>>> + bool direction_may_change)
>>>> {
>>>> unsigned long flags;
>>>> int status;
>>>> - const char *ioname = NULL;
>>>> struct device *dev;
>>>> - int offset;
>>>>
>>>> /* can't export until sysfs is available ... */
>>>> if (!gpio_class.p) {
>>>> @@ -560,10 +559,6 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>>>> direction_may_change = false;
>>>> spin_unlock_irqrestore(&gpio_lock, flags);
>>>>
>>>> - offset = gpio_chip_hwgpio(desc);
>>>> - if (desc->chip->names && desc->chip->names[offset])
>>>> - ioname = desc->chip->names[offset];
>>>> -
>>>> dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
>>>> desc, ioname ? ioname : "gpio%u",
>>>> desc_to_gpio(desc));
>>>> @@ -600,6 +595,20 @@ fail_unlock:
>>>> gpiod_dbg(desc, "%s: status %d\n", __func__, status);
>>>> return status;
>>>> }
>>>> +EXPORT_SYMBOL_GPL(gpiod_export_name);
>>>> +
>>>> +int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>>>> +{
>>>> + const char *ioname = NULL;
>>>> +
>>>> + if (desc) {
>>>> + int offset = gpio_chip_hwgpio(desc);
>>>> +
>>>> + if (desc->chip->names && desc->chip->names[offset])
>>>> + ioname = desc->chip->names[offset];
>>>
>>> I'd add a
>>>
>>> else
>>> ioname = "gpio%u";
>>>
>>> so all the name-chosing code is grouped in the same place. Then you
>>> can remove that same check from gpiod_export_name().
>>>
>>>> + }
>>>> + return gpiod_export_name(desc, ioname, direction_may_change);
>>>> +}
>>>> EXPORT_SYMBOL_GPL(gpiod_export);
>>>>
>>>> static int match_export(struct device *dev, const void *data)
>>>> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>>>> index 05e53cc..986da3e 100644
>>>> --- a/include/linux/gpio/consumer.h
>>>> +++ b/include/linux/gpio/consumer.h
>>>> @@ -260,6 +260,8 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
>>>> #if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_GPIO_SYSFS)
>>>>
>>>> int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
>>>> +int gpiod_export_name(struct gpio_desc *desc, const char *ioname,
>>>> + bool direction_may_change);
>>>> int gpiod_export_link(struct device *dev, const char *name,
>>>> struct gpio_desc *desc);
>>>> int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
>>>> @@ -273,6 +275,13 @@ static inline int gpiod_export(struct gpio_desc *desc,
>>>> return -ENOSYS;
>>>> }
>>>>
>>>> +static inline int gpiod_export_name(struct gpio_desc *desc,
>>>> + const char *ioname,
>>>> + bool direction_may_change)
>>>> +{
>>>> + return -ENOSYS;
>>>> +}
>>>> +
>>>> static inline int gpiod_export_link(struct device *dev, const char *name,
>>>> struct gpio_desc *desc)
>>>> {
>>>> --
>>>> 1.9.1
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 07/25/2014 12:46 AM, Jiří Prchal wrote:
> What about this modification? If is defined label, use it prioritlly, at second use name in chip description.
>
> @@ -842,7 +842,9 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
> spin_unlock_irqrestore(&gpio_lock, flags);
>
> offset = gpio_chip_hwgpio(desc);
> - if (desc->chip->names && desc->chip->names[offset])
> + if (desc->label)
> + ioname = desc->label;
> + else if (desc->chip->names && desc->chip->names[offset])
> ioname = desc->chip->names[offset];
>
Label is not unique. It is, for example, used by the sysfs export function,
and all pins exported from user space have the same label. The first
pin exported through sysfs would be named "sysfs", the second export
would fail.
Guenter