Messages in this thread | | | Date | Tue, 07 Oct 2014 11:20:48 +0900 | From | jonghwa3.lee@samsung ... | Subject | Re: [PATCH] power: charger-manager: Avoid recursive thermal get_temp call |
| |
Hi, On 2014년 10월 07일 01:00, Krzysztof Kozlowski wrote:
> The charger manager supports POWER_SUPPLY_PROP_TEMP property and acts > as a thermal zone if any of these conditions match: > 1. Fuel gauge used by charger manager supports POWER_SUPPLY_PROP_TEMP. > 2. 'cm-thermal-zone' property is present in DTS (then it will supersede > the fuel gauge temperature property). > > However in case 1 (fuel gauge reports temperature and 'cm-thermal-zone' > is not set) the charger manager forwards its get_temp calls to fuel > gauge thermal zone. > > This leads to reporting by lockdep a false positive deadlock for thermal > zone's mutex because of nested calls to thermal_zone_get_temp(). This is > false positive because these are different mutexes: one for charger > manager thermal zone and second for fuel gauge thermal zone. >
Yes, this happens because power_supply_subsystem automatically creates thermal_zone when POWER_SUPPLY_PROP_TEMP is available at the time of power_supply registering. And as you point out, it makes duplicate thermal_zone when some power_supply references other power_supply's. I hope it would become to be a selectable option or change the manner of charger-manager itself (if the charger-manager is only one who references other power_supply's temperature sensing ability).
Anyway, the code looks fine to me.
Acked-by : Jonghwa Lee <jonghwa3.lee@samsung.com>
Thanks, Jonghwa.
> Get rid of false lockdep alert and recursive call by accessing fuel gauge > temperature through power supply property instead of thermal zone API. >
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- > drivers/power/charger-manager.c | 21 +++++++++++---------- > include/linux/power/charger-manager.h | 2 -- > 2 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c > index c1ed3c99c059..871fb91429c8 100644 > --- a/drivers/power/charger-manager.c > +++ b/drivers/power/charger-manager.c > @@ -559,16 +559,18 @@ static int cm_get_battery_temperature(struct charger_manager *cm, > if (!cm->desc->measure_battery_temp) > return -ENODEV; > > -#ifdef CONFIG_THERMAL > - ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp); > - if (!ret) > - /* Calibrate temperature unit */ > - *temp /= 100; > -#else > - ret = cm->fuel_gauge->get_property(cm->fuel_gauge, > + if (IS_ENABLED(CONFIG_THERMAL) && cm->tzd_batt) { > + ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp); > + if (!ret) > + /* Calibrate temperature unit */ > + *temp /= 100; > + } else { > + /* No external thermometer or no CONFIG_THERMAL */ > + ret = cm->fuel_gauge->get_property(cm->fuel_gauge, > POWER_SUPPLY_PROP_TEMP, > (union power_supply_propval *)temp); > -#endif > + } > + > return ret; > } > > @@ -1501,9 +1503,8 @@ static int cm_init_thermal_data(struct charger_manager *cm) > cm->charger_psy.num_properties++; > cm->desc->measure_battery_temp = true; > } > -#ifdef CONFIG_THERMAL > - cm->tzd_batt = cm->fuel_gauge->tzd; > > +#ifdef CONFIG_THERMAL > if (ret && desc->thermal_zone) { > cm->tzd_batt = > thermal_zone_get_zone_by_name(desc->thermal_zone); > diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h > index 07e7945a1ff2..743ed6d472c6 100644 > --- a/include/linux/power/charger-manager.h > +++ b/include/linux/power/charger-manager.h > @@ -256,9 +256,7 @@ struct charger_manager { > struct power_supply *fuel_gauge; > struct power_supply **charger_stat; > > -#ifdef CONFIG_THERMAL > struct thermal_zone_device *tzd_batt; > -#endif > bool charger_enabled; > > unsigned long fullbatt_vchk_jiffies_at;
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |