Skip to contentSkip to conversation

acpi/pmic: Add support for PMIC regs operation region

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 3/3 acpi/pmic: Add support for PMIC regs operation region

Broxton platform firmware has defined new customized operation regions called regs for PMIC chip which is used to handle the PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.

The intel_pmic_regs structure is created also for this purpose of handling the PMIC register read and write.

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

---
Changes in v3: none
Changes in v2: none

 drivers/acpi/pmic/intel_pmic.c | 74 +++++++++++++++++++++++++++++++++++++-----
 drivers/acpi/pmic/intel_pmic.h |  5 +++
 2 files changed, 71 insertions(+), 8 deletions(-)
drivers/acpi/pmic/intel_pmic.c +66 -8
index 410e96f..53b7052 100644
@@ -21,12 +21,14 @@
 
 #define PMIC_POWER_OPREGION_ID		0x8d
 #define PMIC_THERMAL_OPREGION_ID	0x8c
+#define PMIC_REGS_OPREGION_ID		0x8f
 
 struct intel_pmic_opregion {
 	struct mutex lock;
 	struct acpi_lpat_conversion_table *lpat_table;
 	struct regmap *regmap;
 	struct intel_pmic_opregion_data *data;
+	struct pmic_regs_handler	ctx;
 };
 
 static int pmic_get_reg_bit(int address, struct pmic_table *table,

@@ -204,6 +206,52 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
 	return AE_OK;
 }
 
+static acpi_status intel_pmic_regs_handler(u32 function,
+	acpi_physical_address address, u32 bits, u64 *value64,
+		void *handler_context, void *region_context)
+{
+	struct intel_pmic_opregion *opregion = region_context;
+	struct intel_pmic_opregion_data *d = opregion->data;
+	int result = 0;
+
+	switch (address) {
+	case 0:
+		return AE_OK;
+	case 1:
+		opregion->ctx.address |= (*value64 & 0xff) << 8;
+		return AE_OK;
+	case 2:
+		opregion->ctx.address |= *value64 & 0xff;
+		return AE_OK;
+	case 3:
+		opregion->ctx.value = *value64 & 0xff;
+		return AE_OK;
+	case 4:
+		if (*value64) {
+			result = d->regs_write(opregion->regmap,
+					opregion->ctx.address,
+					opregion->ctx.value);
+		} else {
+			result = d->regs_read(opregion->regmap,
+					opregion->ctx.address,
+					&opregion->ctx.value);
+			if (result == 0)
+				*value64 = opregion->ctx.value;
+		}
+		memset(&opregion->ctx, 0x00, sizeof(opregion->ctx));
+	}
+
+	if (result < 0) {
+		if (result == -EINVAL)
+			return AE_BAD_PARAMETER;
+		else
+			return AE_ERROR;
+	}
+
+	return AE_OK;
+}
+
+
 int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 					struct regmap *regmap,
 					struct intel_pmic_opregion_data *d)

@@ -227,21 +275,31 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 	opregion->lpat_table = acpi_lpat_get_conversion_table(handle);
 
 	status = acpi_install_address_space_handler(handle,
-						    PMIC_POWER_OPREGION_ID,
-						    intel_pmic_power_handler,
-						    NULL, opregion);
+				PMIC_POWER_OPREGION_ID,
+				intel_pmic_power_handler,
+					NULL, opregion);
+	if (ACPI_FAILURE(status)) {
+		ret = -ENODEV;
+		goto out_error;
+	}
+
+	status = acpi_install_address_space_handler(handle,
+				PMIC_THERMAL_OPREGION_ID,
+				intel_pmic_thermal_handler,
+					NULL, opregion);
 	if (ACPI_FAILURE(status)) {
+		acpi_remove_address_space_handler(handle,
+				PMIC_POWER_OPREGION_ID,
+				intel_pmic_power_handler);
 		ret = -ENODEV;
 		goto out_error;
 	}
 
 	status = acpi_install_address_space_handler(handle,
-						    PMIC_THERMAL_OPREGION_ID,
-						    intel_pmic_thermal_handler,
-						    NULL, opregion);
+					PMIC_REGS_OPREGION_ID,
+				intel_pmic_regs_handler, NULL,
+			opregion);
 	if (ACPI_FAILURE(status)) {
-		acpi_remove_address_space_handler(handle, PMIC_POWER_OPREGION_ID,
-						  intel_pmic_power_handler);
 		ret = -ENODEV;
 		goto out_error;
 	}
