Re: [PATCHv2 4/6] clk: ti: clkctrl: add API to notify reset status

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

 



On 06/09/2019 23:37, Stephen Boyd wrote:
Quoting Tero Kristo (2019-09-06 12:57:06)
On 06/09/2019 19:15, Stephen Boyd wrote:
Quoting Tero Kristo (2019-08-29 23:06:41)
On 29/08/2019 23:05, Stephen Boyd wrote:
Quoting Tero Kristo (2019-08-27 23:59:27)
diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index e3e0a66a6ce2..47a0d1398c6f 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -680,3 +689,38 @@ u32 ti_clk_is_in_standby(struct clk *clk)
           return false;
    }
    EXPORT_SYMBOL_GPL(ti_clk_is_in_standby);
+
+/**
+ * ti_clk_notify_resets - Notify the clock driver associated reset status

This is completely unused in this patch series. What's going on?

This is needed by the OMAP reset driver. See:

https://lwn.net/Articles/797597/


Ok. I decided to punt this topic forward to next release at the least.
To clarify, TI is not special with regards to coordinating resets and
clk enable/disable state. Every other silicon vendor has the same
requirements and nobody is doing a good job at it.

Please devise a way that avoids making a tight coupling between the clk
driver and the reset driver in this way. Are the two in the same
register space?

No, they do not share register space. One is under a PRM node, one is
under CM node, and there are multiple instances of each following each
other:

prm-1
prm-2
prm-3

So PRM is reset?

PRM is for Power and Reset Manager.



...

cm-1
cm-2
cm-3

And CM is clk?

CM is for Clock Manager.

So basically for both answer is yes.



And the gap between PRM + CM nodes is multiple megabytes in register
space. To make things worse, there are some mutant CM nodes in the
middle of the PRM nodes on certain SoCs.

Ok, sounds fair!


   Perhaps we need to combine the two drivers then. Or can
this be implemented as a genpd that coordinates the resets and clk
controls for various devices?

Generally, ti-sysc bus driver is the one doing the trick combining reset
+ clock handling. However, this is linked at the pm-runtime on device
level so it imposes certain sequencing due to way kernel PM is
implemented. Basically we can't enable the clocks + deassert reset for
remoteproc before the driver is able to load up the firmware for it.
Maybe if I add a custom genpd or just custom PM runtime for the
remoteprocs that would handle both clk + reset...

Another potential change I can think of here is that I would add resets
property under the clkctrl nodes, and link them via DT handles. The
clock driver would get a handle to the reset controller, and query its
state via generic API instead of adding this custom one. I would still
need to add a separate custom API for telling the clocks that reset
controller is in place though... And this would still be a hard link
between reset + clocks.

Do you think fully custom PM implementation would be better here which
would just control reset + clock signals directly?


 From what you're saying it sounds like a job for genpds. Maybe genpds
aren't up to the task yet, but we want devices that have resets and clks
going to them to manage the order of operations somehow without having
to "lie" and say that the resets go to the clk controller when they
don't (or vice versa).

Yeah I am not too sure if genpd would suit this purpose as it has no support for reset control so far I believe. However I think the custom PM implementation might. I will give it a shot next week and see how it fares. Basically the main issue I am trying to tackle is not to introduce any timeouts anywhere due to the hardware level dependencies of these two guys.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux