On 29/03/17 13:08, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thank you for the patch.
>
> On Tuesday 28 Mar 2017 16:08:10 Tomi Valkeinen wrote:
>> From: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
>>
>> Keep the panel_list ordered according to aliases. The DRM connectors will
>> be created following the panel_list. By keeping the list ordered the DRM
>> connectors will be created in the same order regardless of the driver
>> probe order.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
>> ---
>> drivers/gpu/drm/omapdrm/dss/display.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/display.c
>> b/drivers/gpu/drm/omapdrm/dss/display.c index 94c012e0584b..26cb59be045e
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/display.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/display.c
>> @@ -83,6 +83,7 @@ static int disp_num_counter;
>> int omapdss_register_display(struct omap_dss_device *dssdev)
>> {
>> struct omap_dss_driver *drv = dssdev->driver;
>> + struct list_head *cur;
>> int id;
>>
>> /*
>> @@ -118,7 +119,14 @@ int omapdss_register_display(struct omap_dss_device
>> *dssdev) drv->get_timings = omapdss_default_get_timings;
>>
>> mutex_lock(&panel_list_mutex);
>> - list_add_tail(&dssdev->panel_list, &panel_list);
>> + list_for_each(cur, &panel_list) {
>
> How about using list_for_each_entry ?
We need the list iterator after the loop to add the new node. Where
would list_for_each_entry()'s 'pos' point to, if there's nothing in the
list yet? I think list_for_each_entry()'s iterator is supposed to be
used only inside the loop.
>> + struct omap_dss_device *ldev = list_entry(cur,
>> + struct
> omap_dss_device,
>> + panel_list);
>> + if (strcmp(ldev->alias, dssdev->alias) > 0)
>> + break;
>> + }
>> + list_add_tail(&dssdev->panel_list, cur);
>
> What you want to do here is add the new element before cur. list_add_tail()
> does that, but that's not very explicit (at least to me) from the function
> name. I would have used list_add(new, cur->prev), but that's up to you.
list_add_tail's documentation says "Insert a new entry before the
specified head". So while I agree that "tail" is not what comes to my
mind from the description, it sounds like this is exactly the case where
list_add_tail is to be used...
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
