On 01/11/2017 04:31 AM, Marc Gonzalez wrote:
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.)
I do hear that you object to this code.
However, I must admit that you completely lost me here. It is a cast from
one function pointer to another, passed as argument to another function,
with a secondary cast of its argument from a typed pointer to a void pointer.
I don't think C permits for "undefined behavior, strictly speaking".
Besides, that same mechanism is already used elsewhere, which is how I
got the idea. Are you claiming that there are situations where it won't
work ?
Do you really dislike the portable solution I suggested? :-(
It is not more portable than the above. It is more expensive and adds more
code.
Thanks,
Guenter
--
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
