linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Regulator probe
@ 2016-09-01 14:53 Andy Shevchenko
  2016-09-01 15:38 ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-01 14:53 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

Hi!

I have found an issue which I have no idea how to solve properly.
Let's look at the details.

The platform I'm working on (Intel Edison) has the following devices:

Pin control (registered in platform code)
GPIO (PCI device)
SDHCI (PCI device)
WiFi card (SDIO connected to SDHCI)
Regulator for WiFi card (registered in platform code, enumerated via SFI
notifier)

In the kernel configuration
CONFIG_PINCTRL_MERRIFIELD=y
CONFIG_GPIO_MERRIFIELD=y
CONFIG_SDHCI_PCI=y
CONFIG_REGULATOR=y

(All intermediate dependencies also built-in)

Platform has setting
	regulator_has_full_constraints();

During the boot I got the following:

#1

[    0.286099] calling  intel_mid_platform_init+0x0/0x36 @ 1
[    0.290348] Using generic wifi platform data
[    0.291055] initcall intel_mid_platform_init+0x0/0x36 returned 0
after 3906 usecs
[    0.291411] calling  mrfld_pinctrl_init+0x0/0x21 @ 1
[    0.292556] initcall mrfld_pinctrl_init+0x0/0x21 returned 0 after 976
usecs

Regulator for WiFi and pin control are registered.


#2

[    0.555321] calling  mrfld_pinctrl_init+0x0/0x14 @ 1
[    0.563920] initcall mrfld_pinctrl_init+0x0/0x14 returned 0 after
7812 usecs

Pin control is initialized.


#3

[    0.572562] calling  regulator_fixed_voltage_init+0x0/0x14 @ 1
[    0.
573366] bcm43xx-vmmc-regulator: Failed to request enable GPIO96: -517
[  
  0.573708] reg-fixed-voltage reg-fixed-voltage.0.auto: Failed to
register regulator: -517
[    0.574376] initcall
regulator_fixed_voltage_init+0x0/0x14 returned 0 after 1953 usecs

Obviously GPIO device isn't probed yet.


#4

[   12.322718] calling  mrfld_gpio_driver_init+0x0/0x1b @ 1
[   12.349569] initcall mrfld_gpio_driver_init+0x0/0x1b returned 0 after
11716 usecs

GPIO driver is initialized.


# 5

[   20.843973] calling  sdhci_drv_init+0x0/0x20 @ 1
[   20.843976] sdhci: Secure Digital Host Controller Interface driver
[   20.843978] sdhci: Copyright(c) Pierre Ossman
[   20.843989] initcall sdhci_drv_init+0x0/0x20 returned 0 after 5 usecs
[   20.843999] calling  sdhci_driver_init+0x0/0x1b @ 1

[   20.881460] sdhci-pci 0000:00:01.3: SDHCI controller found
[8086:1190] (rev 1)
[   20.881471] sdhci-pci 0000:00:01.3: found 1 slot(s)
[   20.888242] sdhci-pci 0000:00:01.3: No vmmc regulator found

^^^^^ PROBLEM!

Regulator is present, though it's in a waiting list.

[   20.888272] sdhci-pci 0000:00:01.3: No vqmmc regulator found


#6

[   21.520126] calling  deferred_probe_initcall+0x0/0x70 @ 1
[   21.521460] bcm43xx-vmmc-regulator: at 2000 mV
[   21.524831] reg-fixed-voltage reg-fixed-voltage.0.auto: bcm43xx-vmmc-
regulator supplying 2000000uV
[   21.524902] initcall deferred_probe_initcall+0x0/0x70 returned 0
after 4646 usecs

^^^^^ Too late

[   27.479039] calling  regulator_init_complete+0x0/0x30 @ 1
[   27.479048] bcm43xx-vmmc-regulator: disabling


So, my understanding that in _regulator_get() we have to try probe
deferred regulators once again. But I have no idea how to fix this
properly.

P.S. Of course everything works in case of CONFIG_SDHCI=m, but it's
looks like not a (proper?) solution.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: Regulator probe
  2016-09-01 14:53 Regulator probe Andy Shevchenko
@ 2016-09-01 15:38 ` Mark Brown
  2016-09-01 16:15   ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2016-09-01 15:38 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Hunter, Adrian

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

On Thu, Sep 01, 2016 at 05:53:43PM +0300, Andy Shevchenko wrote:

> [   20.843973] calling  sdhci_drv_init+0x0/0x20 @ 1
> [   20.843976] sdhci: Secure Digital Host Controller Interface driver
> [   20.843978] sdhci: Copyright(c) Pierre Ossman
> [   20.843989] initcall sdhci_drv_init+0x0/0x20 returned 0 after 5 usecs
> [   20.843999] calling  sdhci_driver_init+0x0/0x1b @ 1
> 
> [   20.881460] sdhci-pci 0000:00:01.3: SDHCI controller found
> [8086:1190] (rev 1)
> [   20.881471] sdhci-pci 0000:00:01.3: found 1 slot(s)
> [   20.888242] sdhci-pci 0000:00:01.3: No vmmc regulator found

> ^^^^^ PROBLEM!
> 
> Regulator is present, though it's in a waiting list.

The device isn't registered and it's saying it isn't registered, this is
normal.  Since this is an ACPI system we are expecting the firmware or
whatever else registers devices and their supplies to do that, if the
supplies aren't being mapped at device registration time then you're
telling the core not to expect any.  If you had mapped the supply and it
wasn't available the consumer would get an -EPROBE_DEFER.

As I keep saying with all these problems if you want to reimplement DT
instead of using it you need to reimplement *all* of DT, it's there for
a reason.  It would be a lot quicker and simpler to just use DT for
these systems.

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

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

* Re: Regulator probe
  2016-09-01 15:38 ` Mark Brown
@ 2016-09-01 16:15   ` Andy Shevchenko
  2016-09-01 17:02     ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-01 16:15 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

On Thu, 2016-09-01 at 16:38 +0100, Mark Brown wrote:
> On Thu, Sep 01, 2016 at 05:53:43PM +0300, Andy Shevchenko wrote:
> 
> > 
> > [   20.843973] calling  sdhci_drv_init+0x0/0x20 @ 1
> > [   20.843976] sdhci: Secure Digital Host Controller Interface
> > driver
> > [   20.843978] sdhci: Copyright(c) Pierre Ossman
> > [   20.843989] initcall sdhci_drv_init+0x0/0x20 returned 0 after 5
> > usecs
> > [   20.843999] calling  sdhci_driver_init+0x0/0x1b @ 1
> > 
> > [   20.881460] sdhci-pci 0000:00:01.3: SDHCI controller found
> > [8086:1190] (rev 1)
> > [   20.881471] sdhci-pci 0000:00:01.3: found 1 slot(s)
> > [   20.888242] sdhci-pci 0000:00:01.3: No vmmc regulator found
> 
> > 
> > ^^^^^ PROBLEM!
> > 
> > Regulator is present, though it's in a waiting list.
> 
> The device isn't registered and it's saying it isn't registered, this
> is
> normal.  Since this is an ACPI system

Nope, it's SFI. Which basically means we have everything in board files.

>  we are expecting the firmware or
> whatever else registers devices and their supplies to do that, if the
> supplies aren't being mapped at device registration time then you're
> telling the core not to expect any.  If you had mapped the supply and
> it
> wasn't available the consumer would get an -EPROBE_DEFER.

Basically you mean we have not call regulator_has_full_constraints()?
In that case we have to provide stubs for all expected regulators, in
case of SDHCI one real and one dummy, who knows how many them in the
drivers, but for each we have to provide that. Am I right? 

> 
> As I keep saying with all these problems if you want to reimplement DT
> instead of using it you need to reimplement *all* of DT, it's there
> for
> a reason.  It would be a lot quicker and simpler to just use DT for
> these systems.

Here we have platform code. It might be possible to switch to DT for it,
with no firmware support, but I can consider it as a far away from now.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: Regulator probe
  2016-09-01 16:15   ` Andy Shevchenko
