linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH resend 1/2] mfd: intel_soc_pmic: Export separate mfd-cell configs for BYT and CHT
@ 2017-09-04 13:22 Hans de Goede
  2017-09-04 13:22 ` [PATCH resend 2/2] mfd: intel_soc_pmic: Differentiate between Bay and Cherry Trail CRC variants Hans de Goede
  2017-09-05  7:39 ` [PATCH resend 1/2] mfd: intel_soc_pmic: Export separate mfd-cell configs for BYT and CHT Lee Jones
  0 siblings, 2 replies; 4+ messages in thread
From: Hans de Goede @ 2017-09-04 13:22 UTC (permalink / raw)
  To: Lee Jones; +Cc: Hans de Goede, linux-kernel

Both Bay and Cherry Trail devices may be used together with a Crystal Cove
PMIC. Each platform has its own variant of the PMIC, which both use the
same ACPI HID, but they are not 100% compatible.

Looking at the android x86 kernel sources where most of the Crystal Cove
code comes from, it talks about "Valley View", "Bay Trail" and / or BYT
without ever mentioning Cherry Trail, with the exception of the regulator
driver. The Asus Zenfone-2 kernel code has 2 regulator drivers, one
for Crystal Cove and one for what it calls Crystal Cove Plus. The
Crystal Cove Plus regulator driver is the only one to mention Cherry
Trail and that driver uses different register addresses then the
normal (Bay Trail) Crystal Cove regulator driver, showing that at
least the regulator register addresses are different.

The GPIO code should work on both, and the PWM code is known to work on
both and is necessary for backlight control on some Cherry Trail devices.

Testing has shown that the ACPI OpRegion code otoh is causing problems
on Cherry Trail devices, which is not surprising as it deals with the
regulators and those have different register addresses on CHT.

Specifically the ACPI OpRegion code causes the external microsd slot on
a Dell Venue 8 5855 (Cherry Trail version) to not work and the eMMC to
become unreliable and throw lots of errors.

This commit replaces the single mfd_cell array currently used for Crystal
Cove with 2 separate arrays, one for the Bay Trail variant and one for
the Cherry Trail variant, note that the Cherry Trail version of the array
only contains gpio and pwm cells. The PMIC OpRegion cell is deliberately
not included and drivers for the other cells in the Bay Trail cell array
were never upstreamed.

Fixes: 7cf0a66f32 ("mfd: intel_soc_pmic: Crystal Cove support")
Reported-and-tested-by: russianneuromancer <russianneuromancer@ya.ru>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/intel_soc_pmic_core.c |  2 +-
 drivers/mfd/intel_soc_pmic_core.h |  3 ++-
 drivers/mfd/intel_soc_pmic_crc.c  | 27 +++++++++++++++++++++++----
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_core.c b/drivers/mfd/intel_soc_pmic_core.c
index 13737be6df35..2234a847370a 100644
--- a/drivers/mfd/intel_soc_pmic_core.c
+++ b/drivers/mfd/intel_soc_pmic_core.c
@@ -157,7 +157,7 @@ MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
 
 #if defined(CONFIG_ACPI)
 static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
-	{"INT33FD", (kernel_ulong_t)&intel_soc_pmic_config_crc},
+	{"INT33FD", (kernel_ulong_t)&intel_soc_pmic_config_byt_crc},
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
diff --git a/drivers/mfd/intel_soc_pmic_core.h b/drivers/mfd/intel_soc_pmic_core.h
index ff2464bc172f..90a1416d4dac 100644
--- a/drivers/mfd/intel_soc_pmic_core.h
+++ b/drivers/mfd/intel_soc_pmic_core.h
@@ -27,6 +27,7 @@ struct intel_soc_pmic_config {
 	const struct regmap_irq_chip *irq_chip;
 };
 
-extern struct intel_soc_pmic_config intel_soc_pmic_config_crc;
+extern struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc;
+extern struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc;
 
 #endif	/* __INTEL_SOC_PMIC_CORE_H__ */
diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index 4a7494872da2..6d19a6d0fb97 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -80,7 +80,7 @@ static struct resource bcu_resources[] = {
 	},
 };
 
-static struct mfd_cell crystal_cove_dev[] = {
+static struct mfd_cell crystal_cove_byt_dev[] = {
 	{
 		.name = "crystal_cove_pwrsrc",
 		.num_resources = ARRAY_SIZE(pwrsrc_resources),
@@ -114,6 +114,17 @@ static struct mfd_cell crystal_cove_dev[] = {
 	},
 };
 
+static struct mfd_cell crystal_cove_cht_dev[] = {
+	{
+		.name = "crystal_cove_gpio",
+		.num_resources = ARRAY_SIZE(gpio_resources),
+		.resources = gpio_resources,
+	},
+	{
+		.name = "crystal_cove_pwm",
+	},
+};
+
 static const struct regmap_config crystal_cove_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -155,10 +166,18 @@ static const struct regmap_irq_chip crystal_cove_irq_chip = {
 	.mask_base = CRYSTAL_COVE_REG_MIRQLVL1,
 };
 
-struct intel_soc_pmic_config intel_soc_pmic_config_crc = {
+struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
+	.irq_flags = IRQF_TRIGGER_RISING,
+	.cell_dev = crystal_cove_byt_dev,
+	.n_cell_devs = ARRAY_SIZE(crystal_cove_byt_dev),
+	.regmap_config = &crystal_cove_regmap_config,
+	.irq_chip = &crystal_cove_irq_chip,
+};
+
+struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
 	.irq_flags = IRQF_TRIGGER_RISING,
-	.cell_dev = crystal_cove_dev,
-	.n_cell_devs = ARRAY_SIZE(crystal_cove_dev),
+	.cell_dev = crystal_cove_cht_dev,
+	.n_cell_devs = ARRAY_SIZE(crystal_cove_cht_dev),
 	.regmap_config = &crystal_cove_regmap_config,
 	.irq_chip = &crystal_cove_irq_chip,
 };
-- 
2.13.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH resend 2/2] mfd: intel_soc_pmic: Differentiate between Bay and Cherry Trail CRC variants
  2017-09-04 13:22 [PATCH resend 1/2] mfd: intel_soc_pmic: Export separate mfd-cell configs for BYT and CHT Hans de Goede
@ 2017-09-04 13:22 ` Hans de Goede
  2017-09-05  7:39   ` Lee Jones
  2017-09-05  7:39 ` [PATCH resend 1/2] mfd: intel_soc_pmic: Export separate mfd-cell configs for BYT and CHT Lee Jones
  1 sibling, 1 reply; 4+ messages in thread
From: Hans de Goede @ 2017-09-04 13:22 UTC (permalink / raw)
  To: Lee Jones; +Cc: Hans de Goede, linux-kernel

Both Bay and Cherry Trail devices may be used together with a Crystal Cove
PMIC. Each platform has its own variant of the PMIC, which both use the
same ACPI HID, but they are not 100% compatible.

This commits makes the intel_soc_pmic_core code check the _HRV of the
ACPI-firmware-node and selects intel_soc_pmic_config_byt_crc resp.
intel_soc_pmic_config_cht_crc based on this.

This fixes the Bay Trail specific ACPI OpRegion code causing problems
on Cherry Trail devices. Specifically this was causing the external
microsd slot on a Dell Venue 8 5855 (Cherry Trail version) to not work
and the eMMC to become unreliable and throw lots of errors.

Fixes: 5165238460 ("mfd: intel_soc_pmic: Core driver")
Reported-and-tested-by: russianneuromancer <russianneuromancer@ya.ru>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/Kconfig               |  4 ++--
 drivers/mfd/intel_soc_pmic_core.c | 34 ++++++++++++++++++++++++++++------
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 918a8ce54a7c..10c495a73db3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -453,12 +453,12 @@ config LPC_SCH
 
 config INTEL_SOC_PMIC
 	bool "Support for Crystal Cove PMIC"
-	depends on HAS_IOMEM && I2C=y && GPIOLIB && COMMON_CLK
+	depends on ACPI && HAS_IOMEM && I2C=y && GPIOLIB && COMMON_CLK
 	depends on X86 || COMPILE_TEST
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
-	select I2C_DESIGNWARE_PLATFORM if ACPI
+	select I2C_DESIGNWARE_PLATFORM
 	help
 	  Select this option to enable support for Crystal Cove PMIC
 	  on some Intel SoC systems. The PMIC provides ADC, GPIO,
diff --git a/drivers/mfd/intel_soc_pmic_core.c b/drivers/mfd/intel_soc_pmic_core.c
index 2234a847370a..36adf9e8153e 100644
--- a/drivers/mfd/intel_soc_pmic_core.c
+++ b/drivers/mfd/intel_soc_pmic_core.c
@@ -16,6 +16,7 @@
  * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/module.h>
 #include <linux/mfd/core.h>
 #include <linux/i2c.h>
@@ -28,6 +29,10 @@
 #include <linux/pwm.h>
 #include "intel_soc_pmic_core.h"
 
+/* Crystal Cove PMIC shares same ACPI ID between different platforms */
+#define BYT_CRC_HRV		2
+#define CHT_CRC_HRV		3
+
 /* Lookup table for the Panel Enable/Disable line as GPIO signals */
 static struct gpiod_lookup_table panel_gpio_table = {
 	/* Intel GFX is consumer */
@@ -48,16 +53,33 @@ static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
 				    const struct i2c_device_id *i2c_id)
 {
 	struct device *dev = &i2c->dev;
-	const struct acpi_device_id *id;
 	struct intel_soc_pmic_config *config;
 	struct intel_soc_pmic *pmic;
+	unsigned long long hrv;
+	acpi_status status;
 	int ret;
 
-	id = acpi_match_device(dev->driver->acpi_match_table, dev);
-	if (!id || !id->driver_data)
+	/*
+	 * There are 2 different Crystal Cove PMICs a Bay Trail and Cherry
+	 * Trail version, use _HRV to differentiate between the 2.
+	 */
+	status = acpi_evaluate_integer(ACPI_HANDLE(dev), "_HRV", NULL, &hrv);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "Failed to get PMIC hardware revision\n");
 		return -ENODEV;
-
-	config = (struct intel_soc_pmic_config *)id->driver_data;
+	}
+
+	switch (hrv) {
+	case BYT_CRC_HRV:
+		config = &intel_soc_pmic_config_byt_crc;
+		break;
+	case CHT_CRC_HRV:
+		config = &intel_soc_pmic_config_cht_crc;
+		break;
+	default:
+		dev_warn(dev, "Unknown hardware rev %llu, assuming BYT\n", hrv);
+		config = &intel_soc_pmic_config_byt_crc;
+	}
 
 	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
 	if (!pmic)
@@ -157,7 +179,7 @@ MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
 
 #if defined(CONFIG_ACPI)
 static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
-	{"INT33FD", (kernel_ulong_t)&intel_soc_pmic_config_byt_crc},
+	{ "INT33FD" },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
-- 
2.13.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH resend 1/2] mfd: intel_soc_pmic: Export separate mfd-cell configs for BYT and CHT
  2017-09-04 13:22 [PATCH resend 1/2] mfd: intel_soc_pmic: Export separate mfd-cell configs for BYT and CHT Hans de Goede
  2017-09-04 13:22 ` [PATCH resend 2/2] mfd: intel_soc_pmic: Differentiate between Bay and Cherry Trail CRC variants Hans de Goede
