Messages in this thread | | | Date | Fri, 3 Apr 2020 17:53:10 +0200 | From | Michał Mirosław <> | Subject | Re: [PATCH 3/7] power: supply: core: fix HWMON temperature labels |
| |
On Thu, Apr 02, 2020 at 11:29:32AM -0700, Guenter Roeck wrote: > On 4/2/20 8:00 AM, Michał Mirosław wrote: > > On Thu, Apr 02, 2020 at 07:52:19AM -0700, Guenter Roeck wrote: > >> On 4/2/20 7:46 AM, Michał Mirosław wrote: > >>> tempX_label files are swapped compared to what > >>> power_supply_hwmon_temp_to_property() uses. Make them match. > >>> While at it, make room for labeling other channels. > >>> > >>> Fixes: e67d4dfc9ff1 ("power: supply: Add HWMON compatibility layer") > >>> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> > >>> --- > >>> drivers/power/supply/power_supply_hwmon.c | 14 +++++++++++++- > >>> 1 file changed, 13 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c > >>> index 75cf861ba492..83318a21fb52 100644 > >>> --- a/drivers/power/supply/power_supply_hwmon.c > >>> +++ b/drivers/power/supply/power_supply_hwmon.c > >>> @@ -43,6 +43,11 @@ static int power_supply_hwmon_curr_to_property(u32 attr) > >>> } > >>> } > >>> > >>> +static const char *const ps_temp_label[] = { > >>> + "temp", > >>> + "ambient temp", > >>> +}; > >>> + > >>> static int power_supply_hwmon_temp_to_property(u32 attr, int channel) > >>> { > >>> if (channel) { > >>> @@ -144,7 +149,14 @@ static int power_supply_hwmon_read_string(struct device *dev, > >>> u32 attr, int channel, > >>> const char **str) > >>> { > >>> - *str = channel ? "temp" : "temp ambient"; > >>> + switch (type) { > >>> + case hwmon_temp: > >>> + *str = ps_temp_label[channel]; > >>> + break; > >>> + default: > >>> + break; > >> > >> That returns "no error" without setting *str, which is really asking for trouble. > > > > This is carried over from earlier version. No bug though, but I'll add a > > patch while I'm at it. > > > > This is incorrect. The previous version does not check the type and always sets *str. > > - *str = channel ? "temp" : "temp ambient";
The function is called only for channels that have HWMON_T_LABEL set. This is extended in later patches, though, so the type is going to be required then. I'll split the addition so its more clear.
Best Regards, Michał Mirosław
| |