@ 2016-09-01 17:02     ` Mark Brown
  2016-09-05 16:01       ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2016-09-01 17:02 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Hunter, Adrian

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

On Thu, Sep 01, 2016 at 07:15:16PM +0300, Andy Shevchenko wrote:
> On Thu, 2016-09-01 at 16:38 +0100, Mark Brown wrote:
> > On Thu, Sep 01, 2016 at 05:53:43PM +0300, Andy Shevchenko wrote:

> > The device isn't registered and it's saying it isn't registered, this
> > is
> > normal.  Since this is an ACPI system

> Nope, it's SFI. Which basically means we have everything in board files.

Ugh, I'd been assured that SFI had been killed off :(

Anyway if that's the case then it should all be working already then
unless SFI goes down the ACPI code paths or someone changed SFI since
SFI won't flag as having full costraints and therefore any missing
regulator will report as deferring.  If that is indeed the case you've
probably got a broken consumer driver that's not handing probe
deferrals.

> >  we are expecting the firmware or
> > whatever else registers devices and their supplies to do that, if the
> > supplies aren't being mapped at device registration time then you're
> > telling the core not to expect any.  If you had mapped the supply and
> > it
> > wasn't available the consumer would get an -EPROBE_DEFER.

> Basically you mean we have not call regulator_has_full_constraints()?
> In that case we have to provide stubs for all expected regulators, in
> case of SDHCI one real and one dummy, who knows how many them in the
> drivers, but for each we have to provide that. Am I right? 

You have to either not call regulator_has_full_constraints() or supply
all the mappings for device supplies before you register a device.  If
you don't call regulator_has_full_constraints() missing regulators will
always defer so you would need to provide a fixed voltage regulator for
it.  If it's really absent you'll need to provide full constraints, we
can't tell otherwise.

> > As I keep saying with all these problems if you want to reimplement DT
> > instead of using it you need to reimplement *all* of DT, it's there
> > for
> > a reason.  It would be a lot quicker and simpler to just use DT for
> > these systems.

> Here we have platform code. It might be possible to switch to DT for it,
> with no firmware support, but I can consider it as a far away from now.

It would be a lot more sensible than SFI, it's so limited you're
basically just using board files but with the limitations of having to
do bits of it through firmware and then join the two up which seems like
the worst of both worlds.

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

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

* Re: Regulator probe
  2016-09-01 17:02     ` Mark Brown
