lkml.org 
[lkml]   [2021]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v7 2/3] clocksource: Rewrite Xilinx AXI timer driver
Date


On 9/24/21 2:52 AM, Michal Simek wrote:
> Dear Sean,
>
> On 9/16/21 8:05 PM, Sean Anderson wrote:
>> This rewrites the Xilinx AXI timer driver to be more platform agnostic.
>> Some common code has been split off so it can be reused. These routines
>> currently live in drivers/mfd. The largest changes are summarized below:
>>
>> - We now support any number of timer devices, possibly with only one
>> counter each. The first counter will be used as a clocksource. Every
>> other counter will be used as a clockevent. This allocation scheme was
>> chosen arbitrarily.
>> - We do not use timer_of_init because we need to perform some tasks in
>> between different stages. For example, we must ensure that ->read and
>> ->write are initialized before registering the irq. This can only happen
>> after we have gotten the register base (to detect endianness). We also
>> have a rather unusual clock initialization sequence in order to remain
>> backwards compatible. Due to this, it's ok for the initial clock request
>> to fail, and we do not want other initialization to be undone. Lastly, it
>> is more convenient to do one allocation for xilinx_clockevent_device than
>> to do one for timer_of and one for xilinx_timer_priv.
>> - We now pay attention to xlnx,count-width and handle smaller width timers.
>> The default remains 32.
>> - We access registers using regmap. This automatically deals with
>> endianness issues, so we no longer have to use our own wrappers. It
>> also provides locking for clockevents which have to worry about being
>> interrupted in the middle of a read/modify/write.
>>
>> Note that while the existing timer driver always sets the cpumask to cpu
>> 0, this version sets it to all possible CPUs. I believe this is correct
>> for multiprocessor systems where the timer is not physically wired to a
>> particular CPU's interrupt line. For uniprocessor systems (like most
>> microblaze systems) this makes no difference.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>> This has been tested on microblaze qemu.
>>
>> Changes in v7:
>> - Add dependency on OF_ADDRESS
>>
>> Changes in v6:
>> - Add __init* attributes
>> - Export common symbols
>> - Fix goto'ing incorrect label for cleanup
>> - Remove duplicate regmap_config
>> - Round to closest period in xilinx_timer_get_period to ensure proper
>> semantics for xilinx_pwm_get_state
>>
>> Changes in v5:
>> - Fix some overflows when setting the max value for clockevent and
>> sched_clock
>> - Just use clk_register_fixed_rate instead of the "private" version
>> - Remove duplicate register definitions
>> - Remove xilinx_timer_tlr_period
>> - Remove xlnx,axi-timer-2.0 compatible string
>> - Require that callers check arguments to xilinx_timer_tlr_cycles
>> - Use regmap to deal with endianness issues as suggested by Lee
>>
>> Changes in v4:
>> - Break out clock* drivers into their own file
>>
>> MAINTAINERS | 6 +
>> arch/microblaze/kernel/Makefile | 3 +-
>> arch/microblaze/kernel/timer.c | 326 ----------------------
>> drivers/clocksource/Kconfig | 13 +
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-xilinx-common.c | 71 +++++
>> drivers/clocksource/timer-xilinx.c | 323 +++++++++++++++++++++
>> include/clocksource/timer-xilinx.h | 91 ++++++
>> 8 files changed, 506 insertions(+), 328 deletions(-)
>> delete mode 100644 arch/microblaze/kernel/timer.c
>> create mode 100644 drivers/clocksource/timer-xilinx-common.c
>> create mode 100644 drivers/clocksource/timer-xilinx.c
>> create mode 100644 include/clocksource/timer-xilinx.h
>
>
> I have said it couple of times. I won't accept this in this form.
> I have no problem to move this driver out of microblaze. But I want to
> see transition from current state to new state and check it with baby
> steps which are bisectable if any problem happens.
> Because in this style we end in this patch and it will take some time to
> find out what it is failing.

Unfortunately, I do not have the time do do this at the moment. Because
these drivers are independent in nature, I propose to drop these changes
to the timer driver, but leave the common functions split out. In the
future, I (or you) may come back and make the changes in this patch in
an incremental fashion. The only change necessary for this driver would
be to check for #pwm-cells.

--Sean

\
 
 \ /
  Last update: 2021-10-06 00:08    [W:0.215 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site