Messages in this thread | | | Subject | Re: [PATCH v2 4/4] clk: meson: gxbb-aoclk: Add CEC 32k clock | From | Chris Moore <> | Date | Sat, 29 Jul 2017 10:04:39 +0200 |
| |
Hi,
Sorry I forgot to reply to all in my previous post :( I hope this corrects things.
Le 28/07/2017 à 11:53, Neil Armstrong a écrit :
[snip]
> +static long aoclk_cec_32k_round_rate(struct clk_hw *hw, unsigned long > rate, > + unsigned long *prate) > +{ > + const struct cec_32k_freq_table *freq = find_cec_32k_freq(rate, > + *prate); > + > + /* If invalid return first one */ > + if (!freq) > + return freq[0].target_rate;
Wouldn't this dereference a null pointer (or am I being stupid this morning)?
> + > + return freq->target_rate; > +} > + > +static int aoclk_cec_32k_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + const struct cec_32k_freq_table *freq = find_cec_32k_freq(rate, > + parent_rate); > + struct aoclk_cec_32k *cec_32k = to_aoclk_cec_32k(hw); > + u32 reg = 0; > + > + if (!freq) > + return -EINVAL; > + > + /* Disable clock */ > + regmap_update_bits(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0, > + CLK_CNTL0_IN_GATE_EN | CLK_CNTL0_OUT_GATE_EN, 0); > + > + if (freq->dualdiv) > + reg = CLK_CNTL0_DUALDIV_EN | > + FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1) | > + FIELD_PREP(CLK_CNTL0_N2_MASK, freq->n2 - 1); > + else > + reg = FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1); > +
Suggestion:
+ reg = FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1); + if (freq->dualdiv) + reg |= CLK_CNTL0_DUALDIV_EN | + FIELD_PREP(CLK_CNTL0_N2_MASK, freq->n2 - 1);
is shorter but maybe generates the same code.
> + regmap_write(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0, reg); > + > + if (freq->dualdiv) > + reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1) | > + FIELD_PREP(CLK_CNTL1_M2_MASK, freq->m2 - 1); > + else > + reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1); > +
Idem:
+ reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1); + if (freq->dualdiv) + reg |= FIELD_PREP(CLK_CNTL1_M2_MASK, freq->m2 - 1);
Cheers, Chris
| |