@ 2016-09-05 16:01       ` Andy Shevchenko
  2016-09-06 10:24         ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-05 16:01 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

On Thu, 2016-09-01 at 18:02 +0100, Mark Brown wrote:
> On Thu, Sep 01, 2016 at 07:15:16PM +0300, Andy Shevchenko wrote:
> > 
> > On Thu, 2016-09-01 at 16:38 +0100, Mark Brown wrote:
> > > 
> > > On Thu, Sep 01, 2016 at 05:53:43PM +0300, Andy Shevchenko wrote:
> 
> > 
> > > 
> > > The device isn't registered and it's saying it isn't registered,
> > > this
> > > is
> > > normal.  Since this is an ACPI system
> 
> > 
> > Nope, it's SFI. Which basically means we have everything in board
> > files.
> 
> Ugh, I'd been assured that SFI had been killed off :(

Yes, though devices are flying around (I dunno what is used in latest
MID SoC [Annidale] on Asus and Lenovo phone, I'm pretty sure it's still
SFI).

> 
> Anyway if that's the case then it should all be working already then
> unless SFI goes down the ACPI code paths or someone changed SFI since
> SFI won't flag as having full costraints and therefore any missing
> regulator will report as deferring.

But it's a real burden to describe in platform code _every_ regulators
that device driver may need (for example, "vqmmc" is optional and even
"vmmc" is optional for 2 out of 3 SDHCI host controllers!).

>   If that is indeed the case you've
> probably got a broken consumer driver that's not handing probe
> deferrals.

It has, but see above, for 3 SDHCI controllers I have to provide 5 dummy
regulators and 1 real vs. just 1 real.

> > >  we are expecting the firmware or
> > > whatever else registers devices and their supplies to do that, if
> > > the
> > > supplies aren't being mapped at device registration time then
> > > you're
> > > telling the core not to expect any.  If you had mapped the supply
> > > and
> > > it
> > > wasn't available the consumer would get an -EPROBE_DEFER.
> 
> > 
> > Basically you mean we have not call
> > regulator_has_full_constraints()?
> > In that case we have to provide stubs for all expected regulators,
> > in
> > case of SDHCI one real and one dummy, who knows how many them in the
> > drivers, but for each we have to provide that. Am I right? 
> 
> You have to either not call regulator_has_full_constraints() or supply
> all the mappings for device supplies before you register a device.  If
> you don't call regulator_has_full_constraints() missing regulators
> will
> always defer so you would need to provide a fixed voltage regulator
> for
> it.

It would be plenty of dummy regulators.

>   If it's really absent you'll need to provide full constraints, we
> can't tell otherwise.
> 
> > 
> > > 
> > > As I keep saying with all these problems if you want to
> > > reimplement DT
> > > instead of using it you need to reimplement *all* of DT, it's
> > > there
> > > for
> > > a reason.  It would be a lot quicker and simpler to just use DT
> > > for
> > > these systems.
> 
> > 
> > Here we have platform code. It might be possible to switch to DT for
> > it,
> > with no firmware support, but I can consider it as a far away from
> > now.
> 
> It would be a lot more sensible than SFI, it's so limited you're
> basically just using board files but with the limitations of having to
> do bits of it through firmware and then join the two up which seems
> like
> the worst of both worlds.

I dunno if DT has any means of coexistence with e820 BIOS where PCI
devices are enumerated natively.

So, in other words is there any example of something like following:

pinctrl: pinctrl@ {
	// platform device of pinctrl
}

pci: {
	// anonymous bus ?
	gpio: gpio@BDF1 {
		// GPIO device which is enumerated via PCI
		???
	}
	sdhci: sdhci@BDF2 {
		// SDHCI which requires vmmc@BDF2 to be present
		???
	}
}

regulator: vmmc@BDF2 {
	// Fixed regulator with GPIO provided by gpio@BDF1
}

For now I hardly imagine how it should look like or work.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: Regulator probe
  2016-09-05 16:01       ` Andy Shevchenko
@ 2016-09-06 10:24         ` Mark Brown
  2016-09-07 14:24           ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2016-09-06 10:24 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Hunter, Adrian

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

On Mon, Sep 05, 2016 at 07:01:52PM +0300, Andy Shevchenko wrote:
> On Thu, 2016-09-01 at 18:02 +0100, Mark Brown wrote:

> > Anyway if that's the case then it should all be working already then
> > unless SFI goes down the ACPI code paths or someone changed SFI since
> > SFI won't flag as having full costraints and therefore any missing
> > regulator will report as deferring.

> But it's a real burden to describe in platform code _every_ regulators
> that device driver may need (for example, "vqmmc" is optional and even
> "vmmc" is optional for 2 out of 3 SDHCI host controllers!).

Nothing says you have to describe all regulators, you just need to tell
the core you have told it about everything you're going to tell it
about.  Until you do that the core has to assume that something may come
along later and describe that supply.

> >   If that is indeed the case you've
> > probably got a broken consumer driver that's not handing probe
> > deferrals.

> It has, but see above, for 3 SDHCI controllers I have to provide 5 dummy
> regulators and 1 real vs. just 1 real.

The core will substitute in the dummies once it knows that everything is
described, you don't need to do it manually unless you have to provide
information like voltages.

> > > Basically you mean we have not call
> > > regulator_has_full_constraints()?
> > > In that case we have to provide stubs for all expected regulators,
> > > in
> > > case of SDHCI one real and one dummy, who knows how many them in the
> > > drivers, but for each we have to provide that. Am I right? 

> > You have to either not call regulator_has_full_constraints() or supply
> > all the mappings for device supplies before you register a device.  If
> > you don't call regulator_has_full_constraints() missing regulators
> > will
> > always defer so you would need to provide a fixed voltage regulator
> > for
> > it.

> It would be plenty of dummy regulators.

If and only if you don't provide the mappings.

> > It would be a lot more sensible than SFI, it's so limited you're
> > basically just using board files but with the limitations of having to
> > do bits of it through firmware and then join the two up which seems
> > like
> > the worst of both worlds.

> I dunno if DT has any means of coexistence with e820 BIOS where PCI
> devices are enumerated natively.

Of course it does, DT based systems support enumerable buses like USB
and PCI.  Enumerating via BIOS is no different really.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

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

* Re: Regulator probe
  2016-09-06 10:24         ` Mark Brown
@ 2016-09-07 14:24           ` Andy Shevchenko
  2016-09-09 12:17             ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-07 14:24 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

On Tue, 2016-09-06 at 11:24 +0100, Mark Brown wrote:
> 
> > > Anyway if that's the case then it should all be working already
> > > then unless SFI goes down the ACPI code paths or someone changed
> > > SFI since SFI won't flag as having full costraints and therefore
> > > any missing regulator will report as deferring.
> 
> > 
> > But it's a real burden to describe in platform code _every_
> > regulators that device driver may need (for example, "vqmmc" is
> > optional and even "vmmc" is optional for 2 out of 3 SDHCI host
> > controllers!).
> 
> Nothing says you have to describe all regulators, you just need to
> tell the core you have told it about everything you're going to tell
> it about.  Until you do that the core has to assume that something may
> come along later and describe that supply.
> 

That's I would like to make work. For now we have fixed voltage
regulator which returns EPROBE_DEFER since GPIO IP is not initialized
yet at that point. But regulator framework decides that it's not
possible case and overrides the error code.

For me it still looks that regulator framework would not intercept
deferred regulators in case of full constraints. Feels like I'm missing
something...

> > > > Basically you mean we have not call
> > > > regulator_has_full_constraints()? In that case we have to
> > > > provide stubs for all expected regulators, in case of SDHCI one
> > > > real and one dummy, who knows how many them in the drivers, but
> > > > for each we have to provide that. Am I right?

> > > You have to either not call regulator_has_full_constraints() or
> > > supply all the mappings for device supplies before you register a
> > > device.  If you don't call regulator_has_full_constraints()
> > > missing regulators will always defer so you would need to provide
> > > a fixed voltage regulator for it.

...and fixed regulator is also deferred due to GPIO is not ready _yet_.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: Regulator probe
  2016-09-07 14:24           ` Andy Shevchenko
@ 2016-09-09 12:17             ` Mark Brown
  2016-09-09 12:55               ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2016-09-09 12:17 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Hunter, Adrian

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

On Wed, Sep 07, 2016 at 05:24:01PM +0300, Andy Shevchenko wrote:
> On Tue, 2016-09-06 at 11:24 +0100, Mark Brown wrote:

> > Nothing says you have to describe all regulators, you just need to
> > tell the core you have told it about everything you're going to tell
> > it about.  Until you do that the core has to assume that something may
> > come along later and describe that supply.

> That's I would like to make work. For now we have fixed voltage
> regulator which returns EPROBE_DEFER since GPIO IP is not initialized
> yet at that point. But regulator framework decides that it's not
> possible case and overrides the error code.

What do you mean?  Of course we should handle probe deferral if we fail
to get a resource like a GPIO.  Are you trying to say that this doesn't
work for you?

> For me it still looks that regulator framework would not intercept
> deferred regulators in case of full constraints. Feels like I'm missing
> something...

I can't parse the above, sorry.

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

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

* Re: Regulator probe
  2016-09-09 12:17             ` Mark Brown
