An error code returned by devm_clk_get() might have other meanings than "This clock doesn't exist". So use devm_clk_get_optional() and handle all remaining errors as fatal. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello, I sent this back in March and was asked to resend after the merge window closed. Actually I intended to do this earlier, but it got lost in my todo list and I found it just now. (And after all it is still after the merge window and http://vger.kernel.org/~davem/net-next.html claims it's still time ... :-) Best regards Uwe drivers/net/ethernet/freescale/fec_main.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 11227f51404c..2512b68d8545 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -3866,9 +3866,11 @@ fec_probe(struct platform_device *pdev) fep->itr_clk_rate = clk_get_rate(fep->clk_ahb); /* enet_out is optional, depends on board */ - fep->clk_enet_out = devm_clk_get(&pdev->dev, "enet_out"); - if (IS_ERR(fep->clk_enet_out)) - fep->clk_enet_out = NULL; + fep->clk_enet_out = devm_clk_get_optional(&pdev->dev, "enet_out"); + if (IS_ERR(fep->clk_enet_out)) { + ret = PTR_ERR(fep->clk_enet_out); + goto failed_clk; + } fep->ptp_clk_on = false; mutex_init(&fep->ptp_clk_mutex); base-commit: 3123109284176b1532874591f7c81f3837bbdc17 -- 2.35.1
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 11227f51404c..2512b68d8545 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3866,9 +3866,11 @@ fec_probe(struct platform_device *pdev)
> fep->itr_clk_rate = clk_get_rate(fep->clk_ahb);
>
> /* enet_out is optional, depends on board */
> - fep->clk_enet_out = devm_clk_get(&pdev->dev, "enet_out");
> - if (IS_ERR(fep->clk_enet_out))
> - fep->clk_enet_out = NULL;
> + fep->clk_enet_out = devm_clk_get_optional(&pdev->dev, "enet_out");
> + if (IS_ERR(fep->clk_enet_out)) {
> + ret = PTR_ERR(fep->clk_enet_out);
> + goto failed_clk;
> + }
>
> fep->ptp_clk_on = false;
> mutex_init(&fep->ptp_clk_mutex);
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
This is O.K, as far as it goes. But directly after this we have:
/* clk_ref is optional, depends on board */
fep->clk_ref = devm_clk_get(&pdev->dev, "enet_clk_ref");
if (IS_ERR(fep->clk_ref))
fep->clk_ref = NULL;
fep->clk_ref_rate = clk_get_rate(fep->clk_ref);
It would be good to do the same to this clock as well.
Andrew
An error code returned by devm_clk_get() might have other meanings than "This clock doesn't exist". So use devm_clk_get_optional() and handle all remaining errors as fatal. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello, as Andrew pointed out, there are two clocks that can benefit from better error handling during probe. So compared to (implicit) v1 this also cares for "enet_clk_ref". Best regards Uwe drivers/net/ethernet/freescale/fec_main.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 11227f51404c..907b379d2c83 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -3866,17 +3866,21 @@ fec_probe(struct platform_device *pdev) fep->itr_clk_rate = clk_get_rate(fep->clk_ahb); /* enet_out is optional, depends on board */ - fep->clk_enet_out = devm_clk_get(&pdev->dev, "enet_out"); - if (IS_ERR(fep->clk_enet_out)) - fep->clk_enet_out = NULL; + fep->clk_enet_out = devm_clk_get_optional(&pdev->dev, "enet_out"); + if (IS_ERR(fep->clk_enet_out)) { + ret = PTR_ERR(fep->clk_enet_out); + goto failed_clk; + } fep->ptp_clk_on = false; mutex_init(&fep->ptp_clk_mutex); /* clk_ref is optional, depends on board */ - fep->clk_ref = devm_clk_get(&pdev->dev, "enet_clk_ref"); - if (IS_ERR(fep->clk_ref)) - fep->clk_ref = NULL; + fep->clk_ref = devm_clk_get_optional(&pdev->dev, "enet_clk_ref"); + if (IS_ERR(fep->clk_ref)) { + ret = PTR_ERR(fep->clk_ref); + goto failed_clk; + } fep->clk_ref_rate = clk_get_rate(fep->clk_ref); /* clk_2x_txclk is optional, depends on board */ base-commit: 3123109284176b1532874591f7c81f3837bbdc17 -- 2.35.1
On Sat, May 21, 2022 at 10:34:25AM +0200, Uwe Kleine-König wrote:
> An error code returned by devm_clk_get() might have other meanings than
> "This clock doesn't exist". So use devm_clk_get_optional() and handle
> all remaining errors as fatal.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Sat, 21 May 2022 10:34:25 +0200 you wrote: > An error code returned by devm_clk_get() might have other meanings than > "This clock doesn't exist". So use devm_clk_get_optional() and handle > all remaining errors as fatal. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Hello, > > [...] Here is the summary with links: - [v2] net: fec: Do proper error checking for optional clks https://git.kernel.org/netdev/net/c/43252ed15f46 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Hello: This patch was applied to netdev/net-next.git (master) by David S. Miller <davem@davemloft.net>: On Fri, 20 May 2022 08:26:50 +0200 you wrote: > An error code returned by devm_clk_get() might have other meanings than > "This clock doesn't exist". So use devm_clk_get_optional() and handle > all remaining errors as fatal. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Hello, > > [...] Here is the summary with links: - [net-next,RESEND] net: fec: Do proper error checking for enet_out clk https://git.kernel.org/netdev/net-next/c/5ff851b7be75 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html