On Mon, 9 Mar 2009 08:05:12 +0100
"Nurkkala Eero.An (EXT-Offcode/Oulu)" <ext-Eero.Nurkkala@xxxxxxxxx>
wrote:
> From: Eero Nurkkala <ext-eero.nurkkala@xxxxxxxxx>
>
> McBSP clocks are being double enabled in the event the
> McBSP is already active. Also, they are unnecessarily
> disabled when there's no active McBSP in use. Fix this
> phenomenom by enabling and disabling the clocks at a
> proper location.
>
> @@ -226,9 +226,6 @@ int omap_mcbsp_request(unsigned int id)
> if (mcbsp->pdata && mcbsp->pdata->ops &&
> mcbsp->pdata->ops->request) mcbsp->pdata->ops->request(id);
>
> - for (i = 0; i < mcbsp->num_clks; i++)
> - clk_enable(mcbsp->clks[i]);
> -
> spin_lock(&mcbsp->lock);
> if (!mcbsp->free) {
> dev_err(mcbsp->dev, "McBSP%d is currently in use\n",
> @@ -240,6 +237,9 @@ int omap_mcbsp_request(unsigned int id)
> mcbsp->free = 0;
> spin_unlock(&mcbsp->lock);
>
> + for (i = 0; i < mcbsp->num_clks; i++)
> + clk_enable(mcbsp->clks[i]);
> +
Valid fix.
> @@ -319,9 +319,6 @@ void omap_mcbsp_free(unsigned int id)
> if (mcbsp->pdata && mcbsp->pdata->ops &&
> mcbsp->pdata->ops->free) mcbsp->pdata->ops->free(id);
>
> - for (i = mcbsp->num_clks - 1; i >= 0; i--)
> - clk_disable(mcbsp->clks[i]);
> -
> spin_lock(&mcbsp->lock);
> if (mcbsp->free) {
> dev_err(mcbsp->dev, "McBSP%d was not reserved\n",
> @@ -333,6 +330,9 @@ void omap_mcbsp_free(unsigned int id)
> mcbsp->free = 1;
> spin_unlock(&mcbsp->lock);
>
> + for (i = mcbsp->num_clks - 1; i >= 0; i--)
> + clk_disable(mcbsp->clks[i]);
> +
Race here? See, McBSP is marked free before disabling the clocks and
thus omap_mcbsp_request can start enabling them. So should you keep
clock disabling after test for mcbsp->free as here now but mark McBSP
free only after clocks are disabled?
I think you should send this as an separate fix since this should go to
the upstream also.
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
