Hi,
On 1.10.23 г. 13:40 ч., Sean Young wrote:
The pwm-ir-tx driver has to turn the pwm signal on and off, and suffers
from delays as this is done in process context. Make this work in atomic
context.
This makes the driver much more precise.
Signed-off-by: Sean Young <sean@xxxxxxxx>
Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@xxxxxxxxx>
---
drivers/media/rc/pwm-ir-tx.c | 79 ++++++++++++++++++++++++++++--------
1 file changed, 63 insertions(+), 16 deletions(-)
diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
index c5f37c03af9c..557725a07a67 100644
--- a/drivers/media/rc/pwm-ir-tx.c
+++ b/drivers/media/rc/pwm-ir-tx.c
@@ -10,6 +10,8 @@
#include <linux/slab.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/hrtimer.h>
+#include <linux/completion.h>
#include <media/rc-core.h>
#define DRIVER_NAME "pwm-ir-tx"
@@ -17,8 +19,13 @@
struct pwm_ir {
struct pwm_device *pwm;
- unsigned int carrier;
- unsigned int duty_cycle;
+ struct hrtimer timer;
+ struct completion completion;
+ uint carrier;
+ uint duty_cycle;
+ uint *txbuf;
+ uint txbuf_len;
+ uint txbuf_index;
};
static const struct of_device_id pwm_ir_of_match[] = {
@@ -55,33 +62,65 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
struct pwm_ir *pwm_ir = dev->priv;
struct pwm_device *pwm = pwm_ir->pwm;
struct pwm_state state;
- int i;
- ktime_t edge;
- long delta;
+
+ reinit_completion(&pwm_ir->completion);
You should not need that.
pwm_init_state(pwm, &state);
state.period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
pwm_set_relative_duty_cycle(&state, pwm_ir->duty_cycle, 100);
+ state.enabled = false;
- edge = ktime_get();
+ pwm_ir->txbuf = txbuf;
+ pwm_ir->txbuf_len = count;
+ pwm_ir->txbuf_index = 0;
- for (i = 0; i < count; i++) {
- state.enabled = !(i % 2);
- pwm_apply_state(pwm, &state);
+ pwm_apply_state(pwm, &state);
ditto, first pwm control should be in the timer function
- edge = ktime_add_us(edge, txbuf[i]);
- delta = ktime_us_delta(edge, ktime_get());
- if (delta > 0)
- usleep_range(delta, delta + 10);
- }
+ hrtimer_start(&pwm_ir->timer, 1000, HRTIMER_MODE_REL);
why not just call it with 0 time?
- state.enabled = false;
- pwm_apply_state(pwm, &state);
+ wait_for_completion(&pwm_ir->completion);
return count;
}
+static enum hrtimer_restart pwm_ir_timer(struct hrtimer *timer)
+{
+ struct pwm_ir *pwm_ir = container_of(timer, struct pwm_ir, timer);
+ ktime_t now;
+
+ /*
+ * If we happen to hit an odd latency spike, loop through the
+ * pulses until we catch up.
+ */
+ do {
+ u64 ns;
+
+ if (pwm_ir->txbuf_index >= pwm_ir->txbuf_len) {
+ /* Stop TX here */
+ pwm_disable(pwm_ir->pwm);
+
+ complete(&pwm_ir->completion);
+
+ return HRTIMER_NORESTART;
+ }
+
+ if (pwm_ir->txbuf_index % 2)
+ pwm_disable(pwm_ir->pwm);
+ else
+ pwm_enable(pwm_ir->pwm);
+
pwm_ir->pwm->state.enabled = !(pwm_ir->txbuf_index % 2);
pwm_apply_state(pwm_ir->pwm, pwm_ir->state);
+ ns = US_TO_NS(pwm_ir->txbuf[pwm_ir->txbuf_index]);
+ hrtimer_add_expires_ns(timer, ns);
+
+ pwm_ir->txbuf_index++;
+
+ now = timer->base->get_time();
+ } while (hrtimer_get_expires_tv64(timer) < now);
+
+ return HRTIMER_RESTART;
+}
+
static int pwm_ir_probe(struct platform_device *pdev)
{
struct pwm_ir *pwm_ir;
@@ -96,8 +135,16 @@ static int pwm_ir_probe(struct platform_device *pdev)
if (IS_ERR(pwm_ir->pwm))
return PTR_ERR(pwm_ir->pwm);
+ if (pwm_can_sleep(pwm_ir->pwm)) {
+ dev_err(&pdev->dev, "unsupported pwm device: driver can sleep\n");
+ return -ENODEV;
+ }
+
I think we shall not limit, but use high priority thread to support
those drivers. I have that working on n900 with current (sleeping) pwm,
see my reply on the other mail. Maybe we can combine both patches in a
way to support both atomic and sleeping pwm drivers.
pwm_ir->carrier = 38000;
pwm_ir->duty_cycle = 50;
+ init_completion(&pwm_ir->completion);
+ hrtimer_init(&pwm_ir->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ pwm_ir->timer.function = pwm_ir_timer;
rcdev = devm_rc_allocate_device(&pdev->dev, RC_DRIVER_IR_RAW_TX);
if (!rcdev)
Regards,
Ivo
