Re: [RFC]PWM API Update


Liu, Baohong
 

Hi all,

Thank you for your feedback and suggestions on this RFC.

I summarized what were discussed and would like to conclude this RFC.

There will be two APIs in include/pwm.h:

static inline int pwm_pin_set(struct device *dev, uint32_t pwm,
uint32_t period_cycles, uint32_t pulse_cycles);

static inline int pwm_pin_set_usec(struct device *dev, uint32_t pwm,
uint32_t period_usec, uint32_t pulse_usec);

Note: implementation of pwm_pin_set_usec API shall convert the
input (in usec) to cycles and then call pwm_pin_set.

I felt that get_cycles_per_sec() is not needed since there are already
constant definition for this. (e.g. CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC
in boards/arduino_101_sss/arduino_101_sss_defconfig).

The new API would replace the existing APIs so with that, all the existing APIs
(including API like pwm_all_set_xxx and the pwm_pin_configure which is not
Being used) will be marked as deprecated for 2 releases before they are
removed, based on the Zephyr policy.

Again, if any additional comments, please voice.

Thanks
Baohong

-----Original Message-----
From: Liu, Baohong [mailto:baohong.liu(a)intel.com]
Sent: Friday, September 16, 2016 3:18 PM
To: Tomasz Bursztyka <tomasz.bursztyka(a)linux.intel.com>;
devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: [RFC]PWM API Update



-----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.