* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
@ 2012-07-03 10:06 ` Mark Brown
2012-07-03 10:12 ` Guennadi Liakhovetski
` (26 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-07-03 10:06 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 208 bytes --]
On Tue, Jul 03, 2012 at 06:59:30PM +0900, Tetsuyuki Kobayashi wrote:
> +CONFIG_REGULATOR_DUMMY=y
No, this should only ever be enabled as a crutch to get things working.
It should not be enabled by default.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
2012-07-03 10:06 ` Mark Brown
@ 2012-07-03 10:12 ` Guennadi Liakhovetski
2012-07-03 10:31 ` Tetsuyuki Kobayashi
` (25 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-03 10:12 UTC (permalink / raw)
To: linux-sh
Hello
On Tue, 3 Jul 2012, Tetsuyuki Kobayashi wrote:
> Add REGULATOR settings to kernel config file of kzm9g.
>
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> ---
> Hello,
>
> I found Guennadi's
> "ARM: mach-shmobile: add fixed voltage regulators to kzm9g"
> requires REGULATOR setting in kernel config.
> Without this, it causes these errors at boot time.
Thanks for the patch. However, I think, this error will only appear if you
enable REGULATOR in the kernel, but both REGULATOR_FIXED_VOLTAGE and
REGULATOR_DUMMY are disabled. To fix this fixing the defconfig is good,
but not sufficient, I think. How about adding something like
select REGULATOR_FIXED_VOLTAGE if REGULATOR
to either the CPU or the arch section in respective Kconfig? I would
actually propose to add these to both arm/mach-shmobile and sh. Any
objections?
Thanks
Guennadi
> smsc911x: Driver version 2008-10-21
> smsc911x smsc911x.0: Failed to get supply 'vdd33a': -517
> smsc911x smsc911x.0: (unregistered net_device): couldn't get regulators -517
> platform smsc911x.0: Driver smsc911x requests probe deferral
>
>
> After applying this patch,
> make kzm9g_defconfig
> make menuconfig (as you like)
> make uImage
>
> smsc911x ethernet device works well as before.
>
>
> arch/arm/configs/kzm9g_defconfig | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/configs/kzm9g_defconfig b/arch/arm/configs/kzm9g_defconfig
> index 7b2eecd..d2df401 100644
> --- a/arch/arm/configs/kzm9g_defconfig
> +++ b/arch/arm/configs/kzm9g_defconfig
> @@ -82,6 +82,9 @@ CONFIG_I2C_CHARDEV=y
> CONFIG_I2C_SH_MOBILE=y
> CONFIG_GPIO_PCF857X=y
> # CONFIG_HWMON is not set
> +CONFIG_REGULATOR=y
> +CONFIG_REGULATOR_DUMMY=y
> +CONFIG_REGULATOR_FIXED_VOLTAGE=y
> CONFIG_FB=y
> CONFIG_FB_SH_MOBILE_LCDC=y
> CONFIG_FRAMEBUFFER_CONSOLE=y
> -- 1.7.9.5
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
2012-07-03 10:06 ` Mark Brown
2012-07-03 10:12 ` Guennadi Liakhovetski
@ 2012-07-03 10:31 ` Tetsuyuki Kobayashi
2012-07-03 11:15 ` Tetsuyuki Kobayashi
` (24 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-07-03 10:31 UTC (permalink / raw)
To: linux-sh
Hello, Guennadi
(2012/07/03 19:12), Guennadi Liakhovetski wrote:
> Hello
>
> On Tue, 3 Jul 2012, Tetsuyuki Kobayashi wrote:
>
>> Add REGULATOR settings to kernel config file of kzm9g.
>>
>> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>> ---
>> Hello,
>>
>> I found Guennadi's
>> "ARM: mach-shmobile: add fixed voltage regulators to kzm9g"
>> requires REGULATOR setting in kernel config.
>> Without this, it causes these errors at boot time.
>
> Thanks for the patch. However, I think, this error will only appear if you
> enable REGULATOR in the kernel, but both REGULATOR_FIXED_VOLTAGE and
> REGULATOR_DUMMY are disabled. To fix this fixing the defconfig is good,
Oh yes, you are right. I made bad combination as you said.
if CONFIG_REGULATOR is not set, it goes well as before.
> but not sufficient, I think. How about adding something like
>
> select REGULATOR_FIXED_VOLTAGE if REGULATOR
>
> to either the CPU or the arch section in respective Kconfig? I would
> actually propose to add these to both arm/mach-shmobile and sh. Any
> objections?
>
Thank you.
I withdraw my config patch and wait for your proposal to Kconfig.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (2 preceding siblings ...)
2012-07-03 10:31 ` Tetsuyuki Kobayashi
@ 2012-07-03 11:15 ` Tetsuyuki Kobayashi
2012-07-03 12:44 ` Guennadi Liakhovetski
` (23 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-07-03 11:15 UTC (permalink / raw)
To: linux-sh
Hello Mark, Thanks for responding.
(2012/07/03 19:06), Mark Brown wrote:
> On Tue, Jul 03, 2012 at 06:59:30PM +0900, Tetsuyuki Kobayashi wrote:
>
>> +CONFIG_REGULATOR_DUMMY=y
>
> No, this should only ever be enabled as a crutch to get things working.
> It should not be enabled by default.
>
Thank you.
As Guennadi's follow-up, I found this patch is mistake.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (3 preceding siblings ...)
2012-07-03 11:15 ` Tetsuyuki Kobayashi
@ 2012-07-03 12:44 ` Guennadi Liakhovetski
2012-07-03 13:22 ` Mark Brown
` (22 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-03 12:44 UTC (permalink / raw)
To: linux-sh
On Tue, 3 Jul 2012, Tetsuyuki Kobayashi wrote:
> Hello, Guennadi
>
> (2012/07/03 19:12), Guennadi Liakhovetski wrote:
> > Hello
> >
> > On Tue, 3 Jul 2012, Tetsuyuki Kobayashi wrote:
> >
> > > Add REGULATOR settings to kernel config file of kzm9g.
> > >
> > > Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> > > ---
> > > Hello,
> > >
> > > I found Guennadi's
> > > "ARM: mach-shmobile: add fixed voltage regulators to kzm9g"
> > > requires REGULATOR setting in kernel config.
> > > Without this, it causes these errors at boot time.
> >
> > Thanks for the patch. However, I think, this error will only appear if you
> > enable REGULATOR in the kernel, but both REGULATOR_FIXED_VOLTAGE and
> > REGULATOR_DUMMY are disabled. To fix this fixing the defconfig is good,
>
> Oh yes, you are right. I made bad combination as you said.
> if CONFIG_REGULATOR is not set, it goes well as before.
>
> > but not sufficient, I think. How about adding something like
> >
> > select REGULATOR_FIXED_VOLTAGE if REGULATOR
> >
> > to either the CPU or the arch section in respective Kconfig? I would
> > actually propose to add these to both arm/mach-shmobile and sh. Any
> > objections?
> >
>
> Thank you.
> I withdraw my config patch and wait for your proposal to Kconfig.
Well, I'm not sure what approach would be the preferred by the
maintainers, how about the diff below. If it is acceptable, I'll split it
into 2 patches and submit properly.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 84449dd..295ce8e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -720,6 +720,7 @@ config ARCH_SHMOBILE
select MULTI_IRQ_HANDLER
select PM_GENERIC_DOMAINS if PM
select NEED_MACH_MEMORY_H
+ select REGULATOR_FIXED_VOLTAGE if REGULATOR
help
Support for Renesas's SH-Mobile and R-Mobile ARM platforms.
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 31d9db7..dd8f7a7 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -34,6 +34,7 @@ config SUPERH
select GENERIC_CMOS_UPDATE if SH_SH03 || SH_DREAMCAST
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
+ select REGULATOR_FIXED_VOLTAGE if REGULATOR
help
The SuperH is a RISC processor targeted for use in embedded systems
and consumer electronics; it was also used in the Sega Dreamcast
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (4 preceding siblings ...)
2012-07-03 12:44 ` Guennadi Liakhovetski
@ 2012-07-03 13:22 ` Mark Brown
2012-07-03 13:44 ` Guennadi Liakhovetski
` (21 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-07-03 13:22 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 558 bytes --]
On Tue, Jul 03, 2012 at 02:44:33PM +0200, Guennadi Liakhovetski wrote:
> Well, I'm not sure what approach would be the preferred by the
> maintainers, how about the diff below. If it is acceptable, I'll split it
> into 2 patches and submit properly.
This doesn't seem right. If this is useful we should just remove the
configuration for the fixed voltage regulator entirely as there's
nothing platform specific about this, and we probably ought to be going
through all the boards using "real" regulators adding selects for their
drivers too.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (5 preceding siblings ...)
2012-07-03 13:22 ` Mark Brown
@ 2012-07-03 13:44 ` Guennadi Liakhovetski
2012-07-03 14:45 ` Mark Brown
` (20 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-03 13:44 UTC (permalink / raw)
To: linux-sh
On Tue, 3 Jul 2012, Mark Brown wrote:
> On Tue, Jul 03, 2012 at 02:44:33PM +0200, Guennadi Liakhovetski wrote:
>
> > Well, I'm not sure what approach would be the preferred by the
> > maintainers, how about the diff below. If it is acceptable, I'll split it
> > into 2 patches and submit properly.
>
> This doesn't seem right. If this is useful we should just remove the
> configuration for the fixed voltage regulator entirely as there's
> nothing platform specific about this, and we probably ought to be going
> through all the boards using "real" regulators adding selects for their
> drivers too.
Hm, why? Let's take smsc911x. The driver needs "some" regulators for its
two power pins. On different systems different regulators are used for
those supplies. It just happens to be the case, that many sh and
arm/mach-shmobile platform use the fixed voltage regulator for smsc911x
(and other devices). In principle every such board should enable the fixed
regulator individually. But, since there are several of them, I just
proposed a shortcut. Otherwise, if on some other board a different
regulator is used to supply power to devices, enabling the fixed voltage
regulator will not help in any way. To actually make that system work
it'll need to enable the respective regulator.
So, I think, the absolutely correct fix would be to add that "select"
statement to each affected board individually. But maybe enabling it for
sh and shmobile in general will be accepted as a reasonable alternative.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (6 preceding siblings ...)
2012-07-03 13:44 ` Guennadi Liakhovetski
@ 2012-07-03 14:45 ` Mark Brown
2012-07-03 15:05 ` Guennadi Liakhovetski
` (19 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-07-03 14:45 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 858 bytes --]
On Tue, Jul 03, 2012 at 03:44:32PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 3 Jul 2012, Mark Brown wrote:
> > This doesn't seem right. If this is useful we should just remove the
> > configuration for the fixed voltage regulator entirely as there's
> > nothing platform specific about this, and we probably ought to be going
> > through all the boards using "real" regulators adding selects for their
> > drivers too.
> Hm, why? Let's take smsc911x. The driver needs "some" regulators for its
> two power pins. On different systems different regulators are used for
> those supplies. It just happens to be the case, that many sh and
> arm/mach-shmobile platform use the fixed voltage regulator for smsc911x
What I'm saying is that that statement is equally true if you replace
"sh and arm/mach-shmobile platform" with "systems".
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (7 preceding siblings ...)
2012-07-03 14:45 ` Mark Brown
@ 2012-07-03 15:05 ` Guennadi Liakhovetski
2012-07-03 15:06 ` Mark Brown
` (18 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-03 15:05 UTC (permalink / raw)
To: linux-sh
On Tue, 3 Jul 2012, Mark Brown wrote:
> On Tue, Jul 03, 2012 at 03:44:32PM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 3 Jul 2012, Mark Brown wrote:
>
> > > This doesn't seem right. If this is useful we should just remove the
> > > configuration for the fixed voltage regulator entirely as there's
> > > nothing platform specific about this, and we probably ought to be going
> > > through all the boards using "real" regulators adding selects for their
> > > drivers too.
>
> > Hm, why? Let's take smsc911x. The driver needs "some" regulators for its
> > two power pins. On different systems different regulators are used for
> > those supplies. It just happens to be the case, that many sh and
> > arm/mach-shmobile platform use the fixed voltage regulator for smsc911x
>
> What I'm saying is that that statement is equally true if you replace
> "sh and arm/mach-shmobile platform" with "systems".
So, you think, each board should select all required regulators
individually?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (8 preceding siblings ...)
2012-07-03 15:05 ` Guennadi Liakhovetski
@ 2012-07-03 15:06 ` Mark Brown
2012-07-03 15:35 ` Guennadi Liakhovetski
` (17 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-07-03 15:06 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]
On Tue, Jul 03, 2012 at 05:05:46PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 3 Jul 2012, Mark Brown wrote:
> > On Tue, Jul 03, 2012 at 03:44:32PM +0200, Guennadi Liakhovetski wrote:
> > > On Tue, 3 Jul 2012, Mark Brown wrote:
> > > > This doesn't seem right. If this is useful we should just remove the
> > > > configuration for the fixed voltage regulator entirely as there's
> > > > nothing platform specific about this, and we probably ought to be going
> > > > through all the boards using "real" regulators adding selects for their
> > > > drivers too.
> > > Hm, why? Let's take smsc911x. The driver needs "some" regulators for its
> > > two power pins. On different systems different regulators are used for
> > > those supplies. It just happens to be the case, that many sh and
> > > arm/mach-shmobile platform use the fixed voltage regulator for smsc911x
> > What I'm saying is that that statement is equally true if you replace
> > "sh and arm/mach-shmobile platform" with "systems".
> So, you think, each board should select all required regulators
> individually?
That's not what I wrote above (at least not for the fixed voltage
regulator). But in general I'm not sure we're requiring that .configs
that people can generate do more than build.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (9 preceding siblings ...)
2012-07-03 15:06 ` Mark Brown
@ 2012-07-03 15:35 ` Guennadi Liakhovetski
2012-07-03 19:33 ` Mark Brown
` (16 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-03 15:35 UTC (permalink / raw)
To: linux-sh
On Tue, 3 Jul 2012, Mark Brown wrote:
> On Tue, Jul 03, 2012 at 05:05:46PM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 3 Jul 2012, Mark Brown wrote:
> > > On Tue, Jul 03, 2012 at 03:44:32PM +0200, Guennadi Liakhovetski wrote:
> > > > On Tue, 3 Jul 2012, Mark Brown wrote:
>
> > > > > This doesn't seem right. If this is useful we should just remove the
> > > > > configuration for the fixed voltage regulator entirely as there's
> > > > > nothing platform specific about this, and we probably ought to be going
> > > > > through all the boards using "real" regulators adding selects for their
> > > > > drivers too.
>
> > > > Hm, why? Let's take smsc911x. The driver needs "some" regulators for its
> > > > two power pins. On different systems different regulators are used for
> > > > those supplies. It just happens to be the case, that many sh and
> > > > arm/mach-shmobile platform use the fixed voltage regulator for smsc911x
>
> > > What I'm saying is that that statement is equally true if you replace
> > > "sh and arm/mach-shmobile platform" with "systems".
>
> > So, you think, each board should select all required regulators
> > individually?
>
> That's not what I wrote above (at least not for the fixed voltage
> regulator).
No, sorry, that was my interpretation, because I didn't think that we
wanted to treat the fixed regulator any differently than others.
> But in general I'm not sure we're requiring that .configs
> that people can generate do more than build.
Well, you could have a kernel configuration with REGULATOR=y for some
reason with fixed regulator disabled. It would work with earlier kernels.
Now it would begin to fail... Do we want to allow that?
Well, I suspect, other boards must have had the same exact problem, as
they added regulator support. It seems some boards to that, but not all...
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (10 preceding siblings ...)
2012-07-03 15:35 ` Guennadi Liakhovetski
@ 2012-07-03 19:33 ` Mark Brown
2012-07-04 6:39 ` Guennadi Liakhovetski
` (15 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-07-03 19:33 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]
On Tue, Jul 03, 2012 at 05:35:34PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 3 Jul 2012, Mark Brown wrote:
> > But in general I'm not sure we're requiring that .configs
> > that people can generate do more than build.
> Well, you could have a kernel configuration with REGULATOR=y for some
> reason with fixed regulator disabled. It would work with earlier kernels.
> Now it would begin to fail... Do we want to allow that?
Well, it does happen from time to time; for example when moving from
forcing some GPIOs on to making them software managed. It's only going
to affect boards which were previously using regulators but not the
fixed one, I'm not sure if that is common for these systems...
> Well, I suspect, other boards must have had the same exact problem, as
> they added regulator support. It seems some boards to that, but not all...
Handling this really needs some kind of "set on upgrade" thing but I'm
not sure that's worth the trouble, especially since it's not that common.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (11 preceding siblings ...)
2012-07-03 19:33 ` Mark Brown
@ 2012-07-04 6:39 ` Guennadi Liakhovetski
2012-07-04 10:38 ` Mark Brown
` (14 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-04 6:39 UTC (permalink / raw)
To: linux-sh
Paul, Magnus, Rafael
What do you think about this? Shall we force the fixed regulator globally
on for sh and sh-mobile, shall we only force it on for affected boards, or
shall we leave it to the user to shoot themselves in the foot? In general
we're not trying to force all Kconfig options on, that a board needs.
There are enough ways to have a non-working configuration. E.g., you can
leave off the sh I2C driver. Arguably, having to enable the fixed voltage
regulator is less obvious, than needing a correct I2C bus driver.
I think I would go with
select REGULATOR_FIXED_VOLTAGE if REGULATOR
- ideally on a per-board basis, or arch-wide. Mark doesn't favour at least
the latter of these options, IIUC. So, maybe adding entries to defconfigs
would indeed sufice for now (but no dummy regulator!)
Thanks
Guennadi
On Tue, 3 Jul 2012, Mark Brown wrote:
> On Tue, Jul 03, 2012 at 05:35:34PM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 3 Jul 2012, Mark Brown wrote:
>
> > > But in general I'm not sure we're requiring that .configs
> > > that people can generate do more than build.
>
> > Well, you could have a kernel configuration with REGULATOR=y for some
> > reason with fixed regulator disabled. It would work with earlier kernels.
> > Now it would begin to fail... Do we want to allow that?
>
> Well, it does happen from time to time; for example when moving from
> forcing some GPIOs on to making them software managed. It's only going
> to affect boards which were previously using regulators but not the
> fixed one, I'm not sure if that is common for these systems...
>
> > Well, I suspect, other boards must have had the same exact problem, as
> > they added regulator support. It seems some boards to that, but not all...
>
> Handling this really needs some kind of "set on upgrade" thing but I'm
> not sure that's worth the trouble, especially since it's not that common.
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (12 preceding siblings ...)
2012-07-04 6:39 ` Guennadi Liakhovetski
@ 2012-07-04 10:38 ` Mark Brown
2012-07-04 10:40 ` Rafael J. Wysocki
` (13 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-07-04 10:38 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 704 bytes --]
On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
> and we can just make things depending on it depend on REGULATOR directly?
> The meaning of what you're proposing above is pretty much
> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
> when REGULATOR is set?
No, that won't help - the problem is that we want a specific regulator
driver to be built in order to ensure that the services it provides are
available.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (13 preceding siblings ...)
2012-07-04 10:38 ` Mark Brown
@ 2012-07-04 10:40 ` Rafael J. Wysocki
2012-07-04 10:41 ` Magnus Damm
` (12 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2012-07-04 10:40 UTC (permalink / raw)
To: linux-sh
On Wednesday, July 04, 2012, Guennadi Liakhovetski wrote:
> Paul, Magnus, Rafael
>
> What do you think about this? Shall we force the fixed regulator globally
> on for sh and sh-mobile, shall we only force it on for affected boards, or
> shall we leave it to the user to shoot themselves in the foot? In general
> we're not trying to force all Kconfig options on, that a board needs.
> There are enough ways to have a non-working configuration. E.g., you can
> leave off the sh I2C driver. Arguably, having to enable the fixed voltage
> regulator is less obvious, than needing a correct I2C bus driver.
>
> I think I would go with
>
> select REGULATOR_FIXED_VOLTAGE if REGULATOR
>
> - ideally on a per-board basis, or arch-wide. Mark doesn't favour at least
> the latter of these options, IIUC. So, maybe adding entries to defconfigs
> would indeed sufice for now (but no dummy regulator!)
Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
and we can just make things depending on it depend on REGULATOR directly?
The meaning of what you're proposing above is pretty much
"REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
when REGULATOR is set?
Thanks,
Rafael
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (14 preceding siblings ...)
2012-07-04 10:40 ` Rafael J. Wysocki
@ 2012-07-04 10:41 ` Magnus Damm
2012-07-04 10:46 ` Paul Mundt
` (11 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Magnus Damm @ 2012-07-04 10:41 UTC (permalink / raw)
To: linux-sh
On Wed, Jul 4, 2012 at 7:38 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
>
>> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
>> and we can just make things depending on it depend on REGULATOR directly?
>
>> The meaning of what you're proposing above is pretty much
>> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
>> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
>
>> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
>> when REGULATOR is set?
>
> No, that won't help - the problem is that we want a specific regulator
> driver to be built in order to ensure that the services it provides are
> available.
Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
on boards that make use of them? That seems to be the most
straightforward way to me.
Also, I know too little about the regulator subsystem, but to me it is
somewhat special that REGULATOR=n behaves differently than REGULATOR=y
and REGULATOR_DRIVER=n.
Thanks,
/ magnus
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (15 preceding siblings ...)
2012-07-04 10:41 ` Magnus Damm
@ 2012-07-04 10:46 ` Paul Mundt
2012-07-04 10:48 ` Mark Brown
` (10 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Paul Mundt @ 2012-07-04 10:46 UTC (permalink / raw)
To: linux-sh
On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
> On Wed, Jul 4, 2012 at 7:38 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
> >
> >> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
> >> and we can just make things depending on it depend on REGULATOR directly?
> >
> >> The meaning of what you're proposing above is pretty much
> >> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
> >> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
> >
> >> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
> >> when REGULATOR is set?
> >
> > No, that won't help - the problem is that we want a specific regulator
> > driver to be built in order to ensure that the services it provides are
> > available.
>
> Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
> on boards that make use of them? That seems to be the most
> straightforward way to me.
>
Yes, agreed. This is not very different from the gpiolib have/want case,
either, and going that on a board-specific level worked out fine.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (16 preceding siblings ...)
2012-07-04 10:46 ` Paul Mundt
@ 2012-07-04 10:48 ` Mark Brown
2012-07-04 10:51 ` Guennadi Liakhovetski
` (9 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-07-04 10:48 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]
On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
> Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
> on boards that make use of them? That seems to be the most
> straightforward way to me.
I tend to agree; as I've said several times now the only sensible
options I see are:
- Add the selects per board.
- Just enable the fixed voltage driver whenever the regulator core is
enabled.
Anything else seems silly.
> Also, I know too little about the regulator subsystem, but to me it is
> somewhat special that REGULATOR=n behaves differently than REGULATOR=y
> and REGULATOR_DRIVER=n.
What's happening is that the regulator API stubs itself out when it's
not in use in order to avoid having lots of ifdefs every time anyone
tries to use the API in generic code or drivers that get used on
multiple platforms. This is the same behaviour as gpiolib, it's a
similar sort of utility API.
If the power on defaults for the system weren't suitable for whatever is
being done you'd still have an issue even with the stubs but a lot of
the time the main effect of the API is to allow drivers to do additional
power optimisations.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (17 preceding siblings ...)
2012-07-04 10:48 ` Mark Brown
@ 2012-07-04 10:51 ` Guennadi Liakhovetski
2012-07-04 10:53 ` Guennadi Liakhovetski
` (8 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-04 10:51 UTC (permalink / raw)
To: linux-sh
On Wed, 4 Jul 2012, Rafael J. Wysocki wrote:
> On Wednesday, July 04, 2012, Guennadi Liakhovetski wrote:
> > Paul, Magnus, Rafael
> >
> > What do you think about this? Shall we force the fixed regulator globally
> > on for sh and sh-mobile, shall we only force it on for affected boards, or
> > shall we leave it to the user to shoot themselves in the foot? In general
> > we're not trying to force all Kconfig options on, that a board needs.
> > There are enough ways to have a non-working configuration. E.g., you can
> > leave off the sh I2C driver. Arguably, having to enable the fixed voltage
> > regulator is less obvious, than needing a correct I2C bus driver.
> >
> > I think I would go with
> >
> > select REGULATOR_FIXED_VOLTAGE if REGULATOR
> >
> > - ideally on a per-board basis, or arch-wide. Mark doesn't favour at least
> > the latter of these options, IIUC. So, maybe adding entries to defconfigs
> > would indeed sufice for now (but no dummy regulator!)
>
> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
> and we can just make things depending on it depend on REGULATOR directly?
No, we cannot. That's exactly the problem: if REGULATOR is selected and
REGULATOR_FIXED_VOLTAGE is not, then the regulator driver subsystem is
enabled, instead of stubs real regulator functions will be used, and
drivers, using regulators to control power supply to their devices, will
fail obtaining such a regulator, if the respective driver (fixed in our
cases) is not built or loaded. So, we need either - no regulator support
in the kernel at all - then stubs are used and in case of our always-on
fixed voltage power supplies everything just works, or the fixed regulator
driver must be available to satisfy client drivers, e.g., smsc911x,
sh_mmcif, sh_mobile_sdhi.
> The meaning of what you're proposing above is pretty much
> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is"
Exactly.
> and I guess
> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
No, they are not.
> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
> when REGULATOR is set?
Sorry, not sure what you mean "how much code." So far I've got 3 drivers
on my radar - see above. As for platforms - about 10 platforms on each of
sh and arm/mach-shmobile.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (18 preceding siblings ...)
2012-07-04 10:51 ` Guennadi Liakhovetski
@ 2012-07-04 10:53 ` Guennadi Liakhovetski
2012-07-04 10:55 ` Rafael J. Wysocki
` (7 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-04 10:53 UTC (permalink / raw)
To: linux-sh
On Wed, 4 Jul 2012, Paul Mundt wrote:
> On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
> > On Wed, Jul 4, 2012 at 7:38 PM, Mark Brown
> > <broonie@opensource.wolfsonmicro.com> wrote:
> > > On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
> > >
> > >> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
> > >> and we can just make things depending on it depend on REGULATOR directly?
> > >
> > >> The meaning of what you're proposing above is pretty much
> > >> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
> > >> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
> > >
> > >> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
> > >> when REGULATOR is set?
> > >
> > > No, that won't help - the problem is that we want a specific regulator
> > > driver to be built in order to ensure that the services it provides are
> > > available.
> >
> > Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
> > on boards that make use of them? That seems to be the most
> > straightforward way to me.
> >
> Yes, agreed. This is not very different from the gpiolib have/want case,
> either, and going that on a board-specific level worked out fine.
Ok, I'll make 2 patches then - for arch/sh/boards/Kconfig and
arch/arm/mach-shmobile/Kconfig, adding the proposed "select ... if ..."
line for each affected board.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (19 preceding siblings ...)
2012-07-04 10:53 ` Guennadi Liakhovetski
@ 2012-07-04 10:55 ` Rafael J. Wysocki
2012-07-04 10:58 ` Magnus Damm
` (6 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2012-07-04 10:55 UTC (permalink / raw)
To: linux-sh
On Wednesday, July 04, 2012, Mark Brown wrote:
> On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
>
> > Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
> > and we can just make things depending on it depend on REGULATOR directly?
>
> > The meaning of what you're proposing above is pretty much
> > "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
> > REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
>
> > How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
> > when REGULATOR is set?
>
> No, that won't help - the problem is that we want a specific regulator
> driver to be built in order to ensure that the services it provides are
> available.
OK, so if each board requires a specific regulator driver to be used, then
I think we should just select it from arch/arm/mach-shmobile/Kconfig for
that particular board.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (20 preceding siblings ...)
2012-07-04 10:55 ` Rafael J. Wysocki
@ 2012-07-04 10:58 ` Magnus Damm
2012-07-04 11:00 ` Magnus Damm
` (5 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Magnus Damm @ 2012-07-04 10:58 UTC (permalink / raw)
To: linux-sh
On Wed, Jul 4, 2012 at 7:48 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
>
>> Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
>> on boards that make use of them? That seems to be the most
>> straightforward way to me.
>
> I tend to agree; as I've said several times now the only sensible
> options I see are:
>
> - Add the selects per board.
> - Just enable the fixed voltage driver whenever the regulator core is
> enabled.
>
> Anything else seems silly.
Yeah.
>> Also, I know too little about the regulator subsystem, but to me it is
>> somewhat special that REGULATOR=n behaves differently than REGULATOR=y
>> and REGULATOR_DRIVER=n.
>
> What's happening is that the regulator API stubs itself out when it's
> not in use in order to avoid having lots of ifdefs every time anyone
> tries to use the API in generic code or drivers that get used on
> multiple platforms. This is the same behaviour as gpiolib, it's a
> similar sort of utility API.
I very much prefer that way over the way I think I2C (at least used
to) require #ifdef magic.
> If the power on defaults for the system weren't suitable for whatever is
> being done you'd still have an issue even with the stubs but a lot of
> the time the main effect of the API is to allow drivers to do additional
> power optimisations.
I may misunderstand the situation, but I believe the issue here is
that the REGULATOR=n case works fine on the boards with smsc911x, but
it stops "working" whenever REGULATOR=y is turned on and
REGULATOR_DRIVER_FOOBAR=n. People probably enable regulators because
they want to use the PMIC (that most likely isn't hooked up to the
SMSC chip), but they get surprised to see the following SMSC error
message:
smsc911x: Driver version 2008-10-21
smsc911x smsc911x.0: Failed to get supply 'vdd33a': -517
smsc911x smsc911x.0: (unregistered net_device): couldn't get regulators -517
platform smsc911x.0: Driver smsc911x requests probe deferral
Cheers,
/ magnus
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (21 preceding siblings ...)
2012-07-04 10:58 ` Magnus Damm
@ 2012-07-04 11:00 ` Magnus Damm
2012-07-04 11:05 ` Guennadi Liakhovetski
` (4 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Magnus Damm @ 2012-07-04 11:00 UTC (permalink / raw)
To: linux-sh
On Wed, Jul 4, 2012 at 7:53 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Wed, 4 Jul 2012, Paul Mundt wrote:
>
>> On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
>> > On Wed, Jul 4, 2012 at 7:38 PM, Mark Brown
>> > <broonie@opensource.wolfsonmicro.com> wrote:
>> > > On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
>> > >
>> > >> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
>> > >> and we can just make things depending on it depend on REGULATOR directly?
>> > >
>> > >> The meaning of what you're proposing above is pretty much
>> > >> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
>> > >> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
>> > >
>> > >> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
>> > >> when REGULATOR is set?
>> > >
>> > > No, that won't help - the problem is that we want a specific regulator
>> > > driver to be built in order to ensure that the services it provides are
>> > > available.
>> >
>> > Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
>> > on boards that make use of them? That seems to be the most
>> > straightforward way to me.
>> >
>> Yes, agreed. This is not very different from the gpiolib have/want case,
>> either, and going that on a board-specific level worked out fine.
>
> Ok, I'll make 2 patches then - for arch/sh/boards/Kconfig and
> arch/arm/mach-shmobile/Kconfig, adding the proposed "select ... if ..."
> line for each affected board.
Thanks. Is the "if ..." part the preferred way? It seems very verbose to me.
Cheers,
/ magnus
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (22 preceding siblings ...)
2012-07-04 11:00 ` Magnus Damm
@ 2012-07-04 11:05 ` Guennadi Liakhovetski
2012-07-04 11:08 ` Guennadi Liakhovetski
` (3 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-04 11:05 UTC (permalink / raw)
To: linux-sh
On Wed, 4 Jul 2012, Mark Brown wrote:
> On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
>
> > Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
> > on boards that make use of them? That seems to be the most
> > straightforward way to me.
>
> I tend to agree; as I've said several times now the only sensible
> options I see are:
>
> - Add the selects per board.
> - Just enable the fixed voltage driver whenever the regulator core is
> enabled.
>
> Anything else seems silly.
>
> > Also, I know too little about the regulator subsystem, but to me it is
> > somewhat special that REGULATOR=n behaves differently than REGULATOR=y
> > and REGULATOR_DRIVER=n.
>
> What's happening is that the regulator API stubs itself out when it's
> not in use in order to avoid having lots of ifdefs every time anyone
> tries to use the API in generic code or drivers that get used on
> multiple platforms. This is the same behaviour as gpiolib, it's a
> similar sort of utility API.
Right, but there's a difference: the gpio_request() stub in
include/linux/gpio.h returns -ENOSYS, gpio_is_valid() returns false, etc.,
whereas regulator_get() returns NULL (not an error code),
regulator_enable() returns 0 (a success...) etc. Yes, I understand, it's a
design decision and it has its own reasons, but I was a bit surprised when
I first saw that :)
> If the power on defaults for the system weren't suitable for whatever is
> being done you'd still have an issue even with the stubs but a lot of
> the time the main effect of the API is to allow drivers to do additional
> power optimisations.
Yeah... Which is exactly what produces the difference between REGULATOR=n
and (REGULATOR=y && REGULATOR_DRIVER=n). But yes, it probably isn't worth
changing any more, we just have to keep this in mind.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (23 preceding siblings ...)
2012-07-04 11:05 ` Guennadi Liakhovetski
@ 2012-07-04 11:08 ` Guennadi Liakhovetski
2012-07-04 11:22 ` Rafael J. Wysocki
` (2 subsequent siblings)
27 siblings, 0 replies; 29+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-04 11:08 UTC (permalink / raw)
To: linux-sh
On Wed, 4 Jul 2012, Magnus Damm wrote:
> On Wed, Jul 4, 2012 at 7:53 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > On Wed, 4 Jul 2012, Paul Mundt wrote:
> >
> >> On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
> >> > On Wed, Jul 4, 2012 at 7:38 PM, Mark Brown
> >> > <broonie@opensource.wolfsonmicro.com> wrote:
> >> > > On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
> >> > >
> >> > >> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
> >> > >> and we can just make things depending on it depend on REGULATOR directly?
> >> > >
> >> > >> The meaning of what you're proposing above is pretty much
> >> > >> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
> >> > >> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
> >> > >
> >> > >> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
> >> > >> when REGULATOR is set?
> >> > >
> >> > > No, that won't help - the problem is that we want a specific regulator
> >> > > driver to be built in order to ensure that the services it provides are
> >> > > available.
> >> >
> >> > Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
> >> > on boards that make use of them? That seems to be the most
> >> > straightforward way to me.
> >> >
> >> Yes, agreed. This is not very different from the gpiolib have/want case,
> >> either, and going that on a board-specific level worked out fine.
> >
> > Ok, I'll make 2 patches then - for arch/sh/boards/Kconfig and
> > arch/arm/mach-shmobile/Kconfig, adding the proposed "select ... if ..."
> > line for each affected board.
>
> Thanks. Is the "if ..." part the preferred way? It seems very verbose to me.
I tried without the "if," i.e. just
select REGULATOR_FIXED_VOLTAGE
If REGULATOR is not enabled in the .config, this produces a peculiar
configuration of
REGULATOR=n
REGULATOR_FIXED_VOLTAGE=y
Which doesn't seem very meaningful to me. We could "select" both - that's
another possibility. My proposal with the "if" gives more flexibility in
kernel configuration - you can build a kernel without regulator support
that way, for whatever reason you might want to do that.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (24 preceding siblings ...)
2012-07-04 11:08 ` Guennadi Liakhovetski
@ 2012-07-04 11:22 ` Rafael J. Wysocki
2012-07-04 13:31 ` Magnus Damm
2012-07-04 14:29 ` Mark Brown
27 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2012-07-04 11:22 UTC (permalink / raw)
To: linux-sh
On Wednesday, July 04, 2012, Guennadi Liakhovetski wrote:
> On Wed, 4 Jul 2012, Magnus Damm wrote:
>
> > On Wed, Jul 4, 2012 at 7:53 PM, Guennadi Liakhovetski
> > <g.liakhovetski@gmx.de> wrote:
> > > On Wed, 4 Jul 2012, Paul Mundt wrote:
> > >
> > >> On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
> > >> > On Wed, Jul 4, 2012 at 7:38 PM, Mark Brown
> > >> > <broonie@opensource.wolfsonmicro.com> wrote:
> > >> > > On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
> > >> > >
> > >> > >> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
> > >> > >> and we can just make things depending on it depend on REGULATOR directly?
> > >> > >
> > >> > >> The meaning of what you're proposing above is pretty much
> > >> > >> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
> > >> > >> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
> > >> > >
> > >> > >> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
> > >> > >> when REGULATOR is set?
> > >> > >
> > >> > > No, that won't help - the problem is that we want a specific regulator
> > >> > > driver to be built in order to ensure that the services it provides are
> > >> > > available.
> > >> >
> > >> > Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
> > >> > on boards that make use of them? That seems to be the most
> > >> > straightforward way to me.
> > >> >
> > >> Yes, agreed. This is not very different from the gpiolib have/want case,
> > >> either, and going that on a board-specific level worked out fine.
> > >
> > > Ok, I'll make 2 patches then - for arch/sh/boards/Kconfig and
> > > arch/arm/mach-shmobile/Kconfig, adding the proposed "select ... if ..."
> > > line for each affected board.
> >
> > Thanks. Is the "if ..." part the preferred way? It seems very verbose to me.
>
> I tried without the "if," i.e. just
>
> select REGULATOR_FIXED_VOLTAGE
>
> If REGULATOR is not enabled in the .config, this produces a peculiar
> configuration of
>
> REGULATOR=n
> REGULATOR_FIXED_VOLTAGE=y
>
> Which doesn't seem very meaningful to me. We could "select" both - that's
> another possibility. My proposal with the "if" gives more flexibility in
> kernel configuration - you can build a kernel without regulator support
> that way, for whatever reason you might want to do that.
Yes, I think the "if" option is more flexible.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (25 preceding siblings ...)
2012-07-04 11:22 ` Rafael J. Wysocki
@ 2012-07-04 13:31 ` Magnus Damm
2012-07-04 14:29 ` Mark Brown
27 siblings, 0 replies; 29+ messages in thread
From: Magnus Damm @ 2012-07-04 13:31 UTC (permalink / raw)
To: linux-sh
On Wed, Jul 4, 2012 at 8:22 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Wednesday, July 04, 2012, Guennadi Liakhovetski wrote:
>> On Wed, 4 Jul 2012, Magnus Damm wrote:
>>
>> > On Wed, Jul 4, 2012 at 7:53 PM, Guennadi Liakhovetski
>> > <g.liakhovetski@gmx.de> wrote:
>> > > On Wed, 4 Jul 2012, Paul Mundt wrote:
>> > >
>> > >> On Wed, Jul 04, 2012 at 07:41:09PM +0900, Magnus Damm wrote:
>> > >> > On Wed, Jul 4, 2012 at 7:38 PM, Mark Brown
>> > >> > <broonie@opensource.wolfsonmicro.com> wrote:
>> > >> > > On Wed, Jul 04, 2012 at 12:40:56PM +0200, Rafael J. Wysocki wrote:
>> > >> > >
>> > >> > >> Well, if arch-wide, then perhaps we don't need REGULATOR_FIXED_VOLTAGE at all
>> > >> > >> and we can just make things depending on it depend on REGULATOR directly?
>> > >> > >
>> > >> > >> The meaning of what you're proposing above is pretty much
>> > >> > >> "REGULATOR_FIXED_VOLTAGE should be set whenever REGULATOR is" and I guess
>> > >> > >> REGULATOR_FIXED_VOLTAGE depends on REGULATOR, so they appear to be equivalent.
>> > >> > >
>> > >> > >> How much code does really depend on REGULATOR_FIXED_VOLTAGE which is not build
>> > >> > >> when REGULATOR is set?
>> > >> > >
>> > >> > > No, that won't help - the problem is that we want a specific regulator
>> > >> > > driver to be built in order to ensure that the services it provides are
>> > >> > > available.
>> > >> >
>> > >> > Right, so what's wrong with having a "select REGULATOR_FIXED_VOLTAGE"
>> > >> > on boards that make use of them? That seems to be the most
>> > >> > straightforward way to me.
>> > >> >
>> > >> Yes, agreed. This is not very different from the gpiolib have/want case,
>> > >> either, and going that on a board-specific level worked out fine.
>> > >
>> > > Ok, I'll make 2 patches then - for arch/sh/boards/Kconfig and
>> > > arch/arm/mach-shmobile/Kconfig, adding the proposed "select ... if ..."
>> > > line for each affected board.
>> >
>> > Thanks. Is the "if ..." part the preferred way? It seems very verbose to me.
>>
>> I tried without the "if," i.e. just
>>
>> select REGULATOR_FIXED_VOLTAGE
>>
>> If REGULATOR is not enabled in the .config, this produces a peculiar
>> configuration of
>>
>> REGULATOR=n
>> REGULATOR_FIXED_VOLTAGE=y
>>
>> Which doesn't seem very meaningful to me. We could "select" both - that's
>> another possibility. My proposal with the "if" gives more flexibility in
>> kernel configuration - you can build a kernel without regulator support
>> that way, for whatever reason you might want to do that.
>
> Yes, I think the "if" option is more flexible.
Agreed. Thanks.
/ magnus
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig
2012-07-03 9:59 [PATCH] ARM: shmobile: kzm9g: add REGULATOR settings to kzm9g_defconfig Tetsuyuki Kobayashi
` (26 preceding siblings ...)
2012-07-04 13:31 ` Magnus Damm
@ 2012-07-04 14:29 ` Mark Brown
27 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-07-04 14:29 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 897 bytes --]
On Wed, Jul 04, 2012 at 07:58:15PM +0900, Magnus Damm wrote:
> On Wed, Jul 4, 2012 at 7:48 PM, Mark Brown
> > If the power on defaults for the system weren't suitable for whatever is
> > being done you'd still have an issue even with the stubs but a lot of
> > the time the main effect of the API is to allow drivers to do additional
> > power optimisations.
> I may misunderstand the situation, but I believe the issue here is
> that the REGULATOR=n case works fine on the boards with smsc911x, but
> it stops "working" whenever REGULATOR=y is turned on and
> REGULATOR_DRIVER_FOOBAR=n. People probably enable regulators because
> they want to use the PMIC (that most likely isn't hooked up to the
> SMSC chip), but they get surprised to see the following SMSC error
> message:
Yes, of course, but then there's not really much else we can do here
except for ignore errors which isn't awesome.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread