| Message ID | 20181005194206.8925-2-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] pinctrl: rza1: don't manually release devm managed resources | expand |
On Fri 2018-10-05 21:42:06, Uwe Kleine-König wrote: > There is hardly any reason to call devm_gpiochip_remove() because the > driver core handles calling gpiochip_remove() automatically. > > To make it harder to introduce new (and probably unneeded) callers, drop > the function. > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Documentation/driver-model/devres.txt | 1 - > drivers/gpio/gpiolib.c | 18 +----------------- > include/linux/gpio/driver.h | 1 - > 3 files changed, 1 insertion(+), 19 deletions(-) > > diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt > index 7c1bb3d0c222..3f74d645abfa 100644 > --- a/Documentation/driver-model/devres.txt > +++ b/Documentation/driver-model/devres.txt > @@ -254,7 +254,6 @@ GPIO > devm_gpiod_get_optional() > devm_gpiod_put() > devm_gpiochip_add_data() > - devm_gpiochip_remove() > devm_gpio_request() > devm_gpio_request_one() > devm_gpio_free() There's more than one "free" function here, and perhaps this is useful in some cases... Dunno. Renaming to make people think twice sounds ok, but I'm not sure if outright removal is good idea. Pavel
Hello Pavel, On Sat, Oct 06, 2018 at 10:05:57PM +0200, Pavel Machek wrote: > On Fri 2018-10-05 21:42:06, Uwe Kleine-König wrote: > > There is hardly any reason to call devm_gpiochip_remove() because the > > driver core handles calling gpiochip_remove() automatically. > > > > To make it harder to introduce new (and probably unneeded) callers, drop > > the function. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Documentation/driver-model/devres.txt | 1 - > > drivers/gpio/gpiolib.c | 18 +----------------- > > include/linux/gpio/driver.h | 1 - > > 3 files changed, 1 insertion(+), 19 deletions(-) > > > > diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt > > index 7c1bb3d0c222..3f74d645abfa 100644 > > --- a/Documentation/driver-model/devres.txt > > +++ b/Documentation/driver-model/devres.txt > > @@ -254,7 +254,6 @@ GPIO > > devm_gpiod_get_optional() > > devm_gpiod_put() > > devm_gpiochip_add_data() > > - devm_gpiochip_remove() > > devm_gpio_request() > > devm_gpio_request_one() > > devm_gpio_free() > > There's more than one "free" function here, and perhaps this is useful > in some cases... Dunno. Renaming to make people think twice sounds ok, > but I'm not sure if outright removal is good idea. There is currently no user, so there is (IMHO) no good reason to spend .text memory for it. If at some point there is a valid usecase for it, it's a good hurdle to reintroduce this function to make reviewer check if the case is really valid. I also started grepping for devm_gpio_free, and it think the only user (if we let staging aside) could also be fixed to not make use of it. Best regards Uwe
On Fri, Oct 5, 2018 at 9:42 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > There is hardly any reason to call devm_gpiochip_remove() because the > driver core handles calling gpiochip_remove() automatically. > > To make it harder to introduce new (and probably unneeded) callers, drop > the function. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Why not. If somebody legitimately needs it this is easy to reimplement by just looking at this commit. But I have to wait until the next kernel cycle with this because of late in the cycle and cross-tree issues. I don't want to create merge conflicts right now (this patch does due to other changes). Could you resend this at v4.20-rc1 please? Yours, Linus Walleij
Hello Linus,
On Wed, Oct 10, 2018 at 01:35:48PM +0200, Linus Walleij wrote:
> Could you resend this at v4.20-rc1 please?
Sure, will do.
Best regards
Uwe
Hello Linus, On Wed, Oct 10, 2018 at 01:35:48PM +0200, Linus Walleij wrote: > On Fri, Oct 5, 2018 at 9:42 PM Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > > There is hardly any reason to call devm_gpiochip_remove() because the > > driver core handles calling gpiochip_remove() automatically. > > > > To make it harder to introduce new (and probably unneeded) callers, drop > > the function. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Why not. If somebody legitimately needs it this is easy to reimplement by > just looking at this commit. > > But I have to wait until the next kernel cycle with this because of > late in the cycle and cross-tree issues. I don't want to create merge > conflicts right now (this patch does due to other changes). > > Could you resend this at v4.20-rc1 please? I don't see a commit that would conflict with my patch between 4.18 and current next master apart from patches in your tree. What am I missing? Best regards Uwe
On Fri, Oct 5, 2018 at 9:42 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > There is hardly any reason to call devm_gpiochip_remove() because the > driver core handles calling gpiochip_remove() automatically. > > To make it harder to introduce new (and probably unneeded) callers, drop > the function. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Patch applied for v4.21 Yours, Linus Walleij
On Tue, Oct 23, 2018 at 9:23 AM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > Could you resend this at v4.20-rc1 please? > > I don't see a commit that would conflict with my patch between 4.18 and > current next master apart from patches in your tree. > > What am I missing? Some cross-dependence between GPIO and pin control IIRC. Anyways, I applied the patch now, had to do some fuzzing. It will appear in linux-next after v4.20-rc1. Yours, Linus Walleij
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt index 7c1bb3d0c222..3f74d645abfa 100644 --- a/Documentation/driver-model/devres.txt +++ b/Documentation/driver-model/devres.txt @@ -254,7 +254,6 @@ GPIO devm_gpiod_get_optional() devm_gpiod_put() devm_gpiochip_add_data() - devm_gpiochip_remove() devm_gpio_request() devm_gpio_request_one() devm_gpio_free() diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 34617298b896..a6d88d0238f1 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1501,6 +1501,7 @@ static int devm_gpio_chip_match(struct device *dev, void *res, void *data) return *r == data; } + /** * devm_gpiochip_add_data() - Resource manager gpiochip_add_data() * @dev: the device pointer on which gpio_chip belongs to. @@ -1540,23 +1541,6 @@ int devm_gpiochip_add_data(struct device *dev, struct gpio_chip *chip, } EXPORT_SYMBOL_GPL(devm_gpiochip_add_data); -/** - * devm_gpiochip_remove() - Resource manager of gpiochip_remove() - * @dev: device for which which resource was allocated - * @chip: the chip to remove - * - * A gpio_chip with any GPIOs still requested may not be removed. - */ -void devm_gpiochip_remove(struct device *dev, struct gpio_chip *chip) -{ - int ret; - - ret = devres_release(dev, devm_gpio_chip_release, - devm_gpio_chip_match, chip); - WARN_ON(ret); -} -EXPORT_SYMBOL_GPL(devm_gpiochip_remove); - /** * gpiochip_find() - iterator for locating a specific gpio_chip * @data: data to pass to match function diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 5382b5183b7e..7cd23de5d0a9 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -383,7 +383,6 @@ static inline int gpiochip_add(struct gpio_chip *chip) extern void gpiochip_remove(struct gpio_chip *chip); extern int devm_gpiochip_add_data(struct device *dev, struct gpio_chip *chip, void *data); -extern void devm_gpiochip_remove(struct device *dev, struct gpio_chip *chip); extern struct gpio_chip *gpiochip_find(void *data, int (*match)(struct gpio_chip *chip, void *data));
There is hardly any reason to call devm_gpiochip_remove() because the driver core handles calling gpiochip_remove() automatically. To make it harder to introduce new (and probably unneeded) callers, drop the function. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Documentation/driver-model/devres.txt | 1 - drivers/gpio/gpiolib.c | 18 +----------------- include/linux/gpio/driver.h | 1 - 3 files changed, 1 insertion(+), 19 deletions(-)