From: Rajendra Nayak <rnayak@ti.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: grant.likely@secretlab.ca, devicetree-discuss@lists.ozlabs.org,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
tony@atomide.com, lrg@ti.com, b-cousson@ti.com,
patches@linaro.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/5] regulator: map consumer regulator based on device tree
Date: Tue, 11 Oct 2011 11:19:06 +0530 [thread overview]
Message-ID: <4E93D8D2.1050703@ti.com> (raw)
In-Reply-To: <20111010173506.GH3607@opensource.wolfsonmicro.com>
>
>> @@ -1178,6 +1225,10 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
>> goto found;
>> }
>> }
>> + if (!dev)
>> + list_for_each_entry(rdev,®ulator_list, list)
>> + if (strcmp(rdev_get_name(rdev), id))
>> + goto found;
>>
>
> This looks really strange, we didn't remove any old lookup code and the
> DT lookup jumps to found by iself so why are we adding code here?
The old lookup code looks up using the regulator_map_list, and the dt
lookup depends on a dev pointer to extract the of_node.
This above code was needed for cases when a regulator_get() would be
called on dt builds without specifying a device pointer, like the
cpufreq implementations you mentioned.
This is what I tried putting in the comments above the lookup code.
/*
* Lookup happens in 3 ways
* 1. If a dev and dev->of_node exist, look for a
* regulator mapping in dt node.
* 2. Check if a match can be found in regulator_map_list
* if regulator info is still passed in non-dt way
* 3. if !dev, then just look for a matching regulator name.
* Useful for dt builds using regulator_get() without specifying
* a device.
*/
I know its quite complicated but thats because we need to support both
the legacy and the dt based lookups.
>
>> + if (supply) {
>> + struct regulator_dev *r;
>> + struct device_node *node;
>> +
>> + /* first do a dt based lookup */
>> + if (dev) {
>> + node = of_get_regulator(dev, supply);
>> + if (node)
>> + list_for_each_entry(r,®ulator_list, list)
>> + if (node == r->dev.parent->of_node)
>> + goto found;
>> }
>>
>> - if (!found) {
>> - dev_err(dev, "Failed to find supply %s\n",
>> - init_data->supply_regulator);
>> - ret = -ENODEV;
>> - goto scrub;
>> - }
>> + /* next try doing it non-dt way */
>> + list_for_each_entry(r,®ulator_list, list)
>> + if (strcmp(rdev_get_name(r), supply) == 0)
>> + goto found;
>>
>> + dev_err(dev, "Failed to find supply %s\n", supply);
>> + ret = -ENODEV;
>> + goto scrub;
>> +
>> +found:
>
> This is all getting *really* complicated and illegible. I'm not sure
> what the best way to structure is but erroring out early looks useful.
WARNING: multiple messages have this Message-ID (diff)
From: rnayak@ti.com (Rajendra Nayak)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 5/5] regulator: map consumer regulator based on device tree
Date: Tue, 11 Oct 2011 11:19:06 +0530 [thread overview]
Message-ID: <4E93D8D2.1050703@ti.com> (raw)
In-Reply-To: <20111010173506.GH3607@opensource.wolfsonmicro.com>
>
>> @@ -1178,6 +1225,10 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
>> goto found;
>> }
>> }
>> + if (!dev)
>> + list_for_each_entry(rdev,®ulator_list, list)
>> + if (strcmp(rdev_get_name(rdev), id))
>> + goto found;
>>
>
> This looks really strange, we didn't remove any old lookup code and the
> DT lookup jumps to found by iself so why are we adding code here?
The old lookup code looks up using the regulator_map_list, and the dt
lookup depends on a dev pointer to extract the of_node.
This above code was needed for cases when a regulator_get() would be
called on dt builds without specifying a device pointer, like the
cpufreq implementations you mentioned.
This is what I tried putting in the comments above the lookup code.
/*
* Lookup happens in 3 ways
* 1. If a dev and dev->of_node exist, look for a
* regulator mapping in dt node.
* 2. Check if a match can be found in regulator_map_list
* if regulator info is still passed in non-dt way
* 3. if !dev, then just look for a matching regulator name.
* Useful for dt builds using regulator_get() without specifying
* a device.
*/
I know its quite complicated but thats because we need to support both
the legacy and the dt based lookups.
>
>> + if (supply) {
>> + struct regulator_dev *r;
>> + struct device_node *node;
>> +
>> + /* first do a dt based lookup */
>> + if (dev) {
>> + node = of_get_regulator(dev, supply);
>> + if (node)
>> + list_for_each_entry(r,®ulator_list, list)
>> + if (node == r->dev.parent->of_node)
>> + goto found;
>> }
>>
>> - if (!found) {
>> - dev_err(dev, "Failed to find supply %s\n",
>> - init_data->supply_regulator);
>> - ret = -ENODEV;
>> - goto scrub;
>> - }
>> + /* next try doing it non-dt way */
>> + list_for_each_entry(r,®ulator_list, list)
>> + if (strcmp(rdev_get_name(r), supply) == 0)
>> + goto found;
>>
>> + dev_err(dev, "Failed to find supply %s\n", supply);
>> + ret = -ENODEV;
>> + goto scrub;
>> +
>> +found:
>
> This is all getting *really* complicated and illegible. I'm not sure
> what the best way to structure is but erroring out early looks useful.
next prev parent reply other threads:[~2011-10-11 5:49 UTC|newest]
Thread overview: 238+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-10 16:19 [PATCH v2 0/5] Device tree support for regulators Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 16:19 ` [PATCH v2 2/5] dt: add empty dt helpers for non-dt build Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-13 18:32 ` Grant Likely
2011-10-13 18:32 ` Grant Likely
[not found] ` <1318263578-7407-1-git-send-email-rnayak-l0cyMroinI0@public.gmane.org>
2011-10-10 16:19 ` [PATCH v2 1/5] regulator: twl: Remove hardcoded board constraints from driver Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 16:25 ` Mark Brown
2011-10-10 16:25 ` Mark Brown
2011-10-10 16:34 ` Rajendra Nayak
2011-10-10 16:34 ` Rajendra Nayak
2011-10-10 16:19 ` [PATCH v2 3/5] regulator: helper routine to extract regulator_init_data Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 17:22 ` Mark Brown
2011-10-10 17:22 ` Mark Brown
2011-10-10 17:22 ` Mark Brown
2011-10-11 5:59 ` Rajendra Nayak
2011-10-11 5:59 ` Rajendra Nayak
2011-10-13 18:38 ` Grant Likely
2011-10-13 18:38 ` Grant Likely
2011-10-13 18:38 ` Grant Likely
2011-10-13 22:12 ` Mark Brown
2011-10-13 22:12 ` Mark Brown
2011-10-13 22:12 ` Mark Brown
2011-10-13 18:40 ` Grant Likely
2011-10-13 18:40 ` Grant Likely
2011-10-13 18:40 ` Grant Likely
2011-10-16 14:55 ` Shawn Guo
2011-10-16 14:55 ` Shawn Guo
2011-10-16 14:55 ` Shawn Guo
2011-10-17 4:17 ` Rajendra Nayak
2011-10-17 4:17 ` Rajendra Nayak
2011-10-18 11:58 ` Shawn Guo
2011-10-18 11:58 ` Shawn Guo
2011-10-18 11:58 ` Shawn Guo
[not found] ` <20111018115836.GC30703-+NayF8gZjK2ctlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2011-10-18 16:00 ` Mark Brown
2011-10-18 16:00 ` Mark Brown
2011-10-18 16:00 ` Mark Brown
2011-10-19 5:33 ` Shawn Guo
2011-10-19 5:33 ` Shawn Guo
2011-10-19 5:33 ` Shawn Guo
[not found] ` <20111019053354.GB31162-+NayF8gZjK2ctlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2011-10-19 14:47 ` Mark Brown
2011-10-19 14:47 ` Mark Brown
2011-10-19 14:47 ` Mark Brown
2011-10-19 15:04 ` Shawn Guo
2011-10-19 15:04 ` Shawn Guo
2011-10-19 15:04 ` Shawn Guo
2011-10-19 15:10 ` Mark Brown
2011-10-19 15:10 ` Mark Brown
2011-10-20 3:42 ` Rajendra Nayak
2011-10-20 3:42 ` Rajendra Nayak
[not found] ` <4E9F9892.9070007-l0cyMroinI0@public.gmane.org>
2011-10-20 9:41 ` Mark Brown
2011-10-20 9:41 ` Mark Brown
2011-10-20 9:41 ` Mark Brown
[not found] ` <20111020094140.GK18713-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2011-10-20 12:10 ` Rajendra Nayak
2011-10-20 12:10 ` Rajendra Nayak
2011-10-20 12:10 ` Rajendra Nayak
2011-10-20 16:27 ` Tony Lindgren
2011-10-20 16:27 ` Tony Lindgren
2011-10-20 16:27 ` Tony Lindgren
[not found] ` <20111020162743.GB31337-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2011-10-20 16:40 ` Mark Brown
2011-10-20 16:40 ` Mark Brown
2011-10-20 16:40 ` Mark Brown
2011-10-20 17:05 ` Tony Lindgren
2011-10-20 17:05 ` Tony Lindgren
2011-10-20 17:22 ` Tony Lindgren
2011-10-20 17:22 ` Tony Lindgren
2011-10-20 19:57 ` Mark Brown
2011-10-20 19:57 ` Mark Brown
2011-10-20 20:10 ` Tony Lindgren
2011-10-20 20:10 ` Tony Lindgren
2011-10-20 21:42 ` Mark Brown
2011-10-20 21:42 ` Mark Brown
2011-10-20 22:09 ` Tony Lindgren
2011-10-20 22:09 ` Tony Lindgren
2011-10-24 9:07 ` Grant Likely
2011-10-24 9:07 ` Grant Likely
2011-10-20 19:56 ` Mark Brown
2011-10-20 19:56 ` Mark Brown
2011-10-18 13:20 ` Shawn Guo
2011-10-18 13:20 ` Shawn Guo
2011-10-18 13:20 ` Shawn Guo
2011-10-19 11:35 ` Rajendra Nayak
2011-10-19 11:35 ` Rajendra Nayak
2011-10-19 11:35 ` Rajendra Nayak
2011-10-19 14:42 ` Shawn Guo
2011-10-19 14:42 ` Shawn Guo
2011-10-19 14:42 ` Shawn Guo
2011-10-19 14:50 ` Mark Brown
2011-10-19 14:50 ` Mark Brown
2011-10-19 14:50 ` Mark Brown
[not found] ` <20111019144215.GA32007-+NayF8gZjK2ctlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2011-10-20 5:18 ` Rajendra Nayak
2011-10-20 5:18 ` Rajendra Nayak
2011-10-20 5:18 ` Rajendra Nayak
2011-10-20 6:14 ` Shawn Guo
2011-10-20 6:14 ` Shawn Guo
2011-10-20 6:14 ` Shawn Guo
2011-10-20 12:09 ` Rajendra Nayak
2011-10-20 12:09 ` Rajendra Nayak
2011-10-20 12:09 ` Rajendra Nayak
2011-10-21 8:23 ` Shawn Guo
2011-10-21 8:23 ` Shawn Guo
2011-10-21 8:23 ` Shawn Guo
2011-10-21 8:41 ` Rajendra Nayak
2011-10-21 8:41 ` Rajendra Nayak
2011-10-21 11:58 ` Shawn Guo
2011-10-21 11:58 ` Shawn Guo
2011-10-21 11:58 ` Shawn Guo
2011-10-24 6:02 ` Rajendra Nayak
2011-10-24 6:02 ` Rajendra Nayak
2011-10-24 7:34 ` Mark Brown
2011-10-24 7:34 ` Mark Brown
2011-10-24 8:17 ` Grant Likely
2011-10-24 8:17 ` Grant Likely
2011-10-24 8:17 ` Grant Likely
2011-10-24 8:53 ` Rajendra Nayak
2011-10-24 8:53 ` Rajendra Nayak
2011-10-24 8:53 ` Rajendra Nayak
2011-10-24 9:19 ` Mark Brown
2011-10-24 9:19 ` Mark Brown
2011-10-24 9:19 ` Mark Brown
2011-10-24 10:05 ` Rajendra Nayak
2011-10-24 10:05 ` Rajendra Nayak
2011-10-24 9:23 ` Shawn Guo
2011-10-24 9:23 ` Shawn Guo
2011-10-24 9:23 ` Shawn Guo
2011-10-24 9:02 ` Shawn Guo
2011-10-24 9:02 ` Shawn Guo
2011-10-24 9:02 ` Shawn Guo
2011-10-24 8:56 ` Rajendra Nayak
2011-10-24 8:56 ` Rajendra Nayak
2011-10-24 9:11 ` Shawn Guo
2011-10-24 9:11 ` Shawn Guo
2011-10-24 9:11 ` Shawn Guo
2011-10-24 9:13 ` Rajendra Nayak
2011-10-24 9:13 ` Rajendra Nayak
2011-10-24 13:47 ` Shawn Guo
2011-10-24 13:47 ` Shawn Guo
2011-10-24 13:47 ` Shawn Guo
2011-10-25 6:00 ` Rajendra Nayak
2011-10-25 6:00 ` Rajendra Nayak
2011-10-25 6:00 ` Rajendra Nayak
2011-10-25 6:26 ` Rajendra Nayak
2011-10-25 6:26 ` Rajendra Nayak
2011-10-25 6:26 ` Rajendra Nayak
2011-10-25 6:52 ` Shawn Guo
2011-10-25 6:52 ` Shawn Guo
2011-10-25 6:52 ` Shawn Guo
[not found] ` <20111025065216.GD2119-+NayF8gZjK2ctlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2011-10-25 6:56 ` Rajendra Nayak
2011-10-25 6:56 ` Rajendra Nayak
2011-10-25 6:56 ` Rajendra Nayak
2011-10-25 7:20 ` Shawn Guo
2011-10-25 7:20 ` Shawn Guo
2011-10-25 7:20 ` Shawn Guo
2011-10-25 7:13 ` Rajendra Nayak
2011-10-25 7:13 ` Rajendra Nayak
2011-10-25 7:13 ` Rajendra Nayak
2011-10-25 7:42 ` Shawn Guo
2011-10-25 7:42 ` Shawn Guo
2011-10-25 7:42 ` Shawn Guo
2011-10-24 11:35 ` Grant Likely
2011-10-24 11:35 ` Grant Likely
2011-10-24 9:24 ` Grant Likely
2011-10-24 9:24 ` Grant Likely
2011-10-24 9:24 ` Grant Likely
2011-10-24 9:39 ` Mark Brown
2011-10-24 9:39 ` Mark Brown
2011-10-24 13:04 ` Shawn Guo
2011-10-24 13:04 ` Shawn Guo
2011-10-24 13:04 ` Shawn Guo
2011-10-24 13:06 ` Mark Brown
2011-10-24 13:06 ` Mark Brown
2011-10-24 13:40 ` Shawn Guo
2011-10-24 13:40 ` Shawn Guo
2011-10-24 13:40 ` Shawn Guo
2011-10-24 13:49 ` Mark Brown
2011-10-24 13:49 ` Mark Brown
2011-10-24 14:47 ` Shawn Guo
2011-10-24 14:47 ` Shawn Guo
2011-10-24 14:47 ` Shawn Guo
[not found] ` <20111024144716.GG1755-+NayF8gZjK2ctlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2011-10-25 7:11 ` Mark Brown
2011-10-25 7:11 ` Mark Brown
2011-10-25 7:11 ` Mark Brown
2011-10-24 13:59 ` Grant Likely
2011-10-24 13:59 ` Grant Likely
2011-10-24 13:59 ` Grant Likely
2011-10-24 14:51 ` Shawn Guo
2011-10-24 14:51 ` Shawn Guo
2011-10-24 14:51 ` Shawn Guo
2011-10-24 14:56 ` Grant Likely
2011-10-24 14:56 ` Grant Likely
2011-10-24 15:51 ` Shawn Guo
2011-10-24 15:51 ` Shawn Guo
2011-10-24 15:51 ` Shawn Guo
2011-10-24 22:21 ` Grant Likely
2011-10-24 22:21 ` Grant Likely
2011-10-24 22:21 ` Grant Likely
2011-10-25 6:10 ` Rajendra Nayak
2011-10-25 6:10 ` Rajendra Nayak
2011-10-25 7:08 ` Shawn Guo
2011-10-25 7:08 ` Shawn Guo
2011-10-25 7:08 ` Shawn Guo
2011-10-25 7:01 ` Rajendra Nayak
2011-10-25 7:01 ` Rajendra Nayak
2011-10-25 7:01 ` Rajendra Nayak
2011-10-25 7:28 ` Shawn Guo
2011-10-25 7:28 ` Shawn Guo
2011-10-25 7:28 ` Shawn Guo
2011-10-10 16:19 ` [PATCH v2 4/5] regulator: adapt fixed regulator driver to dt Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-13 18:43 ` Grant Likely
2011-10-13 18:43 ` Grant Likely
2011-10-13 18:43 ` Grant Likely
2011-10-10 16:19 ` [PATCH v2 5/5] regulator: map consumer regulator based on device tree Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 16:19 ` Rajendra Nayak
2011-10-10 17:35 ` Mark Brown
2011-10-10 17:35 ` Mark Brown
2011-10-10 17:35 ` Mark Brown
2011-10-11 5:49 ` Rajendra Nayak [this message]
2011-10-11 5:49 ` Rajendra Nayak
2011-10-11 7:08 ` Nayak, Rajendra
2011-10-11 7:08 ` Nayak, Rajendra
2011-10-11 7:08 ` Nayak, Rajendra
[not found] ` <CACYZBRFmLEY_f1UwJa5U9uj8+NVA-WfUVkiuW1yKrpwnrA_00A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-10-13 16:52 ` Mark Brown
2011-10-13 16:52 ` Mark Brown
2011-10-13 16:52 ` Mark Brown
2011-10-13 18:46 ` Grant Likely
2011-10-13 18:46 ` Grant Likely
2011-10-18 13:33 ` Shawn Guo
2011-10-18 13:33 ` Shawn Guo
2011-10-18 13:33 ` Shawn Guo
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=4E93D8D2.1050703@ti.com \
--to=rnayak@ti.com \
--cc=b-cousson@ti.com \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=grant.likely@secretlab.ca \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=lrg@ti.com \
--cc=patches@linaro.org \
--cc=tony@atomide.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.