Re: [PATCH v7 2/3] clocksource: Rewrite Xilinx AXI timer driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxx>
---
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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux