All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] ARM: tegra: set regulator full constraints
@ 2013-10-31  7:05 Wei Ni
       [not found] ` <1383203126-3243-1-git-send-email-wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Wei Ni @ 2013-10-31  7:05 UTC (permalink / raw)
  To: swarren-3lzwWm7+Weoh9ZMKESR00Q, broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, Wei Ni

Set full constraints during machine initialisation, so that the
regulator_get() can get the dummy regulator if the regulator is
physically present and enabled.
Discussed this in the link:
https://lkml.org/lkml/2013/10/8/40

Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/mach-tegra/tegra.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index ce553d5..5c69a75 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -34,6 +34,7 @@
 #include <linux/usb/tegra_usb_phy.h>
 #include <linux/clk/tegra.h>
 #include <linux/irqchip.h>
+#include <linux/regulator/machine.h>
 
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach-types.h>
@@ -135,6 +136,8 @@ static void __init tegra_dt_init(void)
 
 	parent = soc_device_to_device(soc_dev);
 
+	regulator_has_full_constraints();
+
 	/*
 	 * Finished with the static registrations now; fill in the missing
 	 * devices
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found] ` <1383203126-3243-1-git-send-email-wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-10-31 15:56   ` Stephen Warren
       [not found]     ` <52727DB7.9030109-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  2013-10-31 16:20   ` Mark Brown
  1 sibling, 1 reply; 23+ messages in thread
From: Stephen Warren @ 2013-10-31 15:56 UTC (permalink / raw)
  To: Wei Ni, broonie-DgEjT+Ai2ygdnm+yROfE0A; +Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 10/31/2013 01:05 AM, Wei Ni wrote:
> Set full constraints during machine initialisation, so that the
> regulator_get() can get the dummy regulator if the regulator is
> physically present and enabled.
> Discussed this in the link:
> https://lkml.org/lkml/2013/10/8/40

Is this needed to fix some bug that'll be in 3.13 or earlier, or does it
just solve a problem that will only be exposed in new code that will
appear in 3.14 or later?

In other words, do I need to apply this for 3.13, or can I hold off
until after the merge window?

It'd be helpful if the commit description described which
driver/board/DT/commit/... was affected by this. That way, I'd know the
answer to my question just by reading the commit description.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found] ` <1383203126-3243-1-git-send-email-wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2013-10-31 15:56   ` Stephen Warren
@ 2013-10-31 16:20   ` Mark Brown
       [not found]     ` <20131031162029.GF2493-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  1 sibling, 1 reply; 23+ messages in thread
From: Mark Brown @ 2013-10-31 16:20 UTC (permalink / raw)
  To: Wei Ni; +Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q, linux-tegra-u79uwXL29TY76Z2rM5mHXA

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

On Thu, Oct 31, 2013 at 03:05:26PM +0800, Wei Ni wrote:
> Set full constraints during machine initialisation, so that the
> regulator_get() can get the dummy regulator if the regulator is
> physically present and enabled.
> Discussed this in the link:
> https://lkml.org/lkml/2013/10/8/40

This should be done for all DT systems - I'd expect this to be done in
generic DT init code rather than for a specific board.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]     ` <52727DB7.9030109-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-11-01  1:31       ` Wei Ni
       [not found]         ` <52730480.6020607-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Wei Ni @ 2013-11-01  1:31 UTC (permalink / raw)
  To: Stephen Warren, broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 10/31/2013 11:56 PM, Stephen Warren wrote:
> On 10/31/2013 01:05 AM, Wei Ni wrote:
>> Set full constraints during machine initialisation, so that the
>> regulator_get() can get the dummy regulator if the regulator is
>> physically present and enabled.
>> Discussed this in the link:
>> https://lkml.org/lkml/2013/10/8/40
> 
> Is this needed to fix some bug that'll be in 3.13 or earlier, or does it
> just solve a problem that will only be exposed in new code that will
> appear in 3.14 or later?

Let me explain this patch.
I discovered this problem when I worked on lm90 driver.
When I appiled my lm90 power control patches (this patch still in
review), I found the lm90 will be probed failed on Ventana board,
because on this board, the lm90 regulator is physically present and
enabled, so the lm90 driver can't use regulator_get() to get regulator,
and if we didn't set this full contraints, it can't get the dummy
regulator either, then probe failed.
So I think it's better to applied this patch earliy, because this solve
the compatible issue between Mark's dumy regulator patches, device
driver and DT.