@ 2016-09-09 12:55               ` Andy Shevchenko
  2016-09-09 15:29                 ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-09 12:55 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

On Fri, 2016-09-09 at 13:17 +0100, Mark Brown wrote:
> On Wed, Sep 07, 2016 at 05:24:01PM +0300, Andy Shevchenko wrote:
> > 
> > On Tue, 2016-09-06 at 11:24 +0100, Mark Brown wrote:
> 
> > 
> > > 
> > > Nothing says you have to describe all regulators, you just need to
> > > tell the core you have told it about everything you're going to
> > > tell
> > > it about.  Until you do that the core has to assume that something
> > > may
> > > come along later and describe that supply.
> 
> > 
> > That's I would like to make work. For now we have fixed voltage
> > regulator which returns EPROBE_DEFER since GPIO IP is not
> > initialized
> > yet at that point. But regulator framework decides that it's not
> > possible case and overrides the error code.
> 
> What do you mean?  Of course we should handle probe deferral if we
> fail to get a resource like a GPIO.  Are you trying to say that this
> doesn't work for you?

No, it doesn't.

Fixed regulator probe is deferred:

reg-fixed-voltage reg-fixed-voltage.0.auto: Failed to register
regulator: -517

But:
sdhci-pci 0000:00:01.3: No vmmc regulator found

Code in sdhci driver is:
        ret = mmc_regulator_get_supply(mmc);
        if (ret == -EPROBE_DEFER)
                return ret;

mmc_regulator_get_supply():
...
        mmc->supply.vmmc = devm_regulator_get_optional(dev, "vmmc");
        mmc->supply.vqmmc = devm_regulator_get_optional(dev, "vqmmc");

        if (IS_ERR(mmc->supply.vmmc)) {
                if (PTR_ERR(mmc->supply.vmmc) == -EPROBE_DEFER)
                        return -EPROBE_DEFER;
                dev_dbg(dev, "No vmmc regulator found\n");
...


So _regulator_get() returns something else than -EPROBE_DEFER.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: Regulator probe
  2016-09-09 12:55               ` Andy Shevchenko
@ 2016-09-09 15:29                 ` Mark Brown
  2016-09-09 16:10                   ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2016-09-09 15:29 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Hunter, Adrian

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

On Fri, Sep 09, 2016 at 03:55:27PM +0300, Andy Shevchenko wrote:
> On Fri, 2016-09-09 at 13:17 +0100, Mark Brown wrote:

> > What do you mean?  Of course we should handle probe deferral if we
> > fail to get a resource like a GPIO.  Are you trying to say that this
> > doesn't work for you?

> No, it doesn't.

> Fixed regulator probe is deferred:

> reg-fixed-voltage reg-fixed-voltage.0.auto: Failed to register
> regulator: -517

So the regulator probe *does* get deferred as expected...

> But:
> sdhci-pci 0000:00:01.3: No vmmc regulator found

> Code in sdhci driver is:
>         ret = mmc_regulator_get_supply(mmc);
>         if (ret == -EPROBE_DEFER)
>                 return ret;
>
> mmc_regulator_get_supply():
> ...
>         mmc->supply.vmmc = devm_regulator_get_optional(dev, "vmmc");
>         mmc->supply.vqmmc = devm_regulator_get_optional(dev, "vqmmc");

...and then we correctly report that the optional supply that isn't
mapped (as far as I remember) isn't there.

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

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

* Re: Regulator probe
  2016-09-09 15:29                 ` Mark Brown
@ 2016-09-09 16:10                   ` Andy Shevchenko
  2016-09-09 16:38                     ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-09 16:10 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

On Fri, 2016-09-09 at 16:29 +0100, Mark Brown wrote:

> > Fixed regulator probe is deferred:
> 
> > 
> > reg-fixed-voltage reg-fixed-voltage.0.auto: Failed to register
> > regulator: -517
> 
> So the regulator probe *does* get deferred as expected...
> 
> > 
> > But:
> > sdhci-pci 0000:00:01.3: No vmmc regulator found
> 
> > 
> > Code in sdhci driver is:
> >         ret = mmc_regulator_get_supply(mmc);
> >         if (ret == -EPROBE_DEFER)
> >                 return ret;
> > 
> > mmc_regulator_get_supply():
> > ...
> >         mmc->supply.vmmc = devm_regulator_get_optional(dev, "vmmc");
> >         mmc->supply.vqmmc = devm_regulator_get_optional(dev,
> > "vqmmc");
> 
> ...and then we correctly report that the optional supply that isn't
> mapped (as far as I remember) isn't there.

But it *will be* soon there.

Hmm... And the proper fix for this case is... (let's assume there will
not be device tree solution in nearest future)?

If I remove has_full_constraints() call I will get EPROBE_DEFER on all
optional regulators IIRC.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: Regulator probe
  2016-09-09 16:10                   ` Andy Shevchenko
@ 2016-09-09 16:38                     ` Mark Brown
  2016-09-09 17:04                       ` Andy Shevchenko
  2016-09-10 11:03                       ` Andy Shevchenko
  0 siblings, 2 replies; 17+ messages in thread
From: Mark Brown @ 2016-09-09 16:38 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Hunter, Adrian

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

On Fri, Sep 09, 2016 at 07:10:39PM +0300, Andy Shevchenko wrote:
> On Fri, 2016-09-09 at 16:29 +0100, Mark Brown wrote:

> > ...and then we correctly report that the optional supply that isn't
> > mapped (as far as I remember) isn't there.

> But it *will be* soon there.

> Hmm... And the proper fix for this case is... (let's assume there will
> not be device tree solution in nearest future)?

To supply this particular mapping before you set full constraints.

> If I remove has_full_constraints() call I will get EPROBE_DEFER on all
> optional regulators IIRC.

Yes.

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

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

* Re: Regulator probe
  2016-09-09 16:38                     ` Mark Brown
@ 2016-09-09 17:04                       ` Andy Shevchenko
  2016-09-09 17:18                         ` Mark Brown
  2016-09-10 11:03                       ` Andy Shevchenko
  1 sibling, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-09 17:04 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

On Fri, 2016-09-09 at 17:38 +0100, Mark Brown wrote:
> On Fri, Sep 09, 2016 at 07:10:39PM +0300, Andy Shevchenko wrote:
> > 
> > On Fri, 2016-09-09 at 16:29 +0100, Mark Brown wrote:
> 
> > 
> > > 
> > > ...and then we correctly report that the optional supply that
> > > isn't
> > > mapped (as far as I remember) isn't there.
> 
> > 
> > But it *will be* soon there.
> 
> > 
> > Hmm... And the proper fix for this case is... (let's assume there
> > will
> > not be device tree solution in nearest future)?
> 
> To supply this particular mapping before you set full constraints.

Thanks for patience and explanation. I will look later how to achieve
this.

> 
> > 
> > If I remove has_full_constraints() call I will get EPROBE_DEFER on
> > all
> > optional regulators IIRC.
> 
> Yes.

Which basically means that's not a solution I'm trying to get.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: Regulator probe
  2016-09-09 17:04                       ` Andy Shevchenko
@ 2016-09-09 17:18                         ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2016-09-09 17:18 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Hunter, Adrian

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

On Fri, Sep 09, 2016 at 08:04:36PM +0300, Andy Shevchenko wrote:
> On Fri, 2016-09-09 at 17:38 +0100, Mark Brown wrote:

> > > If I remove has_full_constraints() call I will get EPROBE_DEFER on
> > > all
> > > optional regulators IIRC.

> > Yes.

> Which basically means that's not a solution I'm trying to get.

Right, it's not expected to be something that's useful in a finished
system just something that's minimally harmful during development.

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

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

* Re: Regulator probe
  2016-09-09 16:38                     ` Mark Brown
  2016-09-09 17:04                       ` Andy Shevchenko
@ 2016-09-10 11:03                       ` Andy Shevchenko
  2016-09-12 15:27                         ` Mark Brown
  1 sibling, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-10 11:03 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

On Fri, 2016-09-09 at 17:38 +0100, Mark Brown wrote:
> On Fri, Sep 09, 2016 at 07:10:39PM +0300, Andy Shevchenko wrote:
> > On Fri, 2016-09-09 at 16:29 +0100, Mark Brown wrote:
> ...and then we correctly report that the optional supply that
> > > isn't
> > > mapped (as far as I remember) isn't there.
> But it *will be* soon there.
> Hmm... And the proper fix for this case is... (let's assume there
> > will
> > not be device tree solution in nearest future)?
> 
> To supply this particular mapping before you set full constraints.

