| Message ID | 20230912045157.177966-9-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add new Renesas RZ/G3S SoC and RZ/G3S SMARC EVK | expand |
On 9/12/23 7:51 AM, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Initial value of CPG_PL2SDHI_DSEL bits 0..1 or 4..6 is 01b. Hardware user's > manual (r01uh0914ej0130-rzg2l-rzg2lc.pdf) specifies that setting 0 is > prohibited. The rzg2l_cpg_sd_clk_mux_get_parent() should just read > CPG_PL2SDHI_DSEL, trust the value and return the proper clock parent index > based on the read value. Do this. > > Fixes: eaff33646f4cb ("clk: renesas: rzg2l: Add SDHI clk mux support") > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > drivers/clk/renesas/rzg2l-cpg.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c > index 1195d4b1f545..d0d086d6dc51 100644 > --- a/drivers/clk/renesas/rzg2l-cpg.c > +++ b/drivers/clk/renesas/rzg2l-cpg.c > @@ -239,14 +239,8 @@ static u8 rzg2l_cpg_sd_clk_mux_get_parent(struct clk_hw *hw) > > val >>= GET_SHIFT(hwdata->conf); > val &= GENMASK(GET_WIDTH(hwdata->conf) - 1, 0); > - if (val) { > - val--; > - } else { > - /* Prohibited clk source, change it to 533 MHz(reset value) */ > - rzg2l_cpg_sd_clk_mux_set_parent(hw, 0); > - } > > - return val; > + return val ? --val : val; return val ? val - 1 : 0;
On Tue, Sep 12, 2023 at 6:43 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > On 9/12/23 7:51 AM, Claudiu wrote: > > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > > > Initial value of CPG_PL2SDHI_DSEL bits 0..1 or 4..6 is 01b. Hardware user's > > manual (r01uh0914ej0130-rzg2l-rzg2lc.pdf) specifies that setting 0 is > > prohibited. The rzg2l_cpg_sd_clk_mux_get_parent() should just read > > CPG_PL2SDHI_DSEL, trust the value and return the proper clock parent index > > based on the read value. Do this. > > > > Fixes: eaff33646f4cb ("clk: renesas: rzg2l: Add SDHI clk mux support") > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > --- a/drivers/clk/renesas/rzg2l-cpg.c > > +++ b/drivers/clk/renesas/rzg2l-cpg.c > > @@ -239,14 +239,8 @@ static u8 rzg2l_cpg_sd_clk_mux_get_parent(struct clk_hw *hw) > > > > val >>= GET_SHIFT(hwdata->conf); > > val &= GENMASK(GET_WIDTH(hwdata->conf) - 1, 0); > > - if (val) { > > - val--; > > - } else { > > - /* Prohibited clk source, change it to 533 MHz(reset value) */ > > - rzg2l_cpg_sd_clk_mux_set_parent(hw, 0); > > - } > > > > - return val; > > + return val ? --val : val; > > return val ? val - 1 : 0; Definitely, mixing multiple users of the same variable and pre-decrement is ill-defined. Gr{oetje,eeting}s, Geert
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index 1195d4b1f545..d0d086d6dc51 100644 --- a/drivers/clk/renesas/rzg2l-cpg.c +++ b/drivers/clk/renesas/rzg2l-cpg.c @@ -239,14 +239,8 @@ static u8 rzg2l_cpg_sd_clk_mux_get_parent(struct clk_hw *hw) val >>= GET_SHIFT(hwdata->conf); val &= GENMASK(GET_WIDTH(hwdata->conf) - 1, 0); - if (val) { - val--; - } else { - /* Prohibited clk source, change it to 533 MHz(reset value) */ - rzg2l_cpg_sd_clk_mux_set_parent(hw, 0); - } - return val; + return val ? --val : val; } static const struct clk_ops rzg2l_cpg_sd_clk_mux_ops = {