Thanks.
Wei.

> 
> In other words, do I need to apply this for 3.13, or can I hold off
> until after the merge window?
> 
> It'd be helpful if the commit description described which
> driver/board/DT/commit/... was affected by this. That way, I'd know the
> answer to my question just by reading the commit description.
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]         ` <52730480.6020607-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-11-01 16:01           ` Stephen Warren
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Warren @ 2013-11-01 16:01 UTC (permalink / raw)
  To: Wei Ni, broonie-DgEjT+Ai2ygdnm+yROfE0A; +Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 10/31/2013 07:31 PM, Wei Ni wrote:
> On 10/31/2013 11:56 PM, Stephen Warren wrote:
>> On 10/31/2013 01:05 AM, Wei Ni wrote:
>>> Set full constraints during machine initialisation, so that the
>>> regulator_get() can get the dummy regulator if the regulator is
>>> physically present and enabled.
>>> Discussed this in the link:
>>> https://lkml.org/lkml/2013/10/8/40
>>
>> Is this needed to fix some bug that'll be in 3.13 or earlier, or does it
>> just solve a problem that will only be exposed in new code that will
>> appear in 3.14 or later?
> 
> Let me explain this patch.
> I discovered this problem when I worked on lm90 driver.
> When I appiled my lm90 power control patches (this patch still in
> review), I found the lm90 will be probed failed on Ventana board,
> because on this board, the lm90 regulator is physically present and
> enabled, so the lm90 driver can't use regulator_get() to get regulator,
> and if we didn't set this full contraints, it can't get the dummy
> regulator either, then probe failed.
> So I think it's better to applied this patch earliy, because this solve
> the compatible issue between Mark's dumy regulator patches, device
> driver and DT.

To be honest, I don't really understand the answer and I don't think
it's answering my question.

Anyway, Mark requested changes to the patch, so I'll wait for V2 anyway.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]     ` <20131031162029.GF2493-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-11-04  5:59       ` Wei Ni
       [not found]         ` <527737C5.5080901-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Wei Ni @ 2013-11-04  5:59 UTC (permalink / raw)
  To: Mark Brown
  Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q, linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 11/01/2013 12:20 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Oct 31, 2013 at 03:05:26PM +0800, Wei Ni wrote:
>> Set full constraints during machine initialisation, so that the
>> regulator_get() can get the dummy regulator if the regulator is
>> physically present and enabled.
>> Discussed this in the link:
>> https://lkml.org/lkml/2013/10/8/40
> 
> This should be done for all DT systems - I'd expect this to be done in
> generic DT init code rather than for a specific board.

Hi, Mark
I didn't find the generic DT init code, do you mean to add it in the
customize_machine() in arch/arm/kernel/setup.c ? But it isn't only for
DT systems.
How about to add it in the regulator subsystem? I mean we can add
following codes in the regulator_init() routine in driver/regulator/core.c:
+#ifdef CONFIG_OF
+regulator_has_full_constraints();
+#endif

Thanks.
Wei.

> 
> * Unknown Key
> * 0x7EA229BD
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]         ` <527737C5.5080901-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-11-04 16:18           ` Mark Brown
       [not found]             ` <20131104161828.GK2493-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2013-11-04 16:18 UTC (permalink / raw)
  To: Wei Ni; +Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q, linux-tegra-u79uwXL29TY76Z2rM5mHXA

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

On Mon, Nov 04, 2013 at 01:59:33PM +0800, Wei Ni wrote:

> I didn't find the generic DT init code, do you mean to add it in the
> customize_machine() in arch/arm/kernel/setup.c ? But it isn't only for
> DT systems.

We can do it as part of parsing the DT or deciding to parse the DT.

> How about to add it in the regulator subsystem? I mean we can add
> following codes in the regulator_init() routine in driver/regulator/core.c:
> +#ifdef CONFIG_OF
> +regulator_has_full_constraints();
> +#endif

