Re: [PATCH V2 3/4] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Eduardo Valentin]

On Sat, Oct 21, 2017 at 10:14:33AM +0200, Daniel Lezcano wrote:
> On 20/10/2017 22:58, Eduardo Valentin wrote:
> > On Fri, Oct 20, 2017 at 05:11:06PM +0200, Daniel Lezcano wrote:
> >> From: Kevin Wangtao <kevin.wangtao@xxxxxxxxxx>
> >>
> >> For platform compatibility, add the tsensor ops to a thermal data
> >> structure. Each platform has its own probe function to register proper
> >> tsensor ops function to the pointer, platform related resource request
> >> are also implemented in the platform probe function.
> >>
> >> Signed-off-by: Kevin Wangtao <kevin.wangtao@xxxxxxxxxx>
> >> Tested-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> # hikey6220
> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> > 
> > This patch adds this issue to hisi driver (sparse)
> > 
> > drivers/thermal/hisi_thermal.c:398:24: warning: incorrect type in assignment (different modifiers)
> > drivers/thermal/hisi_thermal.c:398:24:    expected int ( *platform_probe )( ... )
> > drivers/thermal/hisi_thermal.c:398:24:    got void const *
> > 
> > essentially you are casting a const into a non const.
> > 
> > Please fix and resend.
> 
> I was not able to reproduce the warning. I tried the C=1, C=2 options,
> cross compiled or compiled on x86 with COMPILE_TEST, the warning does
> not appear.
> 
> Are you using make C=1 or something else to run sparse on the kernel
> sources ?


Yes, this is a make C=1.

The warning is in this code that you add in this patch:
+       platform_probe = of_device_get_match_data(dev);
+       if (!platform_probe) {
+               dev_err(dev, "failed to get probe func\n");
+               return -EINVAL;
        }

platform_probe should be const, because of_device_get_match_data() returns a const:

$ grep -A 10 of_device_get_match_data drivers/of/device.c 
const void *of_device_get_match_data(const struct device *dev)
{
	const struct of_device_id *match;

	match = of_match_device(dev->driver->of_match_table, dev);
	if (!match)
		return NULL;

	return match->data;
}
EXPORT_SYMBOL(of_device_get_match_data);


which I agree, you should use a const to receive the of_device_get_match_data().

> 
> 
> 
> -- 
>  <http://www.linaro.org/> Linaro.org â Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>