Re: [PATCH] clk: samsung: exynos7: Enable clocks for CMU_CCORE and CMU_FSYS0 blocks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On 04/13/2016 05:01 PM, Tomasz Figa wrote:
2016-04-13 13:36 GMT+03:00 Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>:
On 04/13/2016 08:26 AM, Tomasz Figa wrote:
@@ -205,7 +206,11 @@ static struct samsung_cmu_info topc_cmu_info __initdata = {

  static void __init exynos7_clk_topc_init(struct device_node *np)
  {
+       struct clk *clk;
+
         samsung_cmu_register_one(np, &topc_cmu_info);
+       clk = __clk_lookup("aclk_ccore_133");
+       clk_prepare_enable(clk);

Shouldn't this be rather done before calling
samsung_cmu_register_one()? I don't remember exactly, but wouldn't
clock registration trigger reading back current (mux, div) values from
registers?

Also, do we have any guarantees on order of initialization of
particular CMUs? I believe this will happen in order of DT nodes and
so would be not any kind of guarantee at all.

If these clocks need to be kept enabled perhaps it's better to just set
CLK_IS_CRITICAL flag for them? Patches adding this flag are already in
clk-next branch in the clk git tree. This way the clocks would get
enabled within the clk_register() call.


Oh, I didn't know about this. Sounds like a good idea. :)


I was not aware of this flag, Thanks for pointing this out.
This flag indeed solves my problem.
These clocks should not be gated since the CMU block using this clock for SFR access.
Please ignore the $SUBJECT patch, I will send a new patch which uses
CLK_IS_CRITICAL flag instead.

The CMU registration order is enforced by listing input clocks to each
CMU in DT.

Ah, right, I recall now, I even reviewed relevant patch. Thanks for
refreshing my memory.

However, I wouldn't be concerned much about it in context
of this patch. We are enabling here clocks which belong to same CMU.
samsung_cmu_register_one() needs to be called first for subsequent
__clk_lookup() calls to work.

I thought a clock from another CMU has to be enabled for the CPU to be
able to access the CMU being registered.


Perhaps related bits need to be set manually in CMU registers before
registering a clock provider for the CMU, to fulfil the requirements.

Anyway, summary of the $subject patch seems not precise enough:

"This patch enables clocks for CMU_CCORE and CMU_FSYS0 blocks. This is
required before accessing registers of these blocks."

We need to enable selected clocks (i.e. access the CMU's registers)
before accessing this CMU's registers?

Yeah, maybe the explanation could be a bit more precise.

Best regards,
Tomasz


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux