| Message ID | 20210115121601.4xw2kchnkcowxlga@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] bus: sunxi-rsb: make remove callback return void | expand |
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!
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)