Messages in this thread | | | Subject | Re: [PATCH] clocksource: Ingenic: Improve the code. | From | Zhou Yanjie <> | Date | Sat, 24 Jul 2021 23:38:31 +0800 |
| |
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) > <zhouyanjie@wanyeetech.com> a écrit : >> Use "FIELD_GET()" and "FIELD_PREP()" to simplify the code. >> >> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >> --- >> 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) <zhouyanjie@wanyeetech.com> >> */ >> >> +#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.
> >> 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 > >> writel(val, ost_clk->ost->base + info->ostccr_reg); >> >> return 0; >> -- >> 2.7.4 >> >
| |