toggle quoted messageShow quoted text
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
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.
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>;
Subject: [devel] Re: Re: Re: [RFC]PWM API Update
From: Tomasz Bursztyka [mailto:tomasz.bursztyka(a)linux.intel.com]
Sent: Thursday, September 15, 2016 11:41 PM
Subject: [devel] Re: Re: [RFC]PWM API Update
From: Ross, Andrew J
Sent: Thursday, September 15, 2016 8:04 AM
To: Liu, Baohong <baohong.liu(a)intel.com>;
Subject: Re: [devel] [RFC]PWM API Update
Liu, Baohong wrote:
As for the unit for period and pulse width, I understand that bothWhen PWM is used correctly, this shouldn't make any difference at
(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)
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
I personally prefer two APIs. Somebody also suggested two APIs in ZEP-
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
So this piles up more work in driver side. Though, from userI understand what you mean. I compared both methods. In my opinion,
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
pwm_set_pin_usec() as a generic pwm function instead of a driver API
Sounds like the best option to me: you keep the driver API concise,
and you still end up providing both functions in the end.
there is no significant difference.
By the way, it looks odd to me to add pwm_get_cycle_time() into PWM
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