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

Miguel Ojeda miguel.ojeda.sandonis at gmail.com
Wed Jan 9 23:57:09 AEDT 2019


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?

> +       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).

> +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.

Cheers,
Miguel


More information about the openbmc mailing list