Re: [RFC]PWM API Update


Liu, Baohong
 

-----Original Message-----
From: Tomasz Bursztyka [mailto:tomasz.bursztyka(a)linux.intel.com]
Sent: Thursday, September 15, 2016 11:41 PM
To: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: [RFC]PWM API Update

Hi Baohong,
-----Original Message-----
From: Ross, Andrew J
Sent: Thursday, September 15, 2016 8:04 AM
To: Liu, Baohong <baohong.liu(a)intel.com>;
devel(a)lists.zephyrproject.org
Subject: Re: [devel] [RFC]PWM API Update

Liu, Baohong wrote:
As for the unit for period and pulse width, I understand that both
time
(micro-second) and clock cycles are popularly used. To cater for
this, the above-mentioned API will be expanded into two.

pwm_set_pin_usec(uint8_t pin, uint32_t period_usec, uint32_t
pulse_usec) pwm_set_pin_cycles(uint8_t pin, uint32_t period_cycles,
uint32_t pulse_cycles)
When PWM is used correctly, this shouldn't make any difference at all
because the period will be much, much longer than the underlying
clock (which is the whole point).

Why bother with having two ways to do this when they're going to be
exactly equivalent in all but the weirdest apps? Just set them in
arbitrary units of "cycles". And if an application really, truly
needs to know the underlying cycle time of the hardware (which is
going to be device-dependent), give them an API like
"pwm_get_cycle_time()" which returns a cycle time in picoseconds or
something.
I personally prefer two APIs. Somebody also suggested two APIs in ZEP-745.
Looks two APIs is the way to go so far.
In the end, both do the same, but:

there is actually an implementation issue with pwm_set_pin_usec() as the
driver will have to do calculation according to the underlying hw cycle time.
So this piles up more work in driver side. Though, from user perspective, it
might be simpler to use yes.
(no hand-computation needed from the user).

Actually, with Andy's proposal of providing a pwm_get_cycle_time() along
with pwm_set_pin_cycles(), I believe (tell me if I am wrong) you could
implement
pwm_set_pin_usec() as a generic pwm function instead of a driver API
function.
Sounds like the best option to me: you keep the driver API concise, and you
still end up providing both functions in the end.
I understand what you mean. I compared both methods. In my opinion,
there is no significant difference.

By the way, it looks odd to me to add pwm_get_cycle_time() into PWM driver.
It should be a common utility function instead of a PWM API?

For now, let's go with what Andy and you proposed, unless I hear any strong
objection from others. That's:

pwm_set_pin(struct device *dev, uint8_t pin, uint32_t period_cycles,
uint32_t pulse_cycles); /* PWM API */
get_cycles_per_usec(void); /* we can find better name:) */

As you mentioned, the usec one will be a generic PWM function instead of a API.


Tomasz

Join devel@lists.zephyrproject.org to automatically receive all group messages.