| Message ID | 20220128133649.1393201-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: imx8mq-mipi_csi2: Remove unneeded code | expand |
On 28.01.2022 18:36:49, Muhammad Usama Anjum wrote: > ret is constant in imx8mq_mipi_csi_pm_suspend(). This function cannot > return error. Remove the return variable. Simplify other functions which > are using this function. > > Fixes: f0c2ba1ed4ad ("media: imx: imx8mq-mipi_csi2: fix system resume") > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > drivers/staging/media/imx/imx8mq-mipi-csi2.c | 16 ++++------------ > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > index 3b9fa75efac6b..c992b845e63d1 100644 > --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c [...] > static int imx8mq_mipi_csi_pm_resume(struct device *dev) > @@ -742,15 +739,12 @@ static int __maybe_unused imx8mq_mipi_csi_suspend(struct device *dev) > { > struct v4l2_subdev *sd = dev_get_drvdata(dev); > struct csi_state *state = mipi_sd_to_csi2_state(sd); > - int ret; > > - ret = imx8mq_mipi_csi_pm_suspend(dev); > - if (ret) > - return ret; > + imx8mq_mipi_csi_pm_suspend(dev); > > state->state |= ST_SUSPENDED; what about this flag? Marc > > - return ret; > + return 0; > }
On 1/28/22 6:52 PM, Marc Kleine-Budde wrote: > On 28.01.2022 18:36:49, Muhammad Usama Anjum wrote: >> ret is constant in imx8mq_mipi_csi_pm_suspend(). This function cannot >> return error. Remove the return variable. Simplify other functions which >> are using this function. >> >> Fixes: f0c2ba1ed4ad ("media: imx: imx8mq-mipi_csi2: fix system resume") >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >> --- >> drivers/staging/media/imx/imx8mq-mipi-csi2.c | 16 ++++------------ >> 1 file changed, 4 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c >> index 3b9fa75efac6b..c992b845e63d1 100644 >> --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c >> +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > [...] >> static int imx8mq_mipi_csi_pm_resume(struct device *dev) >> @@ -742,15 +739,12 @@ static int __maybe_unused imx8mq_mipi_csi_suspend(struct device *dev) >> { >> struct v4l2_subdev *sd = dev_get_drvdata(dev); >> struct csi_state *state = mipi_sd_to_csi2_state(sd); >> - int ret; >> >> - ret = imx8mq_mipi_csi_pm_suspend(dev); >> - if (ret) >> - return ret; >> + imx8mq_mipi_csi_pm_suspend(dev); >> >> state->state |= ST_SUSPENDED; > > > what about this flag? Now that no error is possible inside this function. The state is being updated each time. This seems correct logically. Thanks, Usama
Hi Muhammad, On Fri, Jan 28, 2022 at 10:38 AM Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: > > ret is constant in imx8mq_mipi_csi_pm_suspend(). This function cannot > return error. Remove the return variable. Simplify other functions which > are using this function. > > Fixes: f0c2ba1ed4ad ("media: imx: imx8mq-mipi_csi2: fix system resume") The patch looks good. I would suggest removing the Fixes tag though as this is more of a clean-up rather than a bug fix.
On 1/28/22 9:23 PM, Fabio Estevam wrote: > Hi Muhammad, > > On Fri, Jan 28, 2022 at 10:38 AM Muhammad Usama Anjum > <usama.anjum@collabora.com> wrote: >> >> ret is constant in imx8mq_mipi_csi_pm_suspend(). This function cannot >> return error. Remove the return variable. Simplify other functions which >> are using this function. >> >> Fixes: f0c2ba1ed4ad ("media: imx: imx8mq-mipi_csi2: fix system resume") > > The patch looks good. > > I would suggest removing the Fixes tag though as this is more of a > clean-up rather than a bug fix. I'll send a V2. Thanks, Usama
diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c index 3b9fa75efac6b..c992b845e63d1 100644 --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c @@ -693,11 +693,10 @@ static int imx8mq_mipi_csi_async_register(struct csi_state *state) * Suspend/resume */ -static int imx8mq_mipi_csi_pm_suspend(struct device *dev) +static void imx8mq_mipi_csi_pm_suspend(struct device *dev) { struct v4l2_subdev *sd = dev_get_drvdata(dev); struct csi_state *state = mipi_sd_to_csi2_state(sd); - int ret = 0; mutex_lock(&state->lock); @@ -708,8 +707,6 @@ static int imx8mq_mipi_csi_pm_suspend(struct device *dev) } mutex_unlock(&state->lock); - - return ret ? -EAGAIN : 0; } static int imx8mq_mipi_csi_pm_resume(struct device *dev) @@ -742,15 +739,12 @@ static int __maybe_unused imx8mq_mipi_csi_suspend(struct device *dev) { struct v4l2_subdev *sd = dev_get_drvdata(dev); struct csi_state *state = mipi_sd_to_csi2_state(sd); - int ret; - ret = imx8mq_mipi_csi_pm_suspend(dev); - if (ret) - return ret; + imx8mq_mipi_csi_pm_suspend(dev); state->state |= ST_SUSPENDED; - return ret; + return 0; } static int __maybe_unused imx8mq_mipi_csi_resume(struct device *dev) @@ -770,9 +764,7 @@ static int __maybe_unused imx8mq_mipi_csi_runtime_suspend(struct device *dev) struct csi_state *state = mipi_sd_to_csi2_state(sd); int ret; - ret = imx8mq_mipi_csi_pm_suspend(dev); - if (ret) - return ret; + imx8mq_mipi_csi_pm_suspend(dev); ret = icc_set_bw(state->icc_path, 0, 0); if (ret)
ret is constant in imx8mq_mipi_csi_pm_suspend(). This function cannot return error. Remove the return variable. Simplify other functions which are using this function. Fixes: f0c2ba1ed4ad ("media: imx: imx8mq-mipi_csi2: fix system resume") Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- drivers/staging/media/imx/imx8mq-mipi-csi2.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)