Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Sun, Aug 31, 2014 at 2:57 AM, Alexandre Courbot wrote: > Linus, please consider my position about this patch as neutral, and > feel free to take it if you think it is worth. I will meditate on this. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Sun, Aug 31, 2014 at 2:57 AM, Alexandre Courbot gnu...@gmail.com wrote: Linus, please consider my position about this patch as neutral, and feel free to take it if you think it is worth. I will meditate on this. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Fri, Aug 29, 2014 at 10:37 AM, Guenter Roeck wrote: > On Fri, Aug 29, 2014 at 10:00:47AM -0700, Alexandre Courbot wrote: >> On Thu, Aug 28, 2014 at 10:54 PM, Guenter Roeck wrote: >> > On 08/28/2014 10:44 PM, Linus Walleij wrote: >> >> >> >> On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot >> >> wrote: >> >>> >> >>> On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck >> >>> wrote: >> >> >> >> >> This is just one of many patches which would make it possible to submit >> the rest, which would make use of it. What you are saying is that it >> won't >> make sense to submit that series into the kernel, because one of the >> very >> first patches needed to enable that won't be accepted. Kind of a >> circular >> argument, but I guess I'll have to live with it. >> >>> >> >>> >> >>> Well I have not seen the other patches you mention and cannot guess >> >>> their existence. If you send the full series it will of course be >> >>> considered as such, but right now this lone patch does not hint any >> >>> upstream user for this interface. >> >>> >> >>> Note that this doesn't change anything to the core of the argument ; >> >>> we have not heard what Linus thinks about named GPIOs in >> >>> /sys/class/gpio yet, maybe he will have a different opinion... >> >> >> >> >> >> The sysfs is sort of broken by design because of things like this and >> >> some other stuff. >> >> >> >> I think the sysfs is scary, for example since it's not hierarchical >> >> but flat and build on the assumption that there is one single >> >> GPIO numberspace. As pointed out in some other message >> >> in the thread it would be nicer to have: >> >> >> >> /dev/gpiochip0/gpio0 >> >> /dev/gpiochip0/gpio1 >> >> >> >> >> >> instead of the horrid sysfs ABI that will have to maintain forever. >> >> >> > Blocking any attempts to make it more useful doesn't help much, though. >> >> This patch is not making it more useful. It just introduces an >> inferior way to do something that is already possible. >> >> I have stated it countless times already, but again: >> "/sys/bus/.../device/gpio_function" is better than >> "/sys/class/gpio/device_function_foo_whatnot" (and actually, >> "/sys/bus/.../device/gpios/function" would be even better). >> >> You can already do the former. I just don't see the need to introduce >> an API to do the latter. >> >> Why is the former better? Because it uses the sysfs hierarchy to make >> the GPIO visible under their consumer's node. That's how sysfs is >> intended to be used. >> >> Just explain me why you cannot live with this or why your proposal is >> better, and my concerns about this patch will be lifted. >> > > "device" is a platform driver and thus platform specific, which defeats > the purpose of having a well defined path and makes it even more difficult > to find a pin across multiple platform variants (which will have different > platform drivers). It means user space will still need platform specific > configuration data to find the pin. If user space needs such configuration > data anyway, it may as well be a "" tuple instead of > "". Even worse, in one of my use > cases some of the platform drivers may have multiple instances with dynamic > instance numbers, meaning I don't even have a well defined path to start with. > > On the other side, as it turns out, most of the gpio chips I deal with are > company specific, and you can not prevent me from populating the 'names' > array for those. It is a bit on the clumsy side, as I would prefer to have > the consumer select the name instead of the provider, but it works. For the > other drivers (so far only one) I only need an out-of-tree patch with a > couple of lines of code to be able to populate the 'names' field. So unless > you take the 'names' field away, which would break the existing ABI and is > thus at least somewhat unlikely, I can still do what I need to do. We won't take the names field away, as you mentioned we will have to support the user-space ABI forever and ever. I hope that at some point we will come with a better alternative, but the old one is here to stay anyway. Linus and myself explained why this ABI is bad ; however, it is also true that there is no better alternative besides implementing a custom solution. Which allows us to reduce the argument about your patch to: "it's a more comfortable way of doing something bad". Since that something bad is not going away, we might as well have another (arguably better) way of doing it. Also: - This patch is limited to gpiolib-sysfs.c, so damage is contained there - The patch is small - I cannot help but notice that Linus did not scream about it - I did not have a strong reject about it either, but was looking for a better way to achieve the desired result. Looks like there is none at the moment. So I'm somehow ok if this patch makes it in ; I still think we should not encourage usage of this ABI, but you do not introduce any new harmful toy - just provide
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Fri, Aug 29, 2014 at 10:37 AM, Guenter Roeck li...@roeck-us.net wrote: On Fri, Aug 29, 2014 at 10:00:47AM -0700, Alexandre Courbot wrote: On Thu, Aug 28, 2014 at 10:54 PM, Guenter Roeck li...@roeck-us.net wrote: On 08/28/2014 10:44 PM, Linus Walleij wrote: On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot gnu...@gmail.com wrote: On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck li...@roeck-us.net wrote: This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. Well I have not seen the other patches you mention and cannot guess their existence. If you send the full series it will of course be considered as such, but right now this lone patch does not hint any upstream user for this interface. Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... The sysfs is sort of broken by design because of things like this and some other stuff. I think the sysfs is scary, for example since it's not hierarchical but flat and build on the assumption that there is one single GPIO numberspace. As pointed out in some other message in the thread it would be nicer to have: /dev/gpiochip0/gpio0 /dev/gpiochip0/gpio1 instead of the horrid sysfs ABI that will have to maintain forever. Blocking any attempts to make it more useful doesn't help much, though. This patch is not making it more useful. It just introduces an inferior way to do something that is already possible. I have stated it countless times already, but again: /sys/bus/.../device/gpio_function is better than /sys/class/gpio/device_function_foo_whatnot (and actually, /sys/bus/.../device/gpios/function would be even better). You can already do the former. I just don't see the need to introduce an API to do the latter. Why is the former better? Because it uses the sysfs hierarchy to make the GPIO visible under their consumer's node. That's how sysfs is intended to be used. Just explain me why you cannot live with this or why your proposal is better, and my concerns about this patch will be lifted. device is a platform driver and thus platform specific, which defeats the purpose of having a well defined path and makes it even more difficult to find a pin across multiple platform variants (which will have different platform drivers). It means user space will still need platform specific configuration data to find the pin. If user space needs such configuration data anyway, it may as well be a gpio chip, pin tuple instead of platform driver path, gpio pin name. Even worse, in one of my use cases some of the platform drivers may have multiple instances with dynamic instance numbers, meaning I don't even have a well defined path to start with. On the other side, as it turns out, most of the gpio chips I deal with are company specific, and you can not prevent me from populating the 'names' array for those. It is a bit on the clumsy side, as I would prefer to have the consumer select the name instead of the provider, but it works. For the other drivers (so far only one) I only need an out-of-tree patch with a couple of lines of code to be able to populate the 'names' field. So unless you take the 'names' field away, which would break the existing ABI and is thus at least somewhat unlikely, I can still do what I need to do. We won't take the names field away, as you mentioned we will have to support the user-space ABI forever and ever. I hope that at some point we will come with a better alternative, but the old one is here to stay anyway. Linus and myself explained why this ABI is bad ; however, it is also true that there is no better alternative besides implementing a custom solution. Which allows us to reduce the argument about your patch to: it's a more comfortable way of doing something bad. Since that something bad is not going away, we might as well have another (arguably better) way of doing it. Also: - This patch is limited to gpiolib-sysfs.c, so damage is contained there - The patch is small - I cannot help but notice that Linus did not scream about it - I did not have a strong reject about it either, but was looking for a better way to achieve the desired result. Looks like there is none at the moment. So I'm somehow ok if this patch makes it in ; I still think we should not encourage usage of this ABI, but you do not introduce any new harmful toy - just provide another way to use one that already exists. Linus, please consider my position about this patch as neutral, and feel free to take it if you think it is
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Fri, Aug 29, 2014 at 10:00:47AM -0700, Alexandre Courbot wrote: > On Thu, Aug 28, 2014 at 10:54 PM, Guenter Roeck wrote: > > On 08/28/2014 10:44 PM, Linus Walleij wrote: > >> > >> On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot > >> wrote: > >>> > >>> On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck > >>> wrote: > >> > >> > This is just one of many patches which would make it possible to submit > the rest, which would make use of it. What you are saying is that it > won't > make sense to submit that series into the kernel, because one of the > very > first patches needed to enable that won't be accepted. Kind of a > circular > argument, but I guess I'll have to live with it. > >>> > >>> > >>> Well I have not seen the other patches you mention and cannot guess > >>> their existence. If you send the full series it will of course be > >>> considered as such, but right now this lone patch does not hint any > >>> upstream user for this interface. > >>> > >>> Note that this doesn't change anything to the core of the argument ; > >>> we have not heard what Linus thinks about named GPIOs in > >>> /sys/class/gpio yet, maybe he will have a different opinion... > >> > >> > >> The sysfs is sort of broken by design because of things like this and > >> some other stuff. > >> > >> I think the sysfs is scary, for example since it's not hierarchical > >> but flat and build on the assumption that there is one single > >> GPIO numberspace. As pointed out in some other message > >> in the thread it would be nicer to have: > >> > >> /dev/gpiochip0/gpio0 > >> /dev/gpiochip0/gpio1 > >> > >> > >> instead of the horrid sysfs ABI that will have to maintain forever. > >> > > Blocking any attempts to make it more useful doesn't help much, though. > > This patch is not making it more useful. It just introduces an > inferior way to do something that is already possible. > > I have stated it countless times already, but again: > "/sys/bus/.../device/gpio_function" is better than > "/sys/class/gpio/device_function_foo_whatnot" (and actually, > "/sys/bus/.../device/gpios/function" would be even better). > > You can already do the former. I just don't see the need to introduce > an API to do the latter. > > Why is the former better? Because it uses the sysfs hierarchy to make > the GPIO visible under their consumer's node. That's how sysfs is > intended to be used. > > Just explain me why you cannot live with this or why your proposal is > better, and my concerns about this patch will be lifted. > "device" is a platform driver and thus platform specific, which defeats the purpose of having a well defined path and makes it even more difficult to find a pin across multiple platform variants (which will have different platform drivers). It means user space will still need platform specific configuration data to find the pin. If user space needs such configuration data anyway, it may as well be a "" tuple instead of "". Even worse, in one of my use cases some of the platform drivers may have multiple instances with dynamic instance numbers, meaning I don't even have a well defined path to start with. On the other side, as it turns out, most of the gpio chips I deal with are company specific, and you can not prevent me from populating the 'names' array for those. It is a bit on the clumsy side, as I would prefer to have the consumer select the name instead of the provider, but it works. For the other drivers (so far only one) I only need an out-of-tree patch with a couple of lines of code to be able to populate the 'names' field. So unless you take the 'names' field away, which would break the existing ABI and is thus at least somewhat unlikely, I can still do what I need to do. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Thu, Aug 28, 2014 at 10:54 PM, Guenter Roeck wrote: > On 08/28/2014 10:44 PM, Linus Walleij wrote: >> >> On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot >> wrote: >>> >>> On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck >>> wrote: >> >> This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. >>> >>> >>> Well I have not seen the other patches you mention and cannot guess >>> their existence. If you send the full series it will of course be >>> considered as such, but right now this lone patch does not hint any >>> upstream user for this interface. >>> >>> Note that this doesn't change anything to the core of the argument ; >>> we have not heard what Linus thinks about named GPIOs in >>> /sys/class/gpio yet, maybe he will have a different opinion... >> >> >> The sysfs is sort of broken by design because of things like this and >> some other stuff. >> >> I think the sysfs is scary, for example since it's not hierarchical >> but flat and build on the assumption that there is one single >> GPIO numberspace. As pointed out in some other message >> in the thread it would be nicer to have: >> >> /dev/gpiochip0/gpio0 >> /dev/gpiochip0/gpio1 >> >> >> instead of the horrid sysfs ABI that will have to maintain forever. >> > Blocking any attempts to make it more useful doesn't help much, though. This patch is not making it more useful. It just introduces an inferior way to do something that is already possible. I have stated it countless times already, but again: "/sys/bus/.../device/gpio_function" is better than "/sys/class/gpio/device_function_foo_whatnot" (and actually, "/sys/bus/.../device/gpios/function" would be even better). You can already do the former. I just don't see the need to introduce an API to do the latter. Why is the former better? Because it uses the sysfs hierarchy to make the GPIO visible under their consumer's node. That's how sysfs is intended to be used. Just explain me why you cannot live with this or why your proposal is better, and my concerns about this patch will be lifted. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Thu, Aug 28, 2014 at 10:54 PM, Guenter Roeck li...@roeck-us.net wrote: On 08/28/2014 10:44 PM, Linus Walleij wrote: On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot gnu...@gmail.com wrote: On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck li...@roeck-us.net wrote: This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. Well I have not seen the other patches you mention and cannot guess their existence. If you send the full series it will of course be considered as such, but right now this lone patch does not hint any upstream user for this interface. Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... The sysfs is sort of broken by design because of things like this and some other stuff. I think the sysfs is scary, for example since it's not hierarchical but flat and build on the assumption that there is one single GPIO numberspace. As pointed out in some other message in the thread it would be nicer to have: /dev/gpiochip0/gpio0 /dev/gpiochip0/gpio1 instead of the horrid sysfs ABI that will have to maintain forever. Blocking any attempts to make it more useful doesn't help much, though. This patch is not making it more useful. It just introduces an inferior way to do something that is already possible. I have stated it countless times already, but again: /sys/bus/.../device/gpio_function is better than /sys/class/gpio/device_function_foo_whatnot (and actually, /sys/bus/.../device/gpios/function would be even better). You can already do the former. I just don't see the need to introduce an API to do the latter. Why is the former better? Because it uses the sysfs hierarchy to make the GPIO visible under their consumer's node. That's how sysfs is intended to be used. Just explain me why you cannot live with this or why your proposal is better, and my concerns about this patch will be lifted. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Fri, Aug 29, 2014 at 10:00:47AM -0700, Alexandre Courbot wrote: On Thu, Aug 28, 2014 at 10:54 PM, Guenter Roeck li...@roeck-us.net wrote: On 08/28/2014 10:44 PM, Linus Walleij wrote: On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot gnu...@gmail.com wrote: On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck li...@roeck-us.net wrote: This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. Well I have not seen the other patches you mention and cannot guess their existence. If you send the full series it will of course be considered as such, but right now this lone patch does not hint any upstream user for this interface. Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... The sysfs is sort of broken by design because of things like this and some other stuff. I think the sysfs is scary, for example since it's not hierarchical but flat and build on the assumption that there is one single GPIO numberspace. As pointed out in some other message in the thread it would be nicer to have: /dev/gpiochip0/gpio0 /dev/gpiochip0/gpio1 instead of the horrid sysfs ABI that will have to maintain forever. Blocking any attempts to make it more useful doesn't help much, though. This patch is not making it more useful. It just introduces an inferior way to do something that is already possible. I have stated it countless times already, but again: /sys/bus/.../device/gpio_function is better than /sys/class/gpio/device_function_foo_whatnot (and actually, /sys/bus/.../device/gpios/function would be even better). You can already do the former. I just don't see the need to introduce an API to do the latter. Why is the former better? Because it uses the sysfs hierarchy to make the GPIO visible under their consumer's node. That's how sysfs is intended to be used. Just explain me why you cannot live with this or why your proposal is better, and my concerns about this patch will be lifted. device is a platform driver and thus platform specific, which defeats the purpose of having a well defined path and makes it even more difficult to find a pin across multiple platform variants (which will have different platform drivers). It means user space will still need platform specific configuration data to find the pin. If user space needs such configuration data anyway, it may as well be a gpio chip, pin tuple instead of platform driver path, gpio pin name. Even worse, in one of my use cases some of the platform drivers may have multiple instances with dynamic instance numbers, meaning I don't even have a well defined path to start with. On the other side, as it turns out, most of the gpio chips I deal with are company specific, and you can not prevent me from populating the 'names' array for those. It is a bit on the clumsy side, as I would prefer to have the consumer select the name instead of the provider, but it works. For the other drivers (so far only one) I only need an out-of-tree patch with a couple of lines of code to be able to populate the 'names' field. So unless you take the 'names' field away, which would break the existing ABI and is thus at least somewhat unlikely, I can still do what I need to do. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 08/28/2014 10:44 PM, Linus Walleij wrote: On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot wrote: On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck wrote: This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. Well I have not seen the other patches you mention and cannot guess their existence. If you send the full series it will of course be considered as such, but right now this lone patch does not hint any upstream user for this interface. Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... The sysfs is sort of broken by design because of things like this and some other stuff. I think the sysfs is scary, for example since it's not hierarchical but flat and build on the assumption that there is one single GPIO numberspace. As pointed out in some other message in the thread it would be nicer to have: /dev/gpiochip0/gpio0 /dev/gpiochip0/gpio1 instead of the horrid sysfs ABI that will have to maintain forever. Blocking any attempts to make it more useful doesn't help much, though. Still it is true that there is a precedent for named GPIOs in sysfs. And in the end, userspace needs a way to figure out how to get what it needs, a unique string is as good as anything. I would be feeling better if userspace got that name from an ioctl() on some /dev/* node than by plain filename search in sysfs. I somewhat feel a named gpio in sysfs is more helpful than just "gpio25". That was the point. The code necessary to find out that a specific function on a specific board is tied to gpiochip X pin Y is quite complex. It seemed to be much easier to be able to rely on a well defined pin name. The argument of "there can be duplicate names" is, in my opinion, quite bogus. Sure, there can be duplicate names. That is why one has error codes. With such an argument you can block anything you want. Regarding ioctls, I thought sysfs was supposed to replace ioctls, and that adding new ioctls was discouraged. Are we reverting on that ? But I'm not happy about merging the patch if Alexandre is hesitant. Guess that's life. I'll just have to live with it. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot wrote: > On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck wrote: >> This is just one of many patches which would make it possible to submit >> the rest, which would make use of it. What you are saying is that it won't >> make sense to submit that series into the kernel, because one of the very >> first patches needed to enable that won't be accepted. Kind of a circular >> argument, but I guess I'll have to live with it. > > Well I have not seen the other patches you mention and cannot guess > their existence. If you send the full series it will of course be > considered as such, but right now this lone patch does not hint any > upstream user for this interface. > > Note that this doesn't change anything to the core of the argument ; > we have not heard what Linus thinks about named GPIOs in > /sys/class/gpio yet, maybe he will have a different opinion... The sysfs is sort of broken by design because of things like this and some other stuff. I think the sysfs is scary, for example since it's not hierarchical but flat and build on the assumption that there is one single GPIO numberspace. As pointed out in some other message in the thread it would be nicer to have: /dev/gpiochip0/gpio0 /dev/gpiochip0/gpio1 instead of the horrid sysfs ABI that will have to maintain forever. Still it is true that there is a precedent for named GPIOs in sysfs. And in the end, userspace needs a way to figure out how to get what it needs, a unique string is as good as anything. I would be feeling better if userspace got that name from an ioctl() on some /dev/* node than by plain filename search in sysfs. I somewhat feel a named gpio in sysfs is more helpful than just "gpio25". But I'm not happy about merging the patch if Alexandre is hesitant. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot gnu...@gmail.com wrote: On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck li...@roeck-us.net wrote: This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. Well I have not seen the other patches you mention and cannot guess their existence. If you send the full series it will of course be considered as such, but right now this lone patch does not hint any upstream user for this interface. Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... The sysfs is sort of broken by design because of things like this and some other stuff. I think the sysfs is scary, for example since it's not hierarchical but flat and build on the assumption that there is one single GPIO numberspace. As pointed out in some other message in the thread it would be nicer to have: /dev/gpiochip0/gpio0 /dev/gpiochip0/gpio1 instead of the horrid sysfs ABI that will have to maintain forever. Still it is true that there is a precedent for named GPIOs in sysfs. And in the end, userspace needs a way to figure out how to get what it needs, a unique string is as good as anything. I would be feeling better if userspace got that name from an ioctl() on some /dev/* node than by plain filename search in sysfs. I somewhat feel a named gpio in sysfs is more helpful than just gpio25. But I'm not happy about merging the patch if Alexandre is hesitant. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 08/28/2014 10:44 PM, Linus Walleij wrote: On Mon, Aug 11, 2014 at 5:20 PM, Alexandre Courbot gnu...@gmail.com wrote: On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck li...@roeck-us.net wrote: This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. Well I have not seen the other patches you mention and cannot guess their existence. If you send the full series it will of course be considered as such, but right now this lone patch does not hint any upstream user for this interface. Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... The sysfs is sort of broken by design because of things like this and some other stuff. I think the sysfs is scary, for example since it's not hierarchical but flat and build on the assumption that there is one single GPIO numberspace. As pointed out in some other message in the thread it would be nicer to have: /dev/gpiochip0/gpio0 /dev/gpiochip0/gpio1 instead of the horrid sysfs ABI that will have to maintain forever. Blocking any attempts to make it more useful doesn't help much, though. Still it is true that there is a precedent for named GPIOs in sysfs. And in the end, userspace needs a way to figure out how to get what it needs, a unique string is as good as anything. I would be feeling better if userspace got that name from an ioctl() on some /dev/* node than by plain filename search in sysfs. I somewhat feel a named gpio in sysfs is more helpful than just gpio25. That was the point. The code necessary to find out that a specific function on a specific board is tied to gpiochip X pin Y is quite complex. It seemed to be much easier to be able to rely on a well defined pin name. The argument of there can be duplicate names is, in my opinion, quite bogus. Sure, there can be duplicate names. That is why one has error codes. With such an argument you can block anything you want. Regarding ioctls, I thought sysfs was supposed to replace ioctls, and that adding new ioctls was discouraged. Are we reverting on that ? But I'm not happy about merging the patch if Alexandre is hesitant. Guess that's life. I'll just have to live with it. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Mon, Aug 11, 2014 at 03:05:54PM -0700, Alexandre Courbot wrote: > On Mon, Aug 11, 2014 at 9:13 AM, Guenter Roeck wrote: > > On 08/11/2014 08:20 AM, Alexandre Courbot wrote: > > [ ... ] > > > >> > >> Note that this doesn't change anything to the core of the argument ; > >> we have not heard what Linus thinks about named GPIOs in > >> /sys/class/gpio yet, maybe he will have a different opinion... > >> > > > > Well, please let me know if/when you are planning to take away > > that existing ABI so I can plan accordingly. FWIW, out-of-kernel > > users I am currently aware of are Juniper Networks and Zodiac > > Aerospace. The latter asked me to help them upstreaming their code. > > > > If you plan to remove the ABI, you might want to inform its current > > in-kernel users. You can look those up yourself. > > We are not taking existing user-space ABIs away. But we are also not > encouraging people to use bad ABIs by introducing new ways to use > them, hence my restraint towards this patch. > You bring it to the point. You believe that the gpio ABI, or maybe just named gpio pins, is a bad idea. Nothing I am going to say will influence that opinion; all we do is to keep turning in circles. So let's stop wasting our time, agree to disagree, and move on. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Mon, Aug 11, 2014 at 03:05:54PM -0700, Alexandre Courbot wrote: On Mon, Aug 11, 2014 at 9:13 AM, Guenter Roeck li...@roeck-us.net wrote: On 08/11/2014 08:20 AM, Alexandre Courbot wrote: [ ... ] Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... Well, please let me know if/when you are planning to take away that existing ABI so I can plan accordingly. FWIW, out-of-kernel users I am currently aware of are Juniper Networks and Zodiac Aerospace. The latter asked me to help them upstreaming their code. If you plan to remove the ABI, you might want to inform its current in-kernel users. You can look those up yourself. We are not taking existing user-space ABIs away. But we are also not encouraging people to use bad ABIs by introducing new ways to use them, hence my restraint towards this patch. You bring it to the point. You believe that the gpio ABI, or maybe just named gpio pins, is a bad idea. Nothing I am going to say will influence that opinion; all we do is to keep turning in circles. So let's stop wasting our time, agree to disagree, and move on. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Mon, Aug 11, 2014 at 9:13 AM, Guenter Roeck wrote: > On 08/11/2014 08:20 AM, Alexandre Courbot wrote: > [ ... ] > >> >> Note that this doesn't change anything to the core of the argument ; >> we have not heard what Linus thinks about named GPIOs in >> /sys/class/gpio yet, maybe he will have a different opinion... >> > > Well, please let me know if/when you are planning to take away > that existing ABI so I can plan accordingly. FWIW, out-of-kernel > users I am currently aware of are Juniper Networks and Zodiac > Aerospace. The latter asked me to help them upstreaming their code. > > If you plan to remove the ABI, you might want to inform its current > in-kernel users. You can look those up yourself. We are not taking existing user-space ABIs away. But we are also not encouraging people to use bad ABIs by introducing new ways to use them, hence my restraint towards this patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 08/11/2014 08:20 AM, Alexandre Courbot wrote: [ ... ] Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... Well, please let me know if/when you are planning to take away that existing ABI so I can plan accordingly. FWIW, out-of-kernel users I am currently aware of are Juniper Networks and Zodiac Aerospace. The latter asked me to help them upstreaming their code. If you plan to remove the ABI, you might want to inform its current in-kernel users. You can look those up yourself. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck wrote: > On 08/11/2014 08:01 AM, Alexandre Courbot wrote: >> >> On Thu, Aug 7, 2014 at 4:34 PM, Guenter Roeck wrote: Or you could have a platform device which sole purpose is to request the GPIOs to export and export them using gpio_export() and gpio_export_link() using itself as the device parameters. All your links would then appear under the same sysfs directory, and that directory name would be consistent across platforms. What is wrong with this approach? >>> The link does not appear in /sys/class/gpio. I understand you don't like >>> the >>> idea of having named gpio pins in that directory, but that is supported >>> today, >>> it works, and others do like it. >> >> >> Who are these "others" that like it? >> >>> >>> As for the patch itself, as I said before I am not a huge fan of putting randomly-named exports under /sys/class/gpio, but since there is a precedent with the driver GPIO names array I don't think this makes the situation much worse. Still I'd like you to explain me what I missed and why you cannot use the technique described above to achieve your goal with the currently existing interfaces. >>> >>> I thought I explained it before; my users (ie the people writing user >>> space >>> applications accessing the pins) expect to see the exported pins in >>> /sys/class/gpio >>> and not in a more or less arbitrary directory. They are used to this >>> approach, and they are less than enthusiastic to change it. The code >>> needed >>> to generate the exported pin names is a bit messy, it being tied to the >>> driver, >>> but it does both exist and work. This patch was an attempt to provide a >>> cleaner API to accomplish the same without having to touch various gpio >>> drivers which don't provide the ability to configure the names array. >>> >>> Note that I don't consider the names "random". They are much less random >>> than gpioX, where X can change each time the system boots or, for OIR >>> capable systems, each time a gpio driver is instantiated or removed. >>> In today's system, without well defined names, one never knows if gpioX >>> points >>> to the pin the user is looking for. If the pin is named >>> "this-is-your-pin" >>> it is much easier to write user space code using it. >>> >>> Oddly enough, if I would use the platform driver approach you suggest, >>> I would still need a well defined directory, say, /sys/class/named_gpios, >> >> >> Which you would have through your platform device, and I guess you are >> not denying that it is possible to do it that way using the existing >> APIs. The discussion now seems to be "let's allow named GPIOs in >> /sys/class/gpio". Why? Because the customer wants it this way. The >> point is, that in mainline we don't merge changes because to make the >> customer happy, but because they generally make sense. I fail to see >> how it makes more sense to allow named GPIOs in /sys/class/gpio >> instead of the node of the device controlled by these GPIOs. >> >> Let me elaborate some more on why: GPIOs are generally tied to fulfil >> a precise function for a given device. Having them appearing under the >> node of the device in question makes it clear what their relationship >> to that device is ; and the name of the node can then be a reflection >> of that function. On the contrary, letting them all bloom under a >> single directory forces you to resort to complex and confusing names >> to differenciate your GPIOs, with everyone coming with their own >> naming pattern and so on. If it doesn't look like a bad idea to you, >> I'm afraid I cannot be any more convincing. >> >> Also, it did not occur to me until now, but with this patch alone you >> are going to be hit by the objection that we don't add APIs that do >> not have upstream users. That policy is quite strongly enforced, and >> I'm afraid this makes this patch even more unlikely to be accepted. >> > This is just one of many patches which would make it possible to submit > the rest, which would make use of it. What you are saying is that it won't > make sense to submit that series into the kernel, because one of the very > first patches needed to enable that won't be accepted. Kind of a circular > argument, but I guess I'll have to live with it. Well I have not seen the other patches you mention and cannot guess their existence. If you send the full series it will of course be considered as such, but right now this lone patch does not hint any upstream user for this interface. Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 08/11/2014 08:01 AM, Alexandre Courbot wrote: On Thu, Aug 7, 2014 at 4:34 PM, Guenter Roeck wrote: Or you could have a platform device which sole purpose is to request the GPIOs to export and export them using gpio_export() and gpio_export_link() using itself as the device parameters. All your links would then appear under the same sysfs directory, and that directory name would be consistent across platforms. What is wrong with this approach? The link does not appear in /sys/class/gpio. I understand you don't like the idea of having named gpio pins in that directory, but that is supported today, it works, and others do like it. Who are these "others" that like it? As for the patch itself, as I said before I am not a huge fan of putting randomly-named exports under /sys/class/gpio, but since there is a precedent with the driver GPIO names array I don't think this makes the situation much worse. Still I'd like you to explain me what I missed and why you cannot use the technique described above to achieve your goal with the currently existing interfaces. I thought I explained it before; my users (ie the people writing user space applications accessing the pins) expect to see the exported pins in /sys/class/gpio and not in a more or less arbitrary directory. They are used to this approach, and they are less than enthusiastic to change it. The code needed to generate the exported pin names is a bit messy, it being tied to the driver, but it does both exist and work. This patch was an attempt to provide a cleaner API to accomplish the same without having to touch various gpio drivers which don't provide the ability to configure the names array. Note that I don't consider the names "random". They are much less random than gpioX, where X can change each time the system boots or, for OIR capable systems, each time a gpio driver is instantiated or removed. In today's system, without well defined names, one never knows if gpioX points to the pin the user is looking for. If the pin is named "this-is-your-pin" it is much easier to write user space code using it. Oddly enough, if I would use the platform driver approach you suggest, I would still need a well defined directory, say, /sys/class/named_gpios, Which you would have through your platform device, and I guess you are not denying that it is possible to do it that way using the existing APIs. The discussion now seems to be "let's allow named GPIOs in /sys/class/gpio". Why? Because the customer wants it this way. The point is, that in mainline we don't merge changes because to make the customer happy, but because they generally make sense. I fail to see how it makes more sense to allow named GPIOs in /sys/class/gpio instead of the node of the device controlled by these GPIOs. Let me elaborate some more on why: GPIOs are generally tied to fulfil a precise function for a given device. Having them appearing under the node of the device in question makes it clear what their relationship to that device is ; and the name of the node can then be a reflection of that function. On the contrary, letting them all bloom under a single directory forces you to resort to complex and confusing names to differenciate your GPIOs, with everyone coming with their own naming pattern and so on. If it doesn't look like a bad idea to you, I'm afraid I cannot be any more convincing. Also, it did not occur to me until now, but with this patch alone you are going to be hit by the objection that we don't add APIs that do not have upstream users. That policy is quite strongly enforced, and I'm afraid this makes this patch even more unlikely to be accepted. This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. So my final word on this is that it seems quite clear that exporting GPIOs under the device they control is a more elegant solution for mainline, and a solution that is already doable using currently existing APIs. I encourage you to explore that direction and work with your customer to make them accept that small change ; if that is not possible this will have to be maintained out of mainline ; sorry. Guess so. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Thu, Aug 7, 2014 at 4:34 PM, Guenter Roeck wrote: >> Or you could have a platform device which sole purpose is to request >> the GPIOs to export and export them using gpio_export() and >> gpio_export_link() using itself as the device parameters. All your >> links would then appear under the same sysfs directory, and that >> directory name would be consistent across platforms. What is wrong >> with this approach? >> > The link does not appear in /sys/class/gpio. I understand you don't like the > idea of having named gpio pins in that directory, but that is supported > today, > it works, and others do like it. Who are these "others" that like it? > > >> As for the patch itself, as I said before I am not a huge fan of >> putting randomly-named exports under /sys/class/gpio, but since there >> is a precedent with the driver GPIO names array I don't think this >> makes the situation much worse. Still I'd like you to explain me what >> I missed and why you cannot use the technique described above to >> achieve your goal with the currently existing interfaces. >> > > I thought I explained it before; my users (ie the people writing user space > applications accessing the pins) expect to see the exported pins in > /sys/class/gpio > and not in a more or less arbitrary directory. They are used to this > approach, and they are less than enthusiastic to change it. The code needed > to generate the exported pin names is a bit messy, it being tied to the > driver, > but it does both exist and work. This patch was an attempt to provide a > cleaner API to accomplish the same without having to touch various gpio > drivers which don't provide the ability to configure the names array. > > Note that I don't consider the names "random". They are much less random > than gpioX, where X can change each time the system boots or, for OIR > capable systems, each time a gpio driver is instantiated or removed. > In today's system, without well defined names, one never knows if gpioX > points > to the pin the user is looking for. If the pin is named "this-is-your-pin" > it is much easier to write user space code using it. > > Oddly enough, if I would use the platform driver approach you suggest, > I would still need a well defined directory, say, /sys/class/named_gpios, Which you would have through your platform device, and I guess you are not denying that it is possible to do it that way using the existing APIs. The discussion now seems to be "let's allow named GPIOs in /sys/class/gpio". Why? Because the customer wants it this way. The point is, that in mainline we don't merge changes because to make the customer happy, but because they generally make sense. I fail to see how it makes more sense to allow named GPIOs in /sys/class/gpio instead of the node of the device controlled by these GPIOs. Let me elaborate some more on why: GPIOs are generally tied to fulfil a precise function for a given device. Having them appearing under the node of the device in question makes it clear what their relationship to that device is ; and the name of the node can then be a reflection of that function. On the contrary, letting them all bloom under a single directory forces you to resort to complex and confusing names to differenciate your GPIOs, with everyone coming with their own naming pattern and so on. If it doesn't look like a bad idea to you, I'm afraid I cannot be any more convincing. Also, it did not occur to me until now, but with this patch alone you are going to be hit by the objection that we don't add APIs that do not have upstream users. That policy is quite strongly enforced, and I'm afraid this makes this patch even more unlikely to be accepted. So my final word on this is that it seems quite clear that exporting GPIOs under the device they control is a more elegant solution for mainline, and a solution that is already doable using currently existing APIs. I encourage you to explore that direction and work with your customer to make them accept that small change ; if that is not possible this will have to be maintained out of mainline ; sorry. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Mon, Aug 11, 2014 at 9:13 AM, Guenter Roeck li...@roeck-us.net wrote: On 08/11/2014 08:20 AM, Alexandre Courbot wrote: [ ... ] Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... Well, please let me know if/when you are planning to take away that existing ABI so I can plan accordingly. FWIW, out-of-kernel users I am currently aware of are Juniper Networks and Zodiac Aerospace. The latter asked me to help them upstreaming their code. If you plan to remove the ABI, you might want to inform its current in-kernel users. You can look those up yourself. We are not taking existing user-space ABIs away. But we are also not encouraging people to use bad ABIs by introducing new ways to use them, hence my restraint towards this patch. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Thu, Aug 7, 2014 at 4:34 PM, Guenter Roeck li...@roeck-us.net wrote: Or you could have a platform device which sole purpose is to request the GPIOs to export and export them using gpio_export() and gpio_export_link() using itself as the device parameters. All your links would then appear under the same sysfs directory, and that directory name would be consistent across platforms. What is wrong with this approach? The link does not appear in /sys/class/gpio. I understand you don't like the idea of having named gpio pins in that directory, but that is supported today, it works, and others do like it. Who are these others that like it? As for the patch itself, as I said before I am not a huge fan of putting randomly-named exports under /sys/class/gpio, but since there is a precedent with the driver GPIO names array I don't think this makes the situation much worse. Still I'd like you to explain me what I missed and why you cannot use the technique described above to achieve your goal with the currently existing interfaces. I thought I explained it before; my users (ie the people writing user space applications accessing the pins) expect to see the exported pins in /sys/class/gpio and not in a more or less arbitrary directory. They are used to this approach, and they are less than enthusiastic to change it. The code needed to generate the exported pin names is a bit messy, it being tied to the driver, but it does both exist and work. This patch was an attempt to provide a cleaner API to accomplish the same without having to touch various gpio drivers which don't provide the ability to configure the names array. Note that I don't consider the names random. They are much less random than gpioX, where X can change each time the system boots or, for OIR capable systems, each time a gpio driver is instantiated or removed. In today's system, without well defined names, one never knows if gpioX points to the pin the user is looking for. If the pin is named this-is-your-pin it is much easier to write user space code using it. Oddly enough, if I would use the platform driver approach you suggest, I would still need a well defined directory, say, /sys/class/named_gpios, Which you would have through your platform device, and I guess you are not denying that it is possible to do it that way using the existing APIs. The discussion now seems to be let's allow named GPIOs in /sys/class/gpio. Why? Because the customer wants it this way. The point is, that in mainline we don't merge changes because to make the customer happy, but because they generally make sense. I fail to see how it makes more sense to allow named GPIOs in /sys/class/gpio instead of the node of the device controlled by these GPIOs. Let me elaborate some more on why: GPIOs are generally tied to fulfil a precise function for a given device. Having them appearing under the node of the device in question makes it clear what their relationship to that device is ; and the name of the node can then be a reflection of that function. On the contrary, letting them all bloom under a single directory forces you to resort to complex and confusing names to differenciate your GPIOs, with everyone coming with their own naming pattern and so on. If it doesn't look like a bad idea to you, I'm afraid I cannot be any more convincing. Also, it did not occur to me until now, but with this patch alone you are going to be hit by the objection that we don't add APIs that do not have upstream users. That policy is quite strongly enforced, and I'm afraid this makes this patch even more unlikely to be accepted. So my final word on this is that it seems quite clear that exporting GPIOs under the device they control is a more elegant solution for mainline, and a solution that is already doable using currently existing APIs. I encourage you to explore that direction and work with your customer to make them accept that small change ; if that is not possible this will have to be maintained out of mainline ; sorry. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 08/11/2014 08:01 AM, Alexandre Courbot wrote: On Thu, Aug 7, 2014 at 4:34 PM, Guenter Roeck li...@roeck-us.net wrote: Or you could have a platform device which sole purpose is to request the GPIOs to export and export them using gpio_export() and gpio_export_link() using itself as the device parameters. All your links would then appear under the same sysfs directory, and that directory name would be consistent across platforms. What is wrong with this approach? The link does not appear in /sys/class/gpio. I understand you don't like the idea of having named gpio pins in that directory, but that is supported today, it works, and others do like it. Who are these others that like it? As for the patch itself, as I said before I am not a huge fan of putting randomly-named exports under /sys/class/gpio, but since there is a precedent with the driver GPIO names array I don't think this makes the situation much worse. Still I'd like you to explain me what I missed and why you cannot use the technique described above to achieve your goal with the currently existing interfaces. I thought I explained it before; my users (ie the people writing user space applications accessing the pins) expect to see the exported pins in /sys/class/gpio and not in a more or less arbitrary directory. They are used to this approach, and they are less than enthusiastic to change it. The code needed to generate the exported pin names is a bit messy, it being tied to the driver, but it does both exist and work. This patch was an attempt to provide a cleaner API to accomplish the same without having to touch various gpio drivers which don't provide the ability to configure the names array. Note that I don't consider the names random. They are much less random than gpioX, where X can change each time the system boots or, for OIR capable systems, each time a gpio driver is instantiated or removed. In today's system, without well defined names, one never knows if gpioX points to the pin the user is looking for. If the pin is named this-is-your-pin it is much easier to write user space code using it. Oddly enough, if I would use the platform driver approach you suggest, I would still need a well defined directory, say, /sys/class/named_gpios, Which you would have through your platform device, and I guess you are not denying that it is possible to do it that way using the existing APIs. The discussion now seems to be let's allow named GPIOs in /sys/class/gpio. Why? Because the customer wants it this way. The point is, that in mainline we don't merge changes because to make the customer happy, but because they generally make sense. I fail to see how it makes more sense to allow named GPIOs in /sys/class/gpio instead of the node of the device controlled by these GPIOs. Let me elaborate some more on why: GPIOs are generally tied to fulfil a precise function for a given device. Having them appearing under the node of the device in question makes it clear what their relationship to that device is ; and the name of the node can then be a reflection of that function. On the contrary, letting them all bloom under a single directory forces you to resort to complex and confusing names to differenciate your GPIOs, with everyone coming with their own naming pattern and so on. If it doesn't look like a bad idea to you, I'm afraid I cannot be any more convincing. Also, it did not occur to me until now, but with this patch alone you are going to be hit by the objection that we don't add APIs that do not have upstream users. That policy is quite strongly enforced, and I'm afraid this makes this patch even more unlikely to be accepted. This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. So my final word on this is that it seems quite clear that exporting GPIOs under the device they control is a more elegant solution for mainline, and a solution that is already doable using currently existing APIs. I encourage you to explore that direction and work with your customer to make them accept that small change ; if that is not possible this will have to be maintained out of mainline ; sorry. Guess so. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Tue, Aug 12, 2014 at 12:15 AM, Guenter Roeck li...@roeck-us.net wrote: On 08/11/2014 08:01 AM, Alexandre Courbot wrote: On Thu, Aug 7, 2014 at 4:34 PM, Guenter Roeck li...@roeck-us.net wrote: Or you could have a platform device which sole purpose is to request the GPIOs to export and export them using gpio_export() and gpio_export_link() using itself as the device parameters. All your links would then appear under the same sysfs directory, and that directory name would be consistent across platforms. What is wrong with this approach? The link does not appear in /sys/class/gpio. I understand you don't like the idea of having named gpio pins in that directory, but that is supported today, it works, and others do like it. Who are these others that like it? As for the patch itself, as I said before I am not a huge fan of putting randomly-named exports under /sys/class/gpio, but since there is a precedent with the driver GPIO names array I don't think this makes the situation much worse. Still I'd like you to explain me what I missed and why you cannot use the technique described above to achieve your goal with the currently existing interfaces. I thought I explained it before; my users (ie the people writing user space applications accessing the pins) expect to see the exported pins in /sys/class/gpio and not in a more or less arbitrary directory. They are used to this approach, and they are less than enthusiastic to change it. The code needed to generate the exported pin names is a bit messy, it being tied to the driver, but it does both exist and work. This patch was an attempt to provide a cleaner API to accomplish the same without having to touch various gpio drivers which don't provide the ability to configure the names array. Note that I don't consider the names random. They are much less random than gpioX, where X can change each time the system boots or, for OIR capable systems, each time a gpio driver is instantiated or removed. In today's system, without well defined names, one never knows if gpioX points to the pin the user is looking for. If the pin is named this-is-your-pin it is much easier to write user space code using it. Oddly enough, if I would use the platform driver approach you suggest, I would still need a well defined directory, say, /sys/class/named_gpios, Which you would have through your platform device, and I guess you are not denying that it is possible to do it that way using the existing APIs. The discussion now seems to be let's allow named GPIOs in /sys/class/gpio. Why? Because the customer wants it this way. The point is, that in mainline we don't merge changes because to make the customer happy, but because they generally make sense. I fail to see how it makes more sense to allow named GPIOs in /sys/class/gpio instead of the node of the device controlled by these GPIOs. Let me elaborate some more on why: GPIOs are generally tied to fulfil a precise function for a given device. Having them appearing under the node of the device in question makes it clear what their relationship to that device is ; and the name of the node can then be a reflection of that function. On the contrary, letting them all bloom under a single directory forces you to resort to complex and confusing names to differenciate your GPIOs, with everyone coming with their own naming pattern and so on. If it doesn't look like a bad idea to you, I'm afraid I cannot be any more convincing. Also, it did not occur to me until now, but with this patch alone you are going to be hit by the objection that we don't add APIs that do not have upstream users. That policy is quite strongly enforced, and I'm afraid this makes this patch even more unlikely to be accepted. This is just one of many patches which would make it possible to submit the rest, which would make use of it. What you are saying is that it won't make sense to submit that series into the kernel, because one of the very first patches needed to enable that won't be accepted. Kind of a circular argument, but I guess I'll have to live with it. Well I have not seen the other patches you mention and cannot guess their existence. If you send the full series it will of course be considered as such, but right now this lone patch does not hint any upstream user for this interface. Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 08/11/2014 08:20 AM, Alexandre Courbot wrote: [ ... ] Note that this doesn't change anything to the core of the argument ; we have not heard what Linus thinks about named GPIOs in /sys/class/gpio yet, maybe he will have a different opinion... Well, please let me know if/when you are planning to take away that existing ABI so I can plan accordingly. FWIW, out-of-kernel users I am currently aware of are Juniper Networks and Zodiac Aerospace. The latter asked me to help them upstreaming their code. If you plan to remove the ABI, you might want to inform its current in-kernel users. You can look those up yourself. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 08/06/2014 11:25 PM, Alexandre Courbot wrote:
Sorry for the delayed reply...
On Thu, Jul 24, 2014 at 3:33 PM, Guenter Roeck wrote:
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)[] = _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.
Or you could have a platform device which sole purpose is to request
the GPIOs to export and export them using gpio_export() and
gpio_export_link() using itself as the device parameters. All your
links would then appear under the same sysfs directory, and that
directory name would be consistent across platforms. What is wrong
with this approach?
The link does not appear in /sys/class/gpio. I understand you don't like the
idea of having named gpio pins in that directory, but that is supported today,
it works, and others do like it.
As for the patch itself, as I said before I am not a huge fan of
putting randomly-named exports under /sys/class/gpio, but since there
is a precedent with the driver GPIO names array I don't think this
makes the situation much worse. Still I'd like you to explain me what
I missed and why you cannot use the technique described above to
achieve your goal with the currently existing interfaces.
I thought I explained it before; my users (ie the people writing user space
applications accessing the pins) expect to see the exported pins in
/sys/class/gpio
and not in a more or less arbitrary directory. They are used to this
approach, and they are less than enthusiastic to change it. The code needed
to generate the
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
Sorry for the delayed reply...
On Thu, Jul 24, 2014 at 3:33 PM, Guenter Roeck wrote:
>> 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)[] = _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.
Or you could have a platform device which sole purpose is to request
the GPIOs to export and export them using gpio_export() and
gpio_export_link() using itself as the device parameters. All your
links would then appear under the same sysfs directory, and that
directory name would be consistent across platforms. What is wrong
with this approach?
As for the patch itself, as I said before I am not a huge fan of
putting randomly-named exports under /sys/class/gpio, but since there
is a precedent with the driver GPIO names array I don't think this
makes the situation much worse. Still I'd like you to explain me what
I missed and why you cannot use the technique described above to
achieve your goal with the currently existing interfaces.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
Sorry for the delayed reply...
On Thu, Jul 24, 2014 at 3:33 PM, Guenter Roeck li...@roeck-us.net wrote:
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.
Or you could have a platform device which sole purpose is to request
the GPIOs to export and export them using gpio_export() and
gpio_export_link() using itself as the device parameters. All your
links would then appear under the same sysfs directory, and that
directory name would be consistent across platforms. What is wrong
with this approach?
As for the patch itself, as I said before I am not a huge fan of
putting randomly-named exports under /sys/class/gpio, but since there
is a precedent with the driver GPIO names array I don't think this
makes the situation much worse. Still I'd like you to explain me what
I missed and why you cannot use the technique described above to
achieve your goal with the currently existing interfaces.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 08/06/2014 11:25 PM, Alexandre Courbot wrote:
Sorry for the delayed reply...
On Thu, Jul 24, 2014 at 3:33 PM, Guenter Roeck li...@roeck-us.net wrote:
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.
Or you could have a platform device which sole purpose is to request
the GPIOs to export and export them using gpio_export() and
gpio_export_link() using itself as the device parameters. All your
links would then appear under the same sysfs directory, and that
directory name would be consistent across platforms. What is wrong
with this approach?
The link does not appear in /sys/class/gpio. I understand you don't like the
idea of having named gpio pins in that directory, but that is supported today,
it works, and others do like it.
As for the patch itself, as I said before I am not a huge fan of
putting randomly-named exports under /sys/class/gpio, but since there
is a precedent with the driver GPIO names array I don't think this
makes the situation much worse. Still I'd like you to explain me what
I missed and why you cannot use the technique described above to
achieve your goal with the currently existing interfaces.
I thought I explained it before; my users (ie the people writing user space
applications accessing the pins) expect to see the exported pins in
/sys/class/gpio
and not in a more or less arbitrary directory. They are used to this
approach, and they are less than enthusiastic to change it. The code needed
to
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
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(_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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
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(_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(_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 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 --- 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
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
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 li...@roeck-us.net 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 li...@roeck-us.net --- 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 ---
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
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 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 --- 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():
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 07/23/2014 10:44 PM, Alexandre Courbot wrote:
On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck 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)[] = _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
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
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 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 --- 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
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
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 li...@roeck-us.net 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 li...@roeck-us.net --- 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
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On 07/23/2014 10:44 PM, Alexandre Courbot wrote:
On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck li...@roeck-us.net 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
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
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 li...@roeck-us.net 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 li...@roeck-us.net --- 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
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck 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 > --- > 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
[RFC PATCH] gpiolib: Provide and export gpiod_export_name
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
---
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(_lock, flags);
- offset = gpio_chip_hwgpio(desc);
- if (desc->chip->names && desc->chip->names[offset])
- ioname = desc->chip->names[offset];
-
dev = device_create(_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,
[RFC PATCH] gpiolib: Provide and export gpiod_export_name
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 li...@roeck-us.net
---
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
Re: [RFC PATCH] gpiolib: Provide and export gpiod_export_name
On Thu, Jul 24, 2014 at 3:12 AM, Guenter Roeck li...@roeck-us.net 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 li...@roeck-us.net --- 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
