Re: [PATCH 3/3] phy: mediatek: phy-mtk-mipi-dsi: Simplify with dev_err_probe()

[AngeloGioacchino Del Regno]

Il 06/01/22 10:13, Chunfeng Yun ha scritto:
> On Mon, 2022-01-03 at 15:53 +0100, AngeloGioacchino Del Regno wrote:
> > Use the dev_err_probe() helper to simplify error handling during
> > probe.
> >
> > Signed-off-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@xxxxxxxxxxxxx>
> > ---
> >   drivers/phy/mediatek/phy-mtk-mipi-dsi.c | 29 +++++++++------------
> > ----
> >   1 file changed, 10 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/phy/mediatek/phy-mtk-mipi-dsi.c
> > b/drivers/phy/mediatek/phy-mtk-mipi-dsi.c
> > index 6f7425b0bf5b..4b77508f5241 100644
> > --- a/drivers/phy/mediatek/phy-mtk-mipi-dsi.c
> > +++ b/drivers/phy/mediatek/phy-mtk-mipi-dsi.c
> > @@ -148,11 +148,9 @@ static int mtk_mipi_tx_probe(struct
> > platform_device *pdev)
> >   		return PTR_ERR(mipi_tx->regmap);
> >   
> >   	ref_clk = devm_clk_get(dev, NULL);
> > -	if (IS_ERR(ref_clk)) {
> > -		ret = PTR_ERR(ref_clk);
> > -		dev_err(dev, "Failed to get reference clock: %d\n",
> > ret);
> > -		return ret;
> > -	}
> > +	if (IS_ERR(ref_clk))
> > +		return dev_err_probe(dev, PTR_ERR(ref_clk),
> > +				     "Failed to get reference
> > clock\n");
> >   
> >   	ret = of_property_read_u32(dev->of_node, "drive-strength-
> > microamp",
> >   				   &mipi_tx->mipitx_drive);
> > @@ -172,27 +170,20 @@ static int mtk_mipi_tx_probe(struct
> > platform_device *pdev)
> >   
> >   	ret = of_property_read_string(dev->of_node, "clock-output-
> > names",
> >   				      &clk_init.name);
> > -	if (ret < 0) {
> > -		dev_err(dev, "Failed to read clock-output-names: %d\n",
> > ret);
> > -		return ret;
> > -	}
> > +	if (ret < 0)
> > +		return dev_err_probe(dev, ret, "Failed to read clock-
> > output-names\n");
> Seems no need change it here. (no EPROBE_DEFER error)

Hello Chunfeng,

pasting from kernel driver-api infrastructure guidelines:

[...]Note that it is deemed acceptable to use this function for error prints during 
probe even if the err is known to never be -EPROBE_DEFER. The benefit compared to a 
normal dev_err() is the standardized format of the error code and the fact that the 
error code is returned.

https://www.kernel.org/doc/html/latest/driver-api/infrastructure.html

Regards,
- Angelo

> Thanks
> >   
> >   	clk_init.ops = mipi_tx->driver_data->mipi_tx_clk_ops;
> >   
> >   	mipi_tx->pll_hw.init = &clk_init;
> >   	mipi_tx->pll = devm_clk_register(dev, &mipi_tx->pll_hw);
> > -	if (IS_ERR(mipi_tx->pll)) {
> > -		ret = PTR_ERR(mipi_tx->pll);
> > -		dev_err(dev, "Failed to register PLL: %d\n", ret);
> > -		return ret;
> > -	}
> > +	if (IS_ERR(mipi_tx->pll))
> > +		return dev_err_probe(dev, PTR_ERR(mipi_tx->pll),
> > "Cannot register PLL\n");
> >   
> >   	phy = devm_phy_create(dev, NULL, &mtk_mipi_tx_ops);
> > -	if (IS_ERR(phy)) {
> > -		ret = PTR_ERR(phy);
> > -		dev_err(dev, "Failed to create MIPI D-PHY: %d\n", ret);
> > -		return ret;
> > -	}
> > +	if (IS_ERR(phy))
> > +		return dev_err_probe(dev, PTR_ERR(phy), "Failed to
> > create MIPI D-PHY\n");
> > +
> >   	phy_set_drvdata(phy, mipi_tx);
> >   
> >   	phy_provider = devm_of_phy_provider_register(dev,
> > of_phy_simple_xlate);