On Mon, Apr 11, 2016 at 6:49 AM, Sekhar Nori <nsekhar@xxxxxx> wrote: > On Monday 11 April 2016 02:21 AM, Paul Walmsley wrote: >> 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. > > Thanks Paul! Although not sure if adding a new compatible for the IP is > the best way (since that would denote a different version of the IP). > How about checking for parent clock iff clk_get() on own device fails > and of_machine_is_compatible() matches the platforms where backward > compatibility needs to be maintained? New compatible strings are acceptable. Rob -- 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
