Hi guys On Tue, 5 Apr 2016, Franklin S Cooper Jr. wrote: > On 04/05/2016 01:08 AM, Sekhar Nori wrote: > > On Tuesday 08 March 2016 06:53 AM, Franklin S Cooper Jr wrote: > > > The eCAP and ePWM doesn't have their own separate clocks. They simply > > > utilize the clock provided directly by the PWMSS. Therefore, they simply > > > need to grab a reference to their parent's clock. > > > > > > Signed-off-by: Franklin S Cooper Jr <fcooper@xxxxxx> > > > > So this assumes that eCAP and eHRPWM are always under the PWMSS > > umbrella. But on TI AM18x, thats not true. These IPs exist independently > > and receive functional clock from PLL sysclk outputs. > > > > > --- > > > drivers/pwm/pwm-tiecap.c | 2 +- > > > drivers/pwm/pwm-tiehrpwm.c | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c > > > index 616af76..9418159 100644 > > > --- a/drivers/pwm/pwm-tiecap.c > > > +++ b/drivers/pwm/pwm-tiecap.c > > > @@ -212,7 +212,7 @@ static int ecap_pwm_probe(struct platform_device *pdev) > > > if (!pc) > > > return -ENOMEM; > > > > > > - clk = devm_clk_get(&pdev->dev, "fck"); > > > + clk = devm_clk_get(pdev->dev.parent, "fck"); > > > > Even keeping the AM18x usecase aside, this seems to be pushing too much > > platform information into the driver. The "fck" is a valid connection id > > for the eCAP IP. Whether its valid for the parent device too is not > > something this driver should need to know. > > > > So it looks like what you need is for the clock hierarchy for the > > platform to have clocks for eHRPWM and eCAP derived out of PWMSS clock? > > So I believe this is a question on if we want to hide the minor > delta between AM18 vs AM335x, AM437x and AM57x/DRA7 in the driver > or within the DT. > > Note that handling this by defining new clocks in DT will then > result in older DTBs not working. I don't think its worth breaking > backwards compatibility for AM335x and AM437x DTBs for fixing support > for AM18 based SOCs. Especially since those SOCs haven't worked with > this driver for several years. By handling things within the driver rather > than DT we can atleast insure that we can get everything working while > avoiding breaking backwards compatibility. I agree with Sekhar that we shouldn't embed this parent clock quirk into the driver. Can you just define a new compatibility string such that the driver can be written with no embedded integration quirks? Then add a workaround in the driver that will use pdev->dev.parent for the old (deprecated) compatibility string and log a warning to the kernel console that the DT needs to be updated. - Paul -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
