Hi Maxime, On Mon, 25 Apr 2016 15:22:46 +0200 Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote: > The Allwinner A10 and subsequent SoCs share the same display pipeline, with > variations in the number of controllers (1 or 2), or the presence or not of > some output (HDMI, TV, VGA) or not. > > Add a driver with a limited set of features for now, and we will hopefully > support all of them eventually > > Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> Just 2 comments below. Once addressed you can add my Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> > --- [...] > + > +static int sun4i_drv_connector_plug_all(struct drm_device *drm) > +{ > + struct drm_connector *connector, *failed; > + int ret; > + > + mutex_lock(&drm->mode_config.mutex); > + list_for_each_entry(connector, &drm->mode_config.connector_list, head) { > + ret = drm_connector_register(connector); > + if (ret) { > + failed = connector; > + goto err; > + } > + } > + mutex_unlock(&drm->mode_config.mutex); > + return 0; > + > +err: > + list_for_each_entry(connector, &drm->mode_config.connector_list, head) { > + if (failed == connector) > + break; > + > + drm_connector_unregister(connector); > + } > + mutex_unlock(&drm->mode_config.mutex); > + > + return ret; > +} You can use the generic drm_connector_register_all() to do that. [...] > + > +static void sun4i_drv_unbind(struct device *dev) > +{ > + struct drm_device *drm = dev_get_drvdata(dev); > + And you probably miss a call to drm_connector_unregister_all() here. > + drm_dev_unregister(drm); > + drm_kms_helper_poll_fini(drm); > + sun4i_framebuffer_free(drm); > + drm_vblank_cleanup(drm); > + drm_dev_unref(drm); > +} -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html