Message ID | cover.1723264979.git.lorenzo@kernel.org |
---|---|
Headers | show |
Series | Add PWM support to EN7581 | expand |
On 10/08/2024 06:48, Lorenzo Bianconi wrote: > From: Benjamin Larsson <benjamin.larsson@genexis.eu> > > Introduce driver for PWM module available on EN7581 SoC. > > Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu> > Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- ... > + > +static void airoha_pwm_config_flash_map(struct airoha_pwm *pc, > + unsigned int hwpwm, int index) > +{ > + u32 addr, mask, val; > + > + if (hwpwm < PWM_NUM_GPIO) { > + addr = REG_GPIO_FLASH_MAP(hwpwm / 8); > + } else { > + addr = REG_SIPO_FLASH_MAP(hwpwm / 8); > + hwpwm -= PWM_NUM_GPIO; > + } > + > + if (index < 0) { > + /* Change of waveform takes effect immediately but This and other comments should be not netdev-style but general Linux style. > + * disabling has some delay so to prevent glitching > + * only the enable bit is touched when disabling > + */ > + airoha_pwm_flash_clear(pc, addr, GPIO_FLASH_EN(hwpwm % 8)); > + return; ... > + > +static const struct of_device_id airoha_pwm_of_match[] = { > + { .compatible = "airoha,en7581-pwm" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, airoha_pwm_of_match); > + > +static struct platform_driver airoha_pwm_driver = { > + .driver = { > + .name = "airoha-pwm", > + .of_match_table = airoha_pwm_of_match, > + }, > + .probe = airoha_pwm_probe, > +}; > +module_platform_driver(airoha_pwm_driver); > + > +MODULE_ALIAS("platform:airoha-pwm"); You should not need MODULE_ALIAS() in normal cases. If you need it, usually it means your device ID table is wrong (e.g. misses either entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute for incomplete ID table. Especially that it does not match compatible, so you cannot use excuse module autoloading does not work for given OF node... Best regards, Krzysztof
> On 10/08/2024 06:48, Lorenzo Bianconi wrote: > > From: Benjamin Larsson <benjamin.larsson@genexis.eu> > > > > Introduce driver for PWM module available on EN7581 SoC. > > > > Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu> > > Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > > ... > > > + > > +static void airoha_pwm_config_flash_map(struct airoha_pwm *pc, > > + unsigned int hwpwm, int index) > > +{ > > + u32 addr, mask, val; > > + > > + if (hwpwm < PWM_NUM_GPIO) { > > + addr = REG_GPIO_FLASH_MAP(hwpwm / 8); > > + } else { > > + addr = REG_SIPO_FLASH_MAP(hwpwm / 8); > > + hwpwm -= PWM_NUM_GPIO; > > + } > > + > > + if (index < 0) { > > + /* Change of waveform takes effect immediately but > > This and other comments should be not netdev-style but general Linux style. ack, I will fix them in v2. > > > + * disabling has some delay so to prevent glitching > > + * only the enable bit is touched when disabling > > + */ > > + airoha_pwm_flash_clear(pc, addr, GPIO_FLASH_EN(hwpwm % 8)); > > + return; > > ... > > > > + > > +static const struct of_device_id airoha_pwm_of_match[] = { > > + { .compatible = "airoha,en7581-pwm" }, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(of, airoha_pwm_of_match); > > + > > +static struct platform_driver airoha_pwm_driver = { > > + .driver = { > > + .name = "airoha-pwm", > > + .of_match_table = airoha_pwm_of_match, > > + }, > > + .probe = airoha_pwm_probe, > > +}; > > +module_platform_driver(airoha_pwm_driver); > > + > > +MODULE_ALIAS("platform:airoha-pwm"); > > You should not need MODULE_ALIAS() in normal cases. If you need it, > usually it means your device ID table is wrong (e.g. misses either > entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute > for incomplete ID table. > > Especially that it does not match compatible, so you cannot use excuse > module autoloading does not work for given OF node... ack, I will fix it in v2. Regards, Lorenzo > > > Best regards, > Krzysztof >