On Mon, 9 Jul 2012, Patrik Jakobsson wrote:
> On Sun, Jul 8, 2012 at 10:16 PM, Alan Cox <alan at lxorguk.ukuu.org.uk> wrote:
>> On Sun, 8 Jul 2012 10:39:43 +0200 (CEST)
>> Julia Lawall <julia.lawall at lip6.fr> wrote:
>>
>>> In the function oaktrail_lvds_mode_set, I don't think that the following
>>> code makes any sense:
>>>
>>> /* Find the connector we're trying to set up */
>>> list_for_each_entry(connector, &mode_config->connector_list, head)
>>> {
>>> if (!connector->encoder || connector->encoder->crtc !=
>>> crtc)
>>> continue;
>>> }
>>>
>>> if (!connector) {
>>> DRM_ERROR("Couldn't find connector when setting mode");
>>> return;
>>> }
>>>
>>> drm_connector_property_get_value(
>>> connector,
>>> dev->mode_config.scaling_mode_property,
>>> &v);
>>>
>>> The initial loop is a no-op, because it always continues. The test
>>> !connector can never be true, because at the end of a list_for_each_entry
>>> connector points to the list head, and calling
>>> drm_connector_property_get_value on the list head probably does not make
>>> sense.
>>
>> We test !connector->encoder rather than !connector ?
>
> It seems we should break on :
> if (connector->encoder && connector->encoder == encoder)
>
> Then do a check after list iteration:
> if (!connector || connector->encoder != encoder)
> DRM_ERROR("Couldn't find connector when setting mode");
Possible. The !connector is still not needed, but the overall logic seems
better.
julia
