On Mon, Oct 27, 2014 at 11:20 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Mon, Oct 27, 2014 at 8:58 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
>>>> @@ -660,8 +662,11 @@ struct drm_bridge_funcs {
>>>> * @driver_private: pointer to the bridge driver's internal context
>>>> */
>>>> struct drm_bridge {
>>>> - struct drm_device *dev;
>>>> + struct device *dev;
>>>
>>> Please don't rename the ->dev pointer into drm. Because _all_ the other
>>> drm structures still call it ->dev. Also, can't we use struct device_node
>>> here like we do in the of helpers Russell added? See 7e435aad38083
>>>
>>
>> I think this is modeled after the naming in drm_panel, FWIW. However,
>> seems reasonable to keep the device_node instead.
>
> Hm, indeed. Tbh I vote to rename drm_panel->drm to ->dev and like with
> drm_crtc drop the struct device and go directly to a struct
> device_node. Since we don't really need the sturct device, the only
> thing we care about is the of_node. For added bonus wrap an #ifdef
> CONFIG_OF around all the various struct device_node in drm_foo.h.
> Should be all fairly simple to pull off with cocci.
>
> Thierry?
Looking at the of_drm_find_panel function I actually wonder how that
works - the drm_panel doesn't really need to stick around afaics.
After all panel_list is global so some other driver can unload.
Russell's of support for possible_crtcs code works differently since
it only looks at per-drm_device lists.
This bridge code here though suffers from the same. So to me this
looks rather fishy.
It doesn't help that we have drm_of.[hc] around but not all the of
code is in there. Adding Russell too.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
