Skip to contentSkip to conversation

acpi/pmic: Add opregion driver for Intel BXT WhiskeyCove PMIC

PATCH Superseded Bin Gao 2 participants 2 messages Jun 23, 2016

Subsystems (2)
ACPI ACPI PMIC drivers

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Bin Gao

author
#0 lore raw

PATCH v3 2/3 acpi/pmic: Add opregion driver for Intel BXT WhiskeyCove PMIC

This patch adds operation region driver for Intel BXT WhiskeyCove PMIC. The register mapping is done as per the BXT WC data sheet.

Signed-off-by Ajay Thomas <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Signed-off-by Chandra Sekhar Anagani <[email protected]>
Signed-off-by Bin Gao <[email protected]>

---
Changes in v3:
- Added regs_read() and regs_write() methods to the intel_pmic_opregion_data{} structure.
Changs in v2:
- Replaced module_init() with device_initcall().

 drivers/acpi/Kconfig                 |   6 +
 drivers/acpi/Makefile                |   1 +
 drivers/acpi/pmic/intel_pmic.h       |   2 +
 drivers/acpi/pmic/intel_pmic_bxtwc.c | 449 +++++++++++++++++++++++++++++++++++
 4 files changed, 458 insertions(+)
 create mode 100644 drivers/acpi/pmic/intel_pmic_bxtwc.c
drivers/acpi/Kconfig +6 -0
index b7e2e77..47cb6f6 100644
@@ -519,6 +519,12 @@ config XPOWER_PMIC_OPREGION
 	help
 	  This config adds ACPI operation region support for XPower AXP288 PMIC.
 
+config BXT_WC_PMIC_OPREGION
+	bool "ACPI operation region support for BXT WhiskeyCove PMIC"
+	depends on INTEL_SOC_PMIC
+	help
+	  This config adds ACPI operation region support for BXT WhiskeyCove PMIC.
+
 endif
 
 endif	# ACPI
drivers/acpi/Makefile +1 -0
index 251ce85..5da9d4b 100644
@@ -99,5 +99,6 @@ obj-$(CONFIG_ACPI_EXTLOG)	+= acpi_extlog.o
 obj-$(CONFIG_PMIC_OPREGION)	+= pmic/intel_pmic.o
 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
 
 video-objs			+= acpi_video.o video_detect.o
drivers/acpi/pmic/intel_pmic.h +2 -0
index e8bfa7b..2f39ee0 100644
@@ -12,6 +12,8 @@ struct intel_pmic_opregion_data {
 	int (*update_power)(struct regmap *r, int reg, int bit, bool on);
 	int (*get_raw_temp)(struct regmap *r, int reg);
 	int (*update_aux)(struct regmap *r, int reg, int raw_temp);
+	int (*regs_read)(struct regmap *r, u16 address, unsigned int *value);
+	int (*regs_write)(struct regmap *r, u16 address, unsigned int value);
 	int (*get_policy)(struct regmap *r, int reg, int bit, u64 *value);
 	int (*update_policy)(struct regmap *r, int reg, int bit, int enable);
 	struct pmic_table *power_table;
drivers/acpi/pmic/intel_pmic_bxtwc.c new file +449 -0

449 lines not on this page · raw message

Signature · 1 line
1.9.1
#1 lore raw
Quoted 28 lines · Bin Gao

This patch adds operation region driver for Intel BXT WhiskeyCove PMIC. The register mapping is done as per the BXT WC data sheet.

Signed-off-by Ajay Thomas <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Signed-off-by Chandra Sekhar Anagani <[email protected]>
Signed-off-by Bin Gao <[email protected]>

---
Changes in v3:
- Added regs_read() and regs_write() methods to the intel_pmic_opregion_data{} structure.
Changs in v2:
- Replaced module_init() with device_initcall().

 drivers/acpi/Kconfig                 |   6 +
 drivers/acpi/Makefile                |   1 +
 drivers/acpi/pmic/intel_pmic.h       |   2 +
 drivers/acpi/pmic/intel_pmic_bxtwc.c | 449 +++++++++++++++++++++++++++++++++++
 4 files changed, 458 insertions(+)
 create mode 100644 drivers/acpi/pmic/intel_pmic_bxtwc.c
drivers/acpi/pmic/intel_pmic_bxtwc.c new file +2 -0
new file mode 100644
index 0000000..51d5bcc
 
+static int
+intel_bxtwc_pmic_regs_read(struct regmap *regmap, u16 address,

drivers/acpi/pmic/intel_pmic_bxtwc.c line 369

Quoted 10 lines
drivers/acpi/pmic/intel_pmic_bxtwc.c new file +10 -0
+					unsigned int *value)
+{
+	unsigned int data;
+
+	if (regmap_read(regmap, address, &data))
+		return -EIO;
+
+	*value = data;
+	return 0;
+}

It looks like you can:
static int
intel_bxtwc_pmic_regs_read(struct regmap *regmap, u16 address,
unsigned int *value)

{
	return regmap_read(regmap, address, value);
}

But then why adding a new function for no actual use? Or you really need to return -EIO for all possible regmap_read's error return values? I don't see such a need.

BTW, the callback functions defined in intel_pmic_opregion_data is there due to different PMICs have different ways to do the same thing like turn on/off a power rail, get raw temperature reading, etc. It's not supposed to host helper functions like the two here.

drivers/acpi/pmic/intel_pmic_bxtwc.c line 379

Quoted 10 lines
drivers/acpi/pmic/intel_pmic_bxtwc.c new file +10 -0
+
+static int
+intel_bxtwc_pmic_regs_write(struct regmap *regmap, u16 address,
+					unsigned int value)
+{
+	if (regmap_write(regmap, address, value))
+		return -EIO;
+
+	return 0;
+}

Ditto.

drivers/acpi/pmic/intel_pmic_bxtwc.c line 409

Quoted 7 lines
drivers/acpi/pmic/intel_pmic_bxtwc.c new file +7 -0
+static struct intel_pmic_opregion_data intel_bxtwc_pmic_opregion_data = {
+	.get_power      = intel_bxtwc_pmic_get_power,
+	.update_power   = intel_bxtwc_pmic_update_power,
+	.get_raw_temp   = intel_bxtwc_pmic_get_raw_temp,
+	.update_aux     = intel_bxtwc_pmic_update_aux,
+	.regs_read	= intel_bxtwc_pmic_regs_read,
+	.regs_write	= intel_bxtwc_pmic_regs_write,

As said above, please drop the two callbacks.

Quoted 7 lines
drivers/acpi/pmic/intel_pmic_bxtwc.c new file +7 -0
+	.get_policy     = intel_bxtwc_pmic_get_policy,
+	.update_policy  = intel_bxtwc_pmic_update_policy,
+	.power_table      = power_table,
+	.power_table_count = ARRAY_SIZE(power_table),
+	.thermal_table     = thermal_table,
+	.thermal_table_count = ARRAY_SIZE(thermal_table),
+};