Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1717043 > unrolled thread

[PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-08-22 08:00 +0200
    Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar  Cove TI PMIC Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-08-22 12:10 +0200
      Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-08-22 12:30 +0200
        Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-08-22 13:10 +0200
          Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar  Cove TI PMIC Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-08-22 13:40 +0200
            Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-08-22 14:10 +0200
            Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar  Cove TI PMIC Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-08-22 14:20 +0200
              Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-08-22 14:30 +0200
                Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar  Cove TI PMIC Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-08-22 15:30 +0200

#1717043 — [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC

This patch adds the opregion driver for Dollar Cove TI PMIC on Intel
Cherry Trail devices.  The patch is based on the original work by
Intel, found at:
      https://github.com/01org/ProductionKernelQuilts
with many cleanups and rewrites.

The driver is currently provided only as built-in to follow other
PMIC opregion drivers convention.

The re-enumeration of devices at probe is required for fixing the
issues on HP x2 210 G2.  See bug#195689.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=193891
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=195689
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/acpi/Kconfig                 |   6 ++
 drivers/acpi/Makefile                |   1 +
 drivers/acpi/pmic/intel_pmic_dc_ti.c | 135 +++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+)
 create mode 100644 drivers/acpi/pmic/intel_pmic_dc_ti.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1ce52f84dc23..9da8dcefde7b 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -521,6 +521,12 @@ config CHT_WC_PMIC_OPREGION
 	help
 	  This config adds ACPI operation region support for CHT Whiskey Cove PMIC.
 
+config DC_TI_PMIC_OPREGION
+	bool "ACPI operation region support for Dollar Cove TI PMIC"
+	depends on INTEL_SOC_PMIC_DC_TI
+	help
+	  This config adds ACPI operation region support for Dollar Cove TI PMIC.
+
 endif
 
 config ACPI_CONFIGFS
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index b1aacfc62b1f..0a9008b971af 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -103,6 +103,7 @@ obj-$(CONFIG_CRC_PMIC_OPREGION) += pmic/intel_pmic_crc.o
 obj-$(CONFIG_XPOWER_PMIC_OPREGION) += pmic/intel_pmic_xpower.o
 obj-$(CONFIG_BXT_WC_PMIC_OPREGION) += pmic/intel_pmic_bxtwc.o
 obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o
+obj-$(CONFIG_DC_TI_PMIC_OPREGION) += pmic/intel_pmic_dc_ti.o
 
 obj-$(CONFIG_ACPI_CONFIGFS)	+= acpi_configfs.o
 
