Hi Laurent, Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 2017-09-01 14:27, Laurent Pinchart wrote: >> -static void omap_disconnect_dssdevs(void) >> +static void omap_disconnect_dssdevs(struct drm_device *ddev) >> { >> - struct omap_dss_device *dssdev = NULL; >> + struct omap_drm_private *priv = ddev->dev_private; >> + int i; >> + >> + for (i = 0; i < priv->num_dssdevs; i++) { > > is is never negative, you can make it an unsigned int. This comment applies > through the whole patch. True, are there any benefits to have unsigned int compared to int? I don't think we are going to hit size limitation with the int, but if you prefer to have unsigned int, then sure, I can change the num_displays and 'i's to unsigned. >> crtc_idx = 0; >> plane_idx = 0; >> - for_each_dss_dev(dssdev) { >> + for (i = 0; i < priv->num_dssdevs; i++) { >> + struct omap_dss_device *dssdev = priv->dssdevs[i]; >> struct drm_connector *connector; >> struct drm_encoder *encoder; >> struct drm_plane *plane; >> struct drm_crtc *crtc; >> >> - if (!omapdss_device_is_connected(dssdev)) >> - continue; >> - > > I believe this hunk is correct as dss devices are only disconnected by calls > to the oma_dss_driver .disconnect() operation, which is only called from > omap_disconnect_dssdevs(), but you should at the very least explain why in the > commit message. The check became irrelevant as we did not add dssdevs to the array if their connect failed and as you have said we only disconnect ddsdevs in omap_disconnect_dssdevs() - in cleanup paths. I will update the commit message with this information. - Péter _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel