[PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge

Daniel Vetter daniel at ffwll.ch
Mon Oct 27 15:26:30 PDT 2014


On Mon, Oct 27, 2014 at 11:20 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Mon, Oct 27, 2014 at 8:58 PM, Sean Paul <seanpaul at chromium.org> 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


More information about the dri-devel mailing list