From: Tomeu Vizoso <tomeu.vizoso@collabora.com>
To: Mark Brown <broonie@kernel.org>
Cc: "linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"Alexander Holler" <holler@ahsoftware.de>,
"Alexandre Courbot" <gnurou@gmail.com>,
"Andrzej Hajda" <a.hajda@samsung.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
"Grant Likely" <grant.likely@linaro.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
"Javier Martinez Canillas" <javier.martinez@collabora.co.uk>,
"Krzysztof Kozlowski" <k.kozlowski@samsung.com>,
"Kumar Gala" <galak@codeaurora.org>,
"Len Brown" <lenb@kernel.org>,
"Linus Walleij" <linus.walleij@linaro.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Lv Zheng" <lv.zheng@intel.com>,
"Mark Rutland" <mark.rutland@arm.com>,
"Pawel Moll" <pawel.moll@arm.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
"Robert Moore" <robert.moore@intel.com>,
"Rob Herring" <robh+dt@kernel.org>,
"Russell King" <linux@arm.linux.org.uk>,
"Stephen Warren" <swarren@wwwdotorg.org>,
"Terje Bergström" <tbergstrom@nvidia.com>,
"Thierry Reding" <thierry.reding@gmail.com>
Subject: Re: [PATCH 13/13] driver-core: probe dependencies before probing
Date: Tue, 30 Jun 2015 17:18:07 +0200 [thread overview]
Message-ID: <CAAObsKAw3m0_iasSJiuVZnCF+uNrf+36K6NbRg4p1c6VE_mAqA@mail.gmail.com> (raw)
In-Reply-To: <20150617181359.GA14071@sirena.org.uk>
On 17 June 2015 at 20:13, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Jun 17, 2015 at 03:42:23PM +0200, Tomeu Vizoso wrote:
>> Before actually probing a device, find out what dependencies it has and
>> do our best to ensure that they are available at this point.
>
>> This is accomplished by finding out what platform devices need to be
>> probed so the dependencies are available.
>
> ...and then trying to probe them first.
>
>> If any dependencies are still unavailable after that (most probably a
>> missing driver or an error in the HW description from the firmware), we
>> print a nice error message so that people don't have to add a zillion of
>> printks to find out why a device asked for its probe to be deferred.
>
> So, I think I like this approach though I've not done a full pass
> through and I'm not sure how expensive it gets (there's definitely room
> for optimisation as the patch notes).
Have measured it and the overhead doesn't seem to be much, in the
version that I'm close to send.
> I'm not 100% sure I see what
> prints this error message you're referring to (I'm just seeing debug
> prints).
Right, so far I have left them as debug messages because I have so far
tested the series on just one platform and I'm not sure if there
wouldn't be lots of noise in others.
>> +static struct fwnode_handle *get_enclosing_platform_dev(
>> + struct fwnode_handle *fwnode)
>
> Only platform devices?
Yes, this code assumes that devices on other buses will be registered
and probed when their enclosing platform devices are.
>> +static void check_dependencies_per_class(struct class *class, void *data)
>> +{
>> + struct fwnode_handle *fwnode = data;
>> + struct list_head *deps;
>> + struct fwnode_dependency *dep, *tmp;
>> +
>> + if (!class->get_dependencies)
>> + return;
>> +
>> + deps = class->get_dependencies(fwnode);
>> + if (!deps)
>> + return;
>> +
>> + list_for_each_entry_safe(dep, tmp, deps, dependency) {
>> + if (!check_dependency(dep->fwnode))
>> + pr_debug("Dependency '%s' not available\n",
>> + fwnode_get_name(dep->fwnode));
>> +
>> + list_del(&dep->dependency);
>> + kfree(dep);
>> + }
>> +
>> + kfree(deps);
>
> OK, so the caller is responsible for freeing everything and the class
> must allocate - this definitely suggests that
>
> I'm not sure there's any benefit in having deps be dynamically allocated
> here, just put it on the stack and iterate through the list - the
> iteration is going to be cheap if we get nothing back (probably the
> common case) and probably cheaper than the alloc/free.
Have done this and I like it more.
> One thing here is that I was under the impression classes were supposed
> to be going away...
Actually, while looking at more firmware node properties to parse for
dependencies, I found a rather common case in which the bindings are
implemented by individual drivers and not subsystems. Some examples
are nvidia,dpaux, nvidia,panel and nvidia,ddc-i2c-bus.
So in my next version I have dropped class callbacks and have gone
with a way for classes, drivers, whatever to just register a function
to extract dependencies.
Thanks,
Tomeu
WARNING: multiple messages have this Message-ID (diff)
From: tomeu.vizoso@collabora.com (Tomeu Vizoso)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 13/13] driver-core: probe dependencies before probing
Date: Tue, 30 Jun 2015 17:18:07 +0200 [thread overview]
Message-ID: <CAAObsKAw3m0_iasSJiuVZnCF+uNrf+36K6NbRg4p1c6VE_mAqA@mail.gmail.com> (raw)
In-Reply-To: <20150617181359.GA14071@sirena.org.uk>
On 17 June 2015 at 20:13, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Jun 17, 2015 at 03:42:23PM +0200, Tomeu Vizoso wrote:
>> Before actually probing a device, find out what dependencies it has and
>> do our best to ensure that they are available at this point.
>
>> This is accomplished by finding out what platform devices need to be
>> probed so the dependencies are available.
>
> ...and then trying to probe them first.
>
>> If any dependencies are still unavailable after that (most probably a
>> missing driver or an error in the HW description from the firmware), we
>> print a nice error message so that people don't have to add a zillion of
>> printks to find out why a device asked for its probe to be deferred.
>
> So, I think I like this approach though I've not done a full pass
> through and I'm not sure how expensive it gets (there's definitely room
> for optimisation as the patch notes).
Have measured it and the overhead doesn't seem to be much, in the
version that I'm close to send.
> I'm not 100% sure I see what
> prints this error message you're referring to (I'm just seeing debug
> prints).
Right, so far I have left them as debug messages because I have so far
tested the series on just one platform and I'm not sure if there
wouldn't be lots of noise in others.
>> +static struct fwnode_handle *get_enclosing_platform_dev(
>> + struct fwnode_handle *fwnode)
>
> Only platform devices?
Yes, this code assumes that devices on other buses will be registered
and probed when their enclosing platform devices are.
>> +static void check_dependencies_per_class(struct class *class, void *data)
>> +{
>> + struct fwnode_handle *fwnode = data;
>> + struct list_head *deps;
>> + struct fwnode_dependency *dep, *tmp;
>> +
>> + if (!class->get_dependencies)
>> + return;
>> +
>> + deps = class->get_dependencies(fwnode);
>> + if (!deps)
>> + return;
>> +
>> + list_for_each_entry_safe(dep, tmp, deps, dependency) {
>> + if (!check_dependency(dep->fwnode))
>> + pr_debug("Dependency '%s' not available\n",
>> + fwnode_get_name(dep->fwnode));
>> +
>> + list_del(&dep->dependency);
>> + kfree(dep);
>> + }
>> +
>> + kfree(deps);
>
> OK, so the caller is responsible for freeing everything and the class
> must allocate - this definitely suggests that
>
> I'm not sure there's any benefit in having deps be dynamically allocated
> here, just put it on the stack and iterate through the list - the
> iteration is going to be cheap if we get nothing back (probably the
> common case) and probably cheaper than the alloc/free.
Have done this and I like it more.
> One thing here is that I was under the impression classes were supposed
> to be going away...
Actually, while looking at more firmware node properties to parse for
dependencies, I found a rather common case in which the bindings are
implemented by individual drivers and not subsystems. Some examples
are nvidia,dpaux, nvidia,panel and nvidia,ddc-i2c-bus.
So in my next version I have dropped class callbacks and have gone
with a way for classes, drivers, whatever to just register a function
to extract dependencies.
Thanks,
Tomeu
next prev parent reply other threads:[~2015-06-30 15:18 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-17 13:42 [PATCH 00/13] Discover and probe dependencies Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 01/13] gpiolib: Fix docs for gpiochip_add_pingroup_range Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-07-13 12:16 ` Linus Walleij
2015-07-13 12:16 ` Linus Walleij
2015-06-17 13:42 ` [PATCH 02/13] driver-core: defer all probes until late_initcall Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-18 21:50 ` Rafael J. Wysocki
2015-06-18 21:50 ` Rafael J. Wysocki
2015-06-19 13:36 ` Tomeu Vizoso
2015-06-19 13:36 ` Tomeu Vizoso
2015-06-19 23:20 ` Rafael J. Wysocki
2015-06-19 23:20 ` Rafael J. Wysocki
2015-06-23 0:07 ` Rob Herring
2015-06-23 0:07 ` Rob Herring
2015-06-23 14:37 ` Rafael J. Wysocki
2015-06-23 14:37 ` Rafael J. Wysocki
2015-06-23 14:17 ` Tomeu Vizoso
2015-06-23 14:17 ` Tomeu Vizoso
2015-06-23 14:51 ` Rafael J. Wysocki
2015-06-23 14:51 ` Rafael J. Wysocki
2015-06-23 14:37 ` Tomeu Vizoso
2015-06-23 14:37 ` Tomeu Vizoso
2015-06-24 0:14 ` Rafael J. Wysocki
2015-06-24 0:14 ` Rafael J. Wysocki
2015-06-17 13:42 ` [PATCH 03/13] ARM: tegra: Add gpio-ranges property Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 17:25 ` Mark Brown
2015-06-17 17:25 ` Mark Brown
2015-06-18 8:06 ` Tomeu Vizoso
2015-06-18 8:06 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 04/13] pinctrl: tegra: Only set the gpio range if needed Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-07-13 20:14 ` Linus Walleij
2015-07-13 20:14 ` Linus Walleij
2015-07-14 8:34 ` Tomeu Vizoso
2015-07-14 8:34 ` Tomeu Vizoso
2015-07-15 3:17 ` Alexandre Courbot
2015-07-15 3:17 ` Alexandre Courbot
2015-07-15 8:13 ` Tomeu Vizoso
2015-07-15 8:13 ` Tomeu Vizoso
2015-07-17 8:04 ` Linus Walleij
2015-07-17 8:04 ` Linus Walleij
2015-07-17 8:19 ` Tomeu Vizoso
2015-07-17 8:19 ` Tomeu Vizoso
2015-07-17 9:36 ` Linus Walleij
2015-07-17 9:36 ` Linus Walleij
2015-06-17 13:42 ` [PATCH 05/13] driver core: fix docbook for device_private.device Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 06/13] of/platform: Set fwnode field for new devices Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 17:27 ` Mark Brown
2015-06-17 17:27 ` Mark Brown
2015-06-17 13:42 ` [PATCH 07/13] driver-core: Add class.get_dependencies() callback Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 08/13] gpio: sysfs: implement class.get_dependencies() Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 17:40 ` Mark Brown
2015-06-17 17:40 ` Mark Brown
2015-06-30 15:00 ` Tomeu Vizoso
2015-06-30 15:00 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 09/13] gpu: host1x: " Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 10/13] driver-core: add for_each_class() Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 11/13] device property: add fwnode_get_parent() Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 12/13] device property: add fwnode_get_name() Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 13/13] driver-core: probe dependencies before probing Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 18:13 ` Mark Brown
2015-06-17 18:13 ` Mark Brown
2015-06-30 15:18 ` Tomeu Vizoso [this message]
2015-06-30 15:18 ` Tomeu Vizoso
2015-06-18 9:42 ` [PATCH 00/13] Discover and probe dependencies Andrzej Hajda
2015-06-18 9:42 ` Andrzej Hajda
2015-06-18 9:57 ` Russell King - ARM Linux
2015-06-18 9:57 ` Russell King - ARM Linux
2015-06-18 10:36 ` Mark Brown
2015-06-18 10:36 ` Mark Brown
2015-06-18 13:14 ` Andrzej Hajda
2015-06-18 13:14 ` Andrzej Hajda
2015-06-18 14:38 ` Tomeu Vizoso
2015-06-18 14:38 ` Tomeu Vizoso
2015-06-18 14:49 ` Russell King - ARM Linux
2015-06-18 14:49 ` Russell King - ARM Linux
2015-06-18 15:32 ` Alexander Holler
2015-06-18 15:32 ` Alexander Holler
2015-06-18 14:57 ` Tomeu Vizoso
2015-06-18 14:57 ` 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=CAAObsKAw3m0_iasSJiuVZnCF+uNrf+36K6NbRg4p1c6VE_mAqA@mail.gmail.com \
--to=tomeu.vizoso@collabora.com \
--cc=a.hajda@samsung.com \
--cc=arnd@arndb.de \
--cc=broonie@kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=galak@codeaurora.org \
--cc=gnurou@gmail.com \
--cc=grant.likely@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=holler@ahsoftware.de \
--cc=ijc+devicetree@hellion.org.uk \
--cc=javier.martinez@collabora.co.uk \
--cc=k.kozlowski@samsung.com \
--cc=lenb@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=lv.zheng@intel.com \
--cc=mark.rutland@arm.com \
--cc=pawel.moll@arm.com \
--cc=rjw@rjwysocki.net \
--cc=robert.moore@intel.com \
--cc=robh+dt@kernel.org \
--cc=swarren@wwwdotorg.org \
--cc=tbergstrom@nvidia.com \
--cc=thierry.reding@gmail.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.