| Message ID | 20191202193230.21310-2-sam@ravnborg.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/panel infrastructure + backlight update | expand |
Hi Sam, Thank you for the patch. On Mon, Dec 02, 2019 at 08:32:05PM +0100, Sam Ravnborg wrote: > The callbacks in drm_panel_funcs are optional, so do not > return an error just because no callback is assigned. Unless I'm mistaken the callbacks are not documented as optional. Should this be fixed too ? > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/drm_panel.c | 35 +++++++++++++++++++++++++---------- > 1 file changed, 25 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index ed7985c0535a..2d59cdd05e50 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -151,10 +151,13 @@ EXPORT_SYMBOL(drm_panel_detach); > */ > int drm_panel_prepare(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->prepare) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->prepare) > return panel->funcs->prepare(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_prepare); > > @@ -171,10 +174,13 @@ EXPORT_SYMBOL(drm_panel_prepare); > */ > int drm_panel_unprepare(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->unprepare) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->unprepare) > return panel->funcs->unprepare(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_unprepare); > > @@ -190,10 +196,13 @@ EXPORT_SYMBOL(drm_panel_unprepare); > */ > int drm_panel_enable(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->enable) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->enable) > return panel->funcs->enable(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_enable); > > @@ -209,10 +218,13 @@ EXPORT_SYMBOL(drm_panel_enable); > */ > int drm_panel_disable(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->disable) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->disable) > return panel->funcs->disable(panel); > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_disable); > > @@ -228,10 +240,13 @@ EXPORT_SYMBOL(drm_panel_disable); > */ > int drm_panel_get_modes(struct drm_panel *panel) > { > - if (panel && panel->funcs && panel->funcs->get_modes) > + if (!panel) > + return -EINVAL; > + > + if (panel->funcs && panel->funcs->get_modes) > return panel->funcs->get_modes(panel); Should get_modes() be optional ? As far as I can tell all the panel drivers in drivers/gpu/drm/panel/ implement it, and I'm not sure to see how a panel could be usable if it can't return its mode. On the other hand you return 0 if the callback isn't implemented, which doesn't mean success here, so I suppose that's fine, but I don't think we should document .get_modes() as being optional. With these issues addressed (if they need to be), Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > - return panel ? -ENOSYS : -EINVAL; > + return 0; > } > EXPORT_SYMBOL(drm_panel_get_modes); >
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index ed7985c0535a..2d59cdd05e50 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -151,10 +151,13 @@ EXPORT_SYMBOL(drm_panel_detach); */ int drm_panel_prepare(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->prepare) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->prepare) return panel->funcs->prepare(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_prepare); @@ -171,10 +174,13 @@ EXPORT_SYMBOL(drm_panel_prepare); */ int drm_panel_unprepare(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->unprepare) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->unprepare) return panel->funcs->unprepare(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_unprepare); @@ -190,10 +196,13 @@ EXPORT_SYMBOL(drm_panel_unprepare); */ int drm_panel_enable(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->enable) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->enable) return panel->funcs->enable(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_enable); @@ -209,10 +218,13 @@ EXPORT_SYMBOL(drm_panel_enable); */ int drm_panel_disable(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->disable) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->disable) return panel->funcs->disable(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_disable); @@ -228,10 +240,13 @@ EXPORT_SYMBOL(drm_panel_disable); */ int drm_panel_get_modes(struct drm_panel *panel) { - if (panel && panel->funcs && panel->funcs->get_modes) + if (!panel) + return -EINVAL; + + if (panel->funcs && panel->funcs->get_modes) return panel->funcs->get_modes(panel); - return panel ? -ENOSYS : -EINVAL; + return 0; } EXPORT_SYMBOL(drm_panel_get_modes);
The callbacks in drm_panel_funcs are optional, so do not return an error just because no callback is assigned. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/drm_panel.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-)