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

From: Thierry Reding <thierry.reding@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	Jingoo Han <jg1.han@samsung.com>, sunil joshi <joshi@samsung.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Ajay kumar <ajaynumb@gmail.com>,
	Thierry Reding <treding@nvidia.com>,
	Prashanth G <prashanth.g@samsung.com>,
	Ajay Kumar <ajaykumar.rs@samsung.com>
Subject: Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
Date: Tue, 28 Oct 2014 15:29:47 +0100	[thread overview]
Message-ID: <20141028142946.GA17770@ulmo> (raw)
In-Reply-To: <CAKMK7uEch65=ZuaG=4JY2u3S3w5RND+uNc5+qL=tWxDLfEqEwQ@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1891 bytes --]

On Mon, Oct 27, 2014 at 11:20:31PM +0100, Daniel Vetter 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?

The struct device * is in DRM panel because there's nothing device tree
specific about the concept. Having a struct device_node * instead would
indicate that it can only be used with a device tree, whereas the
framework doesn't care the tiniest bit what type of device we have.

While the trend clearly is to use more device tree, I don't think we
should make it impossible for anybody else to use these frameworks.

There are other advantages to keeping a struct device *, like having
access to the proper device and its name. Also you get access to the
device_node * via dev->of_node anyway. I don't see any advantage in
switching to just a struct device_node *, only disadvantages.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel