[PATCH v2] bus: sunxi-rsb: make remove callback return void

[PATCH v2] bus: sunxi-rsb: make remove callback return void

[Uwe Kleine-König]

The driver core ignores the return value of struct device_driver::remove
because there is only little that can be done. To simplify the quest to
make this function return void, let struct sunxi_rsb_driver::remove
return void, too. All users already unconditionally return 0, this
commit makes this obvious and ensures future users don't behave
differently. To simplify even further, make axp20x_device_remove()
return void instead of returning 0 unconditionally, too.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

compared to implicit v1 (Message-Id:
20201126074124.1753528-1-u.kleine-koenig@pengutronix.de) the following changed:

 - Sent to a better chosen set of people
 - Add Chen-Yu's Reviewed-by: tag

Chen-Yu wrote in reply to v1:
> Since this touches the mfd tree as well, we either need an Ack from Lee,
> the mfd maintainer (CC-ed), or let Lee take it in his tree. For the latter,
> 
> Acked-by: Chen-Yu Tsai <wens@csie.org>
>
> AFAIK we (sunxi) don't have anything regarding RSB queued up, so there
> won't be any conflicts.

Best regards
Uwe

 drivers/bus/sunxi-rsb.c    | 4 +++-
 drivers/mfd/axp20x-i2c.c   | 4 +++-
 drivers/mfd/axp20x-rsb.c   | 4 ++--
 drivers/mfd/axp20x.c       | 4 +---
 include/linux/mfd/axp20x.h | 2 +-
 include/linux/sunxi-rsb.h  | 2 +-
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index 1bb00a959c67..117716e23ffb 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -170,7 +170,9 @@ static int sunxi_rsb_device_remove(struct device *dev)
 {
 	const struct sunxi_rsb_driver *drv = to_sunxi_rsb_driver(dev->driver);
 
-	return drv->remove(to_sunxi_rsb_device(dev));
+	drv->remove(to_sunxi_rsb_device(dev));
+
+	return 0;
 }
 
 static struct bus_type sunxi_rsb_bus = {
diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
index 068e9c083f13..743e8338b0db 100644
--- a/drivers/mfd/axp20x-i2c.c
+++ b/drivers/mfd/axp20x-i2c.c
@@ -54,7 +54,9 @@ static int axp20x_i2c_remove(struct i2c_client *i2c)
 {
 	struct axp20x_dev *axp20x = i2c_get_clientdata(i2c);
 
-	return axp20x_device_remove(axp20x);
+	axp20x_device_remove(axp20x);
+
+	return 0;
 }
 
 static const struct of_device_id axp20x_i2c_of_match[] = {
diff --git a/drivers/mfd/axp20x-rsb.c b/drivers/mfd/axp20x-rsb.c
index 4cdc79f5cc48..214bc0d84d44 100644
--- a/drivers/mfd/axp20x-rsb.c
+++ b/drivers/mfd/axp20x-rsb.c
@@ -49,11 +49,11 @@ static int axp20x_rsb_probe(struct sunxi_rsb_device *rdev)
 	return axp20x_device_probe(axp20x);
 }
 
-static int axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
+static void axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
 {
 	struct axp20x_dev *axp20x = sunxi_rsb_device_get_drvdata(rdev);
 
-	return axp20x_device_remove(axp20x);
+	axp20x_device_remove(axp20x);
 }
 
 static const struct of_device_id axp20x_rsb_of_match[] = {
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index aa59496e4376..3eae04e24ac8 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -987,7 +987,7 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
 }
 EXPORT_SYMBOL(axp20x_device_probe);
 
-int axp20x_device_remove(struct axp20x_dev *axp20x)
+void axp20x_device_remove(struct axp20x_dev *axp20x)
 {
 	if (axp20x == axp20x_pm_power_off) {
 		axp20x_pm_power_off = NULL;
@@ -996,8 +996,6 @@ int axp20x_device_remove(struct axp20x_dev *axp20x)
 
 	mfd_remove_devices(axp20x->dev);
 	regmap_del_irq_chip(axp20x->irq, axp20x->regmap_irqc);
-
-	return 0;
 }
 EXPORT_SYMBOL(axp20x_device_remove);
 
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index fd5957c042da..9ab0e2fca7ea 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -696,6 +696,6 @@ int axp20x_device_probe(struct axp20x_dev *axp20x);
  *
  * This tells the axp20x core to remove the associated mfd devices
  */
-int axp20x_device_remove(struct axp20x_dev *axp20x);
+void axp20x_device_remove(struct axp20x_dev *axp20x);
 
 #endif /* __LINUX_MFD_AXP20X_H */
diff --git a/include/linux/sunxi-rsb.h b/include/linux/sunxi-rsb.h
index 7e75bb0346d0..bf0d365f471c 100644
--- a/include/linux/sunxi-rsb.h
+++ b/include/linux/sunxi-rsb.h
@@ -59,7 +59,7 @@ static inline void sunxi_rsb_device_set_drvdata(struct sunxi_rsb_device *rdev,
 struct sunxi_rsb_driver {
 	struct device_driver driver;
 	int (*probe)(struct sunxi_rsb_device *rdev);
-	int (*remove)(struct sunxi_rsb_device *rdev);
+	void (*remove)(struct sunxi_rsb_device *rdev);
 };
 
 static inline struct sunxi_rsb_driver *to_sunxi_rsb_driver(struct device_driver *d)
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] bus: sunxi-rsb: make remove callback return void

[Lee Jones]

On Thu, 26 Nov 2020, Uwe Kleine-König wrote:

> The driver core ignores the return value of struct device_driver::remove
> because there is only little that can be done. To simplify the quest to
> make this function return void, let struct sunxi_rsb_driver::remove
> return void, too. All users already unconditionally return 0, this
> commit makes this obvious and ensures future users don't behave
> differently. To simplify even further, make axp20x_device_remove()
> return void instead of returning 0 unconditionally, too.
> 
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> compared to implicit v1 (Message-Id:
> 20201126074124.1753528-1-u.kleine-koenig@pengutronix.de) the following changed:
> 
>  - Sent to a better chosen set of people
>  - Add Chen-Yu's Reviewed-by: tag
> 
> Chen-Yu wrote in reply to v1:
> > Since this touches the mfd tree as well, we either need an Ack from Lee,
> > the mfd maintainer (CC-ed), or let Lee take it in his tree. For the latter,
> > 
> > Acked-by: Chen-Yu Tsai <wens@csie.org>
> >
> > AFAIK we (sunxi) don't have anything regarding RSB queued up, so there
> > won't be any conflicts.
> 
> Best regards
> Uwe
> 
>  drivers/bus/sunxi-rsb.c    | 4 +++-
>  drivers/mfd/axp20x-i2c.c   | 4 +++-
>  drivers/mfd/axp20x-rsb.c   | 4 ++--
>  drivers/mfd/axp20x.c       | 4 +---
>  include/linux/mfd/axp20x.h | 2 +-
>  include/linux/sunxi-rsb.h  | 2 +-
>  6 files changed, 11 insertions(+), 9 deletions(-)

There are no dependencies between the MFD and Bus changes as far as I
can tell.  For the sake of simplicity i.e. to avoid the requirement of
immutable branch maintenance and an associated pull-request, it would
be better to split this out into 2 separate patches.

> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index 1bb00a959c67..117716e23ffb 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
> @@ -170,7 +170,9 @@ static int sunxi_rsb_device_remove(struct device *dev)
>  {
>  	const struct sunxi_rsb_driver *drv = to_sunxi_rsb_driver(dev->driver);
>  
> -	return drv->remove(to_sunxi_rsb_device(dev));
> +	drv->remove(to_sunxi_rsb_device(dev));
> +
> +	return 0;
>  }
>  
>  static struct bus_type sunxi_rsb_bus = {
> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index 068e9c083f13..743e8338b0db 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -54,7 +54,9 @@ static int axp20x_i2c_remove(struct i2c_client *i2c)
>  {
>  	struct axp20x_dev *axp20x = i2c_get_clientdata(i2c);
>  
> -	return axp20x_device_remove(axp20x);
> +	axp20x_device_remove(axp20x);
> +
> +	return 0;
>  }
>  
>  static const struct of_device_id axp20x_i2c_of_match[] = {
> diff --git a/drivers/mfd/axp20x-rsb.c b/drivers/mfd/axp20x-rsb.c
> index 4cdc79f5cc48..214bc0d84d44 100644
> --- a/drivers/mfd/axp20x-rsb.c
> +++ b/drivers/mfd/axp20x-rsb.c
> @@ -49,11 +49,11 @@ static int axp20x_rsb_probe(struct sunxi_rsb_device *rdev)
>  	return axp20x_device_probe(axp20x);
>  }
>  
> -static int axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
> +static void axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
>  {
>  	struct axp20x_dev *axp20x = sunxi_rsb_device_get_drvdata(rdev);
>  
> -	return axp20x_device_remove(axp20x);
> +	axp20x_device_remove(axp20x);
>  }
>  
>  static const struct of_device_id axp20x_rsb_of_match[] = {
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index aa59496e4376..3eae04e24ac8 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -987,7 +987,7 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
>  }
>  EXPORT_SYMBOL(axp20x_device_probe);
>  
> -int axp20x_device_remove(struct axp20x_dev *axp20x)
> +void axp20x_device_remove(struct axp20x_dev *axp20x)
>  {
>  	if (axp20x == axp20x_pm_power_off) {
>  		axp20x_pm_power_off = NULL;
> @@ -996,8 +996,6 @@ int axp20x_device_remove(struct axp20x_dev *axp20x)
>  
>  	mfd_remove_devices(axp20x->dev);
>  	regmap_del_irq_chip(axp20x->irq, axp20x->regmap_irqc);
> -
> -	return 0;
>  }
>  EXPORT_SYMBOL(axp20x_device_remove);
>  
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index fd5957c042da..9ab0e2fca7ea 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -696,6 +696,6 @@ int axp20x_device_probe(struct axp20x_dev *axp20x);
>   *
>   * This tells the axp20x core to remove the associated mfd devices
>   */
> -int axp20x_device_remove(struct axp20x_dev *axp20x);
> +void axp20x_device_remove(struct axp20x_dev *axp20x);
>  
>  #endif /* __LINUX_MFD_AXP20X_H */
> diff --git a/include/linux/sunxi-rsb.h b/include/linux/sunxi-rsb.h
> index 7e75bb0346d0..bf0d365f471c 100644
> --- a/include/linux/sunxi-rsb.h
> +++ b/include/linux/sunxi-rsb.h
> @@ -59,7 +59,7 @@ static inline void sunxi_rsb_device_set_drvdata(struct sunxi_rsb_device *rdev,
>  struct sunxi_rsb_driver {
>  	struct device_driver driver;
>  	int (*probe)(struct sunxi_rsb_device *rdev);
> -	int (*remove)(struct sunxi_rsb_device *rdev);
> +	void (*remove)(struct sunxi_rsb_device *rdev);
>  };
>  
>  static inline struct sunxi_rsb_driver *to_sunxi_rsb_driver(struct device_driver *d)

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] bus: sunxi-rsb: make remove callback return void

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 3346 bytes --]

On Fri, Jan 15, 2021 at 08:11:22AM +0000, Lee Jones wrote:
> On Thu, 26 Nov 2020, Uwe Kleine-König wrote:
> 
> > The driver core ignores the return value of struct device_driver::remove
> > because there is only little that can be done. To simplify the quest to
> > make this function return void, let struct sunxi_rsb_driver::remove
> > return void, too. All users already unconditionally return 0, this
> > commit makes this obvious and ensures future users don't behave
> > differently. To simplify even further, make axp20x_device_remove()
> > return void instead of returning 0 unconditionally, too.
> > 
> > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > compared to implicit v1 (Message-Id:
> > 20201126074124.1753528-1-u.kleine-koenig@pengutronix.de) the following changed:
> > 
> >  - Sent to a better chosen set of people
> >  - Add Chen-Yu's Reviewed-by: tag
> > 
> > Chen-Yu wrote in reply to v1:
> > > Since this touches the mfd tree as well, we either need an Ack from Lee,
> > > the mfd maintainer (CC-ed), or let Lee take it in his tree. For the latter,
> > > 
> > > Acked-by: Chen-Yu Tsai <wens@csie.org>
> > >
> > > AFAIK we (sunxi) don't have anything regarding RSB queued up, so there
> > > won't be any conflicts.
> > 
> > Best regards
> > Uwe
> > 
> >  drivers/bus/sunxi-rsb.c    | 4 +++-
> >  drivers/mfd/axp20x-i2c.c   | 4 +++-
> >  drivers/mfd/axp20x-rsb.c   | 4 ++--
> >  drivers/mfd/axp20x.c       | 4 +---
> >  include/linux/mfd/axp20x.h | 2 +-
> >  include/linux/sunxi-rsb.h  | 2 +-
> >  6 files changed, 11 insertions(+), 9 deletions(-)
> 
> There are no dependencies between the MFD and Bus changes as far as I
> can tell.

There are dependencies, because

-static int axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
+static void axp20x_rsb_remove(struct sunxi_rsb_device *rdev)

in drivers/mfd/axp20x-rsb.c must be done together with

--- a/include/linux/sunxi-rsb.h
+++ b/include/linux/sunxi-rsb.h
@@ -59,7 +59,7 @@ static inline void sunxi_rsb_device_set_drvdata(struct sunxi_rsb_device *rdev,
 struct sunxi_rsb_driver {
 	struct device_driver driver;
 	int (*probe)(struct sunxi_rsb_device *rdev);
-	int (*remove)(struct sunxi_rsb_device *rdev);
+	void (*remove)(struct sunxi_rsb_device *rdev);
 };
 [...]

> For the sake of simplicity i.e. to avoid the requirement of
> immutable branch maintenance and an associated pull-request, it would
> be better to split this out into 2 separate patches.

So the base for this statement is gone and the following questions
remain:

 - Do you insist on splitting out the change to axp20x_device_remove()?
 - Do you prefer to ack the mfd part to let the patch (or patches if
   they get split) go via the sunxi people or do you want to take the
   it (them) via mfd?

Looking at next there are four patches touching drivers/bus/sunxi-rsb.c
and none touching drivers/mfd/axp20x* or include/linux/mfd/axp20x.h
which suggests that letting it go via sunxi might be more sensible. IMHO
an immutable branch is not necessary?!

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] bus: sunxi-rsb: make remove callback return void

[Lee Jones]

On Fri, 15 Jan 2021, Uwe Kleine-König wrote:

> On Fri, Jan 15, 2021 at 08:11:22AM +0000, Lee Jones wrote:
> > On Thu, 26 Nov 2020, Uwe Kleine-König wrote:
> > 
> > > The driver core ignores the return value of struct device_driver::remove
> > > because there is only little that can be done. To simplify the quest to
> > > make this function return void, let struct sunxi_rsb_driver::remove
> > > return void, too. All users already unconditionally return 0, this
> > > commit makes this obvious and ensures future users don't behave
> > > differently. To simplify even further, make axp20x_device_remove()
> > > return void instead of returning 0 unconditionally, too.
> > > 
> > > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Hello,
> > > 
> > > compared to implicit v1 (Message-Id:
> > > 20201126074124.1753528-1-u.kleine-koenig@pengutronix.de) the following changed:
> > > 
> > >  - Sent to a better chosen set of people
> > >  - Add Chen-Yu's Reviewed-by: tag
> > > 
> > > Chen-Yu wrote in reply to v1:
> > > > Since this touches the mfd tree as well, we either need an Ack from Lee,
> > > > the mfd maintainer (CC-ed), or let Lee take it in his tree. For the latter,
> > > > 
> > > > Acked-by: Chen-Yu Tsai <wens@csie.org>
> > > >
> > > > AFAIK we (sunxi) don't have anything regarding RSB queued up, so there
> > > > won't be any conflicts.
> > > 
> > > Best regards
> > > Uwe
> > > 
> > >  drivers/bus/sunxi-rsb.c    | 4 +++-
> > >  drivers/mfd/axp20x-i2c.c   | 4 +++-
> > >  drivers/mfd/axp20x-rsb.c   | 4 ++--
> > >  drivers/mfd/axp20x.c       | 4 +---
> > >  include/linux/mfd/axp20x.h | 2 +-
> > >  include/linux/sunxi-rsb.h  | 2 +-
> > >  6 files changed, 11 insertions(+), 9 deletions(-)
> > 
> > There are no dependencies between the MFD and Bus changes as far as I
> > can tell.
> 
> There are dependencies, because
> 
> -static int axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
> +static void axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
> 
> in drivers/mfd/axp20x-rsb.c must be done together with
> 
> --- a/include/linux/sunxi-rsb.h
> +++ b/include/linux/sunxi-rsb.h
> @@ -59,7 +59,7 @@ static inline void sunxi_rsb_device_set_drvdata(struct sunxi_rsb_device *rdev,
>  struct sunxi_rsb_driver {
>  	struct device_driver driver;
>  	int (*probe)(struct sunxi_rsb_device *rdev);
> -	int (*remove)(struct sunxi_rsb_device *rdev);
> +	void (*remove)(struct sunxi_rsb_device *rdev);
>  };
>  [...]

Yes, this will need to be taken in with the MFD patch.

> > For the sake of simplicity i.e. to avoid the requirement of
> > immutable branch maintenance and an associated pull-request, it would
> > be better to split this out into 2 separate patches.
> 
> So the base for this statement is gone

It still stands.

> and the following questions remain:

>  - Do you insist on splitting out the change to axp20x_device_remove()?

[0] Unless you gave give me a compelling reason why it shouldn't, yes.

>  - Do you prefer to ack the mfd part to let the patch (or patches if
>    they get split) go via the sunxi people or do you want to take the
>    it (them) via mfd?

I'd prefer the MFD (and header only affecting MFD) to go in via MFD.

The Bus patch can do in via it's own tree.

> Looking at next there are four patches touching drivers/bus/sunxi-rsb.c
> and none touching drivers/mfd/axp20x* or include/linux/mfd/axp20x.h
> which suggests that letting it go via sunxi might be more sensible. IMHO
> an immutable branch is not necessary?!

It's only -rc3 and you cannot tell the future.

If you manage to satisfy [0] and they do end up going in together, I
will insist on an immutable branch.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2] bus: sunxi-rsb: make remove callback return void

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 5813 bytes --]

The driver core ignores the return value of struct device_driver::remove
because there is only little that can be done. To simplify the quest to
make this function return void, let struct sunxi_rsb_driver::remove
return void, too.

axp20x_device_remove() always returns 0, so there is no information
lost in axp20x_rsb_remove(). The only other sunxi-rsb driver doesn't
have a remove callback and so doesn't require adaption.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---

Hello Lee,

On Fri, Jan 15, 2021 at 11:05:43AM +0000, Lee Jones wrote:
> On Fri, 15 Jan 2021, Uwe Kleine-König wrote:
> > On Fri, Jan 15, 2021 at 08:11:22AM +0000, Lee Jones wrote:
> > > There are no dependencies between the MFD and Bus changes as far as I
> > > can tell.
> > 
> > There are dependencies, because
> > 
> > -static int axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
> > +static void axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
> > 
> > in drivers/mfd/axp20x-rsb.c must be done together with
> > 
> > --- a/include/linux/sunxi-rsb.h
> > +++ b/include/linux/sunxi-rsb.h
> > @@ -59,7 +59,7 @@ static inline void sunxi_rsb_device_set_drvdata(struct sunxi_rsb_device *rdev,
> >  struct sunxi_rsb_driver {
> >  	struct device_driver driver;
> >  	int (*probe)(struct sunxi_rsb_device *rdev);
> > -	int (*remove)(struct sunxi_rsb_device *rdev);
> > +	void (*remove)(struct sunxi_rsb_device *rdev);
> >  };
> >  [...]
> 
> Yes, this will need to be taken in with the MFD patch.
> 
> > > For the sake of simplicity i.e. to avoid the requirement of
> > > immutable branch maintenance and an associated pull-request, it would
> > > be better to split this out into 2 separate patches.
> > 
> > So the base for this statement is gone
> 
> It still stands.

I don't understand this. Now I dropped the simplification and just kept
the part implementing the change of struct sunxi_rsb_driver::remove to
return void.

Is the need for an immutable branch in your eyes gone now? (If yes, I
don't understand what is the relevant difference compared to the
previous patch; and if not I don't understand why you wrote "For the
sake of simplicity [...] it would be better to split this out into 2
separate patches." if even only one of the two patches you requested
still needs coordination.)

> > and the following questions remain:
> 
> >  - Do you insist on splitting out the change to axp20x_device_remove()?
> 
> [0] Unless you gave give me a compelling reason why it shouldn't, yes.
> 
> >  - Do you prefer to ack the mfd part to let the patch (or patches if
> >    they get split) go via the sunxi people or do you want to take the
> >    it (them) via mfd?
> 
> I'd prefer the MFD (and header only affecting MFD) to go in via MFD.

ok.

> The Bus patch can do in via it's own tree.

I'm not sure what you mean saying "the Bus patch". This v2 that is
still touching drivers/mfd? Probably not, because above you wrote that
the prototype change "will need to be taken in with the MFD patch". /me
is confused.

> > Looking at next there are four patches touching drivers/bus/sunxi-rsb.c
> > and none touching drivers/mfd/axp20x* or include/linux/mfd/axp20x.h
> > which suggests that letting it go via sunxi might be more sensible. IMHO
> > an immutable branch is not necessary?!
> 
> It's only -rc3 and you cannot tell the future.
> 
> If you manage to satisfy [0] and they do end up going in together, I
> will insist on an immutable branch.

I look forward to your position regarding this patch.

If this patch is simple enough to not need coordination and if adding
the simplifcation (as a separate patch) brings back this need, I'd just
go with this patch only.

Best regards
Uwe

 drivers/bus/sunxi-rsb.c   | 4 +++-
 drivers/mfd/axp20x-rsb.c  | 4 ++--
 include/linux/sunxi-rsb.h | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index 1bb00a959c67..117716e23ffb 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -170,7 +170,9 @@ static int sunxi_rsb_device_remove(struct device *dev)
 {
 	const struct sunxi_rsb_driver *drv = to_sunxi_rsb_driver(dev->driver);
 
-	return drv->remove(to_sunxi_rsb_device(dev));
+	drv->remove(to_sunxi_rsb_device(dev));
+
+	return 0;
 }
 
 static struct bus_type sunxi_rsb_bus = {
diff --git a/drivers/mfd/axp20x-rsb.c b/drivers/mfd/axp20x-rsb.c
index 4cdc79f5cc48..214bc0d84d44 100644
--- a/drivers/mfd/axp20x-rsb.c
+++ b/drivers/mfd/axp20x-rsb.c
@@ -49,11 +49,11 @@ static int axp20x_rsb_probe(struct sunxi_rsb_device *rdev)
 	return axp20x_device_probe(axp20x);
 }
 
-static int axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
+static void axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
 {
 	struct axp20x_dev *axp20x = sunxi_rsb_device_get_drvdata(rdev);
 
-	return axp20x_device_remove(axp20x);
+	axp20x_device_remove(axp20x);
 }
 
 static const struct of_device_id axp20x_rsb_of_match[] = {
diff --git a/include/linux/sunxi-rsb.h b/include/linux/sunxi-rsb.h
index 7e75bb0346d0..bf0d365f471c 100644
--- a/include/linux/sunxi-rsb.h
+++ b/include/linux/sunxi-rsb.h
@@ -59,7 +59,7 @@ static inline void sunxi_rsb_device_set_drvdata(struct sunxi_rsb_device *rdev,
 struct sunxi_rsb_driver {
 	struct device_driver driver;
 	int (*probe)(struct sunxi_rsb_device *rdev);
-	int (*remove)(struct sunxi_rsb_device *rdev);
+	void (*remove)(struct sunxi_rsb_device *rdev);
 };
 
 static inline struct sunxi_rsb_driver *to_sunxi_rsb_driver(struct device_driver *d)
-- 
2.29.2


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] bus: sunxi-rsb: make remove callback return void

[Lee Jones]

On Fri, 15 Jan 2021, Uwe Kleine-König wrote:

> The driver core ignores the return value of struct device_driver::remove
> because there is only little that can be done. To simplify the quest to
> make this function return void, let struct sunxi_rsb_driver::remove
> return void, too.
> 
> axp20x_device_remove() always returns 0, so there is no information
> lost in axp20x_rsb_remove(). The only other sunxi-rsb driver doesn't
> have a remove callback and so doesn't require adaption.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> 
> Hello Lee,
> 
> On Fri, Jan 15, 2021 at 11:05:43AM +0000, Lee Jones wrote:
> > On Fri, 15 Jan 2021, Uwe Kleine-König wrote:
> > > On Fri, Jan 15, 2021 at 08:11:22AM +0000, Lee Jones wrote:
> > > > There are no dependencies between the MFD and Bus changes as far as I
> > > > can tell.
> > > 
> > > There are dependencies, because
> > > 
> > > -static int axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
> > > +static void axp20x_rsb_remove(struct sunxi_rsb_device *rdev)
> > > 
> > > in drivers/mfd/axp20x-rsb.c must be done together with
> > > 
> > > --- a/include/linux/sunxi-rsb.h
> > > +++ b/include/linux/sunxi-rsb.h
> > > @@ -59,7 +59,7 @@ static inline void sunxi_rsb_device_set_drvdata(struct sunxi_rsb_device *rdev,
> > >  struct sunxi_rsb_driver {
> > >  	struct device_driver driver;
> > >  	int (*probe)(struct sunxi_rsb_device *rdev);
> > > -	int (*remove)(struct sunxi_rsb_device *rdev);
> > > +	void (*remove)(struct sunxi_rsb_device *rdev);
> > >  };
> > >  [...]
> > 
> > Yes, this will need to be taken in with the MFD patch.
> > 
> > > > For the sake of simplicity i.e. to avoid the requirement of
> > > > immutable branch maintenance and an associated pull-request, it would
> > > > be better to split this out into 2 separate patches.
> > > 
> > > So the base for this statement is gone
> > 
> > It still stands.
> 
> I don't understand this. Now I dropped the simplification and just kept
> the part implementing the change of struct sunxi_rsb_driver::remove to
> return void.
> 
> Is the need for an immutable branch in your eyes gone now? (If yes, I
> don't understand what is the relevant difference compared to the
> previous patch; and if not I don't understand why you wrote "For the
> sake of simplicity [...] it would be better to split this out into 2
> separate patches." if even only one of the two patches you requested
> still needs coordination.)
> 
> > > and the following questions remain:
> > 
> > >  - Do you insist on splitting out the change to axp20x_device_remove()?
> > 
> > [0] Unless you gave give me a compelling reason why it shouldn't, yes.
> > 
> > >  - Do you prefer to ack the mfd part to let the patch (or patches if
> > >    they get split) go via the sunxi people or do you want to take the
> > >    it (them) via mfd?
> > 
> > I'd prefer the MFD (and header only affecting MFD) to go in via MFD.
> 
> ok.
> 
> > The Bus patch can do in via it's own tree.
> 
> I'm not sure what you mean saying "the Bus patch". This v2 that is
> still touching drivers/mfd? Probably not, because above you wrote that
> the prototype change "will need to be taken in with the MFD patch". /me
> is confused.
> 
> > > Looking at next there are four patches touching drivers/bus/sunxi-rsb.c
> > > and none touching drivers/mfd/axp20x* or include/linux/mfd/axp20x.h
> > > which suggests that letting it go via sunxi might be more sensible. IMHO
> > > an immutable branch is not necessary?!
> > 
> > It's only -rc3 and you cannot tell the future.
> > 
> > If you manage to satisfy [0] and they do end up going in together, I
> > will insist on an immutable branch.
> 
> I look forward to your position regarding this patch.
> 
> If this patch is simple enough to not need coordination and if adding
> the simplifcation (as a separate patch) brings back this need, I'd just
> go with this patch only.

Never mind.  I don't have the energy to argue.

I'll apply the patch and send out a pull-request.

It'll probably end up being less hassle!

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[GIT PULL] Immutable branch between MFD and Bus (SunXi) due for the v5.12 merge window

[Lee Jones]

Enjoy!

The following changes since commit 5c8fe583cce542aa0b84adc939ce85293de36e5e:

  Linux 5.11-rc1 (2020-12-27 15:30:22 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-bus-v5.12

for you to fetch changes up to 3c15e00e7b58bc2b37e53d2612f0a0163281be77:

  mfd/bus: sunxi-rsb: Make .remove() callback return void (2021-01-15 13:23:36 +0000)

----------------------------------------------------------------
Immutable branch between MFD and Bus (SunXi) due for the v5.12 merge window

----------------------------------------------------------------
Uwe Kleine-König (1):
      mfd/bus: sunxi-rsb: Make .remove() callback return void

 drivers/bus/sunxi-rsb.c    | 4 +++-
 drivers/mfd/axp20x-i2c.c   | 4 +++-
 drivers/mfd/axp20x-rsb.c   | 4 ++--
 drivers/mfd/axp20x.c       | 4 +---
 include/linux/mfd/axp20x.h | 2 +-
 include/linux/sunxi-rsb.h  | 2 +-
 6 files changed, 11 insertions(+), 9 deletions(-)

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [GIT PULL] Immutable branch between MFD and Bus (SunXi) due for the v5.12 merge window

[Chen-Yu Tsai]

Hi,

On Fri, Jan 15, 2021 at 11:09 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> Enjoy!
>
> The following changes since commit 5c8fe583cce542aa0b84adc939ce85293de36e5e:
>
>   Linux 5.11-rc1 (2020-12-27 15:30:22 -0800)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-bus-v5.12
>
> for you to fetch changes up to 3c15e00e7b58bc2b37e53d2612f0a0163281be77:
>
>   mfd/bus: sunxi-rsb: Make .remove() callback return void (2021-01-15 13:23:36 +0000)
>
> ----------------------------------------------------------------
> Immutable branch between MFD and Bus (SunXi) due for the v5.12 merge window

Thanks. Merged cleanly with other patches for sunxi-rsb we have in the queue.

ChenYu

> ----------------------------------------------------------------
> Uwe Kleine-König (1):
>       mfd/bus: sunxi-rsb: Make .remove() callback return void
>
>  drivers/bus/sunxi-rsb.c    | 4 +++-
>  drivers/mfd/axp20x-i2c.c   | 4 +++-
>  drivers/mfd/axp20x-rsb.c   | 4 ++--
>  drivers/mfd/axp20x.c       | 4 +---
>  include/linux/mfd/axp20x.h | 2 +-
>  include/linux/sunxi-rsb.h  | 2 +-
>  6 files changed, 11 insertions(+), 9 deletions(-)
>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel