mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
* [PATCH v1] clocksource/drivers/arm_global_timer: Simplify prescaler register access
@ 2024-02-21 21:43 Martin Blumenstingl
  2024-02-22 10:02 ` Daniel Lezcano
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Blumenstingl @ 2024-02-21 21:43 UTC (permalink / raw)
  To: daniel.lezcano, tglx
  Cc: linux-arm-kernel, linux-kernel, patrice.chotard, linux-amlogic,
	Martin Blumenstingl

Use GENMASK() to define the prescaler mask and make the whole driver use
the mask (together with helpers such as FIELD_GET and FIELD_PREP)
instead of needed an additional shift and max value constant.

Signed-off-by: Martin Blumenstingl <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..1eb91fa00657 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 || psv > FIELD_GET(GT_CONTROL_PRESCALER_MASK, ~0))
 			return NOTIFY_BAD;
 
 		/*
-- 
2.43.2


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] clocksource/drivers/arm_global_timer: Simplify prescaler register access
  2024-02-21 21:43 [PATCH v1] clocksource/drivers/arm_global_timer: Simplify prescaler register access Martin Blumenstingl
@ 2024-02-22 10:02 ` Daniel Lezcano
  2024-02-22 21:57   ` Martin Blumenstingl
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Lezcano @ 2024-02-22 10:02 UTC (permalink / raw)
  To: Martin Blumenstingl, tglx
  Cc: linux-arm-kernel, linux-kernel, patrice.chotard, linux-amlogic


Hi Martin,

thank you for providing these cleanups.

One question below:


On 21/02/2024 22:43, Martin Blumenstingl wrote:

[ ... ]

>   		/* prescaler within legal range? */
> -		if (psv < 0 || psv > GT_CONTROL_PRESCALER_MAX)
> +		if (psv < 0 || psv > FIELD_GET(GT_CONTROL_PRESCALER_MASK, ~0))

		FIELD_MAX() ?

[ ... ]

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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] clocksource/drivers/arm_global_timer: Simplify prescaler register access
  2024-02-22 10:02 ` Daniel Lezcano
@ 2024-02-22 21:57   ` Martin Blumenstingl
  2024-02-22 22:34     ` Daniel Lezcano
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Blumenstingl @ 2024-02-22 21:57 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: tglx, linux-arm-kernel, linux-kernel, patrice.chotard, linux-amlogic

Hi Daniel,

On Thu, Feb 22, 2024 at 11:02 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
[ ... ]
>
> >               /* prescaler within legal range? */
> > -             if (psv < 0 || psv > GT_CONTROL_PRESCALER_MAX)
> > +             if (psv < 0 || psv > FIELD_GET(GT_CONTROL_PRESCALER_MASK, ~0))
>
>                 FIELD_MAX() ?
Oh, I was not aware of FIELD_MAX() - thank you!
While researching that I found that there's also FIELD_FIT() which I
think is perfect here. What do you think?


Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] clocksource/drivers/arm_global_timer: Simplify prescaler register access
  2024-02-22 21:57   ` Martin Blumenstingl
@ 2024-02-22 22:34     ` Daniel Lezcano
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Lezcano @ 2024-02-22 22:34 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: tglx, linux-arm-kernel, linux-kernel, patrice.chotard, linux-amlogic

On 22/02/2024 22:57, Martin Blumenstingl wrote:
> Hi Daniel,
> 
> On Thu, Feb 22, 2024 at 11:02 AM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
> [ ... ]
>>
>>>                /* prescaler within legal range? */
>>> -             if (psv < 0 || psv > GT_CONTROL_PRESCALER_MAX)
>>> +             if (psv < 0 || psv > FIELD_GET(GT_CONTROL_PRESCALER_MASK, ~0))
>>
>>                  FIELD_MAX() ?
> Oh, I was not aware of FIELD_MAX() - thank you!
> While researching that I found that there's also FIELD_FIT() which I
> think is perfect here. What do you think?

Ah yes, even better :)

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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-22 22:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-21 21:43 [PATCH v1] clocksource/drivers/arm_global_timer: Simplify prescaler register access Martin Blumenstingl
2024-02-22 10:02 ` Daniel Lezcano
2024-02-22 21:57   ` Martin Blumenstingl
2024-02-22 22:34     ` Daniel Lezcano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®