Re: [PATCH v5 2/2] media: rc: remove ir-rx51 in favour of generic pwm-ir-tx

From: Sean Young
Date: Fri Sep 29 2023 - 12:20:17 EST


Hello Uwe,

Just wanted to run an idea by you.

On Fri, Sep 29, 2023 at 11:49:52AM +0300, Ivaylo Dimitrov wrote:
> On 26.09.23 г. 23:18 ч., Sean Young wrote:
> > I've never known of a solution to the pwm-ir-tx driver. If using hrtimers
> > directly improves the situation even a bit, then that would be great.
>
> The issue with hrtimers is that we cannot use them directly, as
> pwm_apply_state() may sleep, but hrtimer function is called in atomic
> context.

I've also been looking at this problem and came to same conclusion: the
fact that pwm_apply_state() sleeps is a huge problem.

1) The vast majority of pwm drivers don't sleep, or could even be converted
to spinlocks (e.g pwm-sifive.c could use spinlocks, as far as I can see).

2) Sure, some pwm devices are on i2c busses, so the driver needs to sleep.
Those devices aren't great for what we're trying to do here, since the
sleeping may cause delays and affect the generated signal.

What would be ideal here is to have pwm-ir-tx work in atomic context if
a non-sleeping pwm device is used, and another (non-optimal) code path
for sleeping pwm drivers. We could even just refuse to run on sleeping pwm
drivers.

Uwe what do you think of this idea? The pwm api could have a
bool pwm_may_sleep(struct pwm *pwm) function, and pwm_apply_state() does
not contain might_sleep() - only the driver-specific apply calls might_sleep().

It would be nice if this could all be done at compile time through e.g. a
device tree attribute.

Thanks,

Sean