| Message ID | 20181205210311.16865-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ACPI-PMIC + i915: Add support for PMIC MIPI sequences | expand |
On Wed, Dec 05, 2018 at 10:03:09PM +0100, Hans de Goede wrote: > Implement the exec_mipi_pmic_seq_element callback for the CHT Whiskey Cove > PMIC. > > On some CHT devices this fixes the LCD panel not lighting up when it was > not initialized by the GOP, because an external monitor was plugged in and > the GOP initialized only the external monitor. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/acpi/pmic/intel_pmic_chtwc.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c > index 078b0448f30a..d035541f0ed2 100644 > --- a/drivers/acpi/pmic/intel_pmic_chtwc.c > +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c > @@ -12,6 +12,7 @@ > #include <linux/mfd/intel_soc_pmic.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > +#include <asm/unaligned.h> > #include "intel_pmic.h" > > #define CHT_WC_V1P05A_CTRL 0x6e3b > @@ -231,6 +232,27 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg, > return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0); > } > > +static void intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap, > + const u8 *data) > +{ > + u16 i2c_client_address, reg_address, address; > + u32 value, mask; > + > + i2c_client_address = get_unaligned_be16(data); > + reg_address = get_unaligned_be16(data + 2); > + value = get_unaligned_be32(data + 4); > + mask = get_unaligned_be32(data + 8); This doesn't match the docs: "Byte0 – PMIC Flag Bits 7:0 = Reserved for future use Byte2,1 – PMIC Slave Address Bits 15:0 = Slave address for PMIC access Each slave can address 256byte register space in general Bytes 6,5,4,3 – DWORD 0 Bits 31:0 = PMIC Register Address Bytes 10,9,8,7 – DWORD 1 Bits 31:0 = PMIC Register Data Bytes 14,13,12,11 – DWORD 2 Bits 31:0 = PMIC Register DataMask" Though I wouldn't be entirely surprised if the docs are simply wrong. There is at least one bug in the docs where it claims the size to be 14 bytes. Another place in the docs says 15 bytes. I'll need to file a bug for that at least. > + > + if ((i2c_client_address & 0xff00) || (reg_address & 0xff00)) { > + pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n", > + __func__, i2c_client_address, reg_address); > + return; > + } > + > + address = (i2c_client_address << 8) | reg_address; > + regmap_update_bits(regmap, address, mask, value); > +} > + > /* > * The thermal table and ops are empty, we do not support the Thermal opregion > * (DPTF) due to lacking documentation. > @@ -238,6 +260,7 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg, > static struct intel_pmic_opregion_data intel_cht_wc_pmic_opregion_data = { > .get_power = intel_cht_wc_pmic_get_power, > .update_power = intel_cht_wc_pmic_update_power, > + .exec_mipi_pmic_seq_element = intel_cht_wc_exec_mipi_pmic_seq_element, > .power_table = power_table, > .power_table_count = ARRAY_SIZE(power_table), > }; > -- > 2.19.2
Hi Ville, Thank you for the review. On 05-12-18 22:25, Ville Syrjälä wrote: > On Wed, Dec 05, 2018 at 10:03:09PM +0100, Hans de Goede wrote: >> Implement the exec_mipi_pmic_seq_element callback for the CHT Whiskey Cove >> PMIC. >> >> On some CHT devices this fixes the LCD panel not lighting up when it was >> not initialized by the GOP, because an external monitor was plugged in and >> the GOP initialized only the external monitor. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/acpi/pmic/intel_pmic_chtwc.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c >> index 078b0448f30a..d035541f0ed2 100644 >> --- a/drivers/acpi/pmic/intel_pmic_chtwc.c >> +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c >> @@ -12,6 +12,7 @@ >> #include <linux/mfd/intel_soc_pmic.h> >> #include <linux/platform_device.h> >> #include <linux/regmap.h> >> +#include <asm/unaligned.h> >> #include "intel_pmic.h" >> >> #define CHT_WC_V1P05A_CTRL 0x6e3b >> @@ -231,6 +232,27 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg, >> return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0); >> } >> >> +static void intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap, >> + const u8 *data) >> +{ >> + u16 i2c_client_address, reg_address, address; >> + u32 value, mask; >> + >> + i2c_client_address = get_unaligned_be16(data); >> + reg_address = get_unaligned_be16(data + 2); >> + value = get_unaligned_be32(data + 4); >> + mask = get_unaligned_be32(data + 8); > > This doesn't match the docs: Well I didn't have access to the docs, I just looked at the data and interpreted that, the first byte being reserved for future use tripped me up. > "Byte0 – PMIC Flag > Bits 7:0 = Reserved for future use > > Byte2,1 – PMIC Slave Address > Bits 15:0 = Slave address for PMIC access > Each slave can address 256byte register space in general > > Bytes 6,5,4,3 – DWORD 0 > Bits 31:0 = PMIC Register Address > > Bytes 10,9,8,7 – DWORD 1 > Bits 31:0 = PMIC Register Data > > Bytes 14,13,12,11 – DWORD 2 > Bits 31:0 = PMIC Register DataMask" > > Though I wouldn't be entirely surprised if the docs are simply > wrong. I think the docs are probably right, of the 2 sequences I'm seeing the OFF sequence has more 00 bytes, so the ON sequence is the most interesting, this is: 00 6e 00 3c 00 00 00 01 00 00 00 01 00 00 00 With my (big endian) interpretation that gives: PMIC Slave Address 00 6e PMIC Register Address 00 3c PMIC Register Data 00 00 00 01 PMIC Register Mask 00 00 00 01 With the docs slightly shifted (little endian) interpretation that gives: Reserverd 00 PMIC Slave Address 00 6e PMIC Register Address 00 00 00 3c PMIC Register Data 00 00 00 01 PMIC Register Mask 00 00 00 01 So the 2 interpretation agree for the data which I'm actually seeing used and the docs interpretation actually consumes all bytes. I'll prepare a v2 of this patch-set using the docs interpretation of the bytes. Regards, Hans > >> + >> + if ((i2c_client_address & 0xff00) || (reg_address & 0xff00)) { >> + pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n", >> + __func__, i2c_client_address, reg_address); >> + return; >> + } >> + >> + address = (i2c_client_address << 8) | reg_address; >> + regmap_update_bits(regmap, address, mask, value); >> +} >> + >> /* >> * The thermal table and ops are empty, we do not support the Thermal opregion >> * (DPTF) due to lacking documentation. >> @@ -238,6 +260,7 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg, >> static struct intel_pmic_opregion_data intel_cht_wc_pmic_opregion_data = { >> .get_power = intel_cht_wc_pmic_get_power, >> .update_power = intel_cht_wc_pmic_update_power, >> + .exec_mipi_pmic_seq_element = intel_cht_wc_exec_mipi_pmic_seq_element, >> .power_table = power_table, >> .power_table_count = ARRAY_SIZE(power_table), >> }; >> -- >> 2.19.2 >
diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c index 078b0448f30a..d035541f0ed2 100644 --- a/drivers/acpi/pmic/intel_pmic_chtwc.c +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c @@ -12,6 +12,7 @@ #include <linux/mfd/intel_soc_pmic.h> #include <linux/platform_device.h> #include <linux/regmap.h> +#include <asm/unaligned.h> #include "intel_pmic.h" #define CHT_WC_V1P05A_CTRL 0x6e3b @@ -231,6 +232,27 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg, return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0); } +static void intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap, + const u8 *data) +{ + u16 i2c_client_address, reg_address, address; + u32 value, mask; + + i2c_client_address = get_unaligned_be16(data); + reg_address = get_unaligned_be16(data + 2); + value = get_unaligned_be32(data + 4); + mask = get_unaligned_be32(data + 8); + + if ((i2c_client_address & 0xff00) || (reg_address & 0xff00)) { + pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n", + __func__, i2c_client_address, reg_address); + return; + } + + address = (i2c_client_address << 8) | reg_address; + regmap_update_bits(regmap, address, mask, value); +} + /* * The thermal table and ops are empty, we do not support the Thermal opregion * (DPTF) due to lacking documentation. @@ -238,6 +260,7 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg, static struct intel_pmic_opregion_data intel_cht_wc_pmic_opregion_data = { .get_power = intel_cht_wc_pmic_get_power, .update_power = intel_cht_wc_pmic_update_power, + .exec_mipi_pmic_seq_element = intel_cht_wc_exec_mipi_pmic_seq_element, .power_table = power_table, .power_table_count = ARRAY_SIZE(power_table), };
Implement the exec_mipi_pmic_seq_element callback for the CHT Whiskey Cove PMIC. On some CHT devices this fixes the LCD panel not lighting up when it was not initialized by the GOP, because an external monitor was plugged in and the GOP initialized only the external monitor. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/acpi/pmic/intel_pmic_chtwc.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)