diff --git a/drivers/acpi/pmic/intel_pmic_dc_ti.c b/drivers/acpi/pmic/intel_pmic_dc_ti.c
new file mode 100644
index 000000000000..236ff31c5a79
--- /dev/null
+++ b/drivers/acpi/pmic/intel_pmic_dc_ti.c
@@ -0,0 +1,135 @@
+/*
+ * intel_pmic_dc_ti.c - TI Dollar Cove PMIC operation region driver
+ * Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *
+ * Rewritten and cleaned up
+ * Copyright (C) 2017 Takashi Iwai <tiwai@suse.de>
+ */
+
+#include <linux/init.h>
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/intel_soc_pmic.h>
+#include "intel_pmic.h"
+
+#define TI_DC_PMICTEMP_LOW	0x57
+#define TI_DC_BATTEMP_LOW	0x59
+#define TI_DC_GPADC_LOW		0x5b
+
+static struct pmic_table dc_ti_power_table[] = {
+	{ .address = 0x00, .reg = 0x41 },
+	{ .address = 0x04, .reg = 0x42 },
+	{ .address = 0x08, .reg = 0x43 },
+	{ .address = 0x0c, .reg = 0x45 },
+	{ .address = 0x10, .reg = 0x46 },
+	{ .address = 0x14, .reg = 0x47 },
+	{ .address = 0x18, .reg = 0x48 },
+	{ .address = 0x1c, .reg = 0x49 },
+	{ .address = 0x20, .reg = 0x4a },
+	{ .address = 0x24, .reg = 0x4b },
+	{ .address = 0x28, .reg = 0x4c },
+	{ .address = 0x2c, .reg = 0x4d },
+	{ .address = 0x30, .reg = 0x4e },
+};
+
+static struct pmic_table dc_ti_thermal_table[] = {
+	{
+		.address = 0x00,
+		.reg = TI_DC_GPADC_LOW
+	},
+	{
+		.address = 0x0c,
+		.reg = TI_DC_GPADC_LOW
+	},
+	/* TMP2 -> SYSTEMP */
+	{
+		.address = 0x18,
+		.reg = TI_DC_GPADC_LOW
+	},
+	/* TMP3 -> BATTEMP */
+	{
+		.address = 0x24,
+		.reg = TI_DC_BATTEMP_LOW
+	},
+	{
+		.address = 0x30,
+		.reg = TI_DC_GPADC_LOW
+	},
+	/* TMP5 -> PMICTEMP */
+	{
+		.address = 0x3c,
+		.reg = TI_DC_PMICTEMP_LOW
+	},
+};
+
+static int dc_ti_pmic_get_power(struct regmap *regmap, int reg, int bit,
+				u64 *value)
+{
+	int data;
+
+	if (regmap_read(regmap, reg, &data))
+		return -EIO;
+
+	*value = data & 1;
+	return 0;
+}
+
+static int dc_ti_pmic_update_power(struct regmap *regmap, int reg, int bit,
+				   bool on)
+{
+	return regmap_update_bits(regmap, reg, 1, on);
+}
+
+static int dc_ti_pmic_get_raw_temp(struct regmap *regmap, int reg)
+{
+	int temp_l, temp_h;
+
+	if (regmap_read(regmap, reg, &temp_l) ||
+	    regmap_read(regmap, reg - 1, &temp_h))
+		return -EIO;
+
+	return temp_l | (temp_h & 0x3) << 8;
+}
+
+static struct intel_pmic_opregion_data dc_ti_pmic_opregion_data = {
+	.get_power = dc_ti_pmic_get_power,
+	.update_power = dc_ti_pmic_update_power,
+	.get_raw_temp = dc_ti_pmic_get_raw_temp,
+	.power_table = dc_ti_power_table,
+	.power_table_count = ARRAY_SIZE(dc_ti_power_table),
+	.thermal_table = dc_ti_thermal_table,
+	.thermal_table_count = ARRAY_SIZE(dc_ti_thermal_table),
+};
+
+static int dc_ti_pmic_opregion_probe(struct platform_device *pdev)
+{
+	struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
+	int err;
+
+	err = intel_pmic_install_opregion_handler(&pdev->dev,
+			ACPI_HANDLE(pdev->dev.parent), pmic->regmap,
+			&dc_ti_pmic_opregion_data);
+	if (err < 0)
+		return err;
+
+	/* Re-enumerate devices depending on PMIC */
+	acpi_walk_dep_device_list(ACPI_HANDLE(pdev->dev.parent));
+	return 0;
+}
+
+static struct platform_device_id dc_ti_pmic_opregion_id_table[] = {
+	{ .name = "dc_ti_region" },
+	{},
+};
+
+static struct platform_driver dc_ti_pmic_opregion_driver = {
+	.probe = dc_ti_pmic_opregion_probe,
+	.driver = {
+		.name = "dollar_cove_ti_pmic",
+	},
+	.id_table = dc_ti_pmic_opregion_id_table,
+};
+module_platform_driver(dc_ti_pmic_opregion_driver);
+
+MODULE_DESCRIPTION("Dollar Cove TI PMIC opregion driver");
+MODULE_LICENSE("GPLv2");
-- 
2.14.0

[toc] | [next] | [standalone]

#1717262 — Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC

On Tue, 2017-08-22 at 07:57 +0200, Takashi Iwai wrote:
> This patch adds the opregion driver for Dollar Cove TI PMIC on Intel
> Cherry Trail devices.  The patch is based on the original work by
> Intel, found at:
>       https://github.com/01org/ProductionKernelQuilts
> with many cleanups and rewrites.
> 
> The driver is currently provided only as built-in to follow other
> PMIC opregion drivers convention.
> 
> The re-enumeration of devices at probe is required for fixing the
> issues on HP x2 210 G2.  See bug#195689.
> 
> 

