| Message ID | 20230507214035.3266438-1-lorenz@brun.one (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: mtk_eth_soc: log clock enable errors | expand |
On Sun, 2023-05-07 at 23:40 +0200, Lorenz Brun wrote: > Currently errors in clk_prepare_enable are silently swallowed. > Add a log stating which clock failed to be enabled and what the error > code was. > > Signed-off-by: Lorenz Brun <lorenz@brun.one> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index e14050e17862..ca66a573cfcb 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -3445,8 +3445,10 @@ static int mtk_clk_enable(struct mtk_eth *eth) > > for (clk = 0; clk < MTK_CLK_MAX ; clk++) { > ret = clk_prepare_enable(eth->clks[clk]); > - if (ret) > + if (ret) { > + dev_err(eth->dev, "enabling clock %s failed with error %d\n", mtk_clks_source_name[clk], ret); I'm sorry for nit-picking, but this lines really exceed any reasonable max len. Please reformat the above as: dev_err(eth->dev, "enabling clock %s failed with error %d\n", mtk_clks_source_name[clk], ret); Thanks! Paolo
On Sun, May 07, 2023 at 11:40:35PM +0200, Lorenz Brun wrote: > Currently errors in clk_prepare_enable are silently swallowed. > Add a log stating which clock failed to be enabled and what the error > code was. > > Signed-off-by: Lorenz Brun <lorenz@brun.one> Hi Lorenz, I think this would be targeted at net-next, and thus that should be noted in the subject. [PATCH net-next] ... Link: https://kernel.org/doc/html/latest/process/maintainer-netdev.html > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index e14050e17862..ca66a573cfcb 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -3445,8 +3445,10 @@ static int mtk_clk_enable(struct mtk_eth *eth) > > for (clk = 0; clk < MTK_CLK_MAX ; clk++) { > ret = clk_prepare_enable(eth->clks[clk]); > - if (ret) > + if (ret) { > + dev_err(eth->dev, "enabling clock %s failed with error %d\n", mtk_clks_source_name[clk], ret); I see that some error logs are generated by clk_prepare_enable(). Is it common practice to also log in the caller, as you are doing above? (Genuine question, I don't know.) > goto err_disable_clks; > + } > } > > return 0;
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index e14050e17862..ca66a573cfcb 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -3445,8 +3445,10 @@ static int mtk_clk_enable(struct mtk_eth *eth) for (clk = 0; clk < MTK_CLK_MAX ; clk++) { ret = clk_prepare_enable(eth->clks[clk]); - if (ret) + if (ret) { + dev_err(eth->dev, "enabling clock %s failed with error %d\n", mtk_clks_source_name[clk], ret); goto err_disable_clks; + } } return 0;
Currently errors in clk_prepare_enable are silently swallowed. Add a log stating which clock failed to be enabled and what the error code was. Signed-off-by: Lorenz Brun <lorenz@brun.one> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)