Re: [PATCH v12 1/2] pwm: add microchip soft ip corePWM driver

From: Uwe Kleine-König
Date: Mon Dec 05 2022 - 13:14:05 EST


Hello Conor,

On Mon, Dec 05, 2022 at 05:13:42PM +0000, Conor Dooley wrote:
> Figured I was better off doing something quickly & asking rather than
> polishing only to find out it was disliked ;)

:-)

> > - in .apply() check the current time, add the current period and store
> > the result to ddata->updatetimestamp
> > - in .get_state do:
> > if (current_time >= ddata->updatetimestamp)
> > process fine
> > else:
> > timeout (or wait until ddata->updatetimestamp?)
> >
> > Actually I'd prefer to wait instead of -ETIMEOUT.
>
> Prefer to wait in get_state() or in both it & apply()?

Prefer to wait where it's necessary ...

> Depending on how far away updatetimestamp is, would we still not want to
> time out if it is going to be a long time, no?

I'd prefer a slow but right value over a quick error.

The sun4i driver has in its .apply():

/* We need a full period to elapse before disabling the channel. */
delay_us = DIV_ROUND_UP_ULL(cstate.period, NSEC_PER_USEC);
if ((delay_us / 500) > MAX_UDELAY_MS)
msleep(delay_us / 1000 + 1);
else
usleep_range(delay_us, delay_us * 2);

so it's at least not new in general to have a waiting time in O(period).

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature