| Message ID | 1541450716-25523-2-git-send-email-festevam@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] media: imx-pxp: Check the return value from clk_prepare_enable() | expand |
On Mon, 2018-11-05 at 18:45 -0200, Fabio Estevam wrote: > pxp_soft_reset() may fail with a timeout, so it is better to propagate > the error in this case. > > Signed-off-by: Fabio Estevam <festevam@gmail.com> > --- > Changes since v1: > - None > > drivers/media/platform/imx-pxp.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c > index 27780f1..b3700b8 100644 > --- a/drivers/media/platform/imx-pxp.c > +++ b/drivers/media/platform/imx-pxp.c > @@ -1607,7 +1607,7 @@ static const struct v4l2_m2m_ops m2m_ops = { > .job_abort = pxp_job_abort, > }; > > -static void pxp_soft_reset(struct pxp_dev *dev) > +static int pxp_soft_reset(struct pxp_dev *dev) > { > int ret; > u32 val; > @@ -1619,11 +1619,15 @@ static void pxp_soft_reset(struct pxp_dev *dev) > > ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val, > val & BM_PXP_CTRL_CLKGATE, 0, 100); > - if (ret < 0) > + if (ret < 0) { > pr_err("PXP reset timeout\n"); > + return ret; > + } > > writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR); I'm not sure if we should clear SFTRST again after a timeout. It probably doesn't matter as something went wrong anyway and the next probe will try to clear it again. > writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR); Clearing CLKGATE if it was not set by the SFTRST in time should have no effect, so we could do this unconditionally as well. > + > + return 0; So you could just "return ret;" here instead of breaking out above. I have no preference either way. > } > > static int pxp_probe(struct platform_device *pdev) > @@ -1670,7 +1674,9 @@ static int pxp_probe(struct platform_device *pdev) > if (ret < 0) > return ret; > > - pxp_soft_reset(dev); > + ret = pxp_soft_reset(dev); > + if (ret < 0) > + return ret; This should "goto err_clk;" instead, though. With that changed, Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c index 27780f1..b3700b8 100644 --- a/drivers/media/platform/imx-pxp.c +++ b/drivers/media/platform/imx-pxp.c @@ -1607,7 +1607,7 @@ static const struct v4l2_m2m_ops m2m_ops = { .job_abort = pxp_job_abort, }; -static void pxp_soft_reset(struct pxp_dev *dev) +static int pxp_soft_reset(struct pxp_dev *dev) { int ret; u32 val; @@ -1619,11 +1619,15 @@ static void pxp_soft_reset(struct pxp_dev *dev) ret = readl_poll_timeout(dev->mmio + HW_PXP_CTRL, val, val & BM_PXP_CTRL_CLKGATE, 0, 100); - if (ret < 0) + if (ret < 0) { pr_err("PXP reset timeout\n"); + return ret; + } writel(BM_PXP_CTRL_SFTRST, dev->mmio + HW_PXP_CTRL_CLR); writel(BM_PXP_CTRL_CLKGATE, dev->mmio + HW_PXP_CTRL_CLR); + + return 0; } static int pxp_probe(struct platform_device *pdev) @@ -1670,7 +1674,9 @@ static int pxp_probe(struct platform_device *pdev) if (ret < 0) return ret; - pxp_soft_reset(dev); + ret = pxp_soft_reset(dev); + if (ret < 0) + return ret; spin_lock_init(&dev->irqlock);
pxp_soft_reset() may fail with a timeout, so it is better to propagate the error in this case. Signed-off-by: Fabio Estevam <festevam@gmail.com> --- Changes since v1: - None drivers/media/platform/imx-pxp.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)