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

[Lee Jones <lee.jones@linaro.org>]

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