> Subject: Re: [PATCH V3 3/4] clk: imx: add i.MX8M composite clk support > > On Fri, Aug 9, 2019 at 5:02 AM Peng Fan <peng....@nxp.com> wrote: > > > + * The clk are not binded to a dev, because it is part of composite > > + clk > > s/binded/bound
Fix in V4. > > > + * use composite clk to get dev > > + */ > > +static ulong imx8m_clk_composite_divider_set_rate(struct clk *clk, > > + unsigned long > rate) > > +{ > > + struct clk_divider *divider = (struct clk_divider > *)to_clk_divider(clk); > > + struct clk_composite *composite = (struct clk_composite > *)clk->data; > > + ulong parent_rate = clk_get_parent_rate(&composite->clk); > > + int prediv_value; > > + int div_value; > > + int ret; > > + u32 val; > > + > > + ret = imx8m_clk_composite_compute_dividers(rate, parent_rate, > > + > &prediv_value, &div_value); > > + if (ret) > > + return -EINVAL; > > What about returning ret directly instead? As wrote in commit log. The code was imported from Linux, I would not modify this. > > > > +struct clk *imx8m_clk_composite_flags(const char *name, > > + const char * const > *parent_names, > > + int num_parents, void > __iomem *reg, > > + unsigned long flags) { > > + struct clk *clk = ERR_PTR(-ENOMEM); > > + struct clk_divider *div = NULL; > > + struct clk_gate *gate = NULL; > > + struct clk_mux *mux = NULL; > > Why all these NULL assignments? Explained above. > > > + > > + mux = kzalloc(sizeof(*mux), GFP_KERNEL); > > + if (!mux) > > + goto fail; > > It would be better like this: Ditto > > mux = kzalloc(sizeof(*mux), GFP_KERNEL); > if (!mux) > return ERR_PTR(-ENOMEM); > > > + > > + mux->reg = reg; > > + mux->shift = PCG_PCS_SHIFT; > > + mux->mask = PCG_PCS_MASK; > > + mux->num_parents = num_parents; > > + mux->flags = flags; > > + mux->parent_names = parent_names; > > + > > + div = kzalloc(sizeof(*div), GFP_KERNEL); > > + if (!div) > > + goto fail; > > + > > + div->reg = reg; > > + div->shift = PCG_PREDIV_SHIFT; > > + div->width = PCG_PREDIV_WIDTH; > > + div->flags = CLK_DIVIDER_ROUND_CLOSEST | flags; > > + > > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > > + if (!gate) > > + goto fail; > > + > > + gate->reg = reg; > > + gate->bit_idx = PCG_CGC_SHIFT; > > + gate->flags = flags; > > + > > + clk = clk_register_composite(NULL, name, > > + parent_names, num_parents, > > + &mux->clk, &clk_mux_ops, > &div->clk, > > + > &imx8m_clk_composite_divider_ops, > > + &gate->clk, &clk_gate_ops, > flags); > > + if (IS_ERR(clk)) > > + goto fail; > > + > > + return clk; > > + > > +fail: > > + kfree(gate); > > + kfree(div); > > + kfree(mux); > > + return ERR_CAST(clk); > > ERR_CAST(clk) is only valid when for the IS_ERR(clk) path. > > Please rework the error handling. Ditto. Thanks, Peng. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot