Mark.Haywood wrote:

> Mark Haywood wrote:
>> Li, Aubrey wrote:
>> 
>>> The following webrev added turbo mode observability support of the
>>> kernel part. http://cr.opensolaris.org/~aubrey/turbo-kernel-part/
>>> It is against [rev #8756] of pad-gate.
>>> 
>>> I almost re-write Vinay's patch, instead of implementing the
>>> ioctl() of cpudrv, I think kstat for observability is better.
>>> 
>>> Before I start the powertop part of turbo mode
> observability support, I really appreciate
>>> your any comments and suggestions.
>>> 
>>> 
>> 
>> Very nice Aubrey. Much cleaner than the ioctl() implementation. I
>> don't know if you are looking for code review feedback on this or
>> not, but in the speedstep_init() routine at lines 321 - 327, I think
>> you want these in an else block for the conditional at 317? Also, do
>> you want to create the kstat if turbo_info->turbo_supported is 0?
>> 
> 
> Actually, I have one more comment. ;-) In an attempt to keep the
> cpupm_mach_state structure vendor agnostic, the cpupm_mach_turbo_info
> structure might be better defined in speedstep.h. And a void * pointer
> added to cpupm_mach_state instead of the cpupm_mach_turbo_info_t.
> 
> Mark

Thanks, I'll improve this after Yufei Zhu's feedback.

-Aubrey

Reply via email to