I'm fine with that. Thanks Rafael. -Bin -----Original Message----- From: Rafael J. Wysocki [mailto:rjw@xxxxxxxxxxxxx] 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> > > >> Signed-off-by: Chandra Sekhar Anagani > > >> <chandra.sekhar.anagani@xxxxxxxxx> > > >> Signed-off-by: Bin Gao <bin.gao@xxxxxxxxx> > > >> --- > > >> 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 ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f