On Mon, Apr 17, 2023 at 09:23:35AM +0200, Neil Armstrong wrote:
> On 13/04/2023 07:54, Heiner Kallweit wrote:
[...]
> > diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
[...]
> > @@ -271,16 +255,16 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> >   			/*
> >   			 * This IP block revision doesn't have an "always high"
> >   			 * setting which we can use for "inverted disabled".
> > -			 * Instead we achieve this using the same settings
> > -			 * that we use a pre_div of 0 (to get the shortest
> > -			 * possible duration for one "count") and
> > +			 * Instead we achieve this by setting an arbitrary,
> > +			 * very high frequency, resulting in the shortest
> > +			 * possible duration for one "count" and
> >   			 * "period == duty_cycle". This results in a signal
> >   			 * which is LOW for one "count", while being HIGH for
> >   			 * the rest of the (so the signal is HIGH for slightly
> >   			 * less than 100% of the period, but this is the best
> >   			 * we can achieve).
> >   			 */
> > -			channel->pre_div = 0;
> > +			channel->rate = 1000000000;
> >   			channel->hi = ~0;
> >   			channel->lo = 0;
> 
> This looks like a really bad idea... please don't do that and instead introduce
> some pinctrl states where we set the PWM pin as GPIO mode high/low state like we
> did for SPI:
> https://lore.kernel.org/r/20221004-up-aml-fix-spi-v4-2-0342d8e10c49@baylibre.com

Yeah, absolutely. If the pin controller can do that, that's the right
way to implement the desired behavior.

Thierry