lkml.org 
[lkml]   [2015]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH RESEND] pwm: ftm: fix clock enable/disable when using PM
    Thanks Thierry for looking into that!

    Added Li as recipient to start discussion.

    Some comments below:

    On 2015-11-23 01:45, Thierry Reding wrote:
    > On Wed, Nov 18, 2015 at 06:04:12PM -0800, Stefan Agner wrote:
    >> Thierry,
    >>
    >> I realized that this patch did not make it into 4.4-rc1, while others,
    >> IMHO less important patches which have been posted later (e.g. sunxi
    >> whitespace fixes) have made it! :-(
    >
    > The reason why I merged them is because they are low-risk, whereas this
    > patch of yours changes existing behaviour, and hasn't received any
    > feedback from anyone. So the choice that I need to make is to either
    > trust the original author to have tested the driver and it was working,
    > or you to have verified that it is still working after the patch on all
    > setups that it used to work on. The first option obviously carries the
    > least risk, and that's the reason why the patch hasn't been merged.
    >
    >> Anything wrong with that? Or am I on your spam list? Note that this is
    >> already a RESEND :-)
    >
    > If you want to get this merged, you should try to get some feedback from
    > at least the original author. Xiubo Li and I extensively discussed this
    > back at the time when he submitted the driver and we came up with the
    > current code as the best approach to making sure that clocks are on and
    > off when they should be. So if it's not working for you, I'm fine taking
    > the patch if Xiubo or somebody else has had the chance to look at it and
    > review or test it. So a Reviewed-by or Tested-by tag will go a long way
    > to convince me that it's safe to apply.

    In mainline, the driver is only used in Freescale Vybrid device trees. I
    think most issues have been introduced with the patches adding suspend
    support:
    http://thread.gmane.org/gmane.linux.kernel/1806940

    Not sure against what suspend/resume implementation Li created the
    patches, so far there is no suspend/resume implementation for Vybrid
    upstream. While working on Vybrid's low power mode LPSTOP3, I noticed
    the issues....

    > Also you enumerate all the various bits that are broken, and it would
    > seem to me that they could each be fixed individually rather than go and
    > implement something completely different which might have undesirable
    > side-effects. Such an approach would also make it more likely for me to
    > merge the patch because it would hopefully be more obvious what is being
    > fixed and that it's a correct fix.

    There are different issues addressed by one solution: Using the clock
    frameworks enable/disable counts... I looked through the code again, it
    is really quite hard to split it up. I could create one patch which only
    switches the PWM enable/disable part to clock framework counts, and
    leave the suspend/resume part broken. Than, in a second patch fix the
    suspend/resume part. But that sounds like the wrong approach to me
    too...

    I took inspiration from other PWM drivers, I think the suspend/resume
    idea was from TI EHRPWM PWM driver. So this shouldn't be far off what
    others are doing.

    >
    > It's not that I mind the rework, but I'd at least like for someone to
    > verify that it's all still working on existing setups. Now, I understand
    > that people can go missing, so if nobody were to give you a Reviewed-by
    > or Tested-by for a couple of weeks I'd even consider applying without,
    > but as it is, you didn't even Cc Xiubo on the patch, so he's likely
    > missed it entirely.

    I have had Li in the initial email, don't know/remember why I removed
    him from the list in RESEND... Will keep him in the loop.

    Just realized that there is now a helper function for
    test_bit(PWMF_ENABLED,... Will send a v2 anyway then.

    --
    Stefan


    \
     
     \ /
      Last update: 2015-11-23 23:01    [W:4.514 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site