lkml.org 
[lkml]   [2017]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] ASoC: mxs-saif: add mclk enable/disable ops
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.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2017-01-23 20:07    [W:0.048 / U:0.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site