[PATCH] ASoC: samsung: i2s: Check before clk_unregister() not needed
clk_unregister() already checks the clk ptr using !clk || WARN_ON_ONCE(IS_ERR(clk)) so there is no need to check it again before calling it.
Signed-off-by: Yihao Han hanyihao@vivo.com --- sound/soc/samsung/i2s.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index 70c827162be4..84e21f7fc179 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -1247,8 +1247,7 @@ static void i2s_unregister_clocks(struct samsung_i2s_priv *priv) int i;
for (i = 0; i < priv->clk_data.clk_num; i++) { - if (!IS_ERR(priv->clk_table[i])) - clk_unregister(priv->clk_table[i]); + clk_unregister(priv->clk_table[i]); } }
On 27/05/2022 08:54, Yihao Han wrote:
clk_unregister() already checks the clk ptr using !clk || WARN_ON_ONCE(IS_ERR(clk)) so there is no need to check it again before calling it.
No, this explanation does not make sense. clk_unregister() warns and this code is not equivalent.
Best regards, Krzysztof
Le 29/05/2022 à 10:06, Krzysztof Kozlowski a écrit :
On 27/05/2022 08:54, Yihao Han wrote:
clk_unregister() already checks the clk ptr using !clk || WARN_ON_ONCE(IS_ERR(clk)) so there is no need to check it again before calling it.
No, this explanation does not make sense. clk_unregister() warns and this code is not equivalent.
Best regards, Krzysztof
Hi,
Moreover, as pointed out by greg in [1] on some plateform the assertion in the commit description is wrong. His message is about clk_disable() but, IIUC, it makes sense for clk_unregister() as well. See [2] on the sh plateform.
CJ
[1]: https://lore.kernel.org/all/YqMIUOTU%2Fk5XpW3I@kroah.com/ [2]: https://elixir.bootlin.com/linux/v5.18.3/source/drivers/sh/clk/core.c#L452
On 10/06/2022 18:15, Christophe JAILLET wrote:
Le 29/05/2022 à 10:06, Krzysztof Kozlowski a écrit :
On 27/05/2022 08:54, Yihao Han wrote:
clk_unregister() already checks the clk ptr using !clk || WARN_ON_ONCE(IS_ERR(clk)) so there is no need to check it again before calling it.
No, this explanation does not make sense. clk_unregister() warns and this code is not equivalent.
Best regards, Krzysztof
Hi,
Moreover, as pointed out by greg in [1] on some plateform the assertion in the commit description is wrong. His message is about clk_disable() but, IIUC, it makes sense for clk_unregister() as well. See [2] on the sh plateform.
Yes, this is true as well, although does not have the practical impact on this driver as it uses platforms with common clock framework.
Best regards, Krzysztof
participants (3)
-
Christophe JAILLET -
Krzysztof Kozlowski -
Yihao Han