Hi Frank, Am Dienstag, 16. Dezember 2025, 15:46:00 CET schrieb Frank Li: > On Tue, Dec 16, 2025 at 08:34:18AM +0100, Alexander Stein wrote: > > Hi, > > > > Am Montag, 15. Dezember 2025, 23:49:53 CET schrieb Frank Li: > > > Use dev_err_probe() to simplify the code. Drop the explicit error message > > > after returning from imx8mq_mipi_csi_parse_dt(), as the error is already > > > reported by this helper. > > > > > > No functional change. > > > > > > Reviewed-by: Laurent Pinchart > > > Signed-off-by: Frank Li > > > --- > > > change in v2: > > > - add Laurent Pinchart review tags > > > - wrap error message to new line > > > - remove error code print in dev_err_probe(); > > > - drop error messaage at imx8mq_mipi_csi_parse_dt() > > > --- > > > drivers/media/platform/nxp/imx-mipi-csis.c | 6 ++--- > > > drivers/media/platform/nxp/imx7-media-csi.c | 14 ++++-------- > > > drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 33 ++++++++++++--------------- > > > 3 files changed, 21 insertions(+), 32 deletions(-) > > > > > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c > > > index 088b2945aee33731c565f049dd17721356300b84..ce93d868746f002c22e2f86b1e0aa84ec1a76061 100644 > > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c > > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c > > > @@ -1547,10 +1547,8 @@ static int mipi_csis_probe(struct platform_device *pdev) > > > /* Now that the hardware is initialized, request the interrupt. */ > > > ret = devm_request_irq(dev, irq, mipi_csis_irq_handler, 0, > > > dev_name(dev), csis); > > > - if (ret) { > > > - dev_err(dev, "Interrupt request failed\n"); > > > - return ret; > > > - } > > > + if (ret) > > > + return dev_err_probe(dev, ret, "Interrupt request failed\n"); > > > > > > /* Initialize and register the subdev. */ > > > ret = mipi_csis_subdev_init(csis); > > > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c > > > index 933a5f39f9f4c9b43ca8d2a1819d0145981266e6..7ddc7ba06e3d4e007013821f67d783898a15461f 100644 > > > --- a/drivers/media/platform/nxp/imx7-media-csi.c > > > +++ b/drivers/media/platform/nxp/imx7-media-csi.c > > > @@ -2218,11 +2218,9 @@ static int imx7_csi_probe(struct platform_device *pdev) > > > > > > /* Acquire resources and install interrupt handler. */ > > > csi->mclk = devm_clk_get(&pdev->dev, "mclk"); > > > - if (IS_ERR(csi->mclk)) { > > > - ret = PTR_ERR(csi->mclk); > > > - dev_err(dev, "Failed to get mclk: %d", ret); > > > - return ret; > > > - } > > > + if (IS_ERR(csi->mclk)) > > > + return dev_err_probe(dev, PTR_ERR(csi->mclk), > > > + "Failed to get mclk\n"); > > > > > > csi->irq = platform_get_irq(pdev, 0); > > > if (csi->irq < 0) > > > @@ -2236,10 +2234,8 @@ static int imx7_csi_probe(struct platform_device *pdev) > > > > > > ret = devm_request_irq(dev, csi->irq, imx7_csi_irq_handler, 0, "csi", > > > (void *)csi); > > > - if (ret < 0) { > > > - dev_err(dev, "Request CSI IRQ failed.\n"); > > > - return ret; > > > - } > > > + if (ret < 0) > > > + return dev_err_probe(dev, ret, "Request CSI IRQ failed.\n"); > > > > > > /* Initialize all the media device infrastructure. */ > > > ret = imx7_csi_media_init(csi); > > > diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c > > > index 0851f4a9ae52d3096f454da643cfdc5017e000b1..a007c582b4d91660a97910a6a0d53c9b6fcd73e9 100644 > > > --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c > > > +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c > > > @@ -951,10 +951,9 @@ static int imx8mq_mipi_csi_parse_dt(struct csi_state *state) > > > int ret = 0; > > > > > > state->rst = devm_reset_control_array_get_exclusive(dev); > > > - if (IS_ERR(state->rst)) { > > > - dev_err(dev, "Failed to get reset: %pe\n", state->rst); > > > - return PTR_ERR(state->rst); > > > - } > > > + if (IS_ERR(state->rst)) > > > + return dev_err_probe(dev, PTR_ERR(state->rst), > > > + "Failed to get reset\n"); > > > > > > if (state->pdata->use_reg_csr) { > > > const struct regmap_config regmap_config = { > > > @@ -977,24 +976,22 @@ static int imx8mq_mipi_csi_parse_dt(struct csi_state *state) > > > > > > ret = of_property_read_u32_array(np, "fsl,mipi-phy-gpr", out_val, > > > ARRAY_SIZE(out_val)); > > > - if (ret) { > > > - dev_err(dev, "no fsl,mipi-phy-gpr property found: %d\n", ret); > > > - return ret; > > > - } > > > + if (ret) > > > + return dev_err_probe(dev, ret, "no %s property found\n", > > > + "fsl,mipi-phy-gpr"); > > > > Do you really need to pass the (fixed) property name as an argument? Why > > not move into the const string directly? I would also rephrase it a bit: > > "property fsl,mipi-phy-gpr not found" > > Laurent Pinchart provide comments at previous version. The same > "fsl,mipi-phy-gpr" share one entry at RO region. slice save RO region > space. > > https://lore.kernel.org/imx/20251119043307.GH17526@pendragon.ideasonboard.com/ "assuming it's worth it". Do you have any numbers? I doubt there is a significant benefit, but I don't have strong opinion here. But the grammar should still be improved. Best regards Alexander > Frank > > > > > With that fixed: > > Reviewed-by: Alexander Stein > > > > > > > > ph = *out_val; > > > > > > node = of_find_node_by_phandle(ph); > > > - if (!node) { > > > - dev_err(dev, "Error finding node by phandle\n"); > > > - return -ENODEV; > > > - } > > > + if (!node) > > > + return dev_err_probe(dev, -ENODEV, > > > + "Error finding node by phandle\n"); > > > + > > > state->phy_gpr = syscon_node_to_regmap(node); > > > of_node_put(node); > > > - if (IS_ERR(state->phy_gpr)) { > > > - dev_err(dev, "failed to get gpr regmap: %pe\n", state->phy_gpr); > > > - return PTR_ERR(state->phy_gpr); > > > - } > > > + if (IS_ERR(state->phy_gpr)) > > > + return dev_err_probe(dev, PTR_ERR(state->phy_gpr), > > > + "failed to get gpr regmap\n"); > > > > > > state->phy_gpr_reg = out_val[1]; > > > dev_dbg(dev, "phy gpr register set to 0x%x\n", state->phy_gpr_reg); > > > @@ -1017,10 +1014,8 @@ static int imx8mq_mipi_csi_probe(struct platform_device *pdev) > > > state->pdata = of_device_get_match_data(dev); > > > > > > ret = imx8mq_mipi_csi_parse_dt(state); > > > - if (ret < 0) { > > > - dev_err(dev, "Failed to parse device tree: %d\n", ret); > > > + if (ret < 0) > > > return ret; > > > - } > > > > > > /* Acquire resources. */ > > > state->regs = devm_platform_ioremap_resource(pdev, 0); > > > > > > > > > > > > -- > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > Amtsgericht München, HRB 105018 > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > http://www.tq-group.com/ > > > -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider http://www.tq-group.com/