linux-kernel - Re: [PATCH v10 3/3] pwm: Add support for Xilinx AXI Timer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51cdffc6-7620-3bfe-2209-6f4abde0c654@xilinx.com>
Date:   Thu, 18 Nov 2021 09:35:47 +0100
From:   Michal Simek <michal.simek@...inx.com>
To:     Thierry Reding <thierry.reding@...il.com>,
        Sean Anderson <sean.anderson@...o.com>,
        Michal Simek <michal.simek@...inx.com>
CC:     <linux-pwm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        Lee Jones <lee.jones@...aro.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        Alvaro Gamez <alvaro.gamez@...ent.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v10 3/3] pwm: Add support for Xilinx AXI Timer



On 11/17/21 17:23, Thierry Reding wrote:
> On Fri, Nov 12, 2021 at 01:55:04PM -0500, Sean Anderson wrote:
>> This adds PWM support for Xilinx LogiCORE IP AXI soft timers commonly
>> found on Xilinx FPGAs. At the moment clock control is very basic: we
>> just enable the clock during probe and pin the frequency. In the future,
>> someone could add support for disabling the clock when not in use.
>>
>> Some common code has been specially demarcated. While currently only
>> used by the PWM driver, it is anticipated that it may be split off in
>> the future to be used by the timer driver as well.
>>
>> 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@...o.com>
>> ---
>>
>> Changes in v10:
>> - Fix compilation error in timer driver
>>
>> Changes in v9:
>> - Refactor "if { return } else if { }" to "if { return } if { }"
>> - Remove drivers/clocksource/timer-xilinx-common.c from MAINTAINERS
>> - Remove xilinx_timer_common_init and integrate it into xilinx_timer_probe
>>
>> Changes in v8:
>> - Drop new timer driver; it has been deferred for future series
>>
>> Changes in v7:
>> - Add dependency on OF_ADDRESS
>> - Fix period_cycles calculation
>> - Fix typo in limitations
>>
>> Changes in v6:
>> - Capitalize error messages
>> - Don't disable regmap locking to allow inspection of registers via
>>    debugfs
>> - Prevent overflow when calculating period_cycles
>> - Remove enabled variable from xilinx_pwm_apply
>> - Swap order of period_cycle range comparisons
>>
>> Changes in v5:
>> - Allow non-zero #pwm-cells
>> - Correctly set duty_cycle in get_state when TLR0=TLR1
>> - Elaborate on limitation section
>> - Perform some additional checks/rounding in apply_state
>> - Remove xlnx,axi-timer-2.0 compatible string
>> - Rework duty-cycle and period calculations with feedback from Uwe
>> - Switch to regmap to abstract endianness issues
>> - Use more verbose error messages
>>
>> Changes in v4:
>> - Don't use volatile in read/write replacements. Some arches have it and
>>    some don't.
>> - Put common timer properties into their own struct to better reuse
>>    code.
>> - Remove references to properties which are not good enough for Linux.
>>
>> Changes in v3:
>> - Add clockevent and clocksource support
>> - Remove old microblaze driver
>> - Rewrite probe to only use a device_node, since timers may need to be
>>    initialized before we have proper devices. This does bloat the code a bit
>>    since we can no longer rely on helpers such as dev_err_probe. We also
>>    cannot rely on device resources being free'd on failure, so we must free
>>    them manually.
>> - We now access registers through xilinx_timer_(read|write). This allows us
>>    to deal with endianness issues, as originally seen in the microblaze
>>    driver. CAVEAT EMPTOR: I have not tested this on big-endian!
>>
>> Changes in v2:
>> - Add comment describing device
>> - Add comment explaining why we depend on !MICROBLAZE
>> - Add dependencies on COMMON_CLK and HAS_IOMEM
>> - Cast dividends to u64 to avoid overflow
>> - Check for over- and underflow when calculating TLR
>> - Check range of xlnx,count-width
>> - Don't compile this module by default for arm64
>> - Don't set pwmchip.base to -1
>> - Ensure the clock is always running when the pwm is registered
>> - Remove debugfs file :l
>> - Rename TCSR_(SET|CLEAR) to TCSR_RUN_(SET|CLEAR)
>> - Report errors with dev_error_probe
>> - Set xilinx_pwm_ops.owner
>> - Use NSEC_TO_SEC instead of defining our own
>> - Use TCSR_RUN_MASK to check if the PWM is enabled, as suggested by Uwe
>>
>>   MAINTAINERS                        |   6 +
>>   arch/microblaze/kernel/timer.c     |   3 +
> 
> Michal,
> 
> do you mind giving an Acked-by for this part. It looks harmless enough,
> but just making sure you're aware of this.

That's fine for me.

Acked-by: Michal Simek <michal.simek@...inx.com>

Thanks,
Michal