Re: [PATCH] backlight: pwm_bl: Set pin to sleep state when powered down

From: Uwe Kleine-König
Date: Wed Jun 26 2019 - 04:58:43 EST

On Tue, Jun 25, 2019 at 11:38:39AM +0200, Thierry Reding wrote:
> On Mon, Jun 24, 2019 at 12:28:44PM +0100, Daniel Thompson wrote:
> > [...] although given pwm-backlight is essentially a wrapper driver
> > round a PWM I wondered why the pinctrl was on the backlight node
> > (rather than the PWM node).
> I agree with this. We're defining the pin control state for the PWM pin,
> so in my opinion it should be the PWM driver that controls it.
> One reason why I think this is important is if we ever end up with a
> device that requires pins from two different controllers to be
> configured at runtime, then how would we model that? Since pin control
> states cannot be aggregated, so you'd have to have multiple "default"
> states, each for the pins that they control.

I thought you can do:

pinctrl-names = "default";
pinctrl-0 = <&pinctrl_in_first_pincontroller>, <&pinctrl_in_another_controller>;

if two (or more) controllers are involved.

> On the other hand if we associate the pin control states with each of
> the resources that need those states, then when those resources are
> controlled, they will automatically know how to deal with the states.
> The top-level device (i.e. backlight) doesn't need to concern itself
> with those details.

So the options are:

a) put "active" and "inactive" pinctrls into the pwm-node, and nothing
related to the involved PWM pins in the consumer

b) put the PWM pin config in the consumer's "default" pinctrl (and
maybe leave it out int "init" if you want smooth taking over).

(Or maybe use "enabled" and "disabled" in a) to match the pwm_states

The advantages I see in b) over a) are:

- "default" and "init" are a known pinctrl concept that most people
should have understood.

- You have all pinctrl config for the backlight in a single place.

- none of the involved driver must explicitly handle pinctrl stuff

You presume that b) being commonly done is a sign of "our device trees
and kernel subsystems still maturing". But maybe it's only that the
capabilities provided by pinctrl subsystem without extra effort is good

Best regards

Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | |