| Message ID | 1390782734-10131-1-git-send-email-broonie@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Mon, Jan 27, 2014 at 01:54:49AM +0100, Rafael J. Wysocki wrote: > On Monday, January 27, 2014 12:32:14 AM Mark Brown wrote: > > @@ -509,6 +510,14 @@ void __init acpi_early_init(void) > > goto error0; > > } > > > > + /* > > + * If the system is using ACPI then we can be reasonably > > + * confident that any regulators are managed by the firmware > > + * so tell the regulator core it has everything it needs to > > + * know. > > + */ > > + regulator_has_full_constraints(); > > + > That's going to be called earlier than it use to be in 3.13 and before. > No problems with that? All that function does is to set a flag - so long as we can assign to a static variable it should be OK.
On Monday, January 27, 2014 12:32:14 AM Mark Brown wrote: > From: Mark Brown <broonie@linaro.org> > > There is currently no facility in ACPI to express the hookup of voltage > regulators, the expectation is that the regulators that exist in the > system will be handled transparently by firmware if they need software > control at all. This means that if for some reason the regulator API is > enabled on such a system it should assume that any supplies that devices > need are provided by the system at all relevant times without any software > intervention. > > Tell the regulator core to make this assumption by calling > regulator_has_full_constraints(). Do this as soon as we know we are using > ACPI so that the information is available to the regulator core as early > as possible. This will cause the regulator core to pretend that there is > an always on regulator supplying any supply that is requested but that has > not otherwise been mapped which is the behaviour expected on a system with > ACPI. > > Should the ability to specify regulators be added in future revisions of > ACPI then once we have support for ACPI mappings in the kernel the same > assumptions will apply. It is also likely that systems will default to a > mode of operation which does not require any interpretation of these > mappings in order to be compatible with existing operating system releases > so it should remain safe to make these assumptions even if the mappings > exist but are not supported by the kernel. > > Signed-off-by: Mark Brown <broonie@linaro.org> > --- > drivers/acpi/bus.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 384da5ab5955..fcb59c21c68d 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -33,6 +33,7 @@ > #include <linux/proc_fs.h> > #include <linux/acpi.h> > #include <linux/slab.h> > +#include <linux/regulator/machine.h> > #ifdef CONFIG_X86 > #include <asm/mpspec.h> > #endif > @@ -509,6 +510,14 @@ void __init acpi_early_init(void) > goto error0; > } > > + /* > + * If the system is using ACPI then we can be reasonably > + * confident that any regulators are managed by the firmware > + * so tell the regulator core it has everything it needs to > + * know. > + */ > + regulator_has_full_constraints(); > + > return; > > error0: That's going to be called earlier than it use to be in 3.13 and before. No problems with that?
On Mon, 27 Jan 2014 00:32:14 +0000, Mark Brown wrote: > From: Mark Brown <broonie@linaro.org> > > There is currently no facility in ACPI to express the hookup of voltage > regulators, the expectation is that the regulators that exist in the > system will be handled transparently by firmware if they need software > control at all. This means that if for some reason the regulator API is > enabled on such a system it should assume that any supplies that devices > need are provided by the system at all relevant times without any software > intervention. > > Tell the regulator core to make this assumption by calling > regulator_has_full_constraints(). Do this as soon as we know we are using > ACPI so that the information is available to the regulator core as early > as possible. This will cause the regulator core to pretend that there is > an always on regulator supplying any supply that is requested but that has > not otherwise been mapped which is the behaviour expected on a system with > ACPI. > > Should the ability to specify regulators be added in future revisions of > ACPI then once we have support for ACPI mappings in the kernel the same > assumptions will apply. It is also likely that systems will default to a > mode of operation which does not require any interpretation of these > mappings in order to be compatible with existing operating system releases > so it should remain safe to make these assumptions even if the mappings > exist but are not supported by the kernel. > > Signed-off-by: Mark Brown <broonie@linaro.org> > --- > drivers/acpi/bus.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 384da5ab5955..fcb59c21c68d 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -33,6 +33,7 @@ > #include <linux/proc_fs.h> > #include <linux/acpi.h> > #include <linux/slab.h> > +#include <linux/regulator/machine.h> > #ifdef CONFIG_X86 > #include <asm/mpspec.h> > #endif > @@ -509,6 +510,14 @@ void __init acpi_early_init(void) > goto error0; > } > > + /* > + * If the system is using ACPI then we can be reasonably > + * confident that any regulators are managed by the firmware > + * so tell the regulator core it has everything it needs to > + * know. > + */ > + regulator_has_full_constraints(); > + > return; > > error0: I think it makes sense to have this included in stable kernel trees? Thanks,
On Mon, Jan 27, 2014 at 09:25:09AM +0100, Jean Delvare wrote: > On Mon, 27 Jan 2014 00:32:14 +0000, Mark Brown wrote: > > control at all. This means that if for some reason the regulator API is > > enabled on such a system it should assume that any supplies that devices > > need are provided by the system at all relevant times without any software > > intervention. > > Tell the regulator core to make this assumption by calling > > regulator_has_full_constraints(). Do this as soon as we know we are using > I think it makes sense to have this included in stable kernel trees? Yes, that should be safe.
On Monday, January 27, 2014 12:44:34 AM Mark Brown wrote: > > --QgvTbcZPsSS/HkXe > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > Content-Transfer-Encoding: quoted-printable > > On Mon, Jan 27, 2014 at 01:54:49AM +0100, Rafael J. Wysocki wrote: > > On Monday, January 27, 2014 12:32:14 AM Mark Brown wrote: > > > > @@ -509,6 +510,14 @@ void __init acpi_early_init(void) > > > goto error0; > > > } > > > =20 > > > + /* > > > + * If the system is using ACPI then we can be reasonably > > > + * confident that any regulators are managed by the firmware > > > + * so tell the regulator core it has everything it needs to > > > + * know. > > > + */ > > > + regulator_has_full_constraints(); > > > + > > > That's going to be called earlier than it use to be in 3.13 and before. > > > No problems with that? > > All that function does is to set a flag - so long as we can assign to a > static variable it should be OK. OK, do you want me to take it? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 27, 2014 at 01:54:48PM +0100, Rafael J. Wysocki wrote: > On Monday, January 27, 2014 12:44:34 AM Mark Brown wrote: > > > That's going to be called earlier than it use to be in 3.13 and before. > > > No problems with that? > > All that function does is to set a flag - so long as we can assign to a > > static variable it should be OK. > OK, do you want me to take it? I'd like someone to take it :) . I'd expected the default thing would be for it to go via the ACPI tree but I can apply it to the regulator tree if you'd prefer, either way is fine by me.
On 01/26/2014 04:32 PM, Mark Brown wrote: > From: Mark Brown <broonie@linaro.org> > > There is currently no facility in ACPI to express the hookup of voltage > regulators, the expectation is that the regulators that exist in the > system will be handled transparently by firmware if they need software > control at all. This means that if for some reason the regulator API is > enabled on such a system it should assume that any supplies that devices > need are provided by the system at all relevant times without any software > intervention. > > Tell the regulator core to make this assumption by calling > regulator_has_full_constraints(). Do this as soon as we know we are using > ACPI so that the information is available to the regulator core as early > as possible. This will cause the regulator core to pretend that there is > an always on regulator supplying any supply that is requested but that has > not otherwise been mapped which is the behaviour expected on a system with > ACPI. > > Should the ability to specify regulators be added in future revisions of > ACPI then once we have support for ACPI mappings in the kernel the same > assumptions will apply. It is also likely that systems will default to a > mode of operation which does not require any interpretation of these > mappings in order to be compatible with existing operating system releases > so it should remain safe to make these assumptions even if the mappings > exist but are not supported by the kernel. > > Signed-off-by: Mark Brown <broonie@linaro.org> > --- > drivers/acpi/bus.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 384da5ab5955..fcb59c21c68d 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -33,6 +33,7 @@ > #include <linux/proc_fs.h> > #include <linux/acpi.h> > #include <linux/slab.h> > +#include <linux/regulator/machine.h> > #ifdef CONFIG_X86 > #include <asm/mpspec.h> > #endif > @@ -509,6 +510,14 @@ void __init acpi_early_init(void) > goto error0; > } > > + /* > + * If the system is using ACPI then we can be reasonably > + * confident that any regulators are managed by the firmware > + * so tell the regulator core it has everything it needs to > + * know. > + */ > + regulator_has_full_constraints(); > + > return; > > error0: > Couple of scenarios where I don't entirely see how this is expected to work. - A system which uses CPU cards which sit on a carrier card. The CPU is shared across multiple carrier cards, and built by an OEM. It supports and uses ACPI, but ACPI only knows about the CPU card and not the carrier cards, and thus does not know anything about the carrier card power or regulator requirements. - A system which supports a large number of complex OIR capable cards. The card connector includes GPIO pins, I2C busses, PCIe busses, and various other functionality such as SERDES lines. The card functionality is determined by the card ID and not known by the time the system ships. - A USB-I2C or USB-SPI adapter connected to a standard PC. Any I2C or SPI device can be connected to the I2C or SPI ports. How is the firmware expected to know about the various devices ? Is the answer "sorry, this scenario is not supported by the kernel" ? Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 27, 2014 at 06:58:35AM -0800, Guenter Roeck wrote: > On 01/26/2014 04:32 PM, Mark Brown wrote: > >control at all. This means that if for some reason the regulator API is > >enabled on such a system it should assume that any supplies that devices > >need are provided by the system at all relevant times without any software > >intervention. > Couple of scenarios where I don't entirely see how this is expected to work. Please do note the above section of the commit log and in general try to understand what happens on systems which have full constraints. As you have been told you appear to have some substantial confusion in this area which is not helping. > - A system which uses CPU cards which sit on a carrier card. The CPU is shared > across multiple carrier cards, and built by an OEM. It supports and uses ACPI, > but ACPI only knows about the CPU card and not the carrier cards, and thus does > not know anything about the carrier card power or regulator requirements. Unless the support for the card says otherwise the system will assume that all power supplies are taken care of transparently in exactly the same way as it will for components on the motherboard. > - A system which supports a large number of complex OIR capable cards. > The card connector includes GPIO pins, I2C busses, PCIe busses, and various other > functionality such as SERDES lines. The card functionality is determined by the > card ID and not known by the time the system ships. Unless the support for the card says otherwise the system will assume that all power supplies are taken care of transparently in exactly the same way as it will for components on the motherboard. > - A USB-I2C or USB-SPI adapter connected to a standard PC. Any I2C or SPI device > can be connected to the I2C or SPI ports. Unless the support for the adaptor says otherwise the system will assume that all power supplies are taken care of transparently in exactly the same way as it will for components on the motherboard. > How is the firmware expected to know about the various devices ? Is the answer > "sorry, this scenario is not supported by the kernel" ? The firmware for the motherboard is not expected to know anything about the supplies for the various devices either on or off the motherboard, you will note that this change does not add any code to parse any data from the firmware (since that data does not exist).
On Monday, January 27, 2014 01:21:06 PM Mark Brown wrote: > > --fzoBs0edN2XpEQF6 > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > > On Mon, Jan 27, 2014 at 01:54:48PM +0100, Rafael J. Wysocki wrote: > > On Monday, January 27, 2014 12:44:34 AM Mark Brown wrote: > > > > > That's going to be called earlier than it use to be in 3.13 and before. > > > > > No problems with that? > > > > All that function does is to set a flag - so long as we can assign to a > > > static variable it should be OK. > > > OK, do you want me to take it? > > I'd like someone to take it :) . I'd expected the default thing would > be for it to go via the ACPI tree but I can apply it to the regulator > tree if you'd prefer, either way is fine by me. I've queued it up for my next pull request, thanks!
On Tue, Jan 28, 2014 at 01:06:45AM +0100, Rafael J. Wysocki wrote: > On Monday, January 27, 2014 01:21:06 PM Mark Brown wrote: > > I'd like someone to take it :) . I'd expected the default thing would > > be for it to go via the ACPI tree but I can apply it to the regulator > > tree if you'd prefer, either way is fine by me. > I've queued it up for my next pull request, thanks! Excellent, thanks! Could you tag it for stable please as Jean requested? In v3.13 it fixes regressions in lm90 on PCs if the regulator API is enabled (which unfortunately Ubuntu did so it's a real scenario) and it should be safe in older kernels though I'm not aware of any actual systems that are affected.
On Tuesday, January 28, 2014 12:21:06 PM Mark Brown wrote: > > --XsQoSWH+UP9D9v3l > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > > On Tue, Jan 28, 2014 at 01:06:45AM +0100, Rafael J. Wysocki wrote: > > On Monday, January 27, 2014 01:21:06 PM Mark Brown wrote: > > > > I'd like someone to take it :) . I'd expected the default thing would > > > be for it to go via the ACPI tree but I can apply it to the regulator > > > tree if you'd prefer, either way is fine by me. > > > I've queued it up for my next pull request, thanks! > > Excellent, thanks! Could you tag it for stable please as Jean > requested? In v3.13 it fixes regressions in lm90 on PCs if the > regulator API is enabled (which unfortunately Ubuntu did so it's > a real scenario) and it should be safe in older kernels though I'm > not aware of any actual systems that are affected. Done, thanks!
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 384da5ab5955..fcb59c21c68d 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -33,6 +33,7 @@ #include <linux/proc_fs.h> #include <linux/acpi.h> #include <linux/slab.h> +#include <linux/regulator/machine.h> #ifdef CONFIG_X86 #include <asm/mpspec.h> #endif @@ -509,6 +510,14 @@ void __init acpi_early_init(void) goto error0; } + /* + * If the system is using ACPI then we can be reasonably + * confident that any regulators are managed by the firmware + * so tell the regulator core it has everything it needs to + * know. + */ + regulator_has_full_constraints(); + return; error0: