lkml.org 
[lkml]   [2016]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 1/6] pwms: pwm-ti*: Get the clock from the PWMSS (parent)
    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@ti.com>
    > >
    > > 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

    \
     
     \ /
      Last update: 2016-04-10 23:21    [W:4.119 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site