Re: [PATCH v2 2/4] watchdog: orion: Use the reference clock on Armada 375 SoC

Thomas Petazzoni
2014-10-31T12:50:16Z

Dear Ezequiel Garcia,

It's really nitpicking, but I find the usage of empty newlines rather
unfortunate in the below function.

On Sat, 25 Oct 2014 11:48:40 -0300, Ezequiel Garcia wrote:

> +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)) {
> +

This empty new line seems not needed.

> + 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;

But one empty new line before the return would be good.

> + }
> +
> + /* Mandatory fallback for proper devicetree backward compatibility */
> + dev->clk = clk_get(&pdev->dev, NULL);
> + if (IS_ERR(dev->clk))
> + return PTR_ERR(dev->clk);

An empty newline would be good here as well.

> + 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;

And here.

With this fixed:

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering