On 11/10/2014 11:41 AM, Gregory CLEMENT wrote:
> Hi Ezequiel,
>
> [...]
>
>> +static int armada375_wdt_clock_init(struct platform_device *pdev,
>> + struct orion_watchdog *dev)
>> +{
>> + int ret;
>> +
>> + dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
>> + if (!IS_ERR(dev->clk)) {
>> + ret = clk_prepare_enable(dev->clk);
>> + if (ret) {
>> + clk_put(dev->clk);
>> + return ret;
>> + }
>> +
>> + atomic_io_modify(dev->reg + TIMER_CTRL,
>> + WDT_AXP_FIXED_ENABLE_BIT,
>> + WDT_AXP_FIXED_ENABLE_BIT);
>> + dev->clk_rate = clk_get_rate(dev->clk);
>> +
>> + return 0;
>> + }
>> +
>> + /* Mandatory fallback for proper devicetree backward compatibility */
>> + dev->clk = clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(dev->clk))
>> + return PTR_ERR(dev->clk);
>> +
>> + ret = clk_prepare_enable(dev->clk);
>> + if (ret) {
>> + clk_put(dev->clk);
>> + return ret;
>> + }
>> +
>> + atomic_io_modify(dev->reg + TIMER_CTRL,
>> + WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT),
>> + WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT));
>> + dev->clk_rate = clk_get_rate(dev->clk) / WDT_A370_RATIO;
>> +
>> + return 0;
>> +}
>
> Shouldn't be possible to do the following:
>
> static int armada375_wdt_clock_init(struct platform_device *pdev,
> struct orion_watchdog *dev)
> {
> if (armadaxp_wdt_clock_init(pdev, dev)) {
> /* Mandatory fallback for proper devicetree backward compatibility */
> return armadaxp_wdt_clock_init(pdev, dev));
I guess you meant armada370_wdt_clock_init for the fallback?
> }
> return 0;
> }
>
> Actually reusing the armadaxp_wdt_clock_init() function was also suggested by Thomas
> on your first version but I didn't find your answer about it.
>
I replied here to the same objection on the clocksource driver:
http://www.spinics.net/lists/linux-watchdog/msg05318.html
I found that it's a fragile practice, just to save a few lines of code.
Someone can go and change the 370/xp clock init, in some way that's
incompatible with 375.
I guess I'm being paranoid, but it's a way to keep the code robust and
we are only duplicating a few lines.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
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
