| Message ID | 20180116222217.240939-1-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sean, On 01/16/2018 11:22 PM, Sean Paul wrote: > Return value for mipi_dsi_shutdown_peripheral() is unchecked. > Check it and return any errors if they come up. Even if > mipi_dsi_shutdown_peripheral() fails, continue attempting to > disable. > > Cc: Philippe Cornu <philippe.cornu@st.com> > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c > index 7f915f706fa6..91dc5a6b14f9 100644 > --- a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c > +++ b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c > @@ -72,11 +72,12 @@ static int wuxga_nt_panel_on(struct wuxga_nt_panel *wuxga_nt) > static int wuxga_nt_panel_disable(struct drm_panel *panel) > { > struct wuxga_nt_panel *wuxga_nt = to_wuxga_nt_panel(panel); > + int ret; > > if (!wuxga_nt->enabled) > return 0; > > - mipi_dsi_shutdown_peripheral(wuxga_nt->dsi); > + ret = mipi_dsi_shutdown_peripheral(wuxga_nt->dsi); > > if (wuxga_nt->backlight) { > wuxga_nt->backlight->props.power = FB_BLANK_POWERDOWN; > @@ -86,7 +87,7 @@ static int wuxga_nt_panel_disable(struct drm_panel *panel) > > wuxga_nt->enabled = false; > > - return 0; > + return ret; > } > > static int wuxga_nt_panel_unprepare(struct drm_panel *panel) > Many thanks for the patch. I agree it is better to continue attempting to disable if mipi_dsi_shutdown_peripheral() fails. Looking more in details in this driver: * backlight_update_status() may also use a ret value... * wuxga_nt_panel_on() should simply do "return mipi_dsi_turn_on_peripheral(dsi);" ... but it is not really part of your patch (and mine initially), maybe TODOs for later : ) Reviewed-by: Philippe Cornu <philippe.cornu@st.com> Philippe :-)
On Wed, Jan 17, 2018 at 05:01:07PM +0000, Philippe CORNU wrote: > Hi Sean, > > On 01/16/2018 11:22 PM, Sean Paul wrote: > > Return value for mipi_dsi_shutdown_peripheral() is unchecked. > > Check it and return any errors if they come up. Even if > > mipi_dsi_shutdown_peripheral() fails, continue attempting to > > disable. > > > > Cc: Philippe Cornu <philippe.cornu@st.com> > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > --- > > drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c > > index 7f915f706fa6..91dc5a6b14f9 100644 > > --- a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c > > +++ b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c > > @@ -72,11 +72,12 @@ static int wuxga_nt_panel_on(struct wuxga_nt_panel *wuxga_nt) > > static int wuxga_nt_panel_disable(struct drm_panel *panel) > > { > > struct wuxga_nt_panel *wuxga_nt = to_wuxga_nt_panel(panel); > > + int ret; > > > > if (!wuxga_nt->enabled) > > return 0; > > > > - mipi_dsi_shutdown_peripheral(wuxga_nt->dsi); > > + ret = mipi_dsi_shutdown_peripheral(wuxga_nt->dsi); > > > > if (wuxga_nt->backlight) { > > wuxga_nt->backlight->props.power = FB_BLANK_POWERDOWN; > > @@ -86,7 +87,7 @@ static int wuxga_nt_panel_disable(struct drm_panel *panel) > > > > wuxga_nt->enabled = false; > > > > - return 0; > > + return ret; > > } > > > > static int wuxga_nt_panel_unprepare(struct drm_panel *panel) > > > > Many thanks for the patch. > > I agree it is better to continue attempting to disable if > mipi_dsi_shutdown_peripheral() fails. > > Looking more in details in this driver: > * backlight_update_status() may also use a ret value... > * wuxga_nt_panel_on() should simply do "return > mipi_dsi_turn_on_peripheral(dsi);" > ... but it is not really part of your patch (and mine initially), maybe > TODOs for later : ) > Thanks for your review, Philippe, I've applied this to -misc-next. I'm stuck in meetings all day today, so there's a good chance I'll get to the rest of the TODOs while I sit here :-) Sean > > Reviewed-by: Philippe Cornu <philippe.cornu@st.com> > > Philippe :-)
diff --git a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c index 7f915f706fa6..91dc5a6b14f9 100644 --- a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c +++ b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c @@ -72,11 +72,12 @@ static int wuxga_nt_panel_on(struct wuxga_nt_panel *wuxga_nt) static int wuxga_nt_panel_disable(struct drm_panel *panel) { struct wuxga_nt_panel *wuxga_nt = to_wuxga_nt_panel(panel); + int ret; if (!wuxga_nt->enabled) return 0; - mipi_dsi_shutdown_peripheral(wuxga_nt->dsi); + ret = mipi_dsi_shutdown_peripheral(wuxga_nt->dsi); if (wuxga_nt->backlight) { wuxga_nt->backlight->props.power = FB_BLANK_POWERDOWN; @@ -86,7 +87,7 @@ static int wuxga_nt_panel_disable(struct drm_panel *panel) wuxga_nt->enabled = false; - return 0; + return ret; } static int wuxga_nt_panel_unprepare(struct drm_panel *panel)
Return value for mipi_dsi_shutdown_peripheral() is unchecked. Check it and return any errors if they come up. Even if mipi_dsi_shutdown_peripheral() fails, continue attempting to disable. Cc: Philippe Cornu <philippe.cornu@st.com> Signed-off-by: Sean Paul <seanpaul@chromium.org> --- drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)