All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region
@ 2016-06-24  0:52 Bin Gao
  2016-06-27  9:26 ` Heikki Krogerus
  0 siblings, 1 reply; 7+ messages in thread
From: Bin Gao @ 2016-06-24  0:52 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 - regs op region is used to handle the
PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.

The intel_gpio_ctx  structure is created for the purpose of handling
the PMIC gpio register read and write.

Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>
Signed-off-by: Bin Gao <bin.gao@intel.com>
---
Changes in v4:
 - various fixes to address Aaron's comments.
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, 76 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index 410e96f..e11d1e0 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_gpio_ctx	ctx;
 };
 
 static int pmic_get_reg_bit(int address, struct pmic_table *table,
@@ -204,6 +206,56 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
 	return AE_OK;
 }
 
+static acpi_status intel_pmic_gpio_handler(u32 function,
+	acpi_physical_address address, u32 bits, u64 *value64,
+		void *handler_context, void *region_context)
+{
+	struct intel_pmic_opregion *opregion = region_context;
+	int result;
+
+	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 = regmap_write(opregion->regmap,
+					 opregion->ctx.address,
+					opregion->ctx.value);
+		} else {
+			result = regmap_read(opregion->regmap,
+					opregion->ctx.address,
+					&opregion->ctx.value);
+			if (result == 0)
+				*value64 = opregion->ctx.value;
+		}
+		memset(&opregion->ctx, 0x00, sizeof(opregion->ctx));
+		break;
+	default:
+		pr_err("%s(): Inavlid address %llu, please check the BIOS\n",
+							 __func__, address);
+		return AE_ERROR;
+	}
+
+	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)
@@ -240,15 +292,31 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 						    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;
+		goto remove_pmic_power_handler;
+	}
+
+	status = acpi_install_address_space_handler(handle,
+						    PMIC_REGS_OPREGION_ID,
+						    intel_pmic_gpio_handler,
+						    NULL, opregion);
+	if (ACPI_FAILURE(status)) {
+		ret = -ENODEV;
+		goto remove_pmic_thermal_handler;
 	}
 
 	opregion->data = d;
 	return 0;
 
+remove_pmic_thermal_handler:
+		acpi_remove_address_space_handler(handle,
+						PMIC_POWER_OPREGION_ID,
+						intel_pmic_thermal_handler);
+remove_pmic_power_handler:
+		acpi_remove_address_space_handler(handle,
+						PMIC_POWER_OPREGION_ID,
+						intel_pmic_power_handler);
+
 out_error:
 	acpi_lpat_free_conversion_table(opregion->lpat_table);
 	return ret;
diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
index e8bfa7b..c87c2ef 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_gpio_ctx {
+	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] 7+ messages in thread

* Re: [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region
  2016-06-24  0:52 [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region Bin Gao
@ 2016-06-27  9:26 ` Heikki Krogerus
  2016-06-27 13:25   ` Rafael J. Wysocki
  0 siblings, 1 reply; 7+ messages in thread
From: Heikki Krogerus @ 2016-06-27  9:26 UTC (permalink / raw)
  To: Bin Gao, Rafael J. Wysocki
  Cc: Aaron Lu, Paul Gortmaker, linux-acpi, linux-kernel, ysiyer,
	Ajay Thomas, Bin Gao

Whoa! Hold on!

On Thu, Jun 23, 2016 at 05:52:53PM -0700, Bin Gao wrote:
> Broxton platform firmware has defined new customized operation regions
> called regs for PMIC chip - regs op region is used to handle the
> PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.
> 
> The intel_gpio_ctx  structure is created for the purpose of handling
> the PMIC gpio register read and write.
> 
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>
> Signed-off-by: Bin Gao <bin.gao@intel.com>
> ---
> Changes in v4:
>  - various fixes to address Aaron's comments.
> 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, 76 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> index 410e96f..e11d1e0 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_gpio_ctx	ctx;

What gpio?

>  };
>  
>  static int pmic_get_reg_bit(int address, struct pmic_table *table,
> @@ -204,6 +206,56 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
>  	return AE_OK;
>  }
>  
> +static acpi_status intel_pmic_gpio_handler(u32 function,
> +	acpi_physical_address address, u32 bits, u64 *value64,
> +		void *handler_context, void *region_context)

What the heck is this? Why is this suddenly a gpio handler?

This is handler for an operation region, not some gpio!



-- 
heikki

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

* Re: [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region
  2016-06-27  9:26 ` Heikki Krogerus
