Messages in this thread | | | Subject | Re: [alsa-devel] [PATCH v7 2/5] clk: x86: Add Atom PMC platform clocks | From | Pierre-Louis Bossart <> | Date | Sat, 21 Jan 2017 10:28:17 -0600 |
| |
Thanks for the review Stephen.
On 1/20/17 5:58 PM, Stephen Boyd wrote: > On 01/17, Pierre-Louis Bossart wrote: >> diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c >> new file mode 100644 >> index 0000000..312d4e9 >> --- /dev/null >> +++ b/drivers/clk/x86/clk-pmc-atom.c > [...] >> + >> +static void plt_clk_reg_update(struct clk_plt *clk, u32 mask, u32 val) >> +{ >> + u32 tmp; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&clk->lock, flags); >> + >> + tmp = clk_readl(clk->reg); > > Do you need to use clk_readl? I'd prefer we deleted that > function/macro because it's just confusing. Please don't use it > unless you need it for some reason.
I just followed Andy's recommendation and will revert to readl/writel, as well as fix the nitpicks below > >> + tmp = (tmp & ~mask) | (val & mask); >> + clk_writel(tmp, clk->reg); >> + >> + spin_unlock_irqrestore(&clk->lock, flags); >> +} >> + > [..] >> + >> +static void plt_clk_unregister_parents(struct clk_plt_data *data) >> +{ >> + plt_clk_unregister_fixed_rate_loop(data, data->nparents); >> +} >> + >> + > > Nitpick: Single newline please
ok
>> +static struct platform_driver plt_clk_driver = { >> + .driver = { >> + .name = PLT_CLK_DRIVER_NAME, > > Nitpick: Just put the string here
ok
| |