Hi Dan
Thank you for your feedback
if (IS_ERR(clk)) { dev_err(dev, "create null clk error\n");
return NULL;
return PTR_ERR(clk);Yes, I think this part is correct. If an error happens, then it should be reported to the user so they can fix it.
Good !
@@ -430,9 +430,9 @@ static int rsnd_adg_get_clkin(struct rsnd_priv *priv) for (i = 0; i < CLKMAX; i++) { clk = devm_clk_get(dev, clk_name[i]);
if (IS_ERR(clk))
if (IS_ERR_OR_NULL(clk)) clk = rsnd_adg_null_clk_get(priv);
if (IS_ERR(clk))
if (IS_ERR_OR_NULL(clk)) goto err;But this is not correct.
If a function like devm_clk_get() returns NULL, then it's not an error, it's something where the user deliberately chose to disable the feature. It shouldn't trigger an error message and the rest of the driver should be written to accomodate it.
adg->clk[i] = clk;So we should assign the NULL pointer here and add NULL checks to make sure that it doesn't lead to a NULL dereference.
Ah, in this driver, if it got error or NULL clk, it try to call rsnd_adg_null_clk_get() and use null_clk instead of NULL. In other words, all adg->clk[i] should not NULL. If one of them was NULL, it is error for this driver. If so, my suggested code was OK, I hope.
Thank you for your help !!
Best regards --- Kuninori Morimoto