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:

> 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