Please, correct me if I'm wrong in the following:
1) mapping is what kept in the regulator_map_list;
2) the only way to list something for this mapping is to use one of
regulator drivers that will call regulator_register() at the end;
3) in case of fixed voltage regulator it prepares configuration and
description (based on platform code for example) of the regulator and
calls devm_regulator_register();
4) regulator_register() _will not_ add fixed regulator to the mapping if
GPIO is deferred;
5) regulator_dev_lookup() didn't see the regulator before
deferred_probe_initcall() happened.

If the above is correct, how to add mapping to be seen in 5) ?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: Regulator probe
  2016-09-10 11:03                       ` Andy Shevchenko
@ 2016-09-12 15:27                         ` Mark Brown
  2016-09-12 16:01                           ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2016-09-12 15:27 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Hunter, Adrian

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

On Sat, Sep 10, 2016 at 02:03:35PM +0300, Andy Shevchenko wrote:

> Please, correct me if I'm wrong in the following:
> 1) mapping is what kept in the regulator_map_list;
> 2) the only way to list something for this mapping is to use one of
> regulator drivers that will call regulator_register() at the end;
> 3) in case of fixed voltage regulator it prepares configuration and
> description (based on platform code for example) of the regulator and
> calls devm_regulator_register();
> 4) regulator_register() _will not_ add fixed regulator to the mapping if
> GPIO is deferred;
> 5) regulator_dev_lookup() didn't see the regulator before
> deferred_probe_initcall() happened.

> If the above is correct, how to add mapping to be seen in 5) ?

You may need to extend the interfaces offered by the core - one of the
great advantages of an open source kernel like Linux is that you can
modify the whole system if needed.

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

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

* Re: Regulator probe
  2016-09-12 15:27                         ` Mark Brown
@ 2016-09-12 16:01                           ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2016-09-12 16:01 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Hunter, Adrian

On Mon, 2016-09-12 at 16:27 +0100, Mark Brown wrote:
> On Sat, Sep 10, 2016 at 02:03:35PM +0300, Andy Shevchenko wrote:
> 
> > 
> > Please, correct me if I'm wrong in the following:
> > 1) mapping is what kept in the regulator_map_list;
> > 2) the only way to list something for this mapping is to use one of
> > regulator drivers that will call regulator_register() at the end;
> > 3) in case of fixed voltage regulator it prepares configuration and
> > description (based on platform code for example) of the regulator
> > and
> > calls devm_regulator_register();
> > 4) regulator_register() _will not_ add fixed regulator to the
> > mapping if
> > GPIO is deferred;
> > 5) regulator_dev_lookup() didn't see the regulator before
> > deferred_probe_initcall() happened.
> 
> > 
> > If the above is correct, how to add mapping to be seen in 5) ?
> 
> You may need to extend the interfaces offered by the core - one of the
> great advantages of an open source kernel like Linux is that you can
> modify the whole system if needed.

Yeah, the core seems not able to handle my case. So, that's what I was
trying to tell.

I will look at the code later and see what I can amend there.

Thanks!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

end of thread, other threads:[~2016-09-12 16:01 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-01 14:53 Regulator probe Andy Shevchenko
2016-09-01 15:38 ` Mark Brown
2016-09-01 16:15   ` Andy Shevchenko
2016-09-01 17:02     ` Mark Brown
2016-09-05 16:01       ` Andy Shevchenko
2016-09-06 10:24         ` Mark Brown
2016-09-07 14:24           ` Andy Shevchenko
2016-09-09 12:17             ` Mark Brown
2016-09-09 12:55               ` Andy Shevchenko
2016-09-09 15:29                 ` Mark Brown
2016-09-09 16:10                   ` Andy Shevchenko
2016-09-09 16:38                     ` Mark Brown
2016-09-09 17:04                       ` Andy Shevchenko
2016-09-09 17:18                         ` Mark Brown
2016-09-10 11:03                       ` Andy Shevchenko
2016-09-12 15:27                         ` Mark Brown
2016-09-12 16:01                           ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).