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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to