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 <khoroshi...@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?
>
