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

From	Guenter Roeck <linux@roeck-us.net>
Newsgroups	linux.kernel
Subject	Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions
Date	2017-01-13 06:20 +0100
Message-ID	<sZ2rn-5d6-3@gated-at.bofh.it> (permalink)
References	(2 earlier) <sYgG5-Sm-17@gated-at.bofh.it> <sYn4R-52M-9@gated-at.bofh.it> <sYoNk-5Rd-13@gated-at.bofh.it> <sYqm6-6WF-27@gated-at.bofh.it> <sYsed-82P-3@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On 01/11/2017 06:39 AM, Uwe Kleine-König wrote:
> On Wed, Jan 11, 2017 at 01:31:47PM +0100, 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;
>
> This looks wrong. There is no clk_unprepare_disable when
> devm_add_action_or_reset fails.
>
>>>>
>>>> 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 would expect it to work on all (Linux) platforms. Anyhow, I wonder if
> there couldn't be found a better solution.
>
> If in the end it looks like the following that would be good I think:
>
> 	clk = devm_clk_get(...);
> 	if (IS_ERR(clk))
> 		...
>
> 	ret = devm_clk_prepare_enable(clk)
> 	if (ret)
> 		return ret;
>

It turns out that at least one static analyzer complains about different
parameter pointer types in situations like this, and at least one embedded
compiler manages to create function names with embedded parameter type
(eg it appends an 'i' to the function name for each integer parameter).

With that, I consider the typecast to be too risky after all. It may work
for all of today's Linux architectures and compilers, but who knows if I
get flooded with static analyzer warnings, and who knows if gcc version
18.0 or binutils 35.0 makes it truly incompatible (following the logic of
"we can, therefore we do"). Since I also dislike the stub function solution,
at least in this situation, I'll drop all patches touching clk_prepare_enable(),
and wait for devm_clk_prepare_enable() to be available.

Guenter

Thread

[PATCH 00/62] watchdog: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 22/62] watchdog: imx2_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 12/62] watchdog: da9055_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 04/62] watchdog: atlas7_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 15/62] watchdog: davinci_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 29/62] watchdog: max77620_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 08/62] watchdog: bcm_kona_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 30/62] watchdog: mena21_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
    Re: [PATCH 30/62] watchdog: mena21_wdt: Convert to use device managed  functions and other improvements Johannes Thumshirn <morbidrsa@gmail.com> - 2017-01-13 09:10 +0100
  [PATCH 02/62] watchdog: aspeed_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
    Re: [PATCH 02/62] watchdog: aspeed_wdt: Convert to use device managed functions Joel Stanley <joel@jms.id.au> - 2017-01-11 06:20 +0100
  [PATCH 03/62] watchdog: at91sam9_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 25/62] watchdog: kempld_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 09/62] watchdog: cadence_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:40 +0100
  [PATCH 05/62] watchdog: bcm2835_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:50 +0100
    Re: [PATCH 05/62] watchdog: bcm2835_wdt: Convert to use device managed functions and other improvements Eric Anholt <eric@anholt.net> - 2017-01-14 07:30 +0100
  [PATCH 21/62] watchdog: imgpdc_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:50 +0100
  [PATCH 19/62] watchdog: gpio_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:50 +0100
  [PATCH 10/62] watchdog: coh901327_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 00:50 +0100
    Re: [PATCH 10/62] watchdog: coh901327_wdt: Convert to use device  managed functions Linus Walleij <linus.walleij@linaro.org> - 2017-01-11 16:50 +0100
  [PATCH 38/62] watchdog: nic7018_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 49/62] watchdog: sama5d4_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 40/62] watchdog: omap_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 39/62] watchdog: of_xilinx_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 36/62] watchdog: mt7621_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 42/62] watchdog: pic32-dmt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 35/62] watchdog: mpc8xxx_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 46/62] watchdog: renesas_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 32/62] watchdog: meson_gxbb_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
    Re: [PATCH 32/62] watchdog: meson_gxbb_wdt: Convert to use device  managed functions and other improvements Neil Armstrong <narmstrong@baylibre.com> - 2017-01-11 09:50 +0100
    Re: [PATCH 32/62] watchdog: meson_gxbb_wdt: Convert to use device managed functions and other improvements Kevin Hilman <khilman@baylibre.com> - 2017-01-11 19:50 +0100
  [PATCH 41/62] watchdog: orion_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 34/62] watchdog: moxart_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 47/62] watchdog: retu_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 37/62] watchdog: mtk_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 50/62] watchdog: sbsa_gwdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 43/62] watchdog: pic32-wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 44/62] watchdog: pnx4008_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
    Re: [PATCH 44/62] watchdog: pnx4008_wdt: Convert to use device  managed functions Vladimir Zapolskiy <vz@mleia.com> - 2017-01-12 01:20 +0100
  [PATCH 45/62] watchdog: qcom-wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
  [PATCH 31/62] watchdog: menf21bmc_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
    [PATCH 48/62] watchdog: rt2880_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
    [PATCH 51/62] watchdog: shwdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
    [PATCH 33/62] watchdog: meson_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 01:50 +0100
      Re: [PATCH 33/62] watchdog: meson_wdt: Convert to use device managed functions and other improvements Kevin Hilman <khilman@baylibre.com> - 2017-01-11 19:50 +0100
  [PATCH 53/62] watchdog: st_lpc_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:10 +0100
  [PATCH 52/62] watchdog: sirfsoc_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:10 +0100
    [PATCH 55/62] watchdog: sunxi_wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:10 +0100
      Re: [PATCH 55/62] watchdog: sunxi_wdt: Convert to use device managed  functions and other improvements Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-01-11 13:20 +0100
    [PATCH 57/62] watchdog: tegra_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:20 +0100
    [PATCH 61/62] watchdog: ux500_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:20 +0100
    [PATCH 60/62] watchdog: txx9wdt: Convert to use device managed functions and other improvements Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:20 +0100
    [PATCH 59/62] watchdog: twl4030_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:20 +0100
    [PATCH 62/62] watchdog: wm831x_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:20 +0100
      Re: [PATCH 62/62] watchdog: wm831x_wdt: Convert to use device  managed functions Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2017-01-12 11:30 +0100
    [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:20 +0100
      Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2017-01-11 10:10 +0100
        Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 12:00 +0100
          Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2017-01-11 13:40 +0100
            Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 15:30 +0100
              Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions Måns Rullgård <mans@mansr.com> - 2017-01-11 15:50 +0100
              Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2017-01-11 16:30 +0100
                Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device  managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 19:00 +0100
                Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2017-01-12 10:50 +0100
                Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device  managed functions Uwe Kleine-König          <u.kleine-koenig@pengutronix.de> - 2017-01-12 11:00 +0100
                Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions Måns Rullgård <mans@mansr.com> - 2017-01-12 12:30 +0100
                Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2017-01-12 13:20 +0100
            Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device  managed functions Uwe Kleine-König          <u.kleine-koenig@pengutronix.de> - 2017-01-11 15:40 +0100
              Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2017-01-11 16:00 +0100
              Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device  managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 18:30 +0100
              Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Guenter Roeck <linux@roeck-us.net> - 2017-01-13 06:20 +0100
          Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-12 01:20 +0100
            Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed  functions Guenter Roeck <linux@roeck-us.net> - 2017-01-12 02:40 +0100
    [PATCH 54/62] watchdog: stmp3xxx_rtc_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:20 +0100
    [PATCH 58/62] watchdog: ts4800_wdt: Convert to use device managed functions Guenter Roeck <linux@roeck-us.net> - 2017-01-11 03:20 +0100

csiph-web