On 5/5/21 2:37 AM, Michal Simek wrote: > > > On 5/4/21 8:49 PM, Sean Anderson wrote: >> This adds PWM support for Xilinx LogiCORE IP AXI soft timers commonly >> found on Xilinx FPGAs. There is another driver for this device located >> at arch/microblaze/kernel/timer.c, but it is only used for timekeeping. >> This driver was written with reference to Xilinx DS764 for v1.03.a [1]. >> >> [1] https://www.xilinx.com/support/documentation/ip_documentation/axi_timer/v1_03_a/axi_timer_ds764.pdf >> >> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx> >> --- >> I tried adding a XILINX_PWM_ prefix to all the defines, but IMO it >> really hurt readability. That prefix almost doubles the size the >> defines, and is particularly excessive in something like >> XILINX_PWM_TCSR_RUN_MASK. >> >> Changes in v2: >> - Don't compile this module by default for arm64 >> - Add dependencies on COMMON_CLK and HAS_IOMEM >> - Add comment explaining why we depend on !MICROBLAZE >> - Add comment describing device >> - Rename TCSR_(SET|CLEAR) to TCSR_RUN_(SET|CLEAR) >> - Use NSEC_TO_SEC instead of defining our own >> - Use TCSR_RUN_MASK to check if the PWM is enabled, as suggested by Uwe >> - Cast dividends to u64 to avoid overflow >> - Check for over- and underflow when calculating TLR >> - Set xilinx_pwm_ops.owner >> - Don't set pwmchip.base to -1 >> - Check range of xlnx,count-width >> - Ensure the clock is always running when the pwm is registered >> - Remove debugfs file :l >> - Report errors with dev_error_probe >> >> drivers/pwm/Kconfig | 13 ++ >> drivers/pwm/Makefile | 1 + >> drivers/pwm/pwm-xilinx.c | 301 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 315 insertions(+) >> create mode 100644 drivers/pwm/pwm-xilinx.c > > Without looking below another driver which target the same IP is just > wrong that's why NACK from me. Can you elaborate on this position a bit more? I don't think a rework of the microblaze driver should hold back this one. They cannot be enabled at the same time. I think it is OK to leave the work of making them coexist for a future series (written by someone with microblaze hardware to test on). --Sean
