* [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get @ 2019-10-15 13:59 Chuhong Yuan 2019-10-16 9:06 ` Marco Felsch 2019-10-17 9:46 ` Rui Miguel Silva 0 siblings, 2 replies; 6+ messages in thread From: Chuhong Yuan @ 2019-10-15 13:59 UTC (permalink / raw) Cc: devel, Fabio Estevam, Pengutronix Kernel Team, Greg Kroah-Hartman, Sascha Hauer, Chuhong Yuan, linux-kernel, Rui Miguel Silva, NXP Linux Team, Philipp Zabel, Steve Longerbeam, Mauro Carvalho Chehab, Shawn Guo, linux-arm-kernel, linux-media devm_regulator_get may return an error but mipi_csis_phy_init misses a check for it. This may lead to problems when regulator_set_voltage uses the unchecked pointer. This patch adds a check for devm_regulator_get to avoid potential risk. Signed-off-by: Chuhong Yuan <hslester96@gmail.com> --- Changes in v2: - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init. drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c index 73d8354e618c..e8a6acaa969e 100644 --- a/drivers/staging/media/imx/imx7-mipi-csis.c +++ b/drivers/staging/media/imx/imx7-mipi-csis.c @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state) static int mipi_csis_phy_init(struct csi_state *state) { state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); + if (IS_ERR(state->mipi_phy_regulator)) + return PTR_ERR(state->mipi_phy_regulator); return regulator_set_voltage(state->mipi_phy_regulator, 1000000, 1000000); @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev) return ret; } - mipi_csis_phy_init(state); + ret = mipi_csis_phy_init(state); + if (ret < 0) + return ret; + mipi_csis_phy_reset(state); mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get 2019-10-15 13:59 [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get Chuhong Yuan @ 2019-10-16 9:06 ` Marco Felsch 2019-10-16 13:43 ` Rui Miguel Silva 2019-10-17 9:46 ` Rui Miguel Silva 1 sibling, 1 reply; 6+ messages in thread From: Marco Felsch @ 2019-10-16 9:06 UTC (permalink / raw) To: Chuhong Yuan Cc: devel, Philipp Zabel, Greg Kroah-Hartman, Sascha Hauer, linux-kernel, Rui Miguel Silva, NXP Linux Team, Pengutronix Kernel Team, Steve Longerbeam, Shawn Guo, Mauro Carvalho Chehab, Fabio Estevam, linux-arm-kernel, linux-media Hi Chuhong, On 19-10-15 21:59, Chuhong Yuan wrote: > devm_regulator_get may return an error but mipi_csis_phy_init misses > a check for it. > This may lead to problems when regulator_set_voltage uses the unchecked > pointer. > This patch adds a check for devm_regulator_get to avoid potential risk. > > Signed-off-by: Chuhong Yuan <hslester96@gmail.com> > --- > Changes in v2: > - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init. Did you miss the check for -EPROBE_DEFER? Regards, Marco > > drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > index 73d8354e618c..e8a6acaa969e 100644 > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state) > static int mipi_csis_phy_init(struct csi_state *state) > { > state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); > + if (IS_ERR(state->mipi_phy_regulator)) > + return PTR_ERR(state->mipi_phy_regulator); > > return regulator_set_voltage(state->mipi_phy_regulator, 1000000, > 1000000); > @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev) > return ret; > } > > - mipi_csis_phy_init(state); > + ret = mipi_csis_phy_init(state); > + if (ret < 0) > + return ret; > + > mipi_csis_phy_reset(state); > > mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > -- > 2.20.1 > > > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get 2019-10-16 9:06 ` Marco Felsch @ 2019-10-16 13:43 ` Rui Miguel Silva 2019-10-17 8:10 ` Marco Felsch 0 siblings, 1 reply; 6+ messages in thread From: Rui Miguel Silva @ 2019-10-16 13:43 UTC (permalink / raw) To: Marco Felsch Cc: devel, Philipp Zabel, Greg Kroah-Hartman, Sascha Hauer, Chuhong Yuan, linux-kernel, NXP Linux Team, Pengutronix Kernel Team, Steve Longerbeam, Shawn Guo, Mauro Carvalho Chehab, Fabio Estevam, linux-arm-kernel, linux-media Hi Marco, On Wed 16 Oct 2019 at 10:06, Marco Felsch wrote: > Hi Chuhong, > > On 19-10-15 21:59, Chuhong Yuan wrote: >> devm_regulator_get may return an error but mipi_csis_phy_init misses >> a check for it. >> This may lead to problems when regulator_set_voltage uses the unchecked >> pointer. >> This patch adds a check for devm_regulator_get to avoid potential risk. >> >> Signed-off-by: Chuhong Yuan <hslester96@gmail.com> >> --- >> Changes in v2: >> - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init. > > Did you miss the check for -EPROBE_DEFER? > I think nothing special is really needed to do in case of EPROBE_DEFER, or am I missing something? It just return to probe and probe returns also. I just talked about it because it was not cover in the original code. --- Cheers, Rui > > Regards, > Marco > >> >> drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c >> index 73d8354e618c..e8a6acaa969e 100644 >> --- a/drivers/staging/media/imx/imx7-mipi-csis.c >> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c >> @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state) >> static int mipi_csis_phy_init(struct csi_state *state) >> { >> state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); >> + if (IS_ERR(state->mipi_phy_regulator)) >> + return PTR_ERR(state->mipi_phy_regulator); >> >> return regulator_set_voltage(state->mipi_phy_regulator, 1000000, >> 1000000); >> @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev) >> return ret; >> } >> >> - mipi_csis_phy_init(state); >> + ret = mipi_csis_phy_init(state); >> + if (ret < 0) >> + return ret; >> + >> mipi_csis_phy_reset(state); >> >> mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> -- >> 2.20.1 >> >> >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get 2019-10-16 13:43 ` Rui Miguel Silva @ 2019-10-17 8:10 ` Marco Felsch 2019-10-17 9:43 ` Rui Miguel Silva 0 siblings, 1 reply; 6+ messages in thread From: Marco Felsch @ 2019-10-17 8:10 UTC (permalink / raw) To: Rui Miguel Silva Cc: devel, Philipp Zabel, Greg Kroah-Hartman, Sascha Hauer, Chuhong Yuan, linux-kernel, NXP Linux Team, Pengutronix Kernel Team, Steve Longerbeam, Shawn Guo, Mauro Carvalho Chehab, Fabio Estevam, linux-arm-kernel, linux-media Hi Rui, On 19-10-16 14:43, Rui Miguel Silva wrote: > Hi Marco, > On Wed 16 Oct 2019 at 10:06, Marco Felsch wrote: > > Hi Chuhong, > > > > On 19-10-15 21:59, Chuhong Yuan wrote: > >> devm_regulator_get may return an error but mipi_csis_phy_init misses > >> a check for it. > >> This may lead to problems when regulator_set_voltage uses the unchecked > >> pointer. > >> This patch adds a check for devm_regulator_get to avoid potential risk. > >> > >> Signed-off-by: Chuhong Yuan <hslester96@gmail.com> > >> --- > >> Changes in v2: > >> - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init. > > > > Did you miss the check for -EPROBE_DEFER? > > > > I think nothing special is really needed to do in case of > EPROBE_DEFER, or am I missing something? > It just return to probe and probe returns also. I just talked > about it because it was not cover in the original code. Yes, your are right... I shouldn't comment on anything I read with one eye. Sorry. Regards, Marco > --- > Cheers, > Rui > > > > > Regards, > > Marco > > > >> > >> drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > >> index 73d8354e618c..e8a6acaa969e 100644 > >> --- a/drivers/staging/media/imx/imx7-mipi-csis.c > >> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > >> @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state) > >> static int mipi_csis_phy_init(struct csi_state *state) > >> { > >> state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); > >> + if (IS_ERR(state->mipi_phy_regulator)) > >> + return PTR_ERR(state->mipi_phy_regulator); > >> > >> return regulator_set_voltage(state->mipi_phy_regulator, 1000000, > >> 1000000); > >> @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev) > >> return ret; > >> } > >> > >> - mipi_csis_phy_init(state); > >> + ret = mipi_csis_phy_init(state); > >> + if (ret < 0) > >> + return ret; > >> + > >> mipi_csis_phy_reset(state); > >> > >> mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> -- > >> 2.20.1 > >> > >> > >> > > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get 2019-10-17 8:10 ` Marco Felsch @ 2019-10-17 9:43 ` Rui Miguel Silva 0 siblings, 0 replies; 6+ messages in thread From: Rui Miguel Silva @ 2019-10-17 9:43 UTC (permalink / raw) To: Marco Felsch Cc: devel, Philipp Zabel, Greg Kroah-Hartman, Sascha Hauer, Chuhong Yuan, linux-kernel, NXP Linux Team, Pengutronix Kernel Team, Steve Longerbeam, Shawn Guo, Mauro Carvalho Chehab, Fabio Estevam, linux-arm-kernel, linux-media Hi Marco, On Thu 17 Oct 2019 at 09:10, Marco Felsch wrote: > Hi Rui, > > On 19-10-16 14:43, Rui Miguel Silva wrote: >> Hi Marco, >> On Wed 16 Oct 2019 at 10:06, Marco Felsch wrote: >> > Hi Chuhong, >> > >> > On 19-10-15 21:59, Chuhong Yuan wrote: >> >> devm_regulator_get may return an error but mipi_csis_phy_init misses >> >> a check for it. >> >> This may lead to problems when regulator_set_voltage uses the unchecked >> >> pointer. >> >> This patch adds a check for devm_regulator_get to avoid potential risk. >> >> >> >> Signed-off-by: Chuhong Yuan <hslester96@gmail.com> >> >> --- >> >> Changes in v2: >> >> - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init. >> > >> > Did you miss the check for -EPROBE_DEFER? >> > >> >> I think nothing special is really needed to do in case of >> EPROBE_DEFER, or am I missing something? >> It just return to probe and probe returns also. I just talked >> about it because it was not cover in the original code. > > Yes, your are right... I shouldn't comment on anything I read with one > eye. Sorry. > ehehe, no problem and thanks for your inputs. --- Cheers, Rui > > Regards, > Marco > >> --- >> Cheers, >> Rui >> >> > >> > Regards, >> > Marco >> > >> >> >> >> drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++++++- >> >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c >> >> index 73d8354e618c..e8a6acaa969e 100644 >> >> --- a/drivers/staging/media/imx/imx7-mipi-csis.c >> >> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c >> >> @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state) >> >> static int mipi_csis_phy_init(struct csi_state *state) >> >> { >> >> state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); >> >> + if (IS_ERR(state->mipi_phy_regulator)) >> >> + return PTR_ERR(state->mipi_phy_regulator); >> >> >> >> return regulator_set_voltage(state->mipi_phy_regulator, 1000000, >> >> 1000000); >> >> @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev) >> >> return ret; >> >> } >> >> >> >> - mipi_csis_phy_init(state); >> >> + ret = mipi_csis_phy_init(state); >> >> + if (ret < 0) >> >> + return ret; >> >> + >> >> mipi_csis_phy_reset(state); >> >> >> >> mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> >> -- >> >> 2.20.1 >> >> >> >> >> >> >> >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get 2019-10-15 13:59 [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get Chuhong Yuan 2019-10-16 9:06 ` Marco Felsch @ 2019-10-17 9:46 ` Rui Miguel Silva 1 sibling, 0 replies; 6+ messages in thread From: Rui Miguel Silva @ 2019-10-17 9:46 UTC (permalink / raw) To: Chuhong Yuan Cc: devel, Fabio Estevam, Pengutronix Kernel Team, Greg Kroah-Hartman, Sascha Hauer, linux-kernel, NXP Linux Team, Philipp Zabel, Steve Longerbeam, Mauro Carvalho Chehab, Shawn Guo, linux-arm-kernel, linux-media Hi Chuhong, many thanks for the patch. On Tue 15 Oct 2019 at 14:59, Chuhong Yuan wrote: > devm_regulator_get may return an error but mipi_csis_phy_init misses > a check for it. > This may lead to problems when regulator_set_voltage uses the unchecked > pointer. > This patch adds a check for devm_regulator_get to avoid potential risk. > > Signed-off-by: Chuhong Yuan <hslester96@gmail.com> Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com> --- Cheers, Rui > --- > Changes in v2: > - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init. > > drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > index 73d8354e618c..e8a6acaa969e 100644 > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state) > static int mipi_csis_phy_init(struct csi_state *state) > { > state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); > + if (IS_ERR(state->mipi_phy_regulator)) > + return PTR_ERR(state->mipi_phy_regulator); > > return regulator_set_voltage(state->mipi_phy_regulator, 1000000, > 1000000); > @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev) > return ret; > } > > - mipi_csis_phy_init(state); > + ret = mipi_csis_phy_init(state); > + if (ret < 0) > + return ret; > + > mipi_csis_phy_reset(state); > > mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-17 9:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-15 13:59 [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get Chuhong Yuan 2019-10-16 9:06 ` Marco Felsch 2019-10-16 13:43 ` Rui Miguel Silva 2019-10-17 8:10 ` Marco Felsch 2019-10-17 9:43 ` Rui Miguel Silva 2019-10-17 9:46 ` Rui Miguel Silva
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).