From: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> To: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>, Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>, Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [PATCH] ARM: tegra: TN7: relax some regulators Date: Fri, 20 Jun 2014 15:44:46 +0900 [thread overview] Message-ID: <53A3D85E.7030704@nvidia.com> (raw) In-Reply-To: <53A3C613.8030207-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> On 06/20/2014 02:26 PM, Alexandre Courbot wrote: > On 06/20/2014 02:56 AM, Mark Brown wrote: >> * PGP Signed by an unknown key >> >> On Thu, Jun 19, 2014 at 09:59:04AM -0600, Stephen Warren wrote: >>> On 06/19/2014 01:49 AM, Alexandre Courbot wrote: >> >>>> Remove the regulator-always-on property from some regulators that do >>>> not >>>> need it. On recent kernels fixed regulators which supply is always on >>>> fail registration. >> >>> That sounds like a bug in the regulator core, which should be fixed >>> there. >> >> Please actually describe the problem you believe you are seeing - I've >> seen no reports and I can't tell anything from what you've described, >> nor can I see any obvious way that a regulator being fixed would have >> any effect on its supply. > > Here is some more information about what happens. > > We have a fixed regulator defined as follows: > > vdd_lcd: regulator@2 { > compatible = "regulator-fixed"; > reg = <2>; > regulator-name = "VD_LCD_1V8"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > gpio = <&palmas_gpio 4 GPIO_ACTIVE_HIGH>; > enable-active-high; > vin-supply = <&vdd_1v8>; > regulator-boot-on; > }; > > Its vin-supply is part of the palmas device: > > vdd_1v8: smps8 { > regulator-name = "vs-pmu-1v8"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > regulator-always-on; > regulator-boot-on; > }; > > When vdd_lcd is registered, set_supply() is called, which creates a new > regulator for vdd_1v8. In create_regulator(), > _regulator_can_change_status() returns false (as it should since the > regulator is always_on) and _regulator_is_enabled() *also* returns > false, so as a result regulator->always_on remains false for vdd_1v8. > > Later in regulator_register(), we try to enable the supply. Since > regulator->always_on is false, _regulator_enable() is called on vdd_1v8, > and the pair _regulator_is_enabled() / _regulator_can_change_status() is > called again with the same result, which causes _regulator_enable() to > return -EPERM. This prevents vdd_lcd from being registered. > > So I can see three questions here: > > 1) Why does _regulator_enable() on vdd_1v8 return 0 while everything > suggests that it is enabled (this regulator powers lot of devices, like > eMMC, which are working fine). This may be an issue with the palmas driver. Ran a bisect eventually, found that reverting this commit led to SMPS8's enabled status to be properly reported at boot time (and consequently the register probe to succeed): dbabd624d regulator: palmas: Reemove open coded functions with helper functions Keerthy, Nishanth, could it be that there is still something wrong with the REGULATOR_LINEAR_RANGE() definitions? This seems to be the cause for our trouble, but the other questions might still stand, in case there is interest in discussing them. > > 2) When an always-on regulator that is not yet enabled is registered, > shouldn't it be switched on by the regulator framework? > > 3) When a boot-on regulator is registered and _regulator_is_enabled() > returns contradictory information, what should be done? > > Note that whether the regulator-boot-on property is present or not does > not change anything. > > I tried to find a recent patch that could have introduced a change of > behavior, but could not find anything so far. Bisecting is made harder > by the fact this happens on a newly-introduced board which requires a > bunch of patches of its own, but it we need more information I can try > to do it anyway. > > Thanks, > Alex. >
WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Courbot <acourbot@nvidia.com> To: Mark Brown <broonie@kernel.org>, Stephen Warren <swarren@wwwdotorg.org>, Keerthy <j-keerthy@ti.com>, Nishanth Menon <nm@ti.com> Cc: Thierry Reding <thierry.reding@gmail.com>, "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] ARM: tegra: TN7: relax some regulators Date: Fri, 20 Jun 2014 15:44:46 +0900 [thread overview] Message-ID: <53A3D85E.7030704@nvidia.com> (raw) In-Reply-To: <53A3C613.8030207@nvidia.com> On 06/20/2014 02:26 PM, Alexandre Courbot wrote: > On 06/20/2014 02:56 AM, Mark Brown wrote: >> * PGP Signed by an unknown key >> >> On Thu, Jun 19, 2014 at 09:59:04AM -0600, Stephen Warren wrote: >>> On 06/19/2014 01:49 AM, Alexandre Courbot wrote: >> >>>> Remove the regulator-always-on property from some regulators that do >>>> not >>>> need it. On recent kernels fixed regulators which supply is always on >>>> fail registration. >> >>> That sounds like a bug in the regulator core, which should be fixed >>> there. >> >> Please actually describe the problem you believe you are seeing - I've >> seen no reports and I can't tell anything from what you've described, >> nor can I see any obvious way that a regulator being fixed would have >> any effect on its supply. > > Here is some more information about what happens. > > We have a fixed regulator defined as follows: > > vdd_lcd: regulator@2 { > compatible = "regulator-fixed"; > reg = <2>; > regulator-name = "VD_LCD_1V8"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > gpio = <&palmas_gpio 4 GPIO_ACTIVE_HIGH>; > enable-active-high; > vin-supply = <&vdd_1v8>; > regulator-boot-on; > }; > > Its vin-supply is part of the palmas device: > > vdd_1v8: smps8 { > regulator-name = "vs-pmu-1v8"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > regulator-always-on; > regulator-boot-on; > }; > > When vdd_lcd is registered, set_supply() is called, which creates a new > regulator for vdd_1v8. In create_regulator(), > _regulator_can_change_status() returns false (as it should since the > regulator is always_on) and _regulator_is_enabled() *also* returns > false, so as a result regulator->always_on remains false for vdd_1v8. > > Later in regulator_register(), we try to enable the supply. Since > regulator->always_on is false, _regulator_enable() is called on vdd_1v8, > and the pair _regulator_is_enabled() / _regulator_can_change_status() is > called again with the same result, which causes _regulator_enable() to > return -EPERM. This prevents vdd_lcd from being registered. > > So I can see three questions here: > > 1) Why does _regulator_enable() on vdd_1v8 return 0 while everything > suggests that it is enabled (this regulator powers lot of devices, like > eMMC, which are working fine). This may be an issue with the palmas driver. Ran a bisect eventually, found that reverting this commit led to SMPS8's enabled status to be properly reported at boot time (and consequently the register probe to succeed): dbabd624d regulator: palmas: Reemove open coded functions with helper functions Keerthy, Nishanth, could it be that there is still something wrong with the REGULATOR_LINEAR_RANGE() definitions? This seems to be the cause for our trouble, but the other questions might still stand, in case there is interest in discussing them. > > 2) When an always-on regulator that is not yet enabled is registered, > shouldn't it be switched on by the regulator framework? > > 3) When a boot-on regulator is registered and _regulator_is_enabled() > returns contradictory information, what should be done? > > Note that whether the regulator-boot-on property is present or not does > not change anything. > > I tried to find a recent patch that could have introduced a change of > behavior, but could not find anything so far. Bisecting is made harder > by the fact this happens on a newly-introduced board which requires a > bunch of patches of its own, but it we need more information I can try > to do it anyway. > > Thanks, > Alex. >
next prev parent reply other threads:[~2014-06-20 6:44 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-06-19 7:49 [PATCH] ARM: tegra: TN7: relax some regulators Alexandre Courbot 2014-06-19 7:49 ` Alexandre Courbot 2014-06-19 15:59 ` Stephen Warren 2014-06-19 17:56 ` Mark Brown [not found] ` <20140619175643.GR5099-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 2014-06-20 5:26 ` Alexandre Courbot 2014-06-20 5:26 ` Alexandre Courbot [not found] ` <53A3C613.8030207-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2014-06-20 6:44 ` Alexandre Courbot [this message] 2014-06-20 6:44 ` Alexandre Courbot [not found] ` <53A3D85E.7030704-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2014-06-20 9:41 ` Mark Brown 2014-06-20 9:41 ` Mark Brown [not found] ` <20140620094119.GT5099-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 2014-06-20 9:49 ` Alexandre Courbot 2014-06-20 9:49 ` Alexandre Courbot 2014-06-20 13:23 ` Palmas regulator broken (was Re: [PATCH] ARM: tegra: TN7: relax some regulators) Nishanth Menon 2014-06-20 13:23 ` Nishanth Menon 2014-06-20 13:54 ` Nishanth Menon 2014-06-20 13:54 ` Nishanth Menon 2014-06-20 14:14 ` Alexandre Courbot [not found] ` <CAAVeFuJ=JTNkwDKBvuoyL=ARQs_UJb_MHzL9S8gOc8EU98znPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-06-20 17:27 ` Nishanth Menon 2014-06-20 17:27 ` Nishanth Menon 2014-06-20 10:16 ` [PATCH] ARM: tegra: TN7: relax some regulators Mark Brown [not found] ` <20140620101618.GU5099-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 2014-06-20 10:33 ` Alexandre Courbot 2014-06-20 10:33 ` Alexandre Courbot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=53A3D85E.7030704@nvidia.com \ --to=acourbot-ddmlm1+adcrqt0dzr+alfa@public.gmane.org \ --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \ --cc=j-keerthy-l0cyMroinI0@public.gmane.org \ --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=nm-l0cyMroinI0@public.gmane.org \ --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org \ --cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.