lkml.org 
[lkml]   [2018]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] mailbox: imx: Fix clk handling in imx_mu_probe()
From
Date
Hi Oleksij,

By chance I took a look at another implementations:

arch/arm/mach-ep93xx/clock.c#L266

int clk_enable(struct clk *clk)
{
unsigned long flags;

if (!clk)
return -EINVAL;
...

arch/c6x/platforms/pll.c#L48

int clk_enable(struct clk *clk)
{
unsigned long flags;

if (clk == NULL || IS_ERR(clk))
return -EINVAL;

So, I am not sure the NULL resistance is a part of the clk_enable()
contract?

--
Alexey


On 17.12.2018 9:01, Oleksij Rempel wrote:
> Hi Alexey,
>
> On Sun, Dec 16, 2018 at 02:01:44AM +0300, Alexey Khoroshilov wrote:
>> Handling of devm_clk_get() suggests that the driver should support
>> lack of priv->clk. But imx_mu_probe() fails on clk_prepare_enable(NULL)
>> in that case.
>>
>> The patch removes the try to enable absent clk and adds error handling
>> for mbox_controller_register().
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
>> ---
>> drivers/mailbox/imx-mailbox.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
>> index 363d35d5e49d..ddde398f576e 100644
>> --- a/drivers/mailbox/imx-mailbox.c
>> +++ b/drivers/mailbox/imx-mailbox.c
>> @@ -292,10 +292,12 @@ static int imx_mu_probe(struct platform_device *pdev)
>> priv->clk = NULL;
>> }
>>
>> - ret = clk_prepare_enable(priv->clk);
>> - if (ret) {
>> - dev_err(dev, "Failed to enable clock\n");
>> - return ret;
>> + if (priv->clk) {
>> + ret = clk_prepare_enable(priv->clk);
>> + if (ret) {
>> + dev_err(dev, "Failed to enable clock\n");
>> + return ret;
>> + }
>> }
>>
>> for (i = 0; i < IMX_MU_CHANS; i++) {
>> @@ -324,7 +326,13 @@ static int imx_mu_probe(struct platform_device *pdev)
>>
>> imx_mu_init_generic(priv);
>>
>> - return mbox_controller_register(&priv->mbox);
>> + ret = mbox_controller_register(&priv->mbox);
>> + if (ret) {
>> + clk_disable_unprepare(priv->clk);
>> + return ret;
>> + }
>> +
>> + return 0;
>> }
>>
>> static int imx_mu_remove(struct platform_device *pdev)
>> --
>> 2.7.4
>>
>>
>
> NACK for this patch.
>
> Here are code snippets from clk framework:
> ============================================================================
> /* clk_prepare_enable helps cases using clk_enable in non-atomic context. */
> static inline int clk_prepare_enable(struct clk *clk)
> {
> int ret;
>
> ret = clk_prepare(clk);
> if (ret)
> return ret;
> ret = clk_enable(clk);
> if (ret)
> clk_unprepare(clk);
>
> return ret;
> }
>
> int clk_prepare(struct clk *clk)
> {
> if (!clk)
> return 0;
>
> return clk_core_prepare_lock(clk->core);
> }
>
> int clk_enable(struct clk *clk)
> {
> if (!clk)
> return 0;
>
> return clk_core_enable_lock(clk->core);
> }
> ============================================================================
>
> As you can see, complete code path is NULL resistant. Are you trying to
> fix some real issue, or it is a theoretical work?
>

\
 
 \ /
  Last update: 2018-12-17 08:24    [W:0.060 / U:0.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site