Re: [PATCH 19/33] thermal: db8500: use match_string() helper

[Yisheng Xie]

Hi Andy,

On 2018/5/22 6:00, Andy Shevchenko wrote:
> On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie <xieyisheng1@xxxxxxxxxx> wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used intead of open coded variant.
> 
>> +       i = match_string((const char **)trip_point->cdev_name,
> 
> Casting looks ugly. You need to constify the variable itself.
When I tried to const cdev_name like:
+++ b/include/linux/platform_data/db8500_thermal.h
@@ -27,7 +27,7 @@
 struct db8500_trip_point {
        unsigned long temp;
        enum thermal_trip_type type;
-       char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH];
+       char const cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH]; // const char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH] will also the same
 };

The compiler will also warning:
drivers/thermal/db8500_thermal.c: In function âdb8500_thermal_match_cdevâ:
drivers/thermal/db8500_thermal.c:53:2: warning: passing argument 1 of âmatch_stringâ from incompatible pointer type [enabled by default]
  i = match_string(trip_point->cdev_name, COOLING_DEV_MAX, cdev->type);
  ^
In file included from include/linux/bitmap.h:9:0,
                 from include/linux/cpumask.h:12,
                 from include/linux/rcupdate.h:44,
                 from include/linux/radix-tree.h:28,
                 from include/linux/idr.h:15,
                 from include/linux/kernfs.h:14,
                 from include/linux/sysfs.h:16,
                 from kernel/include/linux/kobject.h:20,
                 from kernel/include/linux/of.h:17,
                 from include/linux/cpu_cooling.h:27,
                 from drivers/thermal/db8500_thermal.c:20:
include/linux/string.h:184:5: note: expected âconst char * const*â but argument is of type âconst char (*)[20]â

Any idea?

Thanks
Yisheng
> 
>> +                        COOLING_DEV_MAX, cdev->type);
>>
>> -       return -ENODEV;
>> +       return (i < 0) ? -ENODEV : 0;
> 
> I would rather go with
> 
> if (ret < 0)
>  return -ENODEV;
> 
> return 0;
>