[PATCH v1 1/3] gpiolib: Follow usual pattern for gpiod_remove_lookup_table() call

[PATCH v1 1/3] gpiolib: Follow usual pattern for gpiod_remove_lookup_table() call

[Andy Shevchenko]

The usual pattern for the remove calls, like gpiod_remove_lookup_table(),
is to be NULL-aware, i.o.w. become a no-op whenever parameter is NULL.
Update gpiod_remove_lookup_table() call to follow this pattern.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b02cc2abd3b6..611d6ea82d75 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3460,6 +3460,10 @@ EXPORT_SYMBOL_GPL(gpiod_add_lookup_table);
  */
 void gpiod_remove_lookup_table(struct gpiod_lookup_table *table)
 {
+	/* Nothing to remove */
+	if (!table)
+		return;
+
 	mutex_lock(&gpio_lookup_lock);
 
 	list_del(&table->list);
-- 
2.29.2

[PATCH v1 2/3] i2c: i801: Drop duplicate NULL check in i801_del_mux()

[Andy Shevchenko]

Since gpiod_remove_lookup_table() is NULL-aware, no need to have this check
in the caller. Drop duplicate NULL check.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-i801.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index ae90713443fa..7c2569a18f8c 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1487,8 +1487,7 @@ static void i801_del_mux(struct i801_priv *priv)
 {
 	if (priv->mux_pdev)
 		platform_device_unregister(priv->mux_pdev);
-	if (priv->lookup)
-		gpiod_remove_lookup_table(priv->lookup);
+	gpiod_remove_lookup_table(priv->lookup);
 }
 
 static unsigned int i801_get_adapter_class(struct i801_priv *priv)
-- 
2.29.2

Re: [PATCH v1 2/3] i2c: i801: Drop duplicate NULL check in i801_del_mux()

[Jean Delvare]

On Mon, 28 Dec 2020 22:06:17 +0200, Andy Shevchenko wrote:
> Since gpiod_remove_lookup_table() is NULL-aware, no need to have this check
> in the caller. Drop duplicate NULL check.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-i801.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index ae90713443fa..7c2569a18f8c 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1487,8 +1487,7 @@ static void i801_del_mux(struct i801_priv *priv)
>  {
>  	if (priv->mux_pdev)
>  		platform_device_unregister(priv->mux_pdev);
> -	if (priv->lookup)
> -		gpiod_remove_lookup_table(priv->lookup);
> +	gpiod_remove_lookup_table(priv->lookup);
>  }
>  
>  static unsigned int i801_get_adapter_class(struct i801_priv *priv)

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

[PATCH v1 3/3] i2c: i801: Refactor mux code since platform_device_unregister() is NULL aware

[Andy Shevchenko]

platform_device_unregister() is NULL-aware and thus doesn't required a
duplication check in i801_del_mux(). Besides that it's also error pointer
aware, and we may drop unneeded assignment in i801_add_mux() followed by
conversion to PTR_ERR_OR_ZERO() for the returned value.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-i801.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 7c2569a18f8c..1400a8716388 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1433,7 +1433,7 @@ static int i801_add_mux(struct i801_priv *priv)
 	const struct i801_mux_config *mux_config;
 	struct i2c_mux_gpio_platform_data gpio_data;
 	struct gpiod_lookup_table *lookup;
-	int err, i;
+	int i;
 
 	if (!priv->mux_drvdata)
 		return 0;
@@ -1473,20 +1473,16 @@ static int i801_add_mux(struct i801_priv *priv)
 				PLATFORM_DEVID_NONE, &gpio_data,
 				sizeof(struct i2c_mux_gpio_platform_data));
 	if (IS_ERR(priv->mux_pdev)) {
-		err = PTR_ERR(priv->mux_pdev);
 		gpiod_remove_lookup_table(lookup);
-		priv->mux_pdev = NULL;
 		dev_err(dev, "Failed to register i2c-mux-gpio device\n");
-		return err;
 	}
 
-	return 0;
+	return PTR_ERR_OR_ZERO(priv->mux_pdev);
 }
 
 static void i801_del_mux(struct i801_priv *priv)
 {
-	if (priv->mux_pdev)
-		platform_device_unregister(priv->mux_pdev);
+	platform_device_unregister(priv->mux_pdev);
 	gpiod_remove_lookup_table(priv->lookup);
 }
 
-- 
2.29.2

Re: [PATCH v1 3/3] i2c: i801: Refactor mux code since platform_device_unregister() is NULL aware

[Jean Delvare]