@ 2017-09-05  7:39 ` Lee Jones
  1 sibling, 0 replies; 4+ messages in thread
From: Lee Jones @ 2017-09-05  7:39 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-kernel

On Mon, 04 Sep 2017, Hans de Goede wrote:

> Both Bay and Cherry Trail devices may be used together with a Crystal Cove
> PMIC. Each platform has its own variant of the PMIC, which both use the
> same ACPI HID, but they are not 100% compatible.
> 
> Looking at the android x86 kernel sources where most of the Crystal Cove
> code comes from, it talks about "Valley View", "Bay Trail" and / or BYT
> without ever mentioning Cherry Trail, with the exception of the regulator
> driver. The Asus Zenfone-2 kernel code has 2 regulator drivers, one
> for Crystal Cove and one for what it calls Crystal Cove Plus. The
> Crystal Cove Plus regulator driver is the only one to mention Cherry
> Trail and that driver uses different register addresses then the
> normal (Bay Trail) Crystal Cove regulator driver, showing that at
> least the regulator register addresses are different.
> 
> The GPIO code should work on both, and the PWM code is known to work on
> both and is necessary for backlight control on some Cherry Trail devices.
> 
> Testing has shown that the ACPI OpRegion code otoh is causing problems
> on Cherry Trail devices, which is not surprising as it deals with the
> regulators and those have different register addresses on CHT.
> 
> Specifically the ACPI OpRegion code causes the external microsd slot on
> a Dell Venue 8 5855 (Cherry Trail version) to not work and the eMMC to
> become unreliable and throw lots of errors.
> 
> This commit replaces the single mfd_cell array currently used for Crystal
> Cove with 2 separate arrays, one for the Bay Trail variant and one for
> the Cherry Trail variant, note that the Cherry Trail version of the array
> only contains gpio and pwm cells. The PMIC OpRegion cell is deliberately
> not included and drivers for the other cells in the Bay Trail cell array
> were never upstreamed.
> 
> Fixes: 7cf0a66f32 ("mfd: intel_soc_pmic: Crystal Cove support")
> Reported-and-tested-by: russianneuromancer <russianneuromancer@ya.ru>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/intel_soc_pmic_core.c |  2 +-
>  drivers/mfd/intel_soc_pmic_core.h |  3 ++-
>  drivers/mfd/intel_soc_pmic_crc.c  | 27 +++++++++++++++++++++++----
>  3 files changed, 26 insertions(+), 6 deletions(-)

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH resend 2/2] mfd: intel_soc_pmic: Differentiate between Bay and Cherry Trail CRC variants
  2017-09-04 13:22 ` [PATCH resend 2/2] mfd: intel_soc_pmic: Differentiate between Bay and Cherry Trail CRC variants Hans de Goede
@ 2017-09-05  7:39   ` Lee Jones
  0 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2017-09-05  7:39 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-kernel

On Mon, 04 Sep 2017, Hans de Goede wrote:

> Both Bay and Cherry Trail devices may be used together with a Crystal Cove
> PMIC. Each platform has its own variant of the PMIC, which both use the
> same ACPI HID, but they are not 100% compatible.
> 
> This commits makes the intel_soc_pmic_core code check the _HRV of the
> ACPI-firmware-node and selects intel_soc_pmic_config_byt_crc resp.
> intel_soc_pmic_config_cht_crc based on this.
> 
> This fixes the Bay Trail specific ACPI OpRegion code causing problems
> on Cherry Trail devices. Specifically this was causing the external
> microsd slot on a Dell Venue 8 5855 (Cherry Trail version) to not work
> and the eMMC to become unreliable and throw lots of errors.
> 
> Fixes: 5165238460 ("mfd: intel_soc_pmic: Core driver")
> Reported-and-tested-by: russianneuromancer <russianneuromancer@ya.ru>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/Kconfig               |  4 ++--
>  drivers/mfd/intel_soc_pmic_core.c | 34 ++++++++++++++++++++++++++++------
>  2 files changed, 30 insertions(+), 8 deletions(-)

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-09-05  7:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-04 13:22 [PATCH resend 1/2] mfd: intel_soc_pmic: Export separate mfd-cell configs for BYT and CHT Hans de Goede
2017-09-04 13:22 ` [PATCH resend 2/2] mfd: intel_soc_pmic: Differentiate between Bay and Cherry Trail CRC variants Hans de Goede
2017-09-05  7:39   ` Lee Jones
2017-09-05  7:39 ` [PATCH resend 1/2] mfd: intel_soc_pmic: Export separate mfd-cell configs for BYT and CHT Lee Jones

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).