Re: [PATCH v3 7/9] clocksource: realtek: Add timer driver for rtl-otto platforms
From: Marek Behún
Date: Thu Jun 27 2024 - 07:17:22 EST
On Thu, 27 Jun 2024 16:33:15 +1200
Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx> wrote:
> +/* Simple internal register functions */
> +static inline void rttm_set_counter(void __iomem *base, unsigned int counter)
> +{
> + iowrite32(counter, base + RTTM_CNT);
These require #include <asm/io.h>
> +/* Aggregated control functions for kernel clock framework */
> +#define RTTM_DEBUG(base) \
> + pr_debug("------------- %d %p\n", \
> + smp_processor_id(), base)
#include <linux/printk.h>
> +static irqreturn_t rttm_timer_interrupt(int irq, void *dev_id)
> +{
> + struct clock_event_device *clkevt = dev_id;
> + struct timer_of *to = to_timer_of(clkevt);
> +
> + rttm_ack_irq(to->of_base.base);
> + RTTM_DEBUG(to->of_base.base);
> + clkevt->event_handler(clkevt);
Although you include "timer-of.h", which includes clockchips.h, please
do also explicit #include <linux/clockchips.h>
> + rttm_set_period(to->of_base.base, RTTM_TICKS_PER_SEC / HZ);
HZ -> linux/jiffies.h, or maybe asm/param.h
> +static u64 rttm_read_clocksource(struct clocksource *cs)
> +{
> + struct rttm_cs *rcs = container_of(cs, struct rttm_cs, cs);
> +
> + return (u64)rttm_get_counter(rcs->to.of_base.base);
Redundant cast to u64.
> + rttm_enable_timer(rcs->to.of_base.base, RTTM_CTRL_TIMER,
> + rcs->to.of_clk.rate / RTTM_TICKS_PER_SEC);
Is this correct? Sometimes it makes sense to use DIV_ROUND_CLOSEST, but
maybe not here.
> +static u64 notrace rttm_read_clock(void)
> +{
> + return (u64)rttm_get_counter(rttm_cs.to.of_base.base);
Redundant cast to u64.
> +static int __init rttm_probe(struct device_node *np)
> +{
> + int cpu, cpu_rollback;
unsigned int?
> + struct timer_of *to;
> + int clkidx = num_possible_cpus();
linux/cpumask.h, unsigned int
Marek