Hi Paul,

On 2021/7/24 下午7:54, Paul Cercueil wrote:

Hi Zhou,

Thanks! I actually had a similar patch locally that I was eventually 
going to send.


Le sam., juil. 24 2021 at 18:10:07 +0800, 周琰杰 (Zhou Yanjie) 
[off-list ref] a écrit :

quoted

Use "FIELD_GET()" and "FIELD_PREP()" to simplify the code.

Signed-off-by: 周琰杰 (Zhou Yanjie) <redacted>
---
 drivers/clocksource/ingenic-sysost.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/ingenic-sysost.c 

b/drivers/clocksource/ingenic-sysost.c
index a129840..1fbea59 100644

--- a/drivers/clocksource/ingenic-sysost.c
+++ b/drivers/clocksource/ingenic-sysost.c

@@ -4,6 +4,7 @@

  * Copyright (c) 2020 周琰杰 (Zhou Yanjie) [off-list ref]
  */

+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>

@@ -34,8 +35,6 @@

 /* bits within the OSTCCR register */
 #define OSTCCR_PRESCALE1_MASK    0x3
 #define OSTCCR_PRESCALE2_MASK    0xc
-#define OSTCCR_PRESCALE1_LSB    0
-#define OSTCCR_PRESCALE2_LSB    2

 /* bits within the OSTCR register */
 #define OSTCR_OST1CLR            BIT(0)

@@ -98,7 +97,7 @@ static unsigned long 

ingenic_ost_percpu_timer_recalc_rate(struct clk_hw *hw,

     prescale = readl(ost_clk->ost->base + info->ostccr_reg);

-    prescale = (prescale & OSTCCR_PRESCALE1_MASK) >> 
OSTCCR_PRESCALE1_LSB;
+    prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);

     return parent_rate >> (prescale * 2);
 }

@@ -112,7 +111,7 @@ static unsigned long 

ingenic_ost_global_timer_recalc_rate(struct clk_hw *hw,

     prescale = readl(ost_clk->ost->base + info->ostccr_reg);

-    prescale = (prescale & OSTCCR_PRESCALE2_MASK) >> 
OSTCCR_PRESCALE2_LSB;
+    prescale = FIELD_GET(OSTCCR_PRESCALE2_MASK, prescale);

     return parent_rate >> (prescale * 2);
 }

@@ -151,7 +150,7 @@ static int 

ingenic_ost_percpu_timer_set_rate(struct clk_hw *hw, unsigned long re
     int val;

     val = readl(ost_clk->ost->base + info->ostccr_reg);
-    val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale << 
OSTCCR_PRESCALE1_LSB);
+    val = (val & ~OSTCCR_PRESCALE1_MASK) | 
FIELD_PREP(OSTCCR_PRESCALE1_MASK, prescale);

Just one nitpick, I'd prefer this:

val ~= &OSTCCR_PRESCALE1_MASK;
val |= FIELD_PREP(OSTCCR_PRESCALE1_MASK, prescale);

Sure, I will change it in the next version.

quoted

     writel(val, ost_clk->ost->base + info->ostccr_reg);

     return 0;

@@ -166,7 +165,7 @@ static int 

ingenic_ost_global_timer_set_rate(struct clk_hw *hw, unsigned long re
     int val;

     val = readl(ost_clk->ost->base + info->ostccr_reg);
-    val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale << 
OSTCCR_PRESCALE2_LSB);
+    val = (val & ~OSTCCR_PRESCALE2_MASK) | 
FIELD_PREP(OSTCCR_PRESCALE2_MASK, prescale);

Same here.

Sure.


Thanks and  best regards!

Cheers,
-Paul

quoted

     writel(val, ost_clk->ost->base + info->ostccr_reg);

     return 0;
-- 
2.7.4