Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS
From: Paul Walmsley <paul@pwsan.com>
Date: 2016-03-04 06:25:53
Also in:
linux-arm-kernel, linux-clk, linux-omap, linux-pwm, lkml
On Thu, 3 Mar 2016, Franklin S Cooper Jr. wrote:
So I looked into this more and verified that the eCAP and ePWM doesn't have their own unique clock. The PWMSS receives a clock L4PER2_L3_GICLK/2 which is passed through to its sub-devices (ePWM, eCAP and eQEP). The PWMSS is responsible for handling its clock internally while the subdevices have no role in managing this clock. So this explains why we have hwmod entries for PWMSS and why we are planning on removing it from the various subdevices.
It's not whether they have their own unique clock, but whether the submodules have OCP integration registers, speak the idle/standby protocols, have direct L3/L4 ports, etc.
Since ePWM, eCAP and eQEP are subdevices of PWMSS they shouldn't have their own concept of their "own" clock. The ePWM , eCAP and eQEP clocks are all shared and managed by their parent PWMSS. Once the PWMSS is enabled and has its clock running then ePWM, eCAP and eQEP from their main clock perspective have everything they need. So my plan is to strip all references of clocks (including hwmod entries) for ePWM, eCAP and eQEP. The devm_clk_get calls made in the ePWM and eCAP will simply point to their parent's dev (PWMSS). I did a couple of quick test using this approach and it works. I have more testing to do but if that checks out are you ok with the above approach?
I don't know if that should be done or not. I haven't stared at the code yet, but based on your description, it sounds to me that it probably shouldn't be done. In any case, it's not what I meant...
Also I'm not sure how simple-bus fits in this picture. The eCAP, eQEP and ePWM are all separate devices. The only thing that they share is a single clock from their parent. So it doesn't seem like the right approach. I'm basing this on the info in this thread https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg27979.html that talks about the usage of simple-bus. So if its outdated or I"m misinterpreting it incorrectly please let me know.
What I meant is that the ECAP*, EQEP*, EHRPWM* devices don't need to be
registered through the hwmod code, due to the fact that they don't have
the integration mentioned above. Instead, I think those three subdevices
should be listed as child nodes of epwmss* in the DT.
Looking at the DT data from Vignesh, it looks like he's already got
ehrpwm1 as a child node of the epwmss1:
+ epwmss1: epwmss@48440000 {
+ compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+ reg = <0x48440000 0x30>;
+ ti,hwmods = "epwmss1";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ status = "disabled";
+ ranges = <0x48440100 0x48440100 0x80 /* ECAP */
+ 0x48440180 0x48440180 0x80 /* EQEP */
+ 0x48440200 0x48440200 0x80>; /* EHRPWM */
+
+ ehrpwm1: ehrpwm@48440200 {
+ compatible = "ti,dra7xx-ehrpwm",
+ "ti,am33xx-ehrpwm";
+ #pwm-cells = <3>;
+ reg = <0x48440200 0x80>;
+ ti,hwmods = "ehrpwm1";
So, drop the above line, since the subdevices don't have corresponding
hwmods any more.
+ status = "disabled";
+ };
Then, here, you'd add nodes similar to ehrpwm1 for ecap1 and eqep1. I
can't remember at the moment if adding "simple-bus" to the epwmss1 string
would be sufficient to register the subdevices after the epwmss1 is
probed. If so, maybe that's all you need.
+ };
Then repeat for epwmss0, epwmss2.
- Paul