public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: t-kristo@ti•com (Tero Kristo)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC 6/9] clk: ti: add support for omap4 module clocks
Date: Mon, 4 Jan 2016 09:36:05 +0200	[thread overview]
Message-ID: <568A20E5.6040005@ti.com> (raw)
In-Reply-To: <20160101054815.21738.79820@quark.deferred.io>

On 01/01/2016 07:48 AM, Michael Turquette wrote:
> Hi Tero,
>
> Quoting Tero Kristo (2015-12-18 05:58:58)
>> Previously, hwmod core has been used for controlling the hwmod level
>> clocks. This has certain drawbacks, like being unable to share the
>> clocks for multiple users, missing usecounting and generally being
>> totally incompatible with common clock framework.
>>
>> Add support for new clock type under the TI clock driver, which will
>> be used to convert all the existing hwmdo clocks to. This helps to
>> get rid of the clock related hwmod data from kernel and instead
>> parsing this from DT.
>
> I'm really happy to see this series. Looks pretty good to me.
>
>> +static int _omap4_hwmod_clk_enable(struct clk_hw *hw)
>> +{
>> +       struct clk_hw_omap *clk = to_clk_hw_omap(hw);
>> +       u32 val;
>> +       int timeout = 0;
>> +       int ret;
>> +
>> +       if (!clk->enable_bit)
>> +               return 0;
>> +
>> +       if (clk->clkdm) {
>> +               ret = ti_clk_ll_ops->clkdm_clk_enable(clk->clkdm, hw->clk);
>> +               if (ret) {
>> +                       WARN(1,
>> +                            "%s: could not enable %s's clockdomain %s: %d\n",
>> +                            __func__, clk_hw_get_name(hw),
>> +                            clk->clkdm_name, ret);
>> +                       return ret;
>> +               }
>> +       }
>> +
>> +       val = ti_clk_ll_ops->clk_readl(clk->enable_reg);
>> +
>> +       val &= ~OMAP4_MODULEMODE_MASK;
>> +       val |= clk->enable_bit;
>> +
>> +       ti_clk_ll_ops->clk_writel(val, clk->enable_reg);
>> +
>> +       /* Wait until module is enabled */
>> +       while (!_omap4_is_ready(val)) {
>> +               udelay(1);
>
> This should really be a .prepare callback if you plan to keep the delays
> in there.

If this is changed to a .prepare, then all OMAP power management is 
effectively ruined as all clocks are going to be enabled all the time. 
hwmod core doesn't support .prepare/.enable at the moment that well, and 
changing that is going to be a big burden (educated guess, haven't 
checked this yet)... The call chain that comes here is:

device driver -> pm_runtime -> hwmod_core -> hwmod_clk_enable / disable.

The delay within this function should usually be pretty short, just to 
wait that the module comes up from idle.

I recall the discussions regarding the udelays within clk_enable/disable 
calls, but what is the preferred approach then? Typically 
clk_enable/disable just becomes a NOP if it is not allowed to wait for 
hardware to complete transitioning before exiting the function.

-Tero

>
> Regards,
> Mike
>

  reply	other threads:[~2016-01-04  7:36 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-18 13:58 [RFC 0/9] ARM: OMAP4: hwmod clkctrl conversion to DT + clock driver Tero Kristo
2015-12-18 13:58 ` [RFC 1/9] ARM: OMAP2+: omap_device: create clock alias purely from DT data Tero Kristo
2015-12-18 15:16   ` Tony Lindgren
2015-12-18 13:58 ` [RFC 2/9] ARM: OMAP2+: hwmod: initialize main clocks directly from DT Tero Kristo
2015-12-18 13:58 ` [RFC 3/9] clk: ti: remove un-used definitions from public clk_hw_omap struct Tero Kristo
2015-12-18 13:58 ` [RFC 4/9] clk: ti: mux: export mux clock APIs locally Tero Kristo
2015-12-18 13:58 ` [RFC 5/9] dt-bindings: clk: ti: Document module clock type Tero Kristo
2015-12-18 13:58 ` [RFC 6/9] clk: ti: add support for omap4 module clocks Tero Kristo
2015-12-18 15:36   ` Tony Lindgren
2016-01-01  5:48   ` Michael Turquette
2016-01-04  7:36     ` Tero Kristo [this message]
2016-01-04 10:21       ` Geert Uytterhoeven
2016-01-04 13:27         ` Tero Kristo
2016-01-04 14:42           ` Russell King - ARM Linux
2016-01-04 16:37             ` Tony Lindgren
2016-01-04 19:15               ` Tero Kristo
2016-01-05  1:23                 ` Michael Turquette
2016-01-05  1:29       ` Michael Turquette
2015-12-18 13:58 ` [RFC 7/9] ARM: dts: omap4: add hwmod " Tero Kristo
2015-12-18 15:06   ` Tony Lindgren
2015-12-18 13:59 ` [RFC 8/9] ARM: OMAP4: hwmod_data: use module clocks from DT Tero Kristo
2015-12-18 15:11   ` Tony Lindgren
2015-12-18 13:59 ` [RFC 9/9] clk: ti: omap4: update clock aliases to reflect new module clocks Tero Kristo

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=568A20E5.6040005@ti.com \
    --to=t-kristo@ti$(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