From: ezequiel.garcia@free-electrons•com (Ezequiel Garcia)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v3 2/3] watchdog: orion: Use the reference clock on Armada 375 SoC
Date: Mon, 10 Nov 2014 11:47:11 -0300 [thread overview]
Message-ID: <5460CFEF.1090400@free-electrons.com> (raw)
In-Reply-To: <5460CEB0.7080009@free-electrons.com>
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
next prev parent reply other threads:[~2014-11-10 14:47 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-04 13:21 [PATCH v3 0/3] Make Armada 375 use the reference clock when possible Ezequiel Garcia
2014-11-04 13:21 ` [PATCH v3 1/3] clocksource: armada-370-xp: Add missing clock enable Ezequiel Garcia
2014-11-04 13:21 ` [PATCH v3 2/3] watchdog: orion: Use the reference clock on Armada 375 SoC Ezequiel Garcia
2014-11-10 14:41 ` Gregory CLEMENT
2014-11-10 14:47 ` Ezequiel Garcia [this message]
2014-11-10 15:02 ` Gregory CLEMENT
2014-11-04 13:21 ` [PATCH v3 3/3] clocksource: armada-370-xp: Use the reference clock on A375 SoC Ezequiel Garcia
2014-11-19 9:25 ` [PATCH v3 0/3] Make Armada 375 use the reference clock when possible Daniel Lezcano
[not found] ` <20141119104535.GB8973@spo001.leaseweb.nl>
2014-11-24 9:04 ` Daniel Lezcano
[not found] ` <20141125133409.GC19785@spo001.leaseweb.nl>
2014-11-25 15:47 ` Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5460CFEF.1090400@free-electrons.com \
--to=ezequiel.garcia@free-electrons$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox