| Message ID | 1429630951-27082-8-git-send-email-johan@kernel.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: > Rename the gpio-chip export/unexport functions to the more descriptive > names gpiochip_register and gpiochip_unregister. Since these functions are related to sysfs, wouldn't gpiochip_sysfs_export (or gpiochip_sysfs_register, although the former sounds better to me) be even more descriptive? The renaming should probably also cover the non-static gpiod_* functions of gpiolib-sysfs.c which are equally ambiguous. Basically anything non-static from gpiolib-sysfs.c should have that prefix. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 27, 2015 at 12:54:36PM +0900, Alexandre Courbot wrote: > On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: > > Rename the gpio-chip export/unexport functions to the more descriptive > > names gpiochip_register and gpiochip_unregister. > > Since these functions are related to sysfs, wouldn't > gpiochip_sysfs_export (or gpiochip_sysfs_register, although the former > sounds better to me) be even more descriptive? I'm trying to get rid of the made up notion of "exporting" things. What we are doing is to register devices with driver core, and that involves a representation is sysfs. Eventually, a gpio chip should always be registered with driver core and this is not directly related to the (by then hopefully legacy) sysfs-interface. > The renaming should probably also cover the non-static gpiod_* > functions of gpiolib-sysfs.c which are equally ambiguous. Basically > anything non-static from gpiolib-sysfs.c should have that prefix. This would be a different change, and some of those functions are also part of the consumer API. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 27, 2015 at 5:27 PM, Johan Hovold <johan@kernel.org> wrote: > On Mon, Apr 27, 2015 at 12:54:36PM +0900, Alexandre Courbot wrote: >> On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: >> > Rename the gpio-chip export/unexport functions to the more descriptive >> > names gpiochip_register and gpiochip_unregister. >> >> Since these functions are related to sysfs, wouldn't >> gpiochip_sysfs_export (or gpiochip_sysfs_register, although the former >> sounds better to me) be even more descriptive? > > I'm trying to get rid of the made up notion of "exporting" things. What > we are doing is to register devices with driver core, and that involves > a representation is sysfs. > > Eventually, a gpio chip should always be registered with driver core and > this is not directly related to the (by then hopefully legacy) > sysfs-interface. I understand and agree, but even after your patch series, registration of a gpio chip with the driver core is still dependent on the CONFIG_GPIO_SYSFS option. So maybe you could push the logic further and either always register GPIO chips (effectively moving the call to device_create into gpiolib.c) and only keep the legacy bits in gpiolib-sysfs.c? We would then only enable the legacy sysfs interface if CONFIG_GPIO_SYSFS is set, but the gpiochip nodes would still appear as long as core sysfs support is compiled in. >> The renaming should probably also cover the non-static gpiod_* >> functions of gpiolib-sysfs.c which are equally ambiguous. Basically >> anything non-static from gpiolib-sysfs.c should have that prefix. > > This would be a different change, and some of those functions are also > part of the consumer API. That could be another patch. I don't mind if an exported function name changes for consistency as long as all in-kernel users are updated as well. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 27, 2015 at 05:50:54PM +0900, Alexandre Courbot wrote: > On Mon, Apr 27, 2015 at 5:27 PM, Johan Hovold <johan@kernel.org> wrote: > > On Mon, Apr 27, 2015 at 12:54:36PM +0900, Alexandre Courbot wrote: > >> On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: > >> > Rename the gpio-chip export/unexport functions to the more descriptive > >> > names gpiochip_register and gpiochip_unregister. > >> > >> Since these functions are related to sysfs, wouldn't > >> gpiochip_sysfs_export (or gpiochip_sysfs_register, although the former > >> sounds better to me) be even more descriptive? > > > > I'm trying to get rid of the made up notion of "exporting" things. What > > we are doing is to register devices with driver core, and that involves > > a representation is sysfs. > > > > Eventually, a gpio chip should always be registered with driver core and > > this is not directly related to the (by then hopefully legacy) > > sysfs-interface. > > I understand and agree, but even after your patch series, registration > of a gpio chip with the driver core is still dependent on the > CONFIG_GPIO_SYSFS option. So maybe you could push the logic further > and either always register GPIO chips (effectively moving the call to > device_create into gpiolib.c) and only keep the legacy bits in > gpiolib-sysfs.c? That is the plan yes, but there's only so much I can do in one series. ;) The current crazy sysfs API also prevents the decoupling of the sysfs interface from chip device registration. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 27, 2015 at 6:05 PM, Johan Hovold <johan@kernel.org> wrote: > On Mon, Apr 27, 2015 at 05:50:54PM +0900, Alexandre Courbot wrote: >> On Mon, Apr 27, 2015 at 5:27 PM, Johan Hovold <johan@kernel.org> wrote: >> > On Mon, Apr 27, 2015 at 12:54:36PM +0900, Alexandre Courbot wrote: >> >> On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: >> >> > Rename the gpio-chip export/unexport functions to the more descriptive >> >> > names gpiochip_register and gpiochip_unregister. >> >> >> >> Since these functions are related to sysfs, wouldn't >> >> gpiochip_sysfs_export (or gpiochip_sysfs_register, although the former >> >> sounds better to me) be even more descriptive? >> > >> > I'm trying to get rid of the made up notion of "exporting" things. What >> > we are doing is to register devices with driver core, and that involves >> > a representation is sysfs. >> > >> > Eventually, a gpio chip should always be registered with driver core and >> > this is not directly related to the (by then hopefully legacy) >> > sysfs-interface. >> >> I understand and agree, but even after your patch series, registration >> of a gpio chip with the driver core is still dependent on the >> CONFIG_GPIO_SYSFS option. So maybe you could push the logic further >> and either always register GPIO chips (effectively moving the call to >> device_create into gpiolib.c) and only keep the legacy bits in >> gpiolib-sysfs.c? > > That is the plan yes, but there's only so much I can do in one series. > ;) The current crazy sysfs API also prevents the decoupling of the sysfs > interface from chip device registration. Sounds good then. This patch series is great anyway, so if Linus has nothing against it I hope we can merge it quickly. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 28, 2015 at 12:27:16PM +0900, Alexandre Courbot wrote: > On Mon, Apr 27, 2015 at 6:05 PM, Johan Hovold <johan@kernel.org> wrote: > > On Mon, Apr 27, 2015 at 05:50:54PM +0900, Alexandre Courbot wrote: > >> On Mon, Apr 27, 2015 at 5:27 PM, Johan Hovold <johan@kernel.org> wrote: > >> > On Mon, Apr 27, 2015 at 12:54:36PM +0900, Alexandre Courbot wrote: > >> >> On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: > >> >> > Rename the gpio-chip export/unexport functions to the more descriptive > >> >> > names gpiochip_register and gpiochip_unregister. > >> >> > >> >> Since these functions are related to sysfs, wouldn't > >> >> gpiochip_sysfs_export (or gpiochip_sysfs_register, although the former > >> >> sounds better to me) be even more descriptive? > >> > > >> > I'm trying to get rid of the made up notion of "exporting" things. What > >> > we are doing is to register devices with driver core, and that involves > >> > a representation is sysfs. > >> > > >> > Eventually, a gpio chip should always be registered with driver core and > >> > this is not directly related to the (by then hopefully legacy) > >> > sysfs-interface. > >> > >> I understand and agree, but even after your patch series, registration > >> of a gpio chip with the driver core is still dependent on the > >> CONFIG_GPIO_SYSFS option. So maybe you could push the logic further > >> and either always register GPIO chips (effectively moving the call to > >> device_create into gpiolib.c) and only keep the legacy bits in > >> gpiolib-sysfs.c? > > > > That is the plan yes, but there's only so much I can do in one series. > > ;) The current crazy sysfs API also prevents the decoupling of the sysfs > > interface from chip device registration. > > Sounds good then. This patch series is great anyway, so if Linus has > nothing against it I hope we can merge it quickly. Thanks for the review. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c index 2f8bdce792f6..31434c5a90ef 100644 --- a/drivers/gpio/gpiolib-sysfs.c +++ b/drivers/gpio/gpiolib-sysfs.c @@ -750,13 +750,14 @@ void gpiod_unexport(struct gpio_desc *desc) } EXPORT_SYMBOL_GPL(gpiod_unexport); -int gpiochip_export(struct gpio_chip *chip) +int gpiochip_register(struct gpio_chip *chip) { struct device *dev; - /* Many systems register gpio chips for SOC support very early, + /* + * Many systems add gpio chips for SOC support very early, * before driver model support is available. In those cases we - * export this later, in gpiolib_sysfs_init() ... here we just + * register later, in gpiolib_sysfs_init() ... here we just * verify that _some_ field of gpio_class got initialized. */ if (!gpio_class.p) @@ -776,7 +777,7 @@ int gpiochip_export(struct gpio_chip *chip) return 0; } -void gpiochip_unexport(struct gpio_chip *chip) +void gpiochip_unregister(struct gpio_chip *chip) { struct gpio_desc *desc; unsigned int i; @@ -821,7 +822,7 @@ static int __init gpiolib_sysfs_init(void) continue; /* - * TODO we yield gpio_lock here because gpiochip_export() + * TODO we yield gpio_lock here because gpiochip_register() * acquires a mutex. This is unsafe and needs to be fixed. * * Also it would be nice to use gpiochip_find() here so we @@ -829,7 +830,7 @@ static int __init gpiolib_sysfs_init(void) * gpio_lock prevents us from doing this. */ spin_unlock_irqrestore(&gpio_lock, flags); - status = gpiochip_export(chip); + status = gpiochip_register(chip); spin_lock_irqsave(&gpio_lock, flags); } spin_unlock_irqrestore(&gpio_lock, flags); diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 5a5c208d31c7..fefc36211205 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -285,7 +285,7 @@ int gpiochip_add(struct gpio_chip *chip) of_gpiochip_add(chip); acpi_gpiochip_add(chip); - status = gpiochip_export(chip); + status = gpiochip_register(chip); if (status) goto err_remove_chip; @@ -330,7 +330,7 @@ void gpiochip_remove(struct gpio_chip *chip) unsigned id; bool requested = false; - gpiochip_unexport(chip); + gpiochip_unregister(chip); gpiochip_irqchip_remove(chip); diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index 54bc5ec91398..69458a022c90 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -149,17 +149,17 @@ static int __maybe_unused gpio_chip_hwgpio(const struct gpio_desc *desc) #ifdef CONFIG_GPIO_SYSFS -int gpiochip_export(struct gpio_chip *chip); -void gpiochip_unexport(struct gpio_chip *chip); +int gpiochip_register(struct gpio_chip *chip); +void gpiochip_unregister(struct gpio_chip *chip); #else -static inline int gpiochip_export(struct gpio_chip *chip) +static inline int gpiochip_register(struct gpio_chip *chip) { return 0; } -static inline void gpiochip_unexport(struct gpio_chip *chip) +static inline void gpiochip_unregister(struct gpio_chip *chip) { }
Rename the gpio-chip export/unexport functions to the more descriptive names gpiochip_register and gpiochip_unregister. Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/gpio/gpiolib-sysfs.c | 13 +++++++------ drivers/gpio/gpiolib.c | 4 ++-- drivers/gpio/gpiolib.h | 8 ++++---- 3 files changed, 13 insertions(+), 12 deletions(-)