Hi Sebastian, On Wed, Feb 26, 2020 at 10:28:19PM +0100, Sebastian Reichel wrote: > On Tue, Feb 25, 2020 at 01:42:49AM +0200, Laurent Pinchart wrote: > > On Tue, Feb 25, 2020 at 12:20:34AM +0100, Sebastian Reichel wrote: > > > The panel-dsi-cm's ddata->pin_config is always NULL, so this > > > callback is never called. Instead the DSI encoder gets the pin > > > configuration directly from DT. > > > > > > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > > > --- > > > drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 11 ----------- > > > drivers/gpu/drm/omapdrm/dss/dsi.c | 1 - > > > drivers/gpu/drm/omapdrm/dss/omapdss.h | 2 -- > > > 3 files changed, 14 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > > > index 3484b5d4a91c..e7fe5d702337 100644 > > > --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > > > +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > > > @@ -68,8 +68,6 @@ struct panel_drv_data { > > > int width_mm; > > > int height_mm; > > > > > > - struct omap_dsi_pin_config pin_config; > > > - > > > /* runtime variables */ > > > bool enabled; > > > > > > @@ -623,15 +621,6 @@ static int dsicm_power_on(struct panel_drv_data *ddata) > > > } > > > } > > > > > > - if (ddata->pin_config.num_pins > 0) { > > > - r = src->ops->dsi.configure_pins(src, &ddata->pin_config); > > > - if (r) { > > > - dev_err(&ddata->pdev->dev, > > > - "failed to configure DSI pins\n"); > > > - goto err_vddi; > > > - } > > > - } > > > - > > > r = src->ops->dsi.set_config(src, &dsi_config); > > > if (r) { > > > dev_err(&ddata->pdev->dev, "failed to configure DSI\n"); > > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > > > index 79ddfbfd1b58..8c39823a8295 100644 > > > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > > > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > > > @@ -4892,7 +4892,6 @@ static const struct omap_dss_device_ops dsi_ops = { > > > > > > .enable_hs = dsi_vc_enable_hs, > > > > > > - .configure_pins = dsi_configure_pins, > > > .set_config = dsi_set_config, > > > > > > .enable_video_output = dsi_enable_video_output, > > > diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h > > > index cbbe10b2b60d..b0424daaceed 100644 > > > --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h > > > +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h > > > @@ -292,8 +292,6 @@ struct omapdss_dsi_ops { > > > > I think you can drop the definition of the omap_dsi_pin_config structure > > earlier in this file too, as well as the OMAP_DSS_MAX_DSI_PINS macro. > > With this fixed, > > No, the struct is still used by the code setting up the pins from > DT. Indeed, my bad. I think I'd pass the unsigned int num_pins and const int *pins to dsi_configure_pins() directly to drop the structure, but that can be done in a subsequent patch (maybe it is already :-)). > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> This tag holds. > > > /* bus configuration */ > > > int (*set_config)(struct omap_dss_device *dssdev, > > > const struct omap_dss_dsi_config *cfg); > > > - int (*configure_pins)(struct omap_dss_device *dssdev, > > > - const struct omap_dsi_pin_config *pin_cfg); > > > > > > void (*enable_hs)(struct omap_dss_device *dssdev, int channel, > > > bool enable); -- Regards, Laurent Pinchart