All of lore.kernel.org
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
Date: Sun, 9 Feb 2014 10:04:24 +0000	[thread overview]
Message-ID: <20140209100424.GK26684@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20140209102219.3ab40b5e@armhf>

On Sun, Feb 09, 2014 at 10:22:19AM +0100, Jean-Francois Moine wrote:
> On Fri, 7 Feb 2014 20:23:51 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > Here's my changes to the TDA998x driver to add support for the component
> > helper.  The TDA998x driver retains support for the old way so that
> > drivers can be transitioned.  For any one DRM "card" the transition to
> 
> I rewrote the tda998x as a simple encoder+connector (i.e. not a
> slave_encoder) with your component helper, and the code is much clearer
> and simpler: the DRM driver has nothing to do except to know that the
> tda998x is a component and to set the possible_crtcs.

That's exactly what I've done - the slave encoder veneer can be simply
deleted when tilcdc is converted.

> AFAIK, only the tilcdc drm driver is using the tda998x as a
> slave_encoder. It does a (encoder+connector) conversion to
> (slave_encoder). Then, in your changes in the TDA998x, you do a
> (slave_encoder) translation to (encoder+connector).
> This seems rather complicated!

No.  I first split out the slave encoder functions to be a veneer onto
the tda998x backend, and then add the encoder & connector component
support.  Later, the slave encoder veneer can be deleted.

The reason for this is that virtually all the tda998x backend is what's
required by the encoder & connector support - which is completely logical
when you realise that the generic slave encoder support is just a veneer
itself adapting the encoder & connector support to a slave encoder.

So, we're basically getting rid of two veneers, but we end up with exactly
the same functionality.

> > And yes, I'm thinking that maybe moving compare_of() into the component
> > support so that drivers can share this generic function may be a good
> > idea.
> 
> This function exists already in drivers/of/platform.c as
> of_dev_node_match(). It just needs to be exported.

Good, thanks for pointing that out.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Jean-Francois Moine <moinejf@free.fr>
Cc: devel@driverdev.osuosl.org, Daniel Vetter <daniel@ffwll.ch>,
	Takashi Iwai <tiwai@suse.de>,
	dri-devel@lists.freedesktop.org, Rob Clark <robdclark@gmail.com>,
	Sascha Hauer <kernel@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
Date: Sun, 9 Feb 2014 10:04:24 +0000	[thread overview]
Message-ID: <20140209100424.GK26684@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20140209102219.3ab40b5e@armhf>

On Sun, Feb 09, 2014 at 10:22:19AM +0100, Jean-Francois Moine wrote:
> On Fri, 7 Feb 2014 20:23:51 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > Here's my changes to the TDA998x driver to add support for the component
> > helper.  The TDA998x driver retains support for the old way so that
> > drivers can be transitioned.  For any one DRM "card" the transition to
> 
> I rewrote the tda998x as a simple encoder+connector (i.e. not a
> slave_encoder) with your component helper, and the code is much clearer
> and simpler: the DRM driver has nothing to do except to know that the
> tda998x is a component and to set the possible_crtcs.

That's exactly what I've done - the slave encoder veneer can be simply
deleted when tilcdc is converted.

> AFAIK, only the tilcdc drm driver is using the tda998x as a
> slave_encoder. It does a (encoder+connector) conversion to
> (slave_encoder). Then, in your changes in the TDA998x, you do a
> (slave_encoder) translation to (encoder+connector).
> This seems rather complicated!

No.  I first split out the slave encoder functions to be a veneer onto
the tda998x backend, and then add the encoder & connector component
support.  Later, the slave encoder veneer can be deleted.

The reason for this is that virtually all the tda998x backend is what's
required by the encoder & connector support - which is completely logical
when you realise that the generic slave encoder support is just a veneer
itself adapting the encoder & connector support to a slave encoder.

So, we're basically getting rid of two veneers, but we end up with exactly
the same functionality.

> > And yes, I'm thinking that maybe moving compare_of() into the component
> > support so that drivers can share this generic function may be a good
> > idea.
> 
> This function exists already in drivers/of/platform.c as
> of_dev_node_match(). It just needs to be exported.

Good, thanks for pointing that out.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Jean-Francois Moine <moinejf@free.fr>
Cc: Rob Clark <robdclark@gmail.com>,
	devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org,
	Takashi Iwai <tiwai@suse.de>,
	Sascha Hauer <kernel@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org, Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
