Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 2/2] power: charger-manager: Avoid recursive thermal get_temp call | From | Krzysztof Kozlowski <> | Date | Mon, 13 Oct 2014 10:58:01 +0200 |
| |
On pon, 2014-10-13 at 06:01 +0000, MyungJoo Ham wrote: > > > > Add a 'no_thermal' property to the power supply class. If true then > > thermal zone won't be created for this power supply in > > power_supply_register(). > > > > Power supply drivers may want to set it if they support > > POWER_SUPPLY_PROP_TEMP and they are forwarding this get property call to > > other thermal zone. > > > > If they won't set it lockdep may report false positive deadlock for > > thermal zone's mutex because of nested calls to thermal_zone_get_temp(). > > First is the call to thermal_zone_get_temp() of the driver's thermal > > zone. Thermal core gets POWER_SUPPLY_PROP_TEMP property from this > > driver. The driver then calls other thermal zone thermal_zone_get_temp() > > and returns result. > > > > Example of such driver is charger manager. > > > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > Do we really need to add another variable in the psy struct? > In the previous thread, I thought that wasn't needed, though.
I considered the idea mentioned by Jonghwa Lee:
static int psy_register_thermal(struct power_supply *psy) { ... + if (psy->tzd) + return 0;
but there would be problem in determining the ownership of the thermal zone - who should unregister that thermal zone? Charger manager's power supply or fuel gauge's power supply?
We could NULL-ify it manually from charger's remove function before calling power_supply_unregister... but it is really accessing a private field of power_supply structure.
I think this should be done in API-level.
Best regards, Krzysztof
> > > Cheers, > MyungJoo > > > > > --- > > > > Changes since v1: > > 1. New patch (new idea). > > --- > > drivers/power/power_supply_core.c | 3 +++ > > include/linux/power_supply.h | 6 ++++++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c > > index 6cb7fe5c022d..694e8cddd5c1 100644 > > --- a/drivers/power/power_supply_core.c > > +++ b/drivers/power/power_supply_core.c > > @@ -417,6 +417,9 @@ static int psy_register_thermal(struct power_supply *psy) > > { > > int i; > > > > + if (psy->no_thermal) > > + return 0; > > + > > /* Register battery zone device psy reports temperature */ > > for (i = 0; i < psy->num_properties; i++) { > > if (psy->properties[i] == POWER_SUPPLY_PROP_TEMP) { > > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h > > index 3ed049673022..096dbced02ac 100644 > > --- a/include/linux/power_supply.h > > +++ b/include/linux/power_supply.h > > @@ -200,6 +200,12 @@ struct power_supply { > > void (*external_power_changed)(struct power_supply *psy); > > void (*set_charged)(struct power_supply *psy); > > > > + /* > > + * Set if thermal zone should not be created for this power supply. > > + * For example for virtual supplies forwarding calls to actual > > + * sensors or other supplies. > > + */ > > + bool no_thermal; > > /* For APM emulation, think legacy userspace. */ > > int use_for_apm; > > > > -- > > 1.9.1 > >
| |