diff mbox

[v10,2/4] pm: add to_power_supply macro to the API

Message ID 20171223105316.GA4352@thinkpad (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Ognjen Galic Dec. 23, 2017, 10:53 a.m. UTC
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(-)

Comments

Andy Shevchenko Dec. 28, 2017, 8:19 a.m. UTC | #1
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.
Ognjen Galic Dec. 29, 2017, 11:49 p.m. UTC | #2
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.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko Dec. 31, 2017, 9:37 a.m. UTC | #3
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.
Rafael J. Wysocki Dec. 31, 2017, 11:17 a.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko Dec. 31, 2017, 12:40 p.m. UTC | #5
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).
Ognjen Galic Dec. 31, 2017, 12:54 p.m. UTC | #6
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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;