[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eb54b791-05d7-4eee-825d-373cc78df689@linaro.org>
Date: Sat, 24 Feb 2024 22:55:19 +0100
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
tglx@...utronix.de
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
patrice.chotard@...s.st.com, linux-amlogic@...ts.infradead.org
Subject: Re: [PATCH v2] clocksource/drivers/arm_global_timer: Simplify
prescaler register access
On 24/02/2024 22:35, Martin Blumenstingl wrote:
> Use GENMASK() to define the prescaler mask and make the whole driver use
> the mask (together with helpers such as FIELD_{GET,PREP,FIT}) instead of
> needing an additional shift and max value constant.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
> ---
> Changes from v1 at [0]:
> - use FIELD_FIT() to check whether psv overflows the register
> - update the description accordingly
>
>
> [0] https://lore.kernel.org/lkml/20240221214348.2299636-1-martin.blumenstingl@googlemail.com/
>
>
> drivers/clocksource/arm_global_timer.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
> index 8dd1e46b7176..49b094a20717 100644
> --- a/drivers/clocksource/arm_global_timer.c
> +++ b/drivers/clocksource/arm_global_timer.c
> @@ -9,6 +9,7 @@
>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> +#include <linux/bitfield.h>
> #include <linux/clocksource.h>
> #include <linux/clockchips.h>
> #include <linux/cpu.h>
> @@ -31,10 +32,7 @@
> #define GT_CONTROL_COMP_ENABLE BIT(1) /* banked */
> #define GT_CONTROL_IRQ_ENABLE BIT(2) /* banked */
> #define GT_CONTROL_AUTO_INC BIT(3) /* banked */
> -#define GT_CONTROL_PRESCALER_SHIFT 8
> -#define GT_CONTROL_PRESCALER_MAX 0xFF
> -#define GT_CONTROL_PRESCALER_MASK (GT_CONTROL_PRESCALER_MAX << \
> - GT_CONTROL_PRESCALER_SHIFT)
> +#define GT_CONTROL_PRESCALER_MASK GENMASK(15, 8)
>
> #define GT_INT_STATUS 0x0c
> #define GT_INT_STATUS_EVENT_FLAG BIT(0)
> @@ -247,7 +245,7 @@ static void gt_write_presc(u32 psv)
>
> reg = readl(gt_base + GT_CONTROL);
> reg &= ~GT_CONTROL_PRESCALER_MASK;
> - reg |= psv << GT_CONTROL_PRESCALER_SHIFT;
> + reg |= FIELD_PREP(GT_CONTROL_PRESCALER_MASK, psv);
> writel(reg, gt_base + GT_CONTROL);
> }
>
> @@ -256,8 +254,7 @@ static u32 gt_read_presc(void)
> u32 reg;
>
> reg = readl(gt_base + GT_CONTROL);
> - reg &= GT_CONTROL_PRESCALER_MASK;
> - return reg >> GT_CONTROL_PRESCALER_SHIFT;
> + return FIELD_GET(GT_CONTROL_PRESCALER_MASK, reg);
> }
>
> static void __init gt_delay_timer_init(void)
> @@ -272,9 +269,9 @@ static int __init gt_clocksource_init(void)
> writel(0, gt_base + GT_COUNTER0);
> writel(0, gt_base + GT_COUNTER1);
> /* set prescaler and enable timer on all the cores */
> - writel(((CONFIG_ARM_GT_INITIAL_PRESCALER_VAL - 1) <<
> - GT_CONTROL_PRESCALER_SHIFT)
> - | GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL);
> + writel(FIELD_PREP(GT_CONTROL_PRESCALER_MASK,
> + CONFIG_ARM_GT_INITIAL_PRESCALER_VAL - 1) |
> + GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL);
>
> #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> sched_clock_register(gt_sched_clock_read, 64, gt_target_rate);
> @@ -301,7 +298,7 @@ static int gt_clk_rate_change_cb(struct notifier_block *nb,
> psv--;
>
> /* prescaler within legal range? */
> - if (psv < 0 || psv > GT_CONTROL_PRESCALER_MAX)
> + if (psv < 0 || !FIELD_FIT(GT_CONTROL_PRESCALER_MASK, psv))
> return NOTIFY_BAD;
Won't FIELD_FIT cover psv < 0 also ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Powered by blists - more mailing lists