drivers/acpi/pmic/intel_pmic.h +5 -0
index 2f39ee0..bdb7dcc 100644
@@ -7,6 +7,11 @@ struct pmic_table {
 	int bit;	/* control bit for power */
 };
 
+struct pmic_regs_handler {
+	u16 address;		/* pmic regs address */
+	unsigned int value;	/* value to write @regs address */
+};
+
 struct intel_pmic_opregion_data {
 	int (*get_power)(struct regmap *r, int reg, int bit, u64 *value);
 	int (*update_power)(struct regmap *r, int reg, int bit, bool on);
Signature · 1 line
1.9.1
#1 lore raw
Quoted 23 lines · Bin Gao

Broxton platform firmware has defined new customized operation regions called regs for PMIC chip which is used to handle the PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.

The intel_pmic_regs structure is created also for this purpose of handling the PMIC register read and write.

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

---
Changes in v3: none
Changes in v2: none

 drivers/acpi/pmic/intel_pmic.c | 74 +++++++++++++++++++++++++++++++++++++-----
 drivers/acpi/pmic/intel_pmic.h |  5 +++
 2 files changed, 71 insertions(+), 8 deletions(-)
drivers/acpi/pmic/intel_pmic.c +0 -0
index 410e96f..53b7052 100644
@@ -21,12 +21,14 @@

drivers/acpi/pmic/intel_pmic.c line 31

Quoted 10 lines
drivers/acpi/pmic/intel_pmic.c +2 -0
 #define PMIC_POWER_OPREGION_ID		0x8d
 #define PMIC_THERMAL_OPREGION_ID	0x8c
+#define PMIC_REGS_OPREGION_ID		0x8f
 
 struct intel_pmic_opregion {
 	struct mutex lock;
 	struct acpi_lpat_conversion_table *lpat_table;
 	struct regmap *regmap;
 	struct intel_pmic_opregion_data *data;
+	struct pmic_regs_handler	ctx;

What about call it pmic_regs_ctx? pmic_regs_handler sounds like a function to me.

drivers/acpi/pmic/intel_pmic.c static acpi_status intel_pmic_thermal_handler(u32 function, · line 209

Quoted 8 lines
drivers/acpi/pmic/intel_pmic.c +1 -0
 };
 
 static int pmic_get_reg_bit(int address, struct pmic_table *table,

@@ -204,6 +206,52 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
 	return AE_OK;
 }
 
+static acpi_status intel_pmic_regs_handler(u32 function,

It looks like a IOCTL interface, so maybe call it intel_pmic_ioctl_handler, but this is just bikeshedding.
If you decide to change it, then the "struct pmic_regs_handler" should probably be renamed as "struct pmic_ioctl_ctx".

Quoted 22 lines
drivers/acpi/pmic/intel_pmic.c +22 -0
+	acpi_physical_address address, u32 bits, u64 *value64,
+		void *handler_context, void *region_context)
+{
+	struct intel_pmic_opregion *opregion = region_context;
+	struct intel_pmic_opregion_data *d = opregion->data;
+	int result = 0;
+
+	switch (address) {
+	case 0:
+		return AE_OK;
+	case 1:
+		opregion->ctx.address |= (*value64 & 0xff) << 8;
+		return AE_OK;
+	case 2:
+		opregion->ctx.address |= *value64 & 0xff;
+		return AE_OK;
+	case 3:
+		opregion->ctx.value = *value64 & 0xff;
+		return AE_OK;
+	case 4:
+		if (*value64) {
+			result = d->regs_write(opregion->regmap,

drivers/acpi/pmic/intel_pmic.c static acpi_status intel_pmic_thermal_handler(u32 function, · line 241

Quoted 10 lines
drivers/acpi/pmic/intel_pmic.c +10 -0
+					opregion->ctx.address,
+					opregion->ctx.value);
+		} else {
+			result = d->regs_read(opregion->regmap,
+					opregion->ctx.address,
+					&opregion->ctx.value);
+			if (result == 0)
+				*value64 = opregion->ctx.value;
+		}
+		memset(&opregion->ctx, 0x00, sizeof(opregion->ctx));

Perhaps add a default tag and print some warning in case later BIOS updates but our driver doesn't keep up.

Quoted 17 lines
drivers/acpi/pmic/intel_pmic.c +13 -0
+	}
+
+	if (result < 0) {
+		if (result == -EINVAL)
+			return AE_BAD_PARAMETER;
+		else
+			return AE_ERROR;
+	}
+
+	return AE_OK;
+}
+
+
 int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 					struct regmap *regmap,
 					struct intel_pmic_opregion_data *d)

drivers/acpi/pmic/intel_pmic.c int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle, · line 280

Quoted 9 lines
drivers/acpi/pmic/intel_pmic.c +3 -3
@@ -227,21 +275,31 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 	opregion->lpat_table = acpi_lpat_get_conversion_table(handle);
 
 	status = acpi_install_address_space_handler(handle,
-						    PMIC_POWER_OPREGION_ID,
-						    intel_pmic_power_handler,
-						    NULL, opregion);
+				PMIC_POWER_OPREGION_ID,
+				intel_pmic_power_handler,
+					NULL, opregion);

Better not touch these lines if there is no code change. If you want to change the coding style, do it in another patch. But please proper align them.

Quoted 17 lines
drivers/acpi/pmic/intel_pmic.c +12 -0
+	if (ACPI_FAILURE(status)) {
+		ret = -ENODEV;
+		goto out_error;
+	}
+
+	status = acpi_install_address_space_handler(handle,
+				PMIC_THERMAL_OPREGION_ID,
+				intel_pmic_thermal_handler,
+					NULL, opregion);
 	if (ACPI_FAILURE(status)) {
+		acpi_remove_address_space_handler(handle,
+				PMIC_POWER_OPREGION_ID,
+				intel_pmic_power_handler);
 		ret = -ENODEV;
 		goto out_error;
 	}

drivers/acpi/pmic/intel_pmic.c int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle, · line 244

Quoted 10 lines
drivers/acpi/pmic/intel_pmic.c +3 -5
 	status = acpi_install_address_space_handler(handle,
-						    PMIC_THERMAL_OPREGION_ID,
-						    intel_pmic_thermal_handler,
-						    NULL, opregion);
+					PMIC_REGS_OPREGION_ID,
+				intel_pmic_regs_handler, NULL,
+			opregion);
 	if (ACPI_FAILURE(status)) {
-		acpi_remove_address_space_handler(handle, PMIC_POWER_OPREGION_ID,
-						  intel_pmic_power_handler);

Shouldn't the power&thermal operation region handlers be removed here?

Thanks,
Aaron

Quoted 22 lines
drivers/acpi/pmic/intel_pmic.c +0 -0
 		ret = -ENODEV;
 		goto out_error;
 	}
drivers/acpi/pmic/intel_pmic.h +5 -0
index 2f39ee0..bdb7dcc 100644
@@ -7,6 +7,11 @@ struct pmic_table {
 	int bit;	/* control bit for power */
 };
 
+struct pmic_regs_handler {
+	u16 address;		/* pmic regs address */
+	unsigned int value;	/* value to write @regs address */
+};
+
 struct intel_pmic_opregion_data {
 	int (*get_power)(struct regmap *r, int reg, int bit, u64 *value);
 	int (*update_power)(struct regmap *r, int reg, int bit, bool on);
Signature · 1 line
1.9.1