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