Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/01/2017 11:52, Guenter Roeck wrote:

> On 01/11/2017 01:07 AM, Marc Gonzalez wrote:
> 
>>> @@ -134,12 +134,15 @@ static int tangox_wdt_probe(struct platform_device *pdev)
>>>  	err = clk_prepare_enable(dev->clk);
>>>  	if (err)
>>>  		return err;
>>> +	err = devm_add_action_or_reset(&pdev->dev,
>>> +				       (void(*)(void *))clk_disable_unprepare,
>>> +				       dev->clk);
>>> +	if (err)
>>> +		return err;
>>
>> Hello Guenter,
>>
>> I would rather avoid the function pointer cast.
>> How about defining an auxiliary function for the cleanup action?
>>
>> clk_disable_unprepare() is static inline, so gcc will have to
>> define an auxiliary function either way. What do you think?
> 
> Not really. It would just make it more complicated to replace the
> call with devm_clk_prepare_enable(), should it ever find its way
> into the light of day.

More complicated, because the cleanup function will have to be deleted later?
The compiler will warn if someone forgets to do that.

In my opinion, it's not a good idea to rely on the fact that casting
void(*)(struct clk *clk) to void(*)(void *) is likely to work as expected
on most platforms. (It has undefined behavior, strictly speaking.)

Do you really dislike the portable solution I suggested? :-(

Regards.

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux