linux-kernel - Re: [PATCH v3 7/9] clocksource: realtek: Add timer driver for rtl-otto platforms

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]

Date: Thu, 27 Jun 2024 13:17:07 +0200
From: Marek Behún <kabel@...nel.org>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>
Cc: tglx@...utronix.de, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, tsbogend@...ha.franken.de, daniel.lezcano@...aro.org,
 paulburton@...nel.org, peterz@...radead.org, mail@...ger-koblitz.de,
 bert@...t.com, john@...ozen.org, sander@...nheule.net,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-mips@...r.kernel.org, ericwouds@...il.com, Markus Stockhausen
 <markus.stockhausen@....de>
Subject: Re: [PATCH v3 7/9] clocksource: realtek: Add timer driver for
 rtl-otto platforms

On Thu, 27 Jun 2024 16:33:15 +1200
Chris Packham <chris.packham@...iedtelesis.co.nz> 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