* [PATCH v3 3/3] acpi/pmic: Add support for PMIC regs operation region
@ 2016-06-23 8:06 Bin Gao
2016-06-23 9:07 ` Aaron Lu
0 siblings, 1 reply; 2+ messages in thread
From: Bin Gao @ 2016-06-23 8:06 UTC (permalink / raw)
To: Rafael J. Wysocki, Aaron Lu, Paul Gortmaker, linux-acpi
Cc: linux-kernel, ysiyer, Ajay Thomas, 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 <chandra.sekhar.anagani@intel.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Bin Gao <bin.gao@intel.com>
---
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(-)
diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index 410e96f..53b7052 100644
--- a/drivers/acpi/pmic/intel_pmic.c
+++ b/drivers/acpi/pmic/intel_pmic.c
@@ -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;
}
diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
index 2f39ee0..bdb7dcc 100644
--- a/drivers/acpi/pmic/intel_pmic.h
+++ b/drivers/acpi/pmic/intel_pmic.h
@@ -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);
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3 3/3] acpi/pmic: Add support for PMIC regs operation region
2016-06-23 8:06 [PATCH v3 3/3] acpi/pmic: Add support for PMIC regs operation region Bin Gao
@ 2016-06-23 9:07 ` Aaron Lu
0 siblings, 0 replies; 2+ messages in thread
From: Aaron Lu @ 2016-06-23 9:07 UTC (permalink / raw)
To: Bin Gao
Cc: Rafael J. Wysocki, Paul Gortmaker, linux-acpi, linux-kernel,
ysiyer, Ajay Thomas, Bin Gao
On Thu, Jun 23, 2016 at 01:06:30AM -0700, Bin Gao wrote:
> 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 <chandra.sekhar.anagani@intel.com>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Bin Gao <bin.gao@intel.com>
> ---
> 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(-)
>
> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> index 410e96f..53b7052 100644
> --- a/drivers/acpi/pmic/intel_pmic.c
> +++ b/drivers/acpi/pmic/intel_pmic.c
> @@ -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;
What about call it pmic_regs_ctx? pmic_regs_handler sounds like a
function to me.
> };
>
> 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".
> + 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));
Perhaps add a default tag and print some warning in case later BIOS
updates but our driver doesn't keep up.
> + }
> +
> + 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);
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.
> + 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);
Shouldn't the power&thermal operation region handlers be removed here?
Thanks,
Aaron
> ret = -ENODEV;
> goto out_error;
> }
> diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
> index 2f39ee0..bdb7dcc 100644
> --- a/drivers/acpi/pmic/intel_pmic.h
> +++ b/drivers/acpi/pmic/intel_pmic.h
> @@ -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);
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-06-23 9:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-23 8:06 [PATCH v3 3/3] acpi/pmic: Add support for PMIC regs operation region Bin Gao
2016-06-23 9:07 ` Aaron Lu
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).