A37xx PWM pinctrl definitions still (maybe?) not correct

A37xx PWM pinctrl definitions still (maybe?) not correct

[Marek Behún]

Hi Linus,

the commit baf8d6899b1e [1] I sent in July, correcting PWM pinctrl
definitions for A37xx, is not correct (maybe?).

Before my commit, the code defined groups
  pwmN      with funcs   pwm gpio
  lenN_od   with funcs   led gpio

The first group is meant to switch the controller of the pin between
GPIO subsystem and PWM subsystem.
The second group sets what should happen if the output value set is 1:
whether it should be high (func gpio), or tri-stated (func led).

This means that previously it was possible to switch these
settings separately (although I don't know what would happen if both
groups were defined in DT with "gpio" function. Would only the first
setting get applied? Since both groups define the same pin number...)

Anyway, since both groups are controlling the same pin, in my above
mentioned commit I declared it semantically incorrect, and that it
should be only in one group. Thus I removed group ledN_od, and changed
group pwmN to have these functions:
  pwm   - pin controlled by pwm, 0 low, 1 high
  led   - pin STILL controlled by pwm, 0 low, 1 tristate
  gpio  - pin controlled by gpio, 0 low, 1 high

Basically what I am trying to say here is that when user selects the
"led" function, the pin is controlled by PWM. There is no way for the
user to set "tristate on 1" setting but keep the pin controllable via
GPIO subsystem.

What do you think of this? I still think there should be only one group
controlling one pin. But maybe someone will need to control the LED
with "tristate on 1" via GPIO. Should this be a separate, fourth
function, something like "gpio_led"?

Marek

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=baf8d6899b1e8906dc076ef26cc633e96a8bb0c3

Re: A37xx PWM pinctrl definitions still (maybe?) not correct

[Linus Walleij]

On Thu, Sep 16, 2021 at 1:55 PM Marek Behún <kabel@kernel.org> wrote:

> Basically what I am trying to say here is that when user selects the
> "led" function, the pin is controlled by PWM. There is no way for the
> user to set "tristate on 1" setting but keep the pin controllable via
> GPIO subsystem.
>
> What do you think of this? I still think there should be only one group
> controlling one pin. But maybe someone will need to control the LED
> with "tristate on 1" via GPIO. Should this be a separate, fourth
> function, something like "gpio_led"?

There are two ways to do this, either as Qualcomm does and have
the GPIO mode be a unique function, or implement the
three pincontrol override functions on struct pinmux_ops
that makes it possible for the GPIO driver to just override
pinmux entirely at runtime and mux the pins into GPIO mode
on demand, and it seems that am37xx sets those?

Yours,
Linus Walleij