All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	linux-fbdev@vger.kernel.org, linux-acpi@vger.kernel.org,
	alsa-devel@alsa-project.org,
	Stephen Warren <swarren@wwwdotorg.org>,
	Takashi Iwai <tiwai@suse.de>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Liam Girdwood <lgirdwood@gmail.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-gpio@vger.kernel.org,
	Thierry Reding <thierry.reding@gmail.com>,
	Linux PWM List <linux-pwm@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	Alexandre Courbot <gnurou@gmail.com>
Subject: Re: [alsa-devel] [PATCH v2 11/12] ASoC: tegra: register dependency parser for firmware nodes
Date: Fri, 17 Jul 2015 00:04:09 +0100	[thread overview]
Message-ID: <20150716230409.GG1602@sirena.org.uk> (raw)
In-Reply-To: <CAAObsKAHviKKTrEBhuge-KQ115p-kOf69miNnmpR+7knOeg+qg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]

On Tue, Jul 14, 2015 at 02:47:04PM +0200, Tomeu Vizoso wrote:
> On 14 July 2015 at 13:07, Mark Brown <broonie@kernel.org> wrote:

> > I'm not sure how I can be clearer here...  you're replacing something
> > that is currently pure data with open coding in each device.  That seems
> > like a step back in terms of ease of use.

> I could understand that if snd_soc_dai_link had a field with the
> property name, and the core called of_parse_phandle on it, but
> currently what I'm duplicating is:

>     tegra_max98090_dai.cpu_of_node = of_parse_phandle(np,
>             "nvidia,i2s-controller", 0);

> with:

>     add_dependency(fwnode, "nvidia,i2s-controller", deps);

> Admittedly, we could add a cpu_fw_property field to snd_soc_dai_link
> and have the core call of_parse_phandle itself.

Yes, we could - that's really what should be happening here.  The other
bit of this is that we're doing it twice which isn't success.

> But even then, the core doesn't know about a device's snd_soc_dai_link
> until probe() is called and then it's too late for the purposes of
> this series.

That's not a good reason to encourage bad patterns in drivers.  At the
very least the drivers should be able to pass the same struct into both
places, having to open code the same thing in two places is going to be
error prone.

> That's why I mentioned devm_probe, as it would add a common way to
> specify the data needed to acquire resources in each driver, which
> could be made available before probe() is called.

That does avoid the duplication.  However there are issues with the
interface for enumerable buses, it doesn't solve the problem where
embedded systems need you to power up the device manually prior to the
device actually enumerating.  If we're doing early resource acquisition
we probably want to solve that too.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org>
To: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	linux-fbdev@vger.kernel.org, linux-acpi@vger.kernel.org,
	alsa-devel@alsa-project.org,
	Stephen Warren <swarren@wwwdotorg.org>,
	Takashi Iwai <tiwai@suse.de>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Liam Girdwood <lgirdwood@gmail.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-gpio@vger.kernel.org,
	Thierry Reding <thierry.reding@gmail.com>,
	Linux PWM List <linux-pwm@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	Alexandre Courbot <gnurou@gmail.com>
Subject: Re: [alsa-devel] [PATCH v2 11/12] ASoC: tegra: register dependency parser for firmware nodes
Date: Thu, 16 Jul 2015 23:04:09 +0000	[thread overview]
Message-ID: <20150716230409.GG1602@sirena.org.uk> (raw)
In-Reply-To: <CAAObsKAHviKKTrEBhuge-KQ115p-kOf69miNnmpR+7knOeg+qg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]

On Tue, Jul 14, 2015 at 02:47:04PM +0200, Tomeu Vizoso wrote:
> On 14 July 2015 at 13:07, Mark Brown <broonie@kernel.org> wrote:

> > I'm not sure how I can be clearer here...  you're replacing something
> > that is currently pure data with open coding in each device.  That seems
> > like a step back in terms of ease of use.

> I could understand that if snd_soc_dai_link had a field with the
> property name, and the core called of_parse_phandle on it, but
> currently what I'm duplicating is:

>     tegra_max98090_dai.cpu_of_node = of_parse_phandle(np,
>             "nvidia,i2s-controller", 0);

> with:

>     add_dependency(fwnode, "nvidia,i2s-controller", deps);

> Admittedly, we could add a cpu_fw_property field to snd_soc_dai_link
> and have the core call of_parse_phandle itself.

Yes, we could - that's really what should be happening here.  The other
bit of this is that we're doing it twice which isn't success.

> But even then, the core doesn't know about a device's snd_soc_dai_link
> until probe() is called and then it's too late for the purposes of
> this series.

That's not a good reason to encourage bad patterns in drivers.  At the
very least the drivers should be able to pass the same struct into both
places, having to open code the same thing in two places is going to be
error prone.

> That's why I mentioned devm_probe, as it would add a common way to
> specify the data needed to acquire resources in each driver, which
> could be made available before probe() is called.

