| Message ID | 20171223105316.GA4352@thinkpad (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Andy Shevchenko |
| Headers | show |
On Sat, Dec 23, 2017 at 12:53 PM, Ognjen Galic <smclt30p@gmail.com> wrote: > This patch adds the to_power_supply macro to upcast > a device to a power_supply struct. > > This is needed because the same piece of code using > container_of is used in various other places, so we > abstract away such low-level operations via a macro. > --- This is wrong! You have to use *existing* --- line below. Otherwise all mail parsers will cut this out including your SoB tag. > > v9: > * Split the pm changes from the thinkpad_acpi patch > into its own patch > > v10: > * No changes in this patch in v10 > > Signed-off-by: Ognjen Galic <smclt30p@gmail.com> Missed: Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/power/supply/power_supply_core.c | 2 +- > include/linux/power_supply.h | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c > index 82f998a..feac7b0 100644 > --- a/drivers/power/supply/power_supply_core.c > +++ b/drivers/power/supply/power_supply_core.c > @@ -668,7 +668,7 @@ EXPORT_SYMBOL_GPL(power_supply_powers); > > static void power_supply_dev_release(struct device *dev) > { > - struct power_supply *psy = container_of(dev, struct power_supply, dev); > + struct power_supply *psy = to_power_supply(dev); > dev_dbg(dev, "%s\n", __func__); > kfree(psy); > } > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h > index 79e90b3..f0139b4 100644 > --- a/include/linux/power_supply.h > +++ b/include/linux/power_supply.h > @@ -371,6 +371,8 @@ devm_power_supply_register_no_ws(struct device *parent, > extern void power_supply_unregister(struct power_supply *psy); > extern int power_supply_powers(struct power_supply *psy, struct device *dev); > > +#define to_power_supply(device) container_of(device, struct power_supply, dev) > + > extern void *power_supply_get_drvdata(struct power_supply *psy); > /* For APM emulation, think legacy userspace. */ > extern struct class *power_supply_class; Should fold in the changes you sent as a separate patch.
On Čet, 2017-12-28 at 10:19 +0200, Andy Shevchenko wrote: > On Sat, Dec 23, 2017 at 12:53 PM, Ognjen Galic <smclt30p@gmail.com> > wrote: > > > > This patch adds the to_power_supply macro to upcast > > a device to a power_supply struct. > > > > This is needed because the same piece of code using > > container_of is used in various other places, so we > > abstract away such low-level operations via a macro. > > > > --- > This is wrong! You have to use *existing* --- line below. Otherwise > all mail parsers will cut this out including your SoB tag. > My bad I guess. Want another patch revision with that fixed or something? > > > > > > v9: > > * Split the pm changes from the thinkpad_acpi patch > > into its own patch > > > > v10: > > * No changes in this patch in v10 > > > > Signed-off-by: Ognjen Galic <smclt30p@gmail.com> > Missed: > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > > --- > > drivers/power/supply/power_supply_core.c | 2 +- > > include/linux/power_supply.h | 2 ++ > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/power/supply/power_supply_core.c > > b/drivers/power/supply/power_supply_core.c > > index 82f998a..feac7b0 100644 > > --- a/drivers/power/supply/power_supply_core.c > > +++ b/drivers/power/supply/power_supply_core.c > > @@ -668,7 +668,7 @@ EXPORT_SYMBOL_GPL(power_supply_powers); > > > > static void power_supply_dev_release(struct device *dev) > > { > > - struct power_supply *psy = container_of(dev, struct > > power_supply, dev); > > + struct power_supply *psy = to_power_supply(dev); > > dev_dbg(dev, "%s\n", __func__); > > kfree(psy); > > } > > diff --git a/include/linux/power_supply.h > > b/include/linux/power_supply.h > > index 79e90b3..f0139b4 100644 > > --- a/include/linux/power_supply.h > > +++ b/include/linux/power_supply.h > > @@ -371,6 +371,8 @@ devm_power_supply_register_no_ws(struct device > > *parent, > > extern void power_supply_unregister(struct power_supply *psy); > > extern int power_supply_powers(struct power_supply *psy, struct > > device *dev); > > > > +#define to_power_supply(device) container_of(device, struct > > power_supply, dev) > > + > > extern void *power_supply_get_drvdata(struct power_supply *psy); > > /* For APM emulation, think legacy userspace. */ > > extern struct class *power_supply_class; > Should fold in the changes you sent as a separate patch. >
On Sat, Dec 30, 2017 at 1:49 AM, Ognjen Galić <smclt30p@gmail.com> wrote: > On Čet, 2017-12-28 at 10:19 +0200, Andy Shevchenko wrote: >> On Sat, Dec 23, 2017 at 12:53 PM, Ognjen Galic <smclt30p@gmail.com> >> wrote: >> > >> > This patch adds the to_power_supply macro to upcast >> > a device to a power_supply struct. >> > >> > This is needed because the same piece of code using >> > container_of is used in various other places, so we >> > abstract away such low-level operations via a macro. >> > >> > --- >> This is wrong! You have to use *existing* --- line below. Otherwise >> all mail parsers will cut this out including your SoB tag. >> > > My bad I guess. Want another patch revision with that fixed or > something? > You definitely need to send a new revision with all comments addressed. So far you didn't responce to them which I recognize as total agreement that they have to be addressed.
On Sun, Dec 31, 2017 at 10:37 AM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Sat, Dec 30, 2017 at 1:49 AM, Ognjen Galić <smclt30p@gmail.com> wrote: >> On Čet, 2017-12-28 at 10:19 +0200, Andy Shevchenko wrote: >>> On Sat, Dec 23, 2017 at 12:53 PM, Ognjen Galic <smclt30p@gmail.com> >>> wrote: >>> > >>> > This patch adds the to_power_supply macro to upcast >>> > a device to a power_supply struct. >>> > >>> > This is needed because the same piece of code using >>> > container_of is used in various other places, so we >>> > abstract away such low-level operations via a macro. >>> > >>> > --- >>> This is wrong! You have to use *existing* --- line below. Otherwise >>> all mail parsers will cut this out including your SoB tag. >>> >> >> My bad I guess. Want another patch revision with that fixed or >> something? >> > > You definitely need to send a new revision with all comments > addressed. So far you didn't responce to them which I recognize as > total agreement that they have to be addressed. I asked for not sending new versions before I have a chance to look at the current one in detail, but enough time has passed after the last one, so I agree. Sending a new version at this point won't hurt. :-) Thanks, Rafael
On Sun, Dec 31, 2017 at 1:17 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Sun, Dec 31, 2017 at 10:37 AM, Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: >> On Sat, Dec 30, 2017 at 1:49 AM, Ognjen Galić <smclt30p@gmail.com> wrote: >> You definitely need to send a new revision with all comments >> addressed. So far you didn't responce to them which I recognize as >> total agreement that they have to be addressed. > > I asked for not sending new versions before I have a chance to look at > the current one in detail, but enough time has passed after the last > one, so I agree. Sending a new version at this point won't hurt. :-) Thanks, Rafael, for clarification, that's exactly what I kept in mind when proposing a new version (I'm concerned as well of frequency of patch series to be not so high).
On Sun, Dec 31, 2017 at 02:40:41PM +0200, Andy Shevchenko wrote: > On Sun, Dec 31, 2017 at 1:17 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Sun, Dec 31, 2017 at 10:37 AM, Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > >> On Sat, Dec 30, 2017 at 1:49 AM, Ognjen Galić <smclt30p@gmail.com> wrote: > > >> You definitely need to send a new revision with all comments > >> addressed. So far you didn't responce to them which I recognize as > >> total agreement that they have to be addressed. > > > > I asked for not sending new versions before I have a chance to look at > > the current one in detail, but enough time has passed after the last > > one, so I agree. Sending a new version at this point won't hurt. :-) > > Thanks, Rafael, for clarification, that's exactly what I kept in mind > when proposing a new version (I'm concerned as well of frequency of > patch series to be not so high). > Well guys hold on tight, another patch revision is incoming: #11. Oh, and by the way, happy new year! > -- > With Best Regards, > Andy Shevchenko
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c index 82f998a..feac7b0 100644 --- a/drivers/power/supply/power_supply_core.c +++ b/drivers/power/supply/power_supply_core.c @@ -668,7 +668,7 @@ EXPORT_SYMBOL_GPL(power_supply_powers); static void power_supply_dev_release(struct device *dev) { - struct power_supply *psy = container_of(dev, struct power_supply, dev); + struct power_supply *psy = to_power_supply(dev); dev_dbg(dev, "%s\n", __func__); kfree(psy); } diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index 79e90b3..f0139b4 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -371,6 +371,8 @@ devm_power_supply_register_no_ws(struct device *parent, extern void power_supply_unregister(struct power_supply *psy); extern int power_supply_powers(struct power_supply *psy, struct device *dev); +#define to_power_supply(device) container_of(device, struct power_supply, dev) + extern void *power_supply_get_drvdata(struct power_supply *psy); /* For APM emulation, think legacy userspace. */ extern struct class *power_supply_class;
This patch adds the to_power_supply macro to upcast a device to a power_supply struct. This is needed because the same piece of code using container_of is used in various other places, so we abstract away such low-level operations via a macro. --- v9: * Split the pm changes from the thinkpad_acpi patch into its own patch v10: * No changes in this patch in v10 Signed-off-by: Ognjen Galic <smclt30p@gmail.com> --- drivers/power/supply/power_supply_core.c | 2 +- include/linux/power_supply.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)