No, that's not good - we can have DT enabled but boot on a non-DT board.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]             ` <20131104161828.GK2493-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-11-05  9:05               ` Wei Ni
       [not found]                 ` <5278B4CB.9050305-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Wei Ni @ 2013-11-05  9:05 UTC (permalink / raw)
  To: Mark Brown
  Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q, linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 11/05/2013 12:18 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Mon, Nov 04, 2013 at 01:59:33PM +0800, Wei Ni wrote:
> 
>> I didn't find the generic DT init code, do you mean to add it in the
>> customize_machine() in arch/arm/kernel/setup.c ? But it isn't only for
>> DT systems.
> 
> We can do it as part of parsing the DT or deciding to parse the DT.

I checked the kernel codes, it seems the different arch uses different
way to parse DT, and I think these codes are related with DT, it's not
better to add such regulator full constraints codes.

> 
>> How about to add it in the regulator subsystem? I mean we can add
>> following codes in the regulator_init() routine in driver/regulator/core.c:
>> +#ifdef CONFIG_OF
>> +regulator_has_full_constraints();
>> +#endif
> 
> No, that's not good - we can have DT enabled but boot on a non-DT board.

I found in regulator_init_complete(void), there has following codes:
"
/*
 * Since DT doesn't provide an idiomatic mechanism for
 * enabling full constraints and since it's much more natural
 * with DT to provide them just assume that a DT enabled
 * system has full constraints.
 */
if (of_have_populated_dt())
                has_full_constraints = true;
"
According to the comment, this is what we want. Since this routine is
called by late_initcall, it's later than some device probing, so it will
cause the device can't get the dummy regulator.
I think we can move them to the regulator_init() routine, then we can
fix this issue.

Thanks.
Wei.

> 
> * Unknown Key
> * 0x7EA229BD
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                 ` <5278B4CB.9050305-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-11-06  8:51                   ` Mark Brown
       [not found]                     ` <20131106085100.GB11602-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2013-11-06  8:51 UTC (permalink / raw)
  To: Wei Ni; +Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q, linux-tegra-u79uwXL29TY76Z2rM5mHXA

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

On Tue, Nov 05, 2013 at 05:05:15PM +0800, Wei Ni wrote:
> On 11/05/2013 12:18 AM, Mark Brown wrote:

> > We can do it as part of parsing the DT or deciding to parse the DT.

> I checked the kernel codes, it seems the different arch uses different
> way to parse DT, and I think these codes are related with DT, it's not
> better to add such regulator full constraints codes.

There's a lot of shared code there, and it's most likely that any new
platforms will be using the flattened device tree code.  Anything other
than doing this when we start device tree is going to leave a window
where we will be using device tree but not handling that in the
regulator API.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                     ` <20131106085100.GB11602-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-11-06 10:39                       ` Wei Ni
       [not found]                         ` <527A1C47.6050405-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Wei Ni @ 2013-11-06 10:39 UTC (permalink / raw)
  To: Mark Brown
  Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/06/2013 04:51 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Tue, Nov 05, 2013 at 05:05:15PM +0800, Wei Ni wrote:
>> On 11/05/2013 12:18 AM, Mark Brown wrote:
> 
>>> We can do it as part of parsing the DT or deciding to parse the DT.
> 
>> I checked the kernel codes, it seems the different arch uses different
>> way to parse DT, and I think these codes are related with DT, it's not
>> better to add such regulator full constraints codes.
> 
> There's a lot of shared code there, and it's most likely that any new
> platforms will be using the flattened device tree code.  Anything other
> than doing this when we start device tree is going to leave a window
> where we will be using device tree but not handling that in the
> regulator API.

I still can't find a good place to set full_constraints, could you
please show me some reference codes where we can set it?

BTW, add Cc to devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Thanks.
Wei.

> 
> * Unknown Key
> * 0x7EA229BD
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                         ` <527A1C47.6050405-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-11-06 11:01                           ` Mark Brown
       [not found]                             ` <20131106110154.GG11602-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2013-11-06 11:01 UTC (permalink / raw)
  To: Wei Ni
  Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Wed, Nov 06, 2013 at 06:39:03PM +0800, Wei Ni wrote:

> I still can't find a good place to set full_constraints, could you
> please show me some reference codes where we can set it?

