Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Sat, Jan 10, 2015 at 12:34 AM, Andrew Lunn <andrew@xxxxxxx> wrote:

> Armada 370/XP devices can 'blink' gpio lines with a configurable on
> and off period. This can be modelled as a PWM.
>
> However, there are only two sets of PWM configuration registers for
> all the gpio lines. This driver simply allows a single gpio line per
> gpio chip of 32 lines to be used as a PWM. Attempts to use more return
> EBUSY.
>
> Due to the interleaving of registers it is not simple to separate the
> PWM driver from the gpio driver. Thus the gpio driver has been
> extended with a PWM driver.
>
> Signed-off-by: Andrew Lunn <andrew@xxxxxxx>
> ---
>  drivers/gpio/Kconfig          |   5 ++
>  drivers/gpio/Makefile         |   1 +
>  drivers/gpio/gpio-mvebu-pwm.c | 202 ++++++++++++++++++++++++++++++++++++++++++
(...)
> +++ b/drivers/gpio/gpio-mvebu-pwm.c
(...)
> +static const struct pwm_ops mvebu_pwm_ops = {
> +       .request = mvebu_pwm_request,
> +       .free = mvebu_pwm_free,
> +       .config = mvebu_pwm_config,
> +       .enable = mvebu_pwm_enable,
> +       .disable = mvebu_pwm_disable,
> +       .owner = THIS_MODULE,
> +};

So the first basic notes:

- PWM maintainer Thierry Reding has to review and ACK this, and the patch needs
  to be posted to the linux-pwm mailing list too (check To: line)

- Should that driver portion really be in drivers/gpio or rather in drivers/pwm

- Why is this not modeled as an MFD spawning a GPIO and a PWM cell,
  as is custom? (Bringing MFD maintainers into the picture.)

So I am aware that this takes the problem from "quick fix extension to the GPIO
driver" to "really nasty hairy re-engineering of the whole shebang" but there is
a lot of precedents in the kernel for splitting up this type of hardware in
separate drivers under an MFD hub.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html