PWM and rounding for pwm-ir-tx

PWM and rounding for pwm-ir-tx

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

Hello Sean,

I'm currently working on the pwm-bcm2835 driver, converting it to the
waveform PWM API.

A result of that conversation is that calling pwm_apply_atomic() will
result in the period being rounded down instead of rounded to the
nearest possible value. Looking at commit 11fc4edc483b ("pwm: bcm2835:
Improve precision of PWM") I guess you will be unhappy about that.

The obvious way forward is to let the pwm-ir-tx driver make use of the
waveform API which allows better control over the actual hardware
output.

I looked into the pwm-ir-tx driver to check if I can do the conversion,
but didn't understand the protocol good enough to try that.

I wonder if you are still interested enough in the driver to do the
conversion yourself? Alternatively can you point me to some documention
resource explaining the protocol? Maybe there is even an easy way to
test changes?

One thing that I noticed is that

	pwm_ir->state->enabled = !(pwm_ir->txbuf_index % 2);
	pwm_apply_atomic(pwm_ir->pwm, pwm_ir->state);

is conceptually broken because the output of a disabled PWM is
undefined. Many hardwares emit a constant inactive level, but others
might emit active instead, or a random level or might even continue to
toggle. If you want to be sure to drive the inactive level, keep the PWM
enabled but use duty_cycle = 0.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: PWM and rounding for pwm-ir-tx

[Sean Young]

Hello Uwe,

On Fri, Nov 21, 2025 at 05:04:41PM +0100, Uwe Kleine-König wrote:
> Hello Sean,
> 
> I'm currently working on the pwm-bcm2835 driver, converting it to the
> waveform PWM API.
> 
> A result of that conversation is that calling pwm_apply_atomic() will
> result in the period being rounded down instead of rounded to the
> nearest possible value. Looking at commit 11fc4edc483b ("pwm: bcm2835:
> Improve precision of PWM") I guess you will be unhappy about that.
> 
> The obvious way forward is to let the pwm-ir-tx driver make use of the
> waveform API which allows better control over the actual hardware
> output.

That's very interesting, I must have missed the new waveform API. I agree,
this is a great way forward.

> I looked into the pwm-ir-tx driver to check if I can do the conversion,
> but didn't understand the protocol good enough to try that.
> 
> I wonder if you are still interested enough in the driver to do the
> conversion yourself? Alternatively can you point me to some documention
> resource explaining the protocol? Maybe there is even an easy way to
> test changes?

I'm more than happy to do the conversion, then I can also test that works
correctly with a logic analyizer.

> One thing that I noticed is that
> 
> 	pwm_ir->state->enabled = !(pwm_ir->txbuf_index % 2);
> 	pwm_apply_atomic(pwm_ir->pwm, pwm_ir->state);
> 
> is conceptually broken because the output of a disabled PWM is
> undefined. Many hardwares emit a constant inactive level, but others
> might emit active instead, or a random level or might even continue to
> toggle. If you want to be sure to drive the inactive level, keep the PWM
> enabled but use duty_cycle = 0.

That's very useful, I did not know that. I'll write a fix for that.

For the pwm-bcm2835 driver: do you have a draft commit that enables the
waveform API? That way I can start testing any changes I make to pwm-ir-tx
on RPi.

Thank you for keeping in the loop on this, I should pay closer attention
to the list.


Sean

Re: PWM and rounding for pwm-ir-tx

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2213 bytes --]

Hello Sean,

On Fri, Nov 21, 2025 at 10:12:08PM +0000, Sean Young wrote:
> On Fri, Nov 21, 2025 at 05:04:41PM +0100, Uwe Kleine-König wrote:
> > I'm currently working on the pwm-bcm2835 driver, converting it to the
> > waveform PWM API.
> > 
> > A result of that conversation is that calling pwm_apply_atomic() will
> > result in the period being rounded down instead of rounded to the
> > nearest possible value. Looking at commit 11fc4edc483b ("pwm: bcm2835:
> > Improve precision of PWM") I guess you will be unhappy about that.
> > 
> > The obvious way forward is to let the pwm-ir-tx driver make use of the
> > waveform API which allows better control over the actual hardware
> > output.
> 
> That's very interesting, I must have missed the new waveform API. I agree,
> this is a great way forward.

I guess I'm not a good advertiser for the improvements I do :-)

> > I looked into the pwm-ir-tx driver to check if I can do the conversion,
> > but didn't understand the protocol good enough to try that.
> > 
> > I wonder if you are still interested enough in the driver to do the
> > conversion yourself? Alternatively can you point me to some documention
> > resource explaining the protocol? Maybe there is even an easy way to
> > test changes?
> 
> I'm more than happy to do the conversion, then I can also test that works
> correctly with a logic analyizer.

That's great.

The functions that are already there should be enough in theory to pick
the best configuration for your use case. Feel free to create helper
functions in the pwm subsystem that help you. I guess a function
pwm_roundup_waveform_might_sleep() might be useful.

> For the pwm-bcm2835 driver: do you have a draft commit that enables the
> waveform API? That way I can start testing any changes I make to pwm-ir-tx
> on RPi.

I pushed my wip change to:

	https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/wip-bcm2835-waveform

I already did some testing on these changes and I think there are very
little changes needed (if any at all) before posting that. But I want to
apply another round of self-review before sending it out officially.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]