> Unfortunately there is no universal AF value for pwm channels, on some pins > it's 1, on some it's 6 ... , some pins cannot be used as pwm channel outputs > at all and some pins could be used as the output of multiple channels.
Not sure I got it right. So you can configure a given pin using 1 or 6 depending on whether you want it to output a single pwm chan or multiple? Having a single gpio output multiple channels feels a lot like an uncommon feature btw. > It's bad enough that only certain pins work for a given timer/channel but > it's even worse because the driver also needs to know the AF number to use. It definitely does not look great but it feels that you will need either a lot of #ifdefs or have the driver knowing these restrictions. Aren't these restrictions described on some .h from stm? > As an additional crux, the HW timer to be used (TIM1 above) typically gets > defined by the BSP in hal_bsp_init, whereas pwm_chan_config is application > runtime code. I would prefer to break the coupling, or at least make it less > fragile. I see, I'm not sure about that one. I mean it does not make much sense to me, because one might want to reconfigure channels on runtime, just take a look at what we're doing on pwm_test on last PR. It sounds like stm PWM has a bunch of limitations you have to deal with and there are some things you probably won't be able to run from. I already implemented two PWM drivers using the current interface, I think we should be flexible at this stage regarding the API interface so it will be useful for you. However I feel that having an AF field by default doesnt make much sense. Thanks, On Thu, Mar 1, 2018 at 2:51 AM, markus <[email protected]> wrote: > This is the implementation of pwm_configure_channel: > > static int > stm32f3_pwm_configure_channel(struct pwm_dev *dev, uint8_t cnum, struct > pwm_chan_cfg *cfg) > { > stm32f3_pwm_dev_t *pwm; > > assert(dev->pwm_instance_id < PWM_COUNT); > assert(cnum < STM32F3_PWM_CH_COUNT); > > pwm = &stm32f3_pwm_dev[dev->pwm_instance_id]; > > if (STM32F3_PWM_CH_DISABLED != pwm->ch[cnum].config) { > return -1; > } > > if (STM32F3_PWM_CH_NOPIN != cfg->pin) { > if (hal_gpio_init_af(cfg->pin, > (uint8_t)(uintptr_t)cfg->data, > HAL_GPIO_PULL_NONE, > 0)) { > return -1; > } > } > > pwm->ch[cnum].duty = 0; > pwm->ch[cnum].pin = cfg->pin; > pwm->ch[cnum].invert = cfg->inverted; > pwm->ch[cnum].update = 0; > pwm->ch[cnum].enabled = 0; > pwm->ch[cnum].configure = 1; > > return 0; > } > > On an STM to connect the output pad with the PWM channel I have to configure > it for the alternate function (which is done by hal_gpio_init_af). The AF > value, the result of `(uint8_t)(uintptr_t)cfg->data` is a value between 0 and > 15 which determines what the pin is used for. > > Unfortunately there is no universal AF value for pwm channels, on some pins > it's 1, on some it's 6 ... , some pins cannot be used as pwm channel outputs > at all and some pins could be used as the output of multiple channels. > > With the construct above the application has to make a call like: > > struct pwm_chan_cfg cfg; > cfg.pin = 8; > cfg.inverted = false; > cfg.data = (void*)6; > pwm_chan_config(pwm, 0, &cfg); > > But this ONLY works if `pwm` is bound to the mcu's TIM1 and you use channel > 0. For TIM1 and channel 1 the app would have to use `pin=9/data=6`. > > It's bad enough that only certain pins work for a given timer/channel but > it's even worse because the driver also needs to know the AF number to use. > > As an additional crux, the HW timer to be used (TIM1 above) typically gets > defined by the BSP in hal_bsp_init, whereas pwm_chan_config is application > runtime code. I would prefer to break the coupling, or at least make it less > fragile. > > Does this make sense? > Markus > > > On Thu, 1 Mar 2018 01:58:46 -0300 > Miguel Azevedo <[email protected]> wrote: > >> Hi markus, >> >> > `struct pwm_chan_config` only specifies the pin number. I've been >> > using the `data` member to put the burden on the application to >> > specify the "correct" number for the alternate function > - but I >> > find this a little unsatisfactory. >> That sounds a lot driver/hw specific. Can you show us the code for >> that? >> >> Thanks, >> Miguel >> >> On Wed, Feb 28, 2018 at 9:57 PM, markus <[email protected]> wrote: >> > While writing a PWM driver for the STM32s mcu family I came across >> > the issue of assigning output pins for a pwm channel. >> > >> > PWM output in STM32s is done by configuring a pin for "alternate >> > functions" - where specific pins have specific (possible) alternate >> > functions. In other words, only a very limited number of >> > timer:channel:pin combinations are possible, each one requiring a >> > very specific alternate function. >> > >> > `struct pwm_chan_config` only specifies the pin number. I've been >> > using the `data` member to put the burden on the application to >> > specify the "correct" number for the alternate function - but I >> > find this a little unsatisfactory. >> > >> > So I was wondering if somebody has a recommendation, better >> > approach to solving this issue? >> > >> > Have fun, >> > Markus >> >> >> > -- -- Miguel Azevedo
