Hi Peter,
On Monday, 4 September 2017 12:19:19 EEST Peter Ujfalusi wrote:
> 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.
Making it explicit that the variable can't be negative allows the compiler to
warn when encoutering a < 0 test. It also makes the code self-documented,
showing the reader that the variable isn't expected to become negative.
> >> 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.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
