Re: [PATCH 1/6] thermal/intel: Introduce Intel TCC library

[Zhang Rui <rui.zhang@intel.com>]

> > > 
> > > > +
> > > > +       return *tjmax ? 0 : -EINVAL;
> > > 
> > > So the return value of this function could be tjmax if positive
> > > or a
> > > negative error code otherwise.  No return pointers needed.
> > 
> > I tried both. And I think I chose this solution just because it
> > makes
> > the following cleanup patches in this series looks prettier.
> > 
> > I will try your suggestion, and if there is any other reason I
> > wrote it
> > in this way, I will find it out again. :p
> > 
> > > And why do you want to return -EINVAL (rather than any other
> > > error
> > > code) if tjmax turns out to be 0?
> > 
> > Because x86_pkg_temp_thermal driver returns -EINVAL previously.
> > Any suggestions on this?
> 
> I would use -ENODATA.

Sure, will do.

> > > > +}
> > > > +EXPORT_SYMBOL_NS_GPL(intel_tcc_set_offset, INTEL_TCC);
> > > > +
> > > > +/**
> > > > + * intel_tcc_get_temp() - returns the current temperature
> > > > + * @cpu: cpu that the MSR should be run on.
> > > > + * @pkg: true: Package Thermal Sensor. false: Core Thermal
> > > > Sensor.
> > > > + * @temp: a valid pointer to where to store the resulting
> > > > temperature
> > > > + *
> > > > + * Get the current temperature returned by the CPU
> > > > core/package
> > > > level
> > > > + * thermal sensor, in degrees C.
> > > > + *
> > > > + * Return: On success returns 0, an error code otherwise
> > > > + */
> > > > +int intel_tcc_get_temp(int cpu, bool pkg, int *temp)
> > > > +{
> > > > +       u32 eax, edx;
> > > > +       u32 msr = pkg ? MSR_IA32_PACKAGE_THERM_STATUS :
> > > > MSR_IA32_THERM_STATUS;
> > > > +       int tjmax, err;
> > > > +
> > > > +       err = intel_tcc_get_tjmax(cpu, &tjmax);
> > > > +       if (err)
> > > > +               return err;
> > > 
> > > Well, what if somebody change tjmax on this cpu while this
> > > function
> > > is running?
> > 
> > tjmax is read only. Only firmware can change its value at runtime,
> > say
> > this field is updated when SST-PP level is changed.
> 
> Do we get any type of notification on tjmax changes, or do we need to
> poll for it?

I'm not aware of such a notification.
Srinivas, do you know if there is one?

thanks,
rui