On Mon, 28 Dec 2020 22:06:18 +0200, Andy Shevchenko wrote:
> platform_device_unregister() is NULL-aware and thus doesn't required a
> duplication check in i801_del_mux(). Besides that it's also error pointer
> aware, and we may drop unneeded assignment in i801_add_mux() followed by
> conversion to PTR_ERR_OR_ZERO() for the returned value.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-i801.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 7c2569a18f8c..1400a8716388 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1433,7 +1433,7 @@ static int i801_add_mux(struct i801_priv *priv)
>  	const struct i801_mux_config *mux_config;
>  	struct i2c_mux_gpio_platform_data gpio_data;
>  	struct gpiod_lookup_table *lookup;
> -	int err, i;
> +	int i;
>  
>  	if (!priv->mux_drvdata)
>  		return 0;
> @@ -1473,20 +1473,16 @@ static int i801_add_mux(struct i801_priv *priv)
>  				PLATFORM_DEVID_NONE, &gpio_data,
>  				sizeof(struct i2c_mux_gpio_platform_data));
>  	if (IS_ERR(priv->mux_pdev)) {
> -		err = PTR_ERR(priv->mux_pdev);
>  		gpiod_remove_lookup_table(lookup);
> -		priv->mux_pdev = NULL;
>  		dev_err(dev, "Failed to register i2c-mux-gpio device\n");
> -		return err;
>  	}
>  
> -	return 0;
> +	return PTR_ERR_OR_ZERO(priv->mux_pdev);
>  }
>  
>  static void i801_del_mux(struct i801_priv *priv)
>  {
> -	if (priv->mux_pdev)
> -		platform_device_unregister(priv->mux_pdev);
> +	platform_device_unregister(priv->mux_pdev);
>  	gpiod_remove_lookup_table(priv->lookup);
>  }
>  

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v1 1/3] gpiolib: Follow usual pattern for gpiod_remove_lookup_table() call

[Bartosz Golaszewski]

On Mon, Dec 28, 2020 at 9:06 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> The usual pattern for the remove calls, like gpiod_remove_lookup_table(),
> is to be NULL-aware, i.o.w. become a no-op whenever parameter is NULL.
> Update gpiod_remove_lookup_table() call to follow this pattern.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpiolib.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index b02cc2abd3b6..611d6ea82d75 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -3460,6 +3460,10 @@ EXPORT_SYMBOL_GPL(gpiod_add_lookup_table);
>   */
>  void gpiod_remove_lookup_table(struct gpiod_lookup_table *table)
>  {
> +       /* Nothing to remove */
> +       if (!table)
> +               return;
> +
>         mutex_lock(&gpio_lookup_lock);
>
>         list_del(&table->list);
> --
> 2.29.2
>

Applied, thanks!

Bart

Re: [PATCH v1 1/3] gpiolib: Follow usual pattern for gpiod_remove_lookup_table() call

[Andy Shevchenko]

On Mon, Jan 04, 2021 at 03:24:47PM +0100, Bartosz Golaszewski wrote:
> On Mon, Dec 28, 2020 at 9:06 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > The usual pattern for the remove calls, like gpiod_remove_lookup_table(),
> > is to be NULL-aware, i.o.w. become a no-op whenever parameter is NULL.
> > Update gpiod_remove_lookup_table() call to follow this pattern.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/gpio/gpiolib.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index b02cc2abd3b6..611d6ea82d75 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -3460,6 +3460,10 @@ EXPORT_SYMBOL_GPL(gpiod_add_lookup_table);
> >   */
> >  void gpiod_remove_lookup_table(struct gpiod_lookup_table *table)
> >  {
> > +       /* Nothing to remove */
> > +       if (!table)
> > +               return;
> > +
> >         mutex_lock(&gpio_lookup_lock);
> >
> >         list_del(&table->list);
> > --
> > 2.29.2
> >
> 
> Applied, thanks!

Thanks. Though I think it can be also applied to I²C tree since dependency?
Any tags / immutable branch for Wolfram?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/3] gpiolib: Follow usual pattern for gpiod_remove_lookup_table() call

[Bartosz Golaszewski]

On Mon, Jan 4, 2021 at 3:32 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Jan 04, 2021 at 03:24:47PM +0100, Bartosz Golaszewski wrote:
> > On Mon, Dec 28, 2020 at 9:06 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > The usual pattern for the remove calls, like gpiod_remove_lookup_table(),
> > > is to be NULL-aware, i.o.w. become a no-op whenever parameter is NULL.
> > > Update gpiod_remove_lookup_table() call to follow this pattern.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  drivers/gpio/gpiolib.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > index b02cc2abd3b6..611d6ea82d75 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -3460,6 +3460,10 @@ EXPORT_SYMBOL_GPL(gpiod_add_lookup_table);
> > >   */
> > >  void gpiod_remove_lookup_table(struct gpiod_lookup_table *table)
> > >  {
> > > +       /* Nothing to remove */
> > > +       if (!table)
> > > +               return;
> > > +
> > >         mutex_lock(&gpio_lookup_lock);
> > >
> > >         list_del(&table->list);
> > > --
> > > 2.29.2
> > >
> >
> > Applied, thanks!
>
> Thanks. Though I think it can be also applied to I涎 tree since dependency?
> Any tags / immutable branch for Wolfram?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Oops I assumed this is independent. In that case:

Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Wolfram, please take it through the i2c tree.

Bartosz

Re: [PATCH v1 1/3] gpiolib: Follow usual pattern for gpiod_remove_lookup_table() call

[Jean Delvare]

On Mon, 28 Dec 2020 22:06:16 +0200, Andy Shevchenko wrote:
> The usual pattern for the remove calls, like gpiod_remove_lookup_table(),
> is to be NULL-aware, i.o.w. become a no-op whenever parameter is NULL.
> Update gpiod_remove_lookup_table() call to follow this pattern.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpiolib.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index b02cc2abd3b6..611d6ea82d75 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -3460,6 +3460,10 @@ EXPORT_SYMBOL_GPL(gpiod_add_lookup_table);
>   */
>  void gpiod_remove_lookup_table(struct gpiod_lookup_table *table)
>  {
> +	/* Nothing to remove */
> +	if (!table)
> +		return;
> +
>  	mutex_lock(&gpio_lookup_lock);
>  
>  	list_del(&table->list);

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support