* [PATCH v2 1/2] ACPI / PMIC: xpower: Fix power_table addresses
@ 2017-04-21 11:48 Hans de Goede
2017-04-21 11:48 ` [PATCH v2 2/2] ACPI / PMIC: xpower: Add support for the GPI1 regulator to the OpRegion handler Hans de Goede
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2017-04-21 11:48 UTC (permalink / raw)
To: Rafael J . Wysocki, Len Brown
Cc: Hans de Goede, Andy Shevchenko, Srinivas Pandruvada, linux-acpi,
russianneuromancer
The power table addresses should be contiguous, but there was a hole
where 0x34 was missing. On most devices this is not a problem as
addresses above 0x34 are used for the BUC# convertors which are not
used in the DSDTs I've access to but after the BUC# convertors
there is a field named GPI1 in the DSTDs, which does get used in some
cases and ended up turning BUC6 on and off due to the wrong addresses,
resulting in turning the entire device off (or causing it to reboot).
Removing the hole in the addresses fixes this, fixing one of my
Bay Trail tablets turning off while booting the mainline kernel.
While at it add comments with the field names used in the DSDTs to
make it easier to compare the register and bits used at each address
with the datasheet.
Cc: russianneuromancer@ya.ru
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes in v2:
-Add Andy's Reviewed-by
---
drivers/acpi/pmic/intel_pmic_xpower.c | 50 +++++++++++++++++------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
index 55f5111..1a76c78 100644
--- a/drivers/acpi/pmic/intel_pmic_xpower.c
+++ b/drivers/acpi/pmic/intel_pmic_xpower.c
@@ -27,97 +27,97 @@ static struct pmic_table power_table[] = {
.address = 0x00,
.reg = 0x13,
.bit = 0x05,
- },
+ }, /* ALD1 */
{
.address = 0x04,
.reg = 0x13,
.bit = 0x06,
- },
+ }, /* ALD2 */
{
.address = 0x08,
.reg = 0x13,
.bit = 0x07,
- },
+ }, /* ALD3 */
{
.address = 0x0c,
.reg = 0x12,
.bit = 0x03,
- },
+ }, /* DLD1 */
{
.address = 0x10,
.reg = 0x12,
.bit = 0x04,
- },
+ }, /* DLD2 */
{
.address = 0x14,
.reg = 0x12,
.bit = 0x05,
- },
+ }, /* DLD3 */
{
.address = 0x18,
.reg = 0x12,
.bit = 0x06,
- },
+ }, /* DLD4 */
{
.address = 0x1c,
.reg = 0x12,
.bit = 0x00,
- },
+ }, /* ELD1 */
{
.address = 0x20,
.reg = 0x12,
.bit = 0x01,
- },
+ }, /* ELD2 */
{
.address = 0x24,
.reg = 0x12,
.bit = 0x02,
- },
+ }, /* ELD3 */
{
.address = 0x28,
.reg = 0x13,
.bit = 0x02,
- },
+ }, /* FLD1 */
{
.address = 0x2c,
.reg = 0x13,
.bit = 0x03,
- },
+ }, /* FLD2 */
{
.address = 0x30,
.reg = 0x13,
.bit = 0x04,
- },
+ }, /* FLD3 */
{
- .address = 0x38,
+ .address = 0x34,
.reg = 0x10,
.bit = 0x03,
- },
+ }, /* BUC1 */
{
- .address = 0x3c,
+ .address = 0x38,
.reg = 0x10,
.bit = 0x06,
- },
+ }, /* BUC2 */
{
- .address = 0x40,
+ .address = 0x3c,
.reg = 0x10,
.bit = 0x05,
- },
+ }, /* BUC3 */
{
- .address = 0x44,
+ .address = 0x40,
.reg = 0x10,
.bit = 0x04,
- },
+ }, /* BUC4 */
{
- .address = 0x48,
+ .address = 0x44,
.reg = 0x10,
.bit = 0x01,
- },
+ }, /* BUC5 */
{
- .address = 0x4c,
+ .address = 0x48,
.reg = 0x10,
.bit = 0x00
- },
+ }, /* BUC6 */
};
/* TMP0 - TMP5 are the same, all from GPADC */
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] ACPI / PMIC: xpower: Add support for the GPI1 regulator to the OpRegion handler
2017-04-21 11:48 [PATCH v2 1/2] ACPI / PMIC: xpower: Fix power_table addresses Hans de Goede
@ 2017-04-21 11:48 ` Hans de Goede
2017-04-26 22:01 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2017-04-21 11:48 UTC (permalink / raw)
To: Rafael J . Wysocki, Len Brown
Cc: Hans de Goede, Andy Shevchenko, Srinivas Pandruvada, linux-acpi,
russianneuromancer
Some Bay Trail devices use a GPI1 regulator field (address 0x4c) in
their 0x8d power OpRegion, add support for this.
This fixes AE_BAD_PARAMETER errors getting thrown on these devices and
fixes these errors causing these devices to not suspend.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Simplify reg == 0x92 handling (suggested by Andy Shevchenko)
-Add special handling for reg == 0x92 to intel_xpower_pmic_get_power() too
---
drivers/acpi/pmic/intel_pmic_xpower.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
index 1a76c78..c9be430 100644
--- a/drivers/acpi/pmic/intel_pmic_xpower.c
+++ b/drivers/acpi/pmic/intel_pmic_xpower.c
@@ -118,6 +118,10 @@ static struct pmic_table power_table[] = {
.reg = 0x10,
.bit = 0x00
}, /* BUC6 */
+ {
+ .address = 0x4c,
+ .reg = 0x92,
+ }, /* GPI1 */
};
/* TMP0 - TMP5 are the same, all from GPADC */
@@ -156,7 +160,12 @@ static int intel_xpower_pmic_get_power(struct regmap *regmap, int reg,
if (regmap_read(regmap, reg, &data))
return -EIO;
- *value = (data & BIT(bit)) ? 1 : 0;
+ /* GPIO1 LDO regulator needs special handling */
+ if (reg == 0x92)
+ *value = ((data & 0x07) == 0x03);
+ else
+ *value = (data & BIT(bit)) ? 1 : 0;
+
return 0;
}
@@ -165,6 +174,10 @@ static int intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
{
int data;
+ /* GPIO1 LDO regulator needs special handling */
+ if (reg == 0x92)
+ return regmap_update_bits(regmap, reg, 0x07, on ? 0x03 : 0x04);
+
if (regmap_read(regmap, reg, &data))
return -EIO;
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] ACPI / PMIC: xpower: Add support for the GPI1 regulator to the OpRegion handler
2017-04-21 11:48 ` [PATCH v2 2/2] ACPI / PMIC: xpower: Add support for the GPI1 regulator to the OpRegion handler Hans de Goede
@ 2017-04-26 22:01 ` Rafael J. Wysocki
2017-04-27 9:35 ` Andy Shevchenko
2017-04-27 23:20 ` Srinivas Pandruvada
0 siblings, 2 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2017-04-26 22:01 UTC (permalink / raw)
To: Hans de Goede, Andy Shevchenko, Srinivas Pandruvada
Cc: Len Brown, linux-acpi, russianneuromancer
On Friday, April 21, 2017 01:48:09 PM Hans de Goede wrote:
> Some Bay Trail devices use a GPI1 regulator field (address 0x4c) in
> their 0x8d power OpRegion, add support for this.
>
> This fixes AE_BAD_PARAMETER errors getting thrown on these devices and
> fixes these errors causing these devices to not suspend.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Srinivas, Andy, any concerns here?
> ---
> Changes in v2:
> -Simplify reg == 0x92 handling (suggested by Andy Shevchenko)
> -Add special handling for reg == 0x92 to intel_xpower_pmic_get_power() too
> ---
> drivers/acpi/pmic/intel_pmic_xpower.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
> index 1a76c78..c9be430 100644
> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> @@ -118,6 +118,10 @@ static struct pmic_table power_table[] = {
> .reg = 0x10,
> .bit = 0x00
> }, /* BUC6 */
> + {
> + .address = 0x4c,
> + .reg = 0x92,
> + }, /* GPI1 */
> };
>
> /* TMP0 - TMP5 are the same, all from GPADC */
> @@ -156,7 +160,12 @@ static int intel_xpower_pmic_get_power(struct regmap *regmap, int reg,
> if (regmap_read(regmap, reg, &data))
> return -EIO;
>
> - *value = (data & BIT(bit)) ? 1 : 0;
> + /* GPIO1 LDO regulator needs special handling */
> + if (reg == 0x92)
> + *value = ((data & 0x07) == 0x03);
> + else
> + *value = (data & BIT(bit)) ? 1 : 0;
> +
> return 0;
> }
>
> @@ -165,6 +174,10 @@ static int intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
> {
> int data;
>
> + /* GPIO1 LDO regulator needs special handling */
> + if (reg == 0x92)
> + return regmap_update_bits(regmap, reg, 0x07, on ? 0x03 : 0x04);
> +
> if (regmap_read(regmap, reg, &data))
> return -EIO;
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] ACPI / PMIC: xpower: Add support for the GPI1 regulator to the OpRegion handler
2017-04-26 22:01 ` Rafael J. Wysocki
@ 2017-04-27 9:35 ` Andy Shevchenko
2017-04-27 22:28 ` Rafael J. Wysocki
2017-04-27 23:20 ` Srinivas Pandruvada
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2017-04-27 9:35 UTC (permalink / raw)
To: Rafael J. Wysocki, Hans de Goede, Srinivas Pandruvada
Cc: Len Brown, linux-acpi, russianneuromancer
On Thu, 2017-04-27 at 00:01 +0200, Rafael J. Wysocki wrote:
> On Friday, April 21, 2017 01:48:09 PM Hans de Goede wrote:
> > Some Bay Trail devices use a GPI1 regulator field (address 0x4c) in
> > their 0x8d power OpRegion, add support for this.
> >
> > This fixes AE_BAD_PARAMETER errors getting thrown on these devices
> > and
> > fixes these errors causing these devices to not suspend.
> >
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> Srinivas, Andy, any concerns here?
Nothing major.
It might be useful to introduce
GPI1_LDO_MASK GENMASK(2,0)
GPI1_LDO_ON (3 << 0)
GPI1_LDO_OFF (4 << 0)
But it's up to you. I'm fine with either.
> > ---
> > Changes in v2:
> > -Simplify reg == 0x92 handling (suggested by Andy Shevchenko)
> > -Add special handling for reg == 0x92 to
> > intel_xpower_pmic_get_power() too
> > ---
> > drivers/acpi/pmic/intel_pmic_xpower.c | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c
> > b/drivers/acpi/pmic/intel_pmic_xpower.c
> > index 1a76c78..c9be430 100644
> > --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> > +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> > @@ -118,6 +118,10 @@ static struct pmic_table power_table[] = {
> > .reg = 0x10,
> > .bit = 0x00
> > }, /* BUC6 */
> > + {
> > + .address = 0x4c,
> > + .reg = 0x92,
> > + }, /* GPI1 */
> > };
> >
> > /* TMP0 - TMP5 are the same, all from GPADC */
> > @@ -156,7 +160,12 @@ static int intel_xpower_pmic_get_power(struct
> > regmap *regmap, int reg,
> > if (regmap_read(regmap, reg, &data))
> > return -EIO;
> >
> > - *value = (data & BIT(bit)) ? 1 : 0;
> > + /* GPIO1 LDO regulator needs special handling */
> > + if (reg == 0x92)
> > + *value = ((data & 0x07) == 0x03);
> > + else
> > + *value = (data & BIT(bit)) ? 1 : 0;
> > +
> > return 0;
> > }
> >
> > @@ -165,6 +174,10 @@ static int
> > intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
> > {
> > int data;
> >
> > + /* GPIO1 LDO regulator needs special handling */
> > + if (reg == 0x92)
> > + return regmap_update_bits(regmap, reg, 0x07, on ?
> > 0x03 : 0x04);
> > +
> > if (regmap_read(regmap, reg, &data))
> > return -EIO;
> >
> >
>
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] ACPI / PMIC: xpower: Add support for the GPI1 regulator to the OpRegion handler
2017-04-27 9:35 ` Andy Shevchenko
@ 2017-04-27 22:28 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2017-04-27 22:28 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede
Cc: Srinivas Pandruvada, Len Brown, linux-acpi, russianneuromancer
On Thursday, April 27, 2017 12:35:46 PM Andy Shevchenko wrote:
> On Thu, 2017-04-27 at 00:01 +0200, Rafael J. Wysocki wrote:
> > On Friday, April 21, 2017 01:48:09 PM Hans de Goede wrote:
> > > Some Bay Trail devices use a GPI1 regulator field (address 0x4c) in
> > > their 0x8d power OpRegion, add support for this.
> > >
> > > This fixes AE_BAD_PARAMETER errors getting thrown on these devices
> > > and
> > > fixes these errors causing these devices to not suspend.
> > >
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >
> > Srinivas, Andy, any concerns here?
>
> Nothing major.
>
> It might be useful to introduce
>
> GPI1_LDO_MASK GENMASK(2,0)
> GPI1_LDO_ON (3 << 0)
> GPI1_LDO_OFF (4 << 0)
>
> But it's up to you. I'm fine with either.
This is a good point IMO.
Hans, care to use symbols as suggested by Andy?
Thanks,
Rafael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] ACPI / PMIC: xpower: Add support for the GPI1 regulator to the OpRegion handler
2017-04-26 22:01 ` Rafael J. Wysocki
2017-04-27 9:35 ` Andy Shevchenko
@ 2017-04-27 23:20 ` Srinivas Pandruvada
1 sibling, 0 replies; 6+ messages in thread
From: Srinivas Pandruvada @ 2017-04-27 23:20 UTC (permalink / raw)
To: Rafael J. Wysocki, Hans de Goede, Andy Shevchenko
Cc: Len Brown, linux-acpi, russianneuromancer
On Thu, 2017-04-27 at 00:01 +0200, Rafael J. Wysocki wrote:
> On Friday, April 21, 2017 01:48:09 PM Hans de Goede wrote:
> >
> > Some Bay Trail devices use a GPI1 regulator field (address 0x4c) in
> > their 0x8d power OpRegion, add support for this.
> >
> > This fixes AE_BAD_PARAMETER errors getting thrown on these devices
> > and
> > fixes these errors causing these devices to not suspend.
> >
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> Srinivas, Andy, any concerns here?
None from me.
Thanks,
Srinivas
>
> >
> > ---
> > Changes in v2:
> > -Simplify reg == 0x92 handling (suggested by Andy Shevchenko)
> > -Add special handling for reg == 0x92 to
> > intel_xpower_pmic_get_power() too
> > ---
> > drivers/acpi/pmic/intel_pmic_xpower.c | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c
> > b/drivers/acpi/pmic/intel_pmic_xpower.c
> > index 1a76c78..c9be430 100644
> > --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> > +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> > @@ -118,6 +118,10 @@ static struct pmic_table power_table[] = {
> > .reg = 0x10,
> > .bit = 0x00
> > }, /* BUC6 */
> > + {
> > + .address = 0x4c,
> > + .reg = 0x92,
> > + }, /* GPI1 */
> > };
> >
> > /* TMP0 - TMP5 are the same, all from GPADC */
> > @@ -156,7 +160,12 @@ static int intel_xpower_pmic_get_power(struct
> > regmap *regmap, int reg,
> > if (regmap_read(regmap, reg, &data))
> > return -EIO;
> >
> > - *value = (data & BIT(bit)) ? 1 : 0;
> > + /* GPIO1 LDO regulator needs special handling */
> > + if (reg == 0x92)
> > + *value = ((data & 0x07) == 0x03);
> > + else
> > + *value = (data & BIT(bit)) ? 1 : 0;
> > +
> > return 0;
> > }
> >
> > @@ -165,6 +174,10 @@ static int
> > intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
> > {
> > int data;
> >
> > + /* GPIO1 LDO regulator needs special handling */
> > + if (reg == 0x92)
> > + return regmap_update_bits(regmap, reg, 0x07, on ?
> > 0x03 : 0x04);
> > +
> > if (regmap_read(regmap, reg, &data))
> > return -EIO;
> >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-27 23:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-21 11:48 [PATCH v2 1/2] ACPI / PMIC: xpower: Fix power_table addresses Hans de Goede
2017-04-21 11:48 ` [PATCH v2 2/2] ACPI / PMIC: xpower: Add support for the GPI1 regulator to the OpRegion handler Hans de Goede
2017-04-26 22:01 ` Rafael J. Wysocki
2017-04-27 9:35 ` Andy Shevchenko
2017-04-27 22:28 ` Rafael J. Wysocki
2017-04-27 23:20 ` Srinivas Pandruvada
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).