@ 2016-06-27 13:25   ` Rafael J. Wysocki
  2016-06-27 14:37     ` Heikki Krogerus
  0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2016-06-27 13:25 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Bin Gao, Rafael J. Wysocki, Aaron Lu, Paul Gortmaker,
	ACPI Devel Maling List, Linux Kernel Mailing List, ysiyer,
	Ajay Thomas, Bin Gao

On Mon, Jun 27, 2016 at 11:26 AM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> Whoa! Hold on!
>
> On Thu, Jun 23, 2016 at 05:52:53PM -0700, Bin Gao wrote:
>> Broxton platform firmware has defined new customized operation regions
>> called regs for PMIC chip - regs op region is used to handle the
>> PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.
>>
>> The intel_gpio_ctx  structure is created for the purpose of handling
>> the PMIC gpio register read and write.
>>
>> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>> Signed-off-by: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>
>> Signed-off-by: Bin Gao <bin.gao@intel.com>
>> ---
>> Changes in v4:
>>  - various fixes to address Aaron's comments.
>> 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, 76 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
>> index 410e96f..e11d1e0 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_gpio_ctx    ctx;
>
> What gpio?
>
>>  };
>>
>>  static int pmic_get_reg_bit(int address, struct pmic_table *table,
>> @@ -204,6 +206,56 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
>>       return AE_OK;
>>  }
>>
>> +static acpi_status intel_pmic_gpio_handler(u32 function,
>> +     acpi_physical_address address, u32 bits, u64 *value64,
>> +             void *handler_context, void *region_context)
>
> What the heck is this? Why is this suddenly a gpio handler?
>
> This is handler for an operation region, not some gpio!

OK

What about if I replaced this one with the original one from Felipe you sent?

Thanks,
Rafael

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

* Re: [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region
  2016-06-27 13:25   ` Rafael J. Wysocki
@ 2016-06-27 14:37     ` Heikki Krogerus
  2016-06-27 20:29       ` Rafael J. Wysocki
  0 siblings, 1 reply; 7+ messages in thread
From: Heikki Krogerus @ 2016-06-27 14:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Bin Gao, Rafael J. Wysocki, Aaron Lu, Paul Gortmaker,
	ACPI Devel Maling List, Linux Kernel Mailing List, ysiyer,
	Ajay Thomas, Bin Gao

On Mon, Jun 27, 2016 at 03:25:11PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jun 27, 2016 at 11:26 AM, Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> > Whoa! Hold on!
> >
> > On Thu, Jun 23, 2016 at 05:52:53PM -0700, Bin Gao wrote:
> >> Broxton platform firmware has defined new customized operation regions
> >> called regs for PMIC chip - regs op region is used to handle the
> >> PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.
> >>
> >> The intel_gpio_ctx  structure is created for the purpose of handling
> >> the PMIC gpio register read and write.
> >>
> >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> >> Signed-off-by: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>
> >> Signed-off-by: Bin Gao <bin.gao@intel.com>
> >> ---
> >> Changes in v4:
> >>  - various fixes to address Aaron's comments.
> >> 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, 76 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> >> index 410e96f..e11d1e0 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_gpio_ctx    ctx;
> >
> > What gpio?
> >
> >>  };
> >>
> >>  static int pmic_get_reg_bit(int address, struct pmic_table *table,
> >> @@ -204,6 +206,56 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
> >>       return AE_OK;
> >>  }
> >>
> >> +static acpi_status intel_pmic_gpio_handler(u32 function,
> >> +     acpi_physical_address address, u32 bits, u64 *value64,
> >> +             void *handler_context, void *region_context)
> >
> > What the heck is this? Why is this suddenly a gpio handler?
> >
> > This is handler for an operation region, not some gpio!
> 
> OK
> 
> What about if I replaced this one with the original one from Felipe you sent?

That works for me.


Thanks,

-- 
heikki

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

* Re: [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region
  2016-06-27 14:37     ` Heikki Krogerus
@ 2016-06-27 20:29       ` Rafael J. Wysocki
  2016-06-27 21:25           ` Gao, Bin
  0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2016-06-27 20:29 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Rafael J. Wysocki, Bin Gao, Aaron Lu, Paul Gortmaker,
	ACPI Devel Maling List, Linux Kernel Mailing List, ysiyer,
	Ajay Thomas, Bin Gao

On Monday, June 27, 2016 05:37:35 PM Heikki Krogerus wrote:
> On Mon, Jun 27, 2016 at 03:25:11PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jun 27, 2016 at 11:26 AM, Heikki Krogerus
> > <heikki.krogerus@linux.intel.com> wrote:
> > > Whoa! Hold on!
> > >
> > > On Thu, Jun 23, 2016 at 05:52:53PM -0700, Bin Gao wrote:
> > >> Broxton platform firmware has defined new customized operation regions
> > >> called regs for PMIC chip - regs op region is used to handle the
> > >> PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.
> > >>
> > >> The intel_gpio_ctx  structure is created for the purpose of handling
> > >> the PMIC gpio register read and write.
> > >>
> > >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> > >> Signed-off-by: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>
> > >> Signed-off-by: Bin Gao <bin.gao@intel.com>
> > >> ---
> > >> Changes in v4:
> > >>  - various fixes to address Aaron's comments.
> > >> 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, 76 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> > >> index 410e96f..e11d1e0 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_gpio_ctx    ctx;
> > >
> > > What gpio?
> > >
> > >>  };
> > >>
> > >>  static int pmic_get_reg_bit(int address, struct pmic_table *table,
> > >> @@ -204,6 +206,56 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
> > >>       return AE_OK;
> > >>  }
> > >>
> > >> +static acpi_status intel_pmic_gpio_handler(u32 function,
> > >> +     acpi_physical_address address, u32 bits, u64 *value64,
> > >> +             void *handler_context, void *region_context)
> > >
> > > What the heck is this? Why is this suddenly a gpio handler?
> > >
> > > This is handler for an operation region, not some gpio!
> > 
> > OK
> > 
> > What about if I replaced this one with the original one from Felipe you sent?
> 
> That works for me.

OK, let's do that, then.

Thanks,
Rafael

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

* RE: [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region
  2016-06-27 20:29       ` Rafael J. Wysocki
@ 2016-06-27 21:25           ` Gao, Bin
  0 siblings, 0 replies; 7+ messages in thread
From: Gao, Bin @ 2016-06-27 21:25 UTC (permalink / raw)
  To: Rafael J. Wysocki, Heikki Krogerus
  Cc: Rafael J. Wysocki, Bin Gao, Lu, Aaron, Gortmaker,
	Paul (Wind River),
	ACPI Devel Maling List, Linux Kernel Mailing List, Iyer,
	Yegnesh S, David Rajamanickam, Ajay Thomas

I'm fine with that. Thanks Rafael.

-Bin

-----Original Message-----
From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net] 
Sent: Monday, June 27, 2016 1:29 PM
To: Heikki Krogerus
Cc: Rafael J. Wysocki; Bin Gao; Lu, Aaron; Gortmaker, Paul (Wind River); ACPI Devel Maling List; Linux Kernel Mailing List; Iyer, Yegnesh S; David Rajamanickam, Ajay Thomas; Gao, Bin
Subject: Re: [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region

On Monday, June 27, 2016 05:37:35 PM Heikki Krogerus wrote:
> On Mon, Jun 27, 2016 at 03:25:11PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jun 27, 2016 at 11:26 AM, Heikki Krogerus 
> > <heikki.krogerus@linux.intel.com> wrote:
> > > Whoa! Hold on!
> > >
> > > On Thu, Jun 23, 2016 at 05:52:53PM -0700, Bin Gao wrote:
> > >> Broxton platform firmware has defined new customized operation 
> > >> regions called regs for PMIC chip - regs op region is used to 
> > >> handle the PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.
> > >>
> > >> The intel_gpio_ctx  structure is created for the purpose of 
> > >> handling the PMIC gpio register read and write.
> > >>
> > >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> > >> Signed-off-by: Chandra Sekhar Anagani 
> > >> <chandra.sekhar.anagani@intel.com>
> > >> Signed-off-by: Bin Gao <bin.gao@intel.com>
> > >> ---
> > >> Changes in v4:
> > >>  - various fixes to address Aaron's comments.
> > >> 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, 76 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/acpi/pmic/intel_pmic.c 
> > >> b/drivers/acpi/pmic/intel_pmic.c index 410e96f..e11d1e0 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_gpio_ctx    ctx;
> > >
> > > What gpio?
> > >
> > >>  };
> > >>
> > >>  static int pmic_get_reg_bit(int address, struct pmic_table 
> > >> *table, @@ -204,6 +206,56 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
> > >>       return AE_OK;
> > >>  }
> > >>
> > >> +static acpi_status intel_pmic_gpio_handler(u32 function,
> > >> +     acpi_physical_address address, u32 bits, u64 *value64,
> > >> +             void *handler_context, void *region_context)
> > >
> > > What the heck is this? Why is this suddenly a gpio handler?
> > >
> > > This is handler for an operation region, not some gpio!
> > 
> > OK
> > 
> > What about if I replaced this one with the original one from Felipe you sent?
> 
> That works for me.

OK, let's do that, then.

Thanks,
Rafael


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

* RE: [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region
@ 2016-06-27 21:25           ` Gao, Bin
  0 siblings, 0 replies; 7+ messages in thread
From: Gao, Bin @ 2016-06-27 21:25 UTC (permalink / raw)
  To: Rafael J. Wysocki, Heikki Krogerus
  Cc: Rafael J. Wysocki, Bin Gao, Lu, Aaron, Gortmaker,
	Paul (Wind River),
	ACPI Devel Maling List, Linux Kernel Mailing List, Iyer,
	Yegnesh S, David Rajamanickam, Ajay Thomas

I'm fine with that. Thanks Rafael.

-Bin

-----Original Message-----
From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net] 
Sent: Monday, June 27, 2016 1:29 PM
To: Heikki Krogerus
Cc: Rafael J. Wysocki; Bin Gao; Lu, Aaron; Gortmaker, Paul (Wind River); ACPI Devel Maling List; Linux Kernel Mailing List; Iyer, Yegnesh S; David Rajamanickam, Ajay Thomas; Gao, Bin
Subject: Re: [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region

On Monday, June 27, 2016 05:37:35 PM Heikki Krogerus wrote:
> On Mon, Jun 27, 2016 at 03:25:11PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jun 27, 2016 at 11:26 AM, Heikki Krogerus 
> > <heikki.krogerus@linux.intel.com> wrote:
> > > Whoa! Hold on!
> > >
> > > On Thu, Jun 23, 2016 at 05:52:53PM -0700, Bin Gao wrote:
> > >> Broxton platform firmware has defined new customized operation 
> > >> regions called regs for PMIC chip - regs op region is used to 
> > >> handle the PMIC gpio mainly intended for the TYPE-C VBUS and Orientation.
> > >>
> > >> The intel_gpio_ctx  structure is created for the purpose of 
> > >> handling the PMIC gpio register read and write.
> > >>
> > >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> > >> Signed-off-by: Chandra Sekhar Anagani 
> > >> <chandra.sekhar.anagani@intel.com>
> > >> Signed-off-by: Bin Gao <bin.gao@intel.com>
> > >> ---
> > >> Changes in v4:
> > >>  - various fixes to address Aaron's comments.
> > >> 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, 76 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/acpi/pmic/intel_pmic.c 
> > >> b/drivers/acpi/pmic/intel_pmic.c index 410e96f..e11d1e0 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_gpio_ctx    ctx;
> > >
> > > What gpio?
> > >
> > >>  };
> > >>
> > >>  static int pmic_get_reg_bit(int address, struct pmic_table 
> > >> *table, @@ -204,6 +206,56 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
> > >>       return AE_OK;
> > >>  }
> > >>
> > >> +static acpi_status intel_pmic_gpio_handler(u32 function,
> > >> +     acpi_physical_address address, u32 bits, u64 *value64,
> > >> +             void *handler_context, void *region_context)
> > >
> > > What the heck is this? Why is this suddenly a gpio handler?
> > >
> > > This is handler for an operation region, not some gpio!
> > 
> > OK
> > 
> > What about if I replaced this one with the original one from Felipe you sent?
> 
> That works for me.

OK, let's do that, then.

Thanks,
Rafael

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

end of thread, other threads:[~2016-06-27 21:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-24  0:52 [PATCH v4 3/3] acpi/pmic: Add support for PMIC regs operation region Bin Gao
2016-06-27  9:26 ` Heikki Krogerus
2016-06-27 13:25   ` Rafael J. Wysocki
2016-06-27 14:37     ` Heikki Krogerus
2016-06-27 20:29       ` Rafael J. Wysocki
2016-06-27 21:25         ` Gao, Bin
2016-06-27 21:25           ` Gao, Bin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.