That does avoid the duplication.  However there are issues with the
interface for enumerable buses, it doesn't solve the problem where
embedded systems need you to power up the device manually prior to the
device actually enumerating.  If we're doing early resource acquisition
we probably want to solve that too.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  reply	other threads:[~2015-07-16 23:04 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-01  9:40 [PATCH v2 0/12] Discover and probe dependencies Tomeu Vizoso
2015-07-01  9:40 ` Tomeu Vizoso
2015-07-01  9:40 ` Tomeu Vizoso
2015-07-01  9:40 ` [PATCH v2 01/12] device: property: delay device-driver matches Tomeu Vizoso
2015-07-01  9:40   ` Tomeu Vizoso
2015-07-01  9:40   ` Tomeu Vizoso
2015-07-01 23:29   ` Rafael J. Wysocki
2015-07-01 23:29     ` Rafael J. Wysocki
2015-07-10 11:39     ` Tomeu Vizoso
2015-07-10 11:39       ` Tomeu Vizoso
2015-07-10 11:39       ` Tomeu Vizoso
2015-07-16 20:23   ` Mark Brown
2015-07-16 20:23     ` Mark Brown
2015-07-16 23:41     ` Rafael J. Wysocki
2015-07-16 23:41       ` Rafael J. Wysocki
2015-07-16 23:41       ` Rafael J. Wysocki
2015-07-17  0:06       ` Mark Brown
2015-07-17  0:06         ` Mark Brown
2015-07-01  9:40 ` [PATCH v2 02/12] device: property: find dependencies of a firmware node Tomeu Vizoso
2015-07-01  9:40   ` Tomeu Vizoso
2015-07-01  9:40   ` Tomeu Vizoso
2015-07-01 23:36   ` Rafael J. Wysocki
2015-07-02  0:02     ` Rafael J. Wysocki
2015-07-10 13:14     ` [alsa-devel] " Tomeu Vizoso
2015-07-10 13:14       ` Tomeu Vizoso
2015-07-10 13:14       ` Tomeu Vizoso
2015-07-11  2:52       ` Rafael J. Wysocki
2015-07-11  2:52         ` Rafael J. Wysocki
2015-07-11  2:52         ` Rafael J. Wysocki
2015-07-01  9:40 ` [PATCH v2 03/12] string: Introduce strends() Tomeu Vizoso
2015-07-01  9:40   ` Tomeu Vizoso
2015-07-01  9:40   ` Tomeu Vizoso
2015-07-01  9:40 ` [PATCH v2 04/12] gpio: register dependency parser for firmware nodes Tomeu Vizoso
2015-07-01  9:40   ` Tomeu Vizoso
2015-07-01  9:40   ` Tomeu Vizoso
2015-07-01  9:41 ` [PATCH v2 05/12] gpu: host1x: " Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41 ` [PATCH v2 06/12] backlight: Document consumers of backlight nodes Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41 ` [PATCH v2 07/12] backlight: register dependency parser for firmware nodes Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41 ` [PATCH v2 08/12] USB: EHCI: " Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41 ` [PATCH v2 09/12] regulator: " Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-16 21:38   ` Mark Brown
2015-07-16 21:38     ` Mark Brown
2015-07-01  9:41 ` [PATCH v2 10/12] pwm: " Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41 ` [PATCH v2 11/12] ASoC: tegra: " Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01 17:38   ` Mark Brown
2015-07-01 17:38     ` Mark Brown
2015-07-13 12:10     ` [alsa-devel] " Tomeu Vizoso
2015-07-13 12:10       ` Tomeu Vizoso
2015-07-13 12:10       ` Tomeu Vizoso
2015-07-13 15:42       ` Mark Brown
2015-07-13 15:42         ` Mark Brown
2015-07-13 15:42         ` Mark Brown
2015-07-14  7:34         ` Tomeu Vizoso
2015-07-14  7:34           ` Tomeu Vizoso
2015-07-14  7:34           ` Tomeu Vizoso
2015-07-14 11:07           ` Mark Brown
2015-07-14 11:07             ` Mark Brown
2015-07-14 11:07             ` Mark Brown
2015-07-14 12:47             ` Tomeu Vizoso
2015-07-14 12:47               ` Tomeu Vizoso
2015-07-14 12:47               ` Tomeu Vizoso
2015-07-16 23:04               ` Mark Brown [this message]
2015-07-16 23:04                 ` Mark Brown
2015-07-01  9:41 ` [PATCH v2 12/12] driver-core: probe dependencies before probing Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso
2015-07-01  9:41   ` Tomeu Vizoso

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=20150716230409.GG1602@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gnurou@gmail.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=swarren@wwwdotorg.org \
    --cc=thierry.reding@gmail.com \
    --cc=tiwai@suse.de \
    --cc=tomeu.vizoso@collabora.com \
    /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.