Date: Sun, 9 Feb 2014 10:04:24 +0000	[thread overview]
Message-ID: <20140209100424.GK26684@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20140209102219.3ab40b5e@armhf>

On Sun, Feb 09, 2014 at 10:22:19AM +0100, Jean-Francois Moine wrote:
> On Fri, 7 Feb 2014 20:23:51 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > Here's my changes to the TDA998x driver to add support for the component
> > helper.  The TDA998x driver retains support for the old way so that
> > drivers can be transitioned.  For any one DRM "card" the transition to
> 
> I rewrote the tda998x as a simple encoder+connector (i.e. not a
> slave_encoder) with your component helper, and the code is much clearer
> and simpler: the DRM driver has nothing to do except to know that the
> tda998x is a component and to set the possible_crtcs.

That's exactly what I've done - the slave encoder veneer can be simply
deleted when tilcdc is converted.

> AFAIK, only the tilcdc drm driver is using the tda998x as a
> slave_encoder. It does a (encoder+connector) conversion to
> (slave_encoder). Then, in your changes in the TDA998x, you do a
> (slave_encoder) translation to (encoder+connector).
> This seems rather complicated!

No.  I first split out the slave encoder functions to be a veneer onto
the tda998x backend, and then add the encoder & connector component
support.  Later, the slave encoder veneer can be deleted.

The reason for this is that virtually all the tda998x backend is what's
required by the encoder & connector support - which is completely logical
when you realise that the generic slave encoder support is just a veneer
itself adapting the encoder & connector support to a slave encoder.

So, we're basically getting rid of two veneers, but we end up with exactly
the same functionality.

> > And yes, I'm thinking that maybe moving compare_of() into the component
> > support so that drivers can share this generic function may be a good
> > idea.
> 
> This function exists already in drivers/of/platform.c as
> of_dev_node_match(). It just needs to be exported.

Good, thanks for pointing that out.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

  reply	other threads:[~2014-02-09 10:04 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-07 17:11 [PATCH RFC 0/2] drivers/base: simplify simple DT-based components Jean-Francois Moine
2014-02-07 17:11 ` Jean-Francois Moine
2014-02-07 17:11 ` Jean-Francois Moine
2014-02-07 15:55 ` [PATCH RFC 1/2] drivers/base: permit base components to omit the bind/unbind ops Jean-Francois Moine
2014-02-07 15:55   ` Jean-Francois Moine
2014-02-07 15:55   ` Jean-Francois Moine
2014-02-07 17:34   ` Russell King - ARM Linux
2014-02-07 17:34     ` Russell King - ARM Linux
2014-02-07 17:34     ` Russell King - ARM Linux
2014-02-07 16:53 ` [PATCH RFC 2/2] drivers/base: declare phandle DT nodes as components Jean-Francois Moine
2014-02-07 16:53   ` Jean-Francois Moine
2014-02-07 17:43   ` Russell King - ARM Linux
2014-02-07 17:43     ` Russell King - ARM Linux
2014-02-07 17:43     ` Russell King - ARM Linux
2014-02-07 17:33 ` [PATCH RFC 0/2] drivers/base: simplify simple DT-based components Russell King - ARM Linux
2014-02-07 17:33   ` Russell King - ARM Linux
2014-02-07 17:33   ` Russell King - ARM Linux
2014-02-07 18:42   ` Jean-Francois Moine
2014-02-07 18:42     ` Jean-Francois Moine
2014-02-07 18:59     ` Russell King - ARM Linux
2014-02-07 18:59       ` Russell King - ARM Linux
2014-02-07 18:59       ` Russell King - ARM Linux
2014-02-08  0:23       ` Russell King - ARM Linux
2014-02-08  0:23         ` Russell King - ARM Linux
2014-02-08  0:23         ` Russell King - ARM Linux
2014-02-07 20:23 ` Russell King - ARM Linux
2014-02-07 20:23   ` Russell King - ARM Linux
2014-02-07 20:23   ` Russell King - ARM Linux
2014-02-09  9:22   ` Jean-Francois Moine
2014-02-09  9:22     ` Jean-Francois Moine
2014-02-09  9:22     ` Jean-Francois Moine
2014-02-09 10:04     ` Russell King - ARM Linux [this message]
2014-02-09 10:04       ` Russell King - ARM Linux
2014-02-09 10:04       ` Russell King - ARM Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140209100424.GK26684@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.