Thread (24 messages) 24 messages, 7 authors, 2024-09-06

Re: [PATCH v3 5/5] pwm: airoha: Add support for EN7581 SoC

From: Uwe Kleine-König <hidden>
Date: 2024-09-05 09:30:44
Also in: linux-devicetree, linux-gpio, linux-mediatek, linux-pwm

Hello,

On Thu, Sep 05, 2024 at 01:09:48AM +0200, Benjamin Larsson wrote:
On 03/09/2024 17:47, Uwe Kleine-König wrote:
quoted
Hello Benjamin,

On Tue, Sep 03, 2024 at 01:58:30PM +0200, Benjamin Larsson wrote:
quoted
On 2024-09-03 12:46, Uwe Kleine-König wrote:
quoted
Would you please add a "Limitations" paragraph here covering the
following questions:

   - How does the hardware behave on changes of configuration (does it
     complete the currently running period? Are there any glitches?)
   - How does the hardware behave on disabling?

Please stick to the format used in several other drivers such that

	sed -rn '/Limitations:/,/\*\/?$/p' drivers/pwm/*.c

emits the informations.
The answer to your questions are currently unknown. Is this information
needed for a merge of the driver ?
It would be very welcome and typically isn't that hard to work out if
you have an LED connected to the output or a similar means to observe
the output. An oscilloscope makes it still easier.

For example to check if the current period is completed configure the
PWM with period = 1s and duty_cycle = 0 disabling the LED. (I leave it
as an exercise for the reader what to do if duty_cycle = 0 enables the
LED :-) Then do:

	pwm_apply_might_sleep(mypwm, &(struct pwm_state){
		.period = NSEC_PER_SEC,
		.duty_cycle = NSEC_PER_SEC,
		.enabled = true,
	});
	pwm_apply_might_sleep(mypwm, &(struct pwm_state){
		.period = NSEC_PER_SEC,
		.duty_cycle = 0,
		.enabled = true,
	});

Iff that enables the LED for a second, the period is completed. The
question about glitches is a bit harder to answer, but with a tool like
memtool should be possible to answer. Alternatively add delays and
printk output to .apply() in the critical places.
I connected a logic analyzer to a pin and configured the pwm for it.

I then configured the pwm with these parameters (setup for 2Hz).

echo 1000000000 > /sys/class/pwm/pwmchip0/pwm12/period
echo 0 > /sys/class/pwm/pwmchip0/pwm12/duty_cycle

If I then ran the following (in a script) no pulse was detected:

echo 500000000 > /sys/class/pwm/pwmchip0/pwm12/duty_cycle
echo 0 > /sys/class/pwm/pwmchip0/pwm12/duty_cycle

If I added a sleep 1 in between I always got 1 500ms pulse.

I then did the same but with direct register access with the same result.
Setting the duty cycle to 0 disables the pwm function on the pin, it seems
to take a while before it properly activates but before it disables it the
cycle completes.


I also tested with enabling the pwn signal and then setting a 0 duty cycle.
The last observed pulse was always 500ms long.


I am not sure what of your questions this answers and is there some other
tests I should perform ?
IIUC that means to add:

	On configuration the currently running period is completed.

to the Limitations paragraph.
For the record while toggling the registers I noticed that it was actually
possible to generate 1 second long pulses. The documentation is not clear on
this part.
1 second long pulses with a period size of 1 second, so a constant high
signal?

Another thing that would be interesting is, if it can happen that you
get a mixed signal. That is, if you update from 

	.period = A
	.duty_cycle = B

to

	.period = C
	.duty_cycle = D

that you get one period with length C and duty_cycle B when the period
completes after configuring period but before duty_cycle.

Best regards
UWe

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help