[PATCH 2/2] peci-cputemp: label CPU cores from zero instead of one

[Zev Weiss]

On Mon, Sep 28, 2020 at 03:21:23PM CDT, Jae Hyun Yoo wrote:
>On 9/28/2020 12:54 PM, Zev Weiss wrote:
>>On Mon, Sep 28, 2020 at 02:08:24PM CDT, Jae Hyun Yoo wrote:
>>>
>>>
>>>On 9/26/2020 2:27 PM, Zev Weiss wrote:
>>>>Zero-based numbering is more consistent with all other cpu/core
>>>>numbering I'm aware of (including the PECI spec).
>>>>
>>>>Signed-off-by: Zev Weiss <zev at bewilderbeest.net>
>>>>---
>>>> drivers/hwmon/peci-cputemp.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>>diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c
>>>>index b9fe91281d58..78e442f433a7 100644
>>>>--- a/drivers/hwmon/peci-cputemp.c
>>>>+++ b/drivers/hwmon/peci-cputemp.c
>>>>@@ -363,7 +363,7 @@ static int create_core_temp_label(struct 
>>>>peci_cputemp *priv, int idx)
>>>>     if (!priv->coretemp_label[idx])
>>>>         return -ENOMEM;
>>>>-    sprintf(priv->coretemp_label[idx], "Core %d", idx + 1);
>>>>+    sprintf(priv->coretemp_label[idx], "Core %d", idx);
>>>
>>>Differently from low level indexing, it's labeling for users and it
>>>should be synced with other temp or ADC sensors such as
>>>
>>>PVCCIN CPU1
>>>PVDQ ABC CPU1
>>>CPU1 P12V PVCCIN
>>>CPU1 VR Mem ABCD
>>>CPU1 VR P1V8
>>>
>>>These are using indexes starting from '1'.
>>>
>>
>>OK, if it's for consistency with other existing drivers I suppose 
>>that's reasonable, though for my own reference, could you point me 
>>to where those are implemented?  Some rough grepping around the 
>>source tree didn't appear to turn up anything relevant.
>
>Sensor names get assigned through these services
>https://github.com/openbmc/entity-manager
>https://github.com/openbmc/dbus-sensors
>
>and it depends on board configuration of each machine.
>

Oh I see -- I had thought you were referring to other existing hwmon 
drivers in the kernel.

As far as I can tell, all those instances appear to be numbering CPU 
*sockets* though -- which as Jason mentioned in a call earlier today I 
gather is done to line up with motherboard silkscreen labeling.  But in 
the code in question here we're labeling *cores* within a given socket, 
which I don't see arising anywhere in any existing entity-manager 
configs.  So I'm still unclear on why we want to use one-based indexing 
here instead of zero-based -- I'd think we'd want the PECI driver to 
match the PECI spec?


Thanks,
Zev