[lkml]   [2011]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 1/3] PWM: add pwm framework support
    On 04/07/11 17:55, Sascha Hauer wrote:
    > I am tired of discussing this. It seems we can't agree and unless
    > someone else jumps in here we will probably have to wait for another
    > year until something moves in the PWM area.

    If we are going to introduce a new framework for pwms then we should
    create one which meets the needs of at least all of the in kernel
    drivers. This patch series provides no solution for either the atmel or
    ep93xx drivers, so it is not a complete solution. At some point the
    api/framework _must_ be changed. If we can introduce transition layers
    then we should do that now so they we can provide a common framework
    along with some forward thinking about how the other drivers are going
    to be migrated to the new framework. This patch series doesn't even
    migrate _any_ of the existing drivers.

    It doesn't have to be an all or nothing approach. Possibly Bill's series
    is perhaps too involved by changing the api, introducing sysfs support
    and reworking the pwm users. But your series is at the opposite end of
    the spectrum. It does too little. It will take a few release cycles to
    get all of the existing drivers migrated and since we can't change the
    api until that happens the atmel and ep93xx drivers will take longer
    still. At the very least your series should migrate some of the drivers.

    The timeline argument is a bit poor. Yes, there has been discussion for
    a lengthy time about how the pwm api should be developed, but I think
    that is because it is non-trivial to come up with a framework which is
    good enough to support all of the pwm hardware (some of which is already
    in the mainline). Getting something merged now just because it can be
    done quickly is not a good idea if it all has to get reworked in the
    future so that it can support all the hardware.

    The pwm framework needs to incorporate at least the following:
    - sysfs access (ep93xx driver)
    - Multiple channels per device (atmel driver)

    The mxs driver you introduce looks like it could be implemented as a
    single device (continuous mmio space) with multiple channels rather than
    the pwm core/driver approach you have. I also can't see anything in this
    patch set which hooks up the mxs pwms to an actual board (i.e. nothing
    calls mxs_add_mxs_pwm)?

    The other nice things to have for the pwm framework are:
    - More fine grained control of pwms: pwm_period_ns, period_duty_ns, etc
    - Polarity control
    - Synchronisation support for multi-channel devices
    - Interrupt handler support
    - Sleeping vs non-sleeping configuration api

    The fine-grained control api could be added now. pwm_config could be
    left as is for the time being (the new api could be a wrapper around it
    to start with). Polarity control and interrupt handling apis could also
    be defined without affecting the drivers which don't need to implement
    them. Multiple channels and the sleeping/non-sleeping api are the more
    difficult ones, but at least having some sort of indication about how
    these plan to be solved would be useful.


     \ /
      Last update: 2011-07-04 12:45    [W:0.021 / U:6.672 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site