> +static int dc_ti_pmic_get_raw_temp(struct regmap *regmap, int reg)
> +{
> +	int temp_l, temp_h;
> +
> +	if (regmap_read(regmap, reg, &temp_l) ||
> +	    regmap_read(regmap, reg - 1, &temp_h))
> +		return -EIO;
> +
> +	return temp_l | (temp_h & 0x3) << 8;
> +}

I'm not sure I understand this "- 1" part along with choice of l and h
suffixes.

Does it mean the register is big endian?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [next] | [standalone]

#1717276

On Tue, 22 Aug 2017 11:58:35 +0200,
Andy Shevchenko wrote:
> 
> On Tue, 2017-08-22 at 07:57 +0200, Takashi Iwai wrote:
> > This patch adds the opregion driver for Dollar Cove TI PMIC on Intel
> > Cherry Trail devices.  The patch is based on the original work by
> > Intel, found at:
> >       https://github.com/01org/ProductionKernelQuilts
> > with many cleanups and rewrites.
> > 
> > The driver is currently provided only as built-in to follow other
> > PMIC opregion drivers convention.
> > 
> > The re-enumeration of devices at probe is required for fixing the
> > issues on HP x2 210 G2.  See bug#195689.
> > 
> > 
> 
> > +static int dc_ti_pmic_get_raw_temp(struct regmap *regmap, int reg)
> > +{
> > +	int temp_l, temp_h;
> > +
> > +	if (regmap_read(regmap, reg, &temp_l) ||
> > +	    regmap_read(regmap, reg - 1, &temp_h))
> > +		return -EIO;
> > +
> > +	return temp_l | (temp_h & 0x3) << 8;
> > +}
> 
> I'm not sure I understand this "- 1" part along with choice of l and h
> suffixes.
> 
> Does it mean the register is big endian?

Good point, I need to check the original code and the values.
This can be a typo of '+', of course.


thanks,

Takashi

[toc] | [prev] | [next] | [standalone]

#1717311

On Tue, 22 Aug 2017 12:25:12 +0200,
Takashi Iwai wrote:
> 
> On Tue, 22 Aug 2017 11:58:35 +0200,
> Andy Shevchenko wrote:
> > 
> > On Tue, 2017-08-22 at 07:57 +0200, Takashi Iwai wrote:
> > > This patch adds the opregion driver for Dollar Cove TI PMIC on Intel
> > > Cherry Trail devices.  The patch is based on the original work by
> > > Intel, found at:
> > >       https://github.com/01org/ProductionKernelQuilts
> > > with many cleanups and rewrites.
> > > 
> > > The driver is currently provided only as built-in to follow other
> > > PMIC opregion drivers convention.
> > > 
> > > The re-enumeration of devices at probe is required for fixing the
> > > issues on HP x2 210 G2.  See bug#195689.
> > > 
> > > 
> > 
> > > +static int dc_ti_pmic_get_raw_temp(struct regmap *regmap, int reg)
> > > +{
> > > +	int temp_l, temp_h;
> > > +
> > > +	if (regmap_read(regmap, reg, &temp_l) ||
> > > +	    regmap_read(regmap, reg - 1, &temp_h))
> > > +		return -EIO;
> > > +
> > > +	return temp_l | (temp_h & 0x3) << 8;
> > > +}
> > 
> > I'm not sure I understand this "- 1" part along with choice of l and h
> > suffixes.
> > 
> > Does it mean the register is big endian?
> 
> Good point, I need to check the original code and the values.

It's really big-endian, the order is hi:lo.

But, admittedly, the temperature code hasn't been tested, and it's
possibly missing something.  So I'm fine to drop that part in the
first version, too.


thanks,

Takashi

[toc] | [prev] | [next] | [standalone]

#1717324 — Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC

On Tue, 2017-08-22 at 13:01 +0200, Takashi Iwai wrote:
> On Tue, 22 Aug 2017 12:25:12 +0200,
> Takashi Iwai wrote:
> > 
> > On Tue, 22 Aug 2017 11:58:35 +0200,
> > Andy Shevchenko wrote:
> > > 
> > > On Tue, 2017-08-22 at 07:57 +0200, Takashi Iwai wrote:
> > > > This patch adds the opregion driver for Dollar Cove TI PMIC on
> > > > Intel
> > > > Cherry Trail devices.  The patch is based on the original work
> > > > by
> > > > Intel, found at:
> > > >       https://github.com/01org/ProductionKernelQuilts
> > > > with many cleanups and rewrites.
> > > > 
> > > > The driver is currently provided only as built-in to follow
> > > > other
> > > > PMIC opregion drivers convention.
> > > > 
> > > > The re-enumeration of devices at probe is required for fixing
> > > > the
> > > > issues on HP x2 210 G2.  See bug#195689.
> > > > 
> > > > 
> > > > +static int dc_ti_pmic_get_raw_temp(struct regmap *regmap, int
> > > > reg)
> > > > +{
> > > > +	int temp_l, temp_h;
> > > > +
> > > > +	if (regmap_read(regmap, reg, &temp_l) ||
> > > > +	    regmap_read(regmap, reg - 1, &temp_h))
> > > > +		return -EIO;
> > > > +
> > > > +	return temp_l | (temp_h & 0x3) << 8;
> > > > +}
> > > 
> > > I'm not sure I understand this "- 1" part along with choice of l
> > > and h
> > > suffixes.
> > > 
> > > Does it mean the register is big endian?
> > 
> > Good point, I need to check the original code and the values.
> 
> It's really big-endian, the order is hi:lo.
> 
> But, admittedly, the temperature code hasn't been tested, and it's
> possibly missing something.  So I'm fine to drop that part in the
> first version, too.

I don't know if regmap allows you to define registers with different
sizes for same chip, perhaps it would make sense to start register from
hi part (and not doing non-intuitive "- 1", or maybe "+ 1" instead) and
mark it in comment that is BE16.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [next] | [standalone]

#1717349

On Tue, 22 Aug 2017 13:37:21 +0200,
Andy Shevchenko wrote:
> 
> On Tue, 2017-08-22 at 13:01 +0200, Takashi Iwai wrote:
> > On Tue, 22 Aug 2017 12:25:12 +0200,
> > Takashi Iwai wrote:
> > > 
> > > On Tue, 22 Aug 2017 11:58:35 +0200,
> > > Andy Shevchenko wrote:
> > > > 
> > > > On Tue, 2017-08-22 at 07:57 +0200, Takashi Iwai wrote:
> > > > > This patch adds the opregion driver for Dollar Cove TI PMIC on
> > > > > Intel
> > > > > Cherry Trail devices.  The patch is based on the original work
> > > > > by
> > > > > Intel, found at:
> > > > >       https://github.com/01org/ProductionKernelQuilts
> > > > > with many cleanups and rewrites.
> > > > > 
> > > > > The driver is currently provided only as built-in to follow
> > > > > other
> > > > > PMIC opregion drivers convention.
> > > > > 
> > > > > The re-enumeration of devices at probe is required for fixing
> > > > > the
> > > > > issues on HP x2 210 G2.  See bug#195689.
> > > > > 
> > > > > 
> > > > > +static int dc_ti_pmic_get_raw_temp(struct regmap *regmap, int
> > > > > reg)
> > > > > +{
> > > > > +	int temp_l, temp_h;
> > > > > +
> > > > > +	if (regmap_read(regmap, reg, &temp_l) ||
> > > > > +	    regmap_read(regmap, reg - 1, &temp_h))
> > > > > +		return -EIO;
> > > > > +
> > > > > +	return temp_l | (temp_h & 0x3) << 8;
> > > > > +}
> > > > 
> > > > I'm not sure I understand this "- 1" part along with choice of l
> > > > and h
> > > > suffixes.
> > > > 
> > > > Does it mean the register is big endian?
> > > 
> > > Good point, I need to check the original code and the values.
> > 
> > It's really big-endian, the order is hi:lo.
> > 
> > But, admittedly, the temperature code hasn't been tested, and it's
> > possibly missing something.  So I'm fine to drop that part in the
> > first version, too.
> 
> I don't know if regmap allows you to define registers with different
> sizes for same chip, perhaps it would make sense to start register from
> hi part (and not doing non-intuitive "- 1", or maybe "+ 1" instead) and
> mark it in comment that is BE16.

