On Fri, 2016-03-04 at 19:53 +0100, Bjørn Mork wrote: > Bjørn Mork <bjorn@xxxxxxx> writes: > > > > +static void iwl_mvm_thermal_zone_unregister(struct iwl_mvm *mvm) > > > +{ > > > + if (!iwl_mvm_is_tt_in_fw(mvm)) > > > + return; > > > + > > > + if (mvm->tz_device.tzone) { > > > + IWL_DEBUG_TEMP(mvm, "Thermal zone device > > > unregister\n"); > > > + thermal_zone_device_unregister(mvm- > > > >tz_device.tzone); > > > > Won't that ERR_PTR blow up when dereferenced by > > thermal_zone_device_unregister() ? > > To answer myself: No, it won't. I was tricked by the oddly placed > tzp = tz->tzp; > line here, but we will return before that becomes a problem: > > void thermal_zone_device_unregister(struct thermal_zone_device *tz) > { > int i; > const struct thermal_zone_params *tzp; > struct thermal_cooling_device *cdev; > struct thermal_zone_device *pos = NULL; > > if (!tz) > return; > > tzp = tz->tzp; > > mutex_lock(&thermal_list_lock); > list_for_each_entry(pos, &thermal_tz_list, node) > if (pos == tz) > break; > if (pos != tz) { > /* thermal zone device not found */ > mutex_unlock(&thermal_list_lock); > return; > } > > > Still think it's unwise to leave ERR_PTR's around though. The main point is that we don't want to fail the entire driver loading if the thermal zone (or cooling device) registration fails. We have a fallback for that, which is what was used before this new implementation (namely, the thermal throttling code in the driver). IIRC Emmanuel has a patch in the queue to clean this up a bit... -- Cheers, Luca.��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f