All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andrew F. Davis" <afd@ti.com>
To: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Lee Jones <lee.jones@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 3/4] regulator: tps65086: Add regulator driver for the TPS65086 PMIC
Date: Mon, 23 Nov 2015 13:18:15 -0600	[thread overview]
Message-ID: <56536677.5070102@ti.com> (raw)
In-Reply-To: <20151123190026.GS26072@sirena.org.uk>

On 11/23/2015 01:00 PM, Mark Brown wrote:
> On Mon, Nov 23, 2015 at 11:40:55AM -0600, Andrew F. Davis wrote:
>
>> But which of_node?
>
>> regulator_config->of_node
>> regulator_config->dev->of_node
>
>> The second is the only one I see getting used, the first is only
>> used when drivers provide their own init_data and automatic init
>> data getting fails.
>
> The configuration of_node is there to override the device one if there
> were some reason to do it.  This should only happen in a situation where
> we weren't able to use the core parsing, with modern drivers it
> indicates a problematic binding so the code deliberately doesn't handle
> it.  Anything with a problematic binding will have generated the
> init_data in driver code anyway.  If we come up with a reason to extend
> the interface we can do that but for now there is no need.
>

Right, so this is the kind of description that would be nice with the
declaration.

>> The same issue is present in GPIO (gpiolib.c:612), where the of_node
>
> Line number references are complately unhelpful if you don't say what
> you're looking at (for me that's a call to irq_find_mapping() which I'm
> guessing isn't what you were talking about).
>

My bad, that is for v4.4-rc1 line 694. The lines are:

> 	of_node = gpiochip->dev->of_node;
> #ifdef CONFIG_OF_GPIO
> 	/*
> 	 * If the gpiochip has an assigned OF node this takes precedence
> 	 * FIXME: get rid of this and use gpiochip->dev->of_node everywhere
> 	 */
> 	if (gpiochip->of_node)
> 		of_node = gpiochip->of_node;
> #endif

So, if we have a config->of_node it is used over config->dev->of_node.

>> in the config takes precedence over the one in config->dev, the
>> opposite is true for regulators, this is very confusing and should be
>> standardized.
>
> No, they both do the same thing.
>

I don't see that, config->dev->of_node is checked for the init data in
regulator_of_get_init_data, then config->of_node is then ignored if that
succeeds.

WARNING: multiple messages have this Message-ID (diff)
From: "Andrew F. Davis" <afd@ti.com>
To: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Lee Jones <lee.jones@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>, <devicetree@vger.kernel.org>,
	<linux-gpio@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 3/4] regulator: tps65086: Add regulator driver for the TPS65086 PMIC
Date: Mon, 23 Nov 2015 13:18:15 -0600	[thread overview]
Message-ID: <56536677.5070102@ti.com> (raw)
In-Reply-To: <20151123190026.GS26072@sirena.org.uk>

On 11/23/2015 01:00 PM, Mark Brown wrote:
> On Mon, Nov 23, 2015 at 11:40:55AM -0600, Andrew F. Davis wrote:
>
>> But which of_node?
>
>> regulator_config->of_node
>> regulator_config->dev->of_node
>
>> The second is the only one I see getting used, the first is only
>> used when drivers provide their own init_data and automatic init
>> data getting fails.
>
> The configuration of_node is there to override the device one if there
> were some reason to do it.  This should only happen in a situation where
> we weren't able to use the core parsing, with modern drivers it
> indicates a problematic binding so the code deliberately doesn't handle
> it.  Anything with a problematic binding will have generated the
> init_data in driver code anyway.  If we come up with a reason to extend
> the interface we can do that but for now there is no need.
>

Right, so this is the kind of description that would be nice with the
declaration.

>> The same issue is present in GPIO (gpiolib.c:612), where the of_node
>
> Line number references are complately unhelpful if you don't say what
> you're looking at (for me that's a call to irq_find_mapping() which I'm
> guessing isn't what you were talking about).
>

My bad, that is for v4.4-rc1 line 694. The lines are:

> 	of_node = gpiochip->dev->of_node;
> #ifdef CONFIG_OF_GPIO
> 	/*
> 	 * If the gpiochip has an assigned OF node this takes precedence
> 	 * FIXME: get rid of this and use gpiochip->dev->of_node everywhere
> 	 */
> 	if (gpiochip->of_node)
> 		of_node = gpiochip->of_node;
> #endif

So, if we have a config->of_node it is used over config->dev->of_node.

>> in the config takes precedence over the one in config->dev, the
>> opposite is true for regulators, this is very confusing and should be
>> standardized.
>
> No, they both do the same thing.
>

I don't see that, config->dev->of_node is checked for the init data in
regulator_of_get_init_data, then config->of_node is then ignored if that
succeeds.

  reply	other threads:[~2015-11-23 19:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-19 23:01 [PATCH v4 0/4] Add support for the TI TPS65086 PMIC Andrew F. Davis
2015-11-19 23:01 ` Andrew F. Davis
2015-11-19 23:01 ` [PATCH v4 1/4] Documentation: tps65086: Add DT bindings for the " Andrew F. Davis
2015-11-19 23:01   ` Andrew F. Davis
2015-11-20 16:33   ` Rob Herring
2015-11-23 16:09   ` Lee Jones
2015-11-23 16:09     ` Lee Jones
2015-11-19 23:01 ` [PATCH v4 2/4] mfd: tps65086: Add driver " Andrew F. Davis
2015-11-19 23:01   ` Andrew F. Davis
2015-11-23 16:20   ` Lee Jones
2015-11-19 23:01 ` [PATCH v4 3/4] regulator: tps65086: Add regulator " Andrew F. Davis
2015-11-19 23:01   ` Andrew F. Davis
2015-11-21 13:37   ` Mark Brown
2015-11-21 20:40     ` Andrew F. Davis
2015-11-21 20:40       ` Andrew F. Davis
2015-11-22 13:13       ` Mark Brown
2015-11-23 17:40         ` Andrew F. Davis
2015-11-23 17:40           ` Andrew F. Davis
2015-11-23 19:00           ` Mark Brown
2015-11-23 19:18             ` Andrew F. Davis [this message]
2015-11-23 19:18               ` Andrew F. Davis
2015-11-24 12:11               ` Mark Brown
2015-11-19 23:01 ` [PATCH v4 4/4] gpio: tps65086: Add GPO " Andrew F. Davis
2015-11-19 23:01   ` Andrew F. Davis
     [not found]   ` <1447974102-24938-5-git-send-email-afd-l0cyMroinI0@public.gmane.org>
2015-11-30  8:50     ` Linus Walleij
2015-11-30  8:50       ` Linus Walleij

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=56536677.5070102@ti.com \
    --to=afd@ti.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=gnurou@gmail.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=sameo@linux.intel.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.