Hi again Markus, > I think your enhancements should be their own driver. Plain PWM is such a > basic need and feature that I think the interface for it should be as simple > as possible. I'm also concerned about the memory footprint increase, which is > 3x that of the original interface (and that's just the data structure).
Agreed. Although if you re-use the structure it isn't such a dramatic increase in memory footprint. Regarding the datastructure size I believe we can keep the previous interface, without the callback associated fields and the cycle(I don't love this one either) I suggest we have a specific datastructure defined on pwm_nrf52.h or even pwm.h(given these are common features on PWM devices, any driver would be eligible to implement them on a standardized way). This struct could either be a struct containing a pwm_chan_cfg which we would 'upcast' and 'downcast' (like we do with dev and odev) or simply something we pass through the data field. > For the nrf52 specifically you could implement the old interface driver on > top of a driver with the new proposed interface. I don't understand what you mean by using the old interface but I think one of the the ways I described will sort out the pwm_chan_cfg memory footprint problem. Thanks for your feedback! :) Miguel On Wed, Feb 28, 2018 at 9:24 PM, markus <mar...@bibi.ca> wrote: > Hi Miguel, > > I think your enhancements should be their own driver. Plain PWM is such a > basic need and feature that I think the interface for it should be as simple > as possible. I'm also concerned about the memory footprint increase, which is > 3x that of the original interface (and that's just the data structure). > > For the nrf52 specifically you could implement the old interface driver on > top of a driver with the new proposed interface. > > My 2 cents, > Markus > > > > On Wed, 28 Feb 2018 17:51:50 -0300 > Miguel Azevedo <miguella...@gmail.com> wrote: > >> Hi everyone, >> >> I recently implemented a few new features for the nRF52 PWM driver and >> will probably implement them for soft PWM as well. >> >> The features: >> * Interrupts fired every cycle with user handler and parameter data; >> * A mode where the device plays for n cycles; >> * Interrupts fired at the end of a sequence of n cycles with user >> handler and parameter data. >> >> These changes are PRed here: >> https://github.com/apache/mynewt-core/pull/836 >> >> There is an example (pwm_test) which uses the also PRed easing >> library: https://github.com/apache/mynewt-core/pull/822 >> >> These features change the pwm_chan_cfg datastructure on pwm.h, wich is >> general to all drivers. >> I can argue that these features are implementable on virtually any PWM >> device since every hardware PWM implementation I've seen so far uses >> timers and counters, however there might be (and probably are) >> exceptions. >> Anyway a driver which doesn't use these new fields on this >> datastructure is still implementable(well, we have soft PWM using the >> sabe API interface and it works). >> >> The alternative to this would be to write a datastructure on the >> driver's implementation header file (pwm_nrf52.h in this case) with >> the new fields. >> >> What do you think? >> >> Best, >> >> Miguel Azevedo > -- -- Miguel Azevedo
