Re: [PATCH] cpuidle/powernv : Add Description for cpuidle state

From: Michael Ellerman
Date: Tue May 29 2018 - 08:31:14 EST


Stewart Smith <stewart@xxxxxxxxxxxxx> writes:

> Abhishek Goel <huntbag@xxxxxxxxxxxxxxxxxx> writes:
>> @@ -215,7 +216,7 @@ static inline void add_powernv_state(int index, const char *name,
>> u64 psscr_val, u64 psscr_mask)
>> {
>> strlcpy(powernv_states[index].name, name, CPUIDLE_NAME_LEN);
>> - strlcpy(powernv_states[index].desc, name, CPUIDLE_NAME_LEN);
>> + strlcpy(powernv_states[index].desc, desc, CPUIDLE_DESC_LEN);
>
> We should still fall back to using name in the event of desc being null,
> as not all firmware will expose the descriptions.
>
>> @@ -311,6 +313,11 @@ static int powernv_add_idle_states(void)
>> pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
>> goto out;
>> }
>> + if (of_property_read_string_array(power_mgt,
>> + "ibm,cpu-idle-state-descs", descs, dt_idle_states) < 0) {
>> + pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-descs in DT\n");
>> + goto out;
>> + }
>
> I don't think pr_warn is appropriate here, as for all current released
> firmware we don't have that property. I think perhaps just silently
> continuing on is okay, as we have to keep compatibility with that firmware.

+1

Anyone who's wondering why they're not seeing the names can just look at
the device tree on the machine to see if the property is there or not,
they don't need a printk in dmesg to tell them.

cheers