The places where we unflatten the device tree for example, or start
instantiating devices from DT.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                             ` <20131106110154.GG11602-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-11-12  5:52                               ` Wei Ni
       [not found]                                 ` <5281C228.3000404-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Wei Ni @ 2013-11-12  5:52 UTC (permalink / raw)
  To: Mark Brown
  Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/06/2013 07:01 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Wed, Nov 06, 2013 at 06:39:03PM +0800, Wei Ni wrote:
> 
>> I still can't find a good place to set full_constraints, could you
>> please show me some reference codes where we can set it?
> 
> The places where we unflatten the device tree for example, or start
> instantiating devices from DT.

Hi, Mark
Sorry, I really don't familiar with the DT, could you please take this
by youself?

Thanks.
Wei.

> 
> * Unknown Key
> * 0x7EA229BD
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                 ` <5281C228.3000404-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-11-12 18:20                                   ` Stephen Warren
       [not found]                                     ` <5282717C.3050502-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Warren @ 2013-11-12 18:20 UTC (permalink / raw)
  To: Wei Ni, Mark Brown
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/11/2013 10:52 PM, Wei Ni wrote:
> On 11/06/2013 07:01 PM, Mark Brown wrote:
>> * PGP Signed by an unknown key
>>
>> On Wed, Nov 06, 2013 at 06:39:03PM +0800, Wei Ni wrote:
>>
>>> I still can't find a good place to set full_constraints, could you
>>> please show me some reference codes where we can set it?
>>
>> The places where we unflatten the device tree for example, or start
>> instantiating devices from DT.
> 
> Hi, Mark
> Sorry, I really don't familiar with the DT, could you please take this
> by youself?

I think if you want a feature implemented or a patch created, it's up to
you to do so.

Isn't this as simple as moving the following code:

> 	/*
> 	 * Since DT doesn't provide an idiomatic mechanism for
> 	 * enabling full constraints and since it's much more natural
> 	 * with DT to provide them just assume that a DT enabled
> 	 * system has full constraints.
> 	 */
> 	if (of_have_populated_dt())
> 		has_full_constraints = true;

... out of regulator_init_complete() and into regulator_init()?

The only issue you may have to watch out for is: When is
regulator_init() called (i.e. when does core_initcall happen) relative
to when driver probe()s can be called? If it's earlier, then
core_initcall is early enough I suspect.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                     ` <5282717C.3050502-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-11-13 10:12                                       ` Wei Ni
  2013-11-13 12:23                                       ` Mark Brown
  1 sibling, 0 replies; 23+ messages in thread
From: Wei Ni @ 2013-11-13 10:12 UTC (permalink / raw)
  To: Stephen Warren, Mark Brown
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/13/2013 02:20 AM, Stephen Warren wrote:
> On 11/11/2013 10:52 PM, Wei Ni wrote:
>> On 11/06/2013 07:01 PM, Mark Brown wrote:
>>> * PGP Signed by an unknown key
>>>
>>> On Wed, Nov 06, 2013 at 06:39:03PM +0800, Wei Ni wrote:
>>>
>>>> I still can't find a good place to set full_constraints, could you
>>>> please show me some reference codes where we can set it?
>>>
>>> The places where we unflatten the device tree for example, or start
>>> instantiating devices from DT.
>>
>> Hi, Mark
>> Sorry, I really don't familiar with the DT, could you please take this
>> by youself?
> 
> I think if you want a feature implemented or a patch created, it's up to
> you to do so.
> 
> Isn't this as simple as moving the following code:

Yes, I had sent patch:
[PATCH v2] regulator: core: set full constraints in regulator_init
http://www.mail-archive.com/linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg13788.html .
It's same as you said, but Mark think it should be set in DT.
If we set it in DT, it will affect all DT system, and I still didn't
find a good place to set it.

So I think it's better to let Mark to handle it.

Thanks.
Wei.

> 
>> 	/*
>> 	 * Since DT doesn't provide an idiomatic mechanism for
>> 	 * enabling full constraints and since it's much more natural
>> 	 * with DT to provide them just assume that a DT enabled
>> 	 * system has full constraints.
>> 	 */
>> 	if (of_have_populated_dt())
>> 		has_full_constraints = true;
> 
> ... out of regulator_init_complete() and into regulator_init()?
> 
> The only issue you may have to watch out for is: When is
> regulator_init() called (i.e. when does core_initcall happen) relative
> to when driver probe()s can be called? If it's earlier, then
> core_initcall is early enough I suspect.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                     ` <5282717C.3050502-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  2013-11-13 10:12                                       ` Wei Ni
@ 2013-11-13 12:23                                       ` Mark Brown
       [not found]                                         ` <20131113122329.GC878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  1 sibling, 1 reply; 23+ messages in thread
From: Mark Brown @ 2013-11-13 12:23 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Tue, Nov 12, 2013 at 11:20:44AM -0700, Stephen Warren wrote:

> The only issue you may have to watch out for is: When is
> regulator_init() called (i.e. when does core_initcall happen) relative
> to when driver probe()s can be called? If it's earlier, then
> core_initcall is early enough I suspect.

What I said was to set this up when we hand the DT over to the DT code
to be parsed so that we don't need to worry about any gaps like that,
it seems like a more direct solution than worrying about initcall
ordering.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                         ` <20131113122329.GC878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-11-13 17:29                                           ` Stephen Warren
       [not found]                                             ` <5283B6E3.9070206-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Warren @ 2013-11-13 17:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/13/2013 05:23 AM, Mark Brown wrote:
> On Tue, Nov 12, 2013 at 11:20:44AM -0700, Stephen Warren wrote:
> 
>> The only issue you may have to watch out for is: When is 
>> regulator_init() called (i.e. when does core_initcall happen)
>> relative to when driver probe()s can be called? If it's earlier,
>> then core_initcall is early enough I suspect.
> 
> What I said was to set this up when we hand the DT over to the DT
> code to be parsed so that we don't need to worry about any gaps
> like that, it seems like a more direct solution than worrying about
> initcall ordering.

I guess this means putting the call to
regulator_has_full_constraints() somewhere inside
early_init_dt_scan(), unflatten_device_tree(), or just perhaps inside
of_platform_populate() (all in drivers/of/*.c).

It seems slightly odd to tightly link the drivers/of and
drivers/regulator code like that, but I guess with the appropriate
ifdefs it'll work out OK.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                             ` <5283B6E3.9070206-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-11-13 19:07                                               ` Mark Brown
       [not found]                                                 ` <20131113190745.GD878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2013-11-13 19:07 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Wed, Nov 13, 2013 at 10:29:07AM -0700, Stephen Warren wrote:

> It seems slightly odd to tightly link the drivers/of and
> drivers/regulator code like that, but I guess with the appropriate
> ifdefs it'll work out OK.

This isn't really regulator specific - it's something that applies in
general to things implementing deferred probing - so we ought to have a
generic "we know if devices can appear later or not" flag that
subsystems can check.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                                 ` <20131113190745.GD878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-11-13 20:13                                                   ` Stephen Warren
       [not found]                                                     ` <5283DD52.5050903-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Warren @ 2013-11-13 20:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/13/2013 12:07 PM, Mark Brown wrote:
> On Wed, Nov 13, 2013 at 10:29:07AM -0700, Stephen Warren wrote:
> 
>> It seems slightly odd to tightly link the drivers/of and 
>> drivers/regulator code like that, but I guess with the
>> appropriate ifdefs it'll work out OK.
> 
> This isn't really regulator specific - it's something that applies
> in general to things implementing deferred probing - so we ought to
> have a generic "we know if devices can appear later or not" flag
> that subsystems can check.

I guess I misunderstand then, since given that modules exist, wouldn't
that flag always be true?

IIUC, the issue being discussed here isn't about deferred probe at
all. You always must defer probe if an object is specified as existing
yet the provider isn't available yet. The issue here is when a
regulator isn't specified as existing, yet something asks for that
regulator, should the regulator subsystem automatically provide a
dummy regulator instead, rather than erroring out.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                                     ` <5283DD52.5050903-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-11-13 20:49                                                       ` Mark Brown
       [not found]                                                         ` <20131113204923.GG878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2013-11-13 20:49 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Wed, Nov 13, 2013 at 01:13:06PM -0700, Stephen Warren wrote:
> On 11/13/2013 12:07 PM, Mark Brown wrote:

> > This isn't really regulator specific - it's something that applies
> > in general to things implementing deferred probing - so we ought to
> > have a generic "we know if devices can appear later or not" flag
> > that subsystems can check.

> I guess I misunderstand then, since given that modules exist, wouldn't
> that flag always be true?

No, with DT you can say that if there is no DT binding configuring a
given thing (clock, regulator, GPIO or whatever) then no amount of
module loading will ever cause it to appear - this is what the flag in
question controls.

> IIUC, the issue being discussed here isn't about deferred probe at
> all. You always must defer probe if an object is specified as existing
> yet the provider isn't available yet. The issue here is when a
> regulator isn't specified as existing, yet something asks for that
> regulator, should the regulator subsystem automatically provide a
> dummy regulator instead, rather than erroring out.

No, it's a deferred probing thing - one of the effects here is that
we're choosing between deferring waiting for the resource we know is
bound to appear and substituting a dummy for something we know exists
physically and we know can never be provided by software.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                                         ` <20131113204923.GG878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-11-13 21:59                                                           ` Stephen Warren
       [not found]                                                             ` <5283F625.403-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Warren @ 2013-11-13 21:59 UTC (permalink / raw)
  To: Mark Brown
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/13/2013 01:49 PM, Mark Brown wrote:
> On Wed, Nov 13, 2013 at 01:13:06PM -0700, Stephen Warren wrote:
>> On 11/13/2013 12:07 PM, Mark Brown wrote:
> 
>>> This isn't really regulator specific - it's something that
>>> applies in general to things implementing deferred probing - so
>>> we ought to have a generic "we know if devices can appear later
>>> or not" flag that subsystems can check.
> 
>> I guess I misunderstand then, since given that modules exist,
>> wouldn't that flag always be true?
> 
> No, with DT you can say that if there is no DT binding configuring
> a given thing (clock, regulator, GPIO or whatever) then no amount
> of module loading will ever cause it to appear - this is what the
> flag in question controls.

But we do have a binding for regulators, so wouldn't that flag always
be true?

Perhaps you can suggest a name for the flag, and a specific set of
conditions when it will have specific values. That might help me
understand what you mean.

Are you confusing having a binding (schema definition) with having DT
content ("foo-supply" property)?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                                             ` <5283F625.403-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-11-13 22:29                                                               ` Mark Brown
       [not found]                                                                 ` <20131113222953.GA26614-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2013-11-13 22:29 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Wed, Nov 13, 2013 at 02:59:01PM -0700, Stephen Warren wrote:
> On 11/13/2013 01:49 PM, Mark Brown wrote:

> > No, with DT you can say that if there is no DT binding configuring
> > a given thing (clock, regulator, GPIO or whatever) then no amount
> > of module loading will ever cause it to appear - this is what the
> > flag in question controls.

> But we do have a binding for regulators, so wouldn't that flag always
> be true?

In theory.  In practice people often add bindings for devices without
including the regulators and then someone comes along and adds the
regulators later, perhaps not even using a system with DT, and renders
all existing DTs buggy.  This is generally miserable for everyone so
it's better if we're liberal in what we accept.

> Perhaps you can suggest a name for the flag, and a specific set of
> conditions when it will have specific values. That might help me
> understand what you mean.

Well, of_have_populated_dt() is essentially doing the same thing
(probably, I don't know if it's set at quite the right time) - it's
saying we have a DT.  We could even change the users to check that as
well if it's doing the right thing.

> Are you confusing having a binding (schema definition) with having DT
> content ("foo-supply" property)?

No.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                                                 ` <20131113222953.GA26614-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-11-13 23:41                                                                   ` Stephen Warren
       [not found]                                                                     ` <52840E23.5080105-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Warren @ 2013-11-13 23:41 UTC (permalink / raw)
  To: Mark Brown
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/13/2013 03:29 PM, Mark Brown wrote:
> On Wed, Nov 13, 2013 at 02:59:01PM -0700, Stephen Warren wrote:
>> On 11/13/2013 01:49 PM, Mark Brown wrote:
> 
>>> No, with DT you can say that if there is no DT binding configuring
>>> a given thing (clock, regulator, GPIO or whatever) then no amount
>>> of module loading will ever cause it to appear - this is what the
>>> flag in question controls.
> 
>> But we do have a binding for regulators, so wouldn't that flag always
>> be true?
> 
> In theory.  In practice people often add bindings for devices without
> including the regulators and then someone comes along and adds the
> regulators later, perhaps not even using a system with DT, and renders
> all existing DTs buggy.  This is generally miserable for everyone so
> it's better if we're liberal in what we accept.

But that's a per-binding issue. Earlier, you wrote:

>> The only issue you may have to watch out for is: When is
>> regulator_init() called (i.e. when does core_initcall happen) relative
>> to when driver probe()s can be called? If it's earlier, then
>> core_initcall is early enough I suspect.
> 
> What I said was to set this up when we hand the DT over to the DT code
> to be parsed so that we don't need to worry about any gaps like that,
> it seems like a more direct solution than worrying about initcall
> ordering.

That sounds like a system-global flag, not a per-device/binding flag.
Has the conversation shifted topics? And indeed, I've been talking about
the system-global has_full_constraints flag all along here.

>> Perhaps you can suggest a name for the flag, and a specific set of
>> conditions when it will have specific values. That might help me
>> understand what you mean.
> 
> Well, of_have_populated_dt() is essentially doing the same thing
> (probably, I don't know if it's set at quite the right time) - it's
> saying we have a DT.  We could even change the users to check that as
> well if it's doing the right thing.

But the regulator code already calls of_have_populated_dt() in order to
determine whether to set has_full_constraints = true; The only issue is
that it does it too late. If of_have_populated_dt() is the flag that the
code should key off, what's wrong with the suggestion I made yesterday
to simply move the existing code (that uses of_have_populated_dt()) to
an earlier location?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH RESEND] ARM: tegra: set regulator full constraints
       [not found]                                                                     ` <52840E23.5080105-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-11-14 11:36                                                                       ` Mark Brown
  0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2013-11-14 11:36 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Wei Ni, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Wed, Nov 13, 2013 at 04:41:23PM -0700, Stephen Warren wrote:
> On 11/13/2013 03:29 PM, Mark Brown wrote:

> > In theory.  In practice people often add bindings for devices without
> > including the regulators and then someone comes along and adds the
> > regulators later, perhaps not even using a system with DT, and renders
> > all existing DTs buggy.  This is generally miserable for everyone so
> > it's better if we're liberal in what we accept.

> But that's a per-binding issue. Earlier, you wrote:

No, it's an issue that affects running systems every time a binding gets
updated.  The binding doesn't care.

> That sounds like a system-global flag, not a per-device/binding flag.
> Has the conversation shifted topics? And indeed, I've been talking about
> the system-global has_full_constraints flag all along here.

Yes, this is a behaviour which is implemented by the system in order to
help keep device trees valid.

> But the regulator code already calls of_have_populated_dt() in order to
> determine whether to set has_full_constraints = true; The only issue is
> that it does it too late. If of_have_populated_dt() is the flag that the
> code should key off, what's wrong with the suggestion I made yesterday
> to simply move the existing code (that uses of_have_populated_dt()) to
> an earlier location?

This is a third suggestion which is to check have_populated_dt() at use
time not at some random point during initialisation which may or may not
be the right one.  And someone needs to check that this is actually the
check we want to use earlier on.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2013-11-14 11:36 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-31  7:05 [PATCH RESEND] ARM: tegra: set regulator full constraints Wei Ni
     [not found] ` <1383203126-3243-1-git-send-email-wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-10-31 15:56   ` Stephen Warren
     [not found]     ` <52727DB7.9030109-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-01  1:31       ` Wei Ni
     [not found]         ` <52730480.6020607-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-11-01 16:01           ` Stephen Warren
2013-10-31 16:20   ` Mark Brown
     [not found]     ` <20131031162029.GF2493-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-04  5:59       ` Wei Ni
     [not found]         ` <527737C5.5080901-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-11-04 16:18           ` Mark Brown
     [not found]             ` <20131104161828.GK2493-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-05  9:05               ` Wei Ni
     [not found]                 ` <5278B4CB.9050305-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-11-06  8:51                   ` Mark Brown
     [not found]                     ` <20131106085100.GB11602-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-06 10:39                       ` Wei Ni
     [not found]                         ` <527A1C47.6050405-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-11-06 11:01                           ` Mark Brown
     [not found]                             ` <20131106110154.GG11602-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-12  5:52                               ` Wei Ni
     [not found]                                 ` <5281C228.3000404-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-11-12 18:20                                   ` Stephen Warren
     [not found]                                     ` <5282717C.3050502-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-13 10:12                                       ` Wei Ni
2013-11-13 12:23                                       ` Mark Brown
     [not found]                                         ` <20131113122329.GC878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-13 17:29                                           ` Stephen Warren
     [not found]                                             ` <5283B6E3.9070206-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-13 19:07                                               ` Mark Brown
     [not found]                                                 ` <20131113190745.GD878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-13 20:13                                                   ` Stephen Warren
     [not found]                                                     ` <5283DD52.5050903-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-13 20:49                                                       ` Mark Brown
     [not found]                                                         ` <20131113204923.GG878-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-13 21:59                                                           ` Stephen Warren
     [not found]                                                             ` <5283F625.403-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-13 22:29                                                               ` Mark Brown
     [not found]                                                                 ` <20131113222953.GA26614-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-13 23:41                                                                   ` Stephen Warren
     [not found]                                                                     ` <52840E23.5080105-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-14 11:36                                                                       ` Mark Brown

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.