On Thu, Dec 22, 2016 at 04:49:26PM +0000, Mans Rullgard wrote:
> int mxs_saif_put_mclk(unsigned int saif_id)
> {
> - struct mxs_saif *saif = mxs_saif[saif_id];
> - u32 stat;
> + struct clk *clk;
>
> - if (!saif)
> - return -EINVAL;
> + clk = clk_get(NULL, "mxs_saif_mclk");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
So, this *is* an in place refactoring but it's only half done in that
we don't have any followup patches that move the clk_get() to device
probe where it should be.
> +static void mxs_saif_mclk_disable(struct clk_hw *hw)
> +{
> + struct mxs_saif *saif = to_mxs_saif(hw);
> +
> + if (!saif->ongoing)
> + __raw_writel(BM_SAIF_CTRL_RUN,
> + saif->base + SAIF_CTRL + MXS_CLR_ADDR);
> +
> + saif->mclk_in_use = 0;
> +}
We silently ignore disables if the clock is in use? That seems error
prone. I'd expect us to at least warn in that case.
> +static unsigned long mxs_saif_mclk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + return clk_divider_ops.recalc_rate(hw, parent_rate);
> +}
Can't we just assign these ops directly? Having to write wrapper
functions like this looks like we're doing something wrong here.
signature.asc
Description: PGP signature
