[PATCH v10 10/12] hwmon: Add PECI cputemp driver

[Jae Hyun Yoo]

Hi Miguel,

On 1/9/2019 4:57 AM, Miguel Ojeda wrote:
> Hi,
> 
> A few minor nitpicks that you may consider.
> 
> On Mon, Jan 7, 2019 at 10:41 PM Jae Hyun Yoo
> <jae.hyun.yoo at linux.intel.com> wrote:
>>
>> +       if (priv->temp_config[channel] & BIT(attr))
>> +               if (channel < DEFAULT_CHANNEL_NUMS ||
>> +                   (channel >= DEFAULT_CHANNEL_NUMS &&
>> +                    (priv->core_mask & BIT(channel - DEFAULT_CHANNEL_NUMS))))
>> +                       return 0444;
> 
> Maybe if ((...) && (...)) instead of double if?
> 

Okay, that would be neater. Will change it.

>> +       for (i = 0; i < priv->gen_info->core_max; i++)
>> +               if (priv->core_mask & BIT(i))
>> +                       while (i + DEFAULT_CHANNEL_NUMS >= priv->config_idx)
>> +                               priv->temp_config[priv->config_idx++] =
>> +                                       config_table[channel_core];
> 
> priv->config_idx <= i + DEFAULT_CHANNEL_NUMS seems more readable (i.e.
> the while-loop variable first).
> 

Yeah, that would be better for readability. Will change it too.

>> +static inline bool peci_temp_need_update(struct temp_data *temp)
>> +{
>> +       if (temp->valid &&
>> +           time_before(jiffies, temp->last_updated + UPDATE_INTERVAL))
>> +               return false;
>> +
>> +       return true;
>> +}
> 
> Simply return the condition result.
> 

I'm assuming you meant:

return !(temp->valid && time_before(jiffies, temp->last_updated +
                                              UPDATE_INTERVAL));

Will change it as well.

Thanks for your review!

Regards,
Jae