I don't think regmap would allow different bit size, so combining
still needed.  But yeah, it's better to start from the high register
with the lower address and use +1.


thanks,

Takashi

[toc] | [prev] | [next] | [standalone]

#1717355 — Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC

On Tue, 2017-08-22 at 14:37 +0300, Andy Shevchenko wrote:
> On Tue, 2017-08-22 at 13:01 +0200, Takashi Iwai wrote:
> > On Tue, 22 Aug 2017 12:25:12 +0200,
> > Takashi Iwai wrote:

> > > > Does it mean the register is big endian?
> > > 
> > > Good point, I need to check the original code and the values.
> > 
> > It's really big-endian, the order is hi:lo.
> > 
> > But, admittedly, the temperature code hasn't been tested, and it's
> > possibly missing something.  So I'm fine to drop that part in the
> > first version, too.
> 
> I don't know if regmap allows you to define registers with different
> sizes for same chip, perhaps it would make sense to start register
> from
> hi part (and not doing non-intuitive "- 1", or maybe "+ 1" instead)
> and
> mark it in comment that is BE16.

I have just checked datasheet, so, there are 4 pairs of BE16 registers. 

VBAT (Hi:Lo) 0x54
DIETEMP 0x56
BPTHERM 0x58
GPADC 0x5a 

So, I would create a separate address mapping for them, dropping out
that _LOW suffix and put a comment that they are BE16 since ADC has 10-
bit resolution.

Or even do a separate ADC driver under drivers/iio/adc for PMIC(s).

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [next] | [standalone]

#1717365

On Tue, 22 Aug 2017 14:08:27 +0200,
Andy Shevchenko wrote:
> 
> On Tue, 2017-08-22 at 14:37 +0300, Andy Shevchenko wrote:
> > On Tue, 2017-08-22 at 13:01 +0200, Takashi Iwai wrote:
> > > On Tue, 22 Aug 2017 12:25:12 +0200,
> > > Takashi Iwai wrote:
> 
> > > > > Does it mean the register is big endian?
> > > > 
> > > > Good point, I need to check the original code and the values.
> > > 
> > > It's really big-endian, the order is hi:lo.
> > > 
> > > But, admittedly, the temperature code hasn't been tested, and it's
> > > possibly missing something.  So I'm fine to drop that part in the
> > > first version, too.
> > 
> > I don't know if regmap allows you to define registers with different
> > sizes for same chip, perhaps it would make sense to start register
> > from
> > hi part (and not doing non-intuitive "- 1", or maybe "+ 1" instead)
> > and
> > mark it in comment that is BE16.
> 
> I have just checked datasheet, so, there are 4 pairs of BE16 registers. 
> 
> VBAT (Hi:Lo) 0x54
> DIETEMP 0x56
> BPTHERM 0x58
> GPADC 0x5a 
> 
> So, I would create a separate address mapping for them, dropping out
> that _LOW suffix and put a comment that they are BE16 since ADC has 10-
> bit resolution.

Yep, done in that way now.  Also I changed the register names to
follow your reference.

> Or even do a separate ADC driver under drivers/iio/adc for PMIC(s).

That's another option, but I took an easier path as the first step.


thanks,

Takashi

[toc] | [prev] | [next] | [standalone]

#1717413 — Re: [PATCH 3/3] ACPI / PMIC: Add opregion driver for Intel Dollar Cove TI PMIC

On Tue, 2017-08-22 at 14:26 +0200, Takashi Iwai wrote:
> On Tue, 22 Aug 2017 14:08:27 +0200,
> Andy Shevchenko wrote:
> > 

> 
> > Or even do a separate ADC driver under drivers/iio/adc for PMIC(s).
> 
> That's another option, but I took an easier path as the first step.

Looking to the current pattern we have ADC being referred from ACPI PMIC
 OpRegion drivers, so, I have to withdraw my proposal.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web