diff mbox

[v1,3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

Message ID 1496750118-5570-4-git-send-email-rajmohan.mani@intel.com
State New
Headers show

Commit Message

Mani, Rajmohan June 6, 2017, 11:55 a.m. UTC
The Kabylake platform coreboot (Chrome OS equivalent of
BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
These operation regions are to enable/disable voltage
regulators, configure voltage regulators, enable/disable
clocks and to configure clocks.

This config adds ACPI operation region support for TI TPS68470 PMIC.
TPS68470 device is an advanced power management unit that powers
a Compact Camera Module (CCM), generates clocks for image sensors,
drives a dual LED for flash and incorporates two LED drivers for
general purpose indicators.
This driver enables ACPI operation region support to control voltage
regulators and clocks for the TPS68470 PMIC.

Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
---
 drivers/acpi/Kconfig              |  12 +
 drivers/acpi/Makefile             |   2 +
 drivers/acpi/pmic/pmic_tps68470.c | 454 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 468 insertions(+)
 create mode 100644 drivers/acpi/pmic/pmic_tps68470.c

Comments

Andy Shevchenko June 6, 2017, 2:23 p.m. UTC | #1
+Cc Hans (that's why didn't delete anything from original mail, just
adding my comments).

Hans, if you have few minutes it would be appreciated to glance on the
below for some issues if any since you did pass quite a good quest
with other PMIC drivers.

On Tue, Jun 6, 2017 at 2:55 PM, Rajmohan Mani <rajmohan.mani@intel.com> wrote:
> The Kabylake platform coreboot (Chrome OS equivalent of
> BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
> These operation regions are to enable/disable voltage
> regulators, configure voltage regulators, enable/disable
> clocks and to configure clocks.
>
> This config adds ACPI operation region support for TI TPS68470 PMIC.
> TPS68470 device is an advanced power management unit that powers
> a Compact Camera Module (CCM), generates clocks for image sensors,
> drives a dual LED for flash and incorporates two LED drivers for
> general purpose indicators.
> This driver enables ACPI operation region support to control voltage
> regulators and clocks for the TPS68470 PMIC.
>
> Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
> ---
>  drivers/acpi/Kconfig              |  12 +
>  drivers/acpi/Makefile             |   2 +

>  drivers/acpi/pmic/pmic_tps68470.c | 454 ++++++++++++++++++++++++++++++++++++++

Follow the pattern, please, I suppose
ti_pmic_tps68470.c

>  3 files changed, 468 insertions(+)
>  create mode 100644 drivers/acpi/pmic/pmic_tps68470.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 1ce52f8..218d22d 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -535,4 +535,16 @@ if ARM64
>  source "drivers/acpi/arm64/Kconfig"
>  endif
>
> +config TPS68470_PMIC_OPREGION
> +       bool "ACPI operation region support for TPS68470 PMIC"
> +       help
> +         This config adds ACPI operation region support for TI TPS68470 PMIC.
> +         TPS68470 device is an advanced power management unit that powers
> +         a Compact Camera Module (CCM), generates clocks for image sensors,
> +         drives a dual LED for flash and incorporates two LED drivers for
> +         general purpose indicators.
> +         This driver enables ACPI operation region support control voltage
> +         regulators and clocks.
> +

> +

Extra line, remove.

>  endif  # ACPI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index b1aacfc..7113d05 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -106,6 +106,8 @@ obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o
>
>  obj-$(CONFIG_ACPI_CONFIGFS)    += acpi_configfs.o
>
> +obj-$(CONFIG_TPS68470_PMIC_OPREGION)   += pmic/pmic_tps68470.o
> +
>  video-objs                     += acpi_video.o video_detect.o
>  obj-y                          += dptf/
>
> diff --git a/drivers/acpi/pmic/pmic_tps68470.c b/drivers/acpi/pmic/pmic_tps68470.c
> new file mode 100644
> index 0000000..b2d608b
> --- /dev/null
> +++ b/drivers/acpi/pmic/pmic_tps68470.c
> @@ -0,0 +1,454 @@
> +/*
> + * TI TPS68470 PMIC operation region driver
> + *
> + * Copyright (C) 2017 Intel Corporation. All rights reserved.
> + * Author: Rajmohan Mani <rajmohan.mani@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Based on drivers/acpi/pmic/intel_pmic* drivers
> + *
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/mfd/tps68470.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +struct ti_pmic_table {
> +       u32 address;            /* operation region address */
> +       u32 reg;                /* corresponding register */
> +       u32 bitmask;            /* bit mask for power, clock */
> +};
> +
> +#define TI_PMIC_POWER_OPREGION_ID              0xB0
> +#define TI_PMIC_VR_VAL_OPREGION_ID             0xB1
> +#define TI_PMIC_CLOCK_OPREGION_ID              0xB2
> +#define TI_PMIC_CLKFREQ_OPREGION_ID            0xB3
> +
> +struct ti_pmic_opregion {
> +       struct mutex lock;
> +       struct regmap *regmap;
> +};
> +
> +#define S_IO_I2C_EN    (BIT(0) | BIT(1))
> +
> +static const struct ti_pmic_table power_table[] = {
> +       {
> +               .address = 0x00,
> +               .reg = TPS68470_REG_S_I2C_CTL,
> +               .bitmask = S_IO_I2C_EN,
> +               /* S_I2C_CTL */
> +       },
> +       {
> +               .address = 0x04,
> +               .reg = TPS68470_REG_VCMCTL,
> +               .bitmask = BIT(0),
> +               /* VCMCTL */
> +       },
> +       {
> +               .address = 0x08,
> +               .reg = TPS68470_REG_VAUX1CTL,
> +               .bitmask = BIT(0),
> +               /* VAUX1_CTL */
> +       },
> +       {
> +               .address = 0x0C,
> +               .reg = TPS68470_REG_VAUX2CTL,
> +               .bitmask = BIT(0),
> +               /* VAUX2CTL */
> +       },
> +       {
> +               .address = 0x10,
> +               .reg = TPS68470_REG_VACTL,
> +               .bitmask = BIT(0),
> +               /* VACTL */
> +       },
> +       {
> +               .address = 0x14,
> +               .reg = TPS68470_REG_VDCTL,
> +               .bitmask = BIT(0),
> +               /* VDCTL */
> +       },
> +};
> +
> +/* Table to set voltage regulator value */
> +static const struct ti_pmic_table vr_val_table[] = {
> +       {
> +               .address = 0x00,
> +               .reg = TPS68470_REG_VSIOVAL,
> +               .bitmask = TPS68470_VSIOVAL_IOVOLT_MASK,
> +               /* TPS68470_REG_VSIOVAL */
> +       },
> +       {
> +               .address = 0x04,
> +               .reg = TPS68470_REG_VIOVAL,
> +               .bitmask = TPS68470_VIOVAL_IOVOLT_MASK,
> +               /* TPS68470_REG_VIOVAL */
> +       },
> +       {
> +               .address = 0x08,
> +               .reg = TPS68470_REG_VCMVAL,
> +               .bitmask = TPS68470_VCMVAL_VCVOLT_MASK,
> +               /* TPS68470_REG_VCMVAL */
> +       },
> +       {
> +               .address = 0x0C,
> +               .reg = TPS68470_REG_VAUX1VAL,
> +               .bitmask = TPS68470_VAUX1VAL_AUX1VOLT_MASK,
> +               /* TPS68470_REG_VAUX1VAL */
> +       },
> +       {
> +               .address = 0x10,
> +               .reg = TPS68470_REG_VAUX2VAL,
> +               .bitmask = TPS68470_VAUX2VAL_AUX2VOLT_MASK,
> +               /* TPS68470_REG_VAUX2VAL */
> +       },
> +       {
> +               .address = 0x14,
> +               .reg = TPS68470_REG_VAVAL,
> +               .bitmask = TPS68470_VAVAL_AVOLT_MASK,
> +               /* TPS68470_REG_VAVAL */
> +       },
> +       {
> +               .address = 0x18,
> +               .reg = TPS68470_REG_VDVAL,
> +               .bitmask = TPS68470_VDVAL_DVOLT_MASK,
> +               /* TPS68470_REG_VDVAL */
> +       },
> +};
> +
> +/* Table to configure clock frequency */
> +static const struct ti_pmic_table clk_freq_table[] = {
> +       {
> +               .address = 0x00,
> +               .reg = TPS68470_REG_POSTDIV2,
> +               .bitmask = BIT(0) | BIT(1),
> +               /* TPS68470_REG_POSTDIV2 */
> +       },
> +       {
> +               .address = 0x04,
> +               .reg = TPS68470_REG_BOOSTDIV,
> +               .bitmask = 0x1F,
> +               /* TPS68470_REG_BOOSTDIV */
> +       },
> +       {
> +               .address = 0x08,
> +               .reg = TPS68470_REG_BUCKDIV,
> +               .bitmask = 0x0F,
> +               /* TPS68470_REG_BUCKDIV */
> +       },
> +       {
> +               .address = 0x0C,
> +               .reg = TPS68470_REG_PLLSWR,
> +               .bitmask = 0x13,
> +               /* TPS68470_REG_PLLSWR */
> +       },
> +       {
> +               .address = 0x10,
> +               .reg = TPS68470_REG_XTALDIV,
> +               .bitmask = 0xFF,
> +               /* TPS68470_REG_XTALDIV */
> +       },
> +       {
> +               .address = 0x14,
> +               .reg = TPS68470_REG_PLLDIV,
> +               .bitmask = 0xFF,
> +               /* TPS68470_REG_PLLDIV */
> +       },
> +       {
> +               .address = 0x18,
> +               .reg = TPS68470_REG_POSTDIV,
> +               .bitmask = 0x83,
> +               /* TPS68470_REG_POSTDIV */
> +       },
> +};
> +
> +/* Table to configure and enable clocks */
> +static const struct ti_pmic_table clk_table[] = {
> +       {
> +               .address = 0x00,
> +               .reg = TPS68470_REG_PLLCTL,
> +               .bitmask = 0xF5,
> +               /* TPS68470_REG_PLLCTL */
> +       },
> +       {
> +               .address = 0x04,
> +               .reg = TPS68470_REG_PLLCTL2,
> +               .bitmask = BIT(0),
> +               /* TPS68470_REG_PLLCTL2 */
> +       },
> +       {
> +               .address = 0x08,
> +               .reg = TPS68470_REG_CLKCFG1,
> +               .bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
> +                       TPS68470_CLKCFG1_MODE_B_MASK,
> +               /* TPS68470_REG_CLKCFG1 */
> +       },
> +       {
> +               .address = 0x0C,
> +               .reg = TPS68470_REG_CLKCFG2,
> +               .bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
> +                       TPS68470_CLKCFG1_MODE_B_MASK,
> +               /* TPS68470_REG_CLKCFG2 */
> +       },
> +};
> +
> +static int pmic_get_reg_bit(u64 address, struct ti_pmic_table *table,
> +                           int count, int *reg, int *bitmask)
> +{
> +       u64 i;
> +

> +       i = address / 4;


> +

Remove this line.

> +       if (i >= count)
> +               return -ENOENT;
> +
> +       if (!reg || !bitmask)
> +               return -EINVAL;
> +
> +       *reg = table[i].reg;
> +       *bitmask = table[i].bitmask;
> +
> +       return 0;
> +}
> +
> +static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg,
> +                                      int bitmask, u64 *value)
> +{
> +       int data;
> +
> +       if (regmap_read(regmap, reg, &data))
> +               return -EIO;
> +
> +       *value = (data & bitmask) ? 1 : 0;
> +       return 0;
> +}
> +
> +static int ti_tps68470_pmic_get_vr_val(struct regmap *regmap, int reg,
> +                                      int bitmask, u64 *value)
> +{
> +       int data;
> +
> +       if (regmap_read(regmap, reg, &data))
> +               return -EIO;
> +
> +       *value = data & bitmask;
> +       return 0;
> +}
> +
> +static int ti_tps68470_pmic_get_clk(struct regmap *regmap, int reg,
> +                                      int bitmask, u64 *value)
> +{
> +       int data;
> +
> +       if (regmap_read(regmap, reg, &data))
> +               return -EIO;
> +
> +       *value = (data & bitmask) ? 1 : 0;
> +       return 0;
> +}
> +
> +static int ti_tps68470_pmic_get_clk_freq(struct regmap *regmap, int reg,
> +                                      int bitmask, u64 *value)
> +{
> +       int data;
> +
> +       if (regmap_read(regmap, reg, &data))
> +               return -EIO;
> +
> +       *value = data & bitmask;
> +       return 0;
> +}
> +
> +static int ti_tps68470_regmap_update_bits(struct regmap *regmap, int reg,
> +                                       int bitmask, u64 value)
> +{
> +       return regmap_update_bits(regmap, reg, bitmask, value);
> +}
> +
> +static acpi_status ti_pmic_common_handler(u32 function,
> +                                         acpi_physical_address address,
> +                                         u32 bits, u64 *value,
> +                                         void *handler_context,
> +                                         void *region_context,
> +                                         int (*get)(struct regmap *,
> +                                                    int, int, u64 *),
> +                                         int (*update)(struct regmap *,
> +                                                       int, int, u64),
> +                                         struct ti_pmic_table *table,
> +                                         int table_size)
> +{
> +       struct ti_pmic_opregion *opregion = region_context;
> +       struct regmap *regmap = opregion->regmap;
> +       int reg, ret, bitmask;
> +
> +       if (bits != 32)
> +               return AE_BAD_PARAMETER;
> +
> +       ret = pmic_get_reg_bit(address, table,
> +                                 table_size, &reg, &bitmask);
> +       if (ret < 0)
> +               return AE_BAD_PARAMETER;
> +
> +       if (function == ACPI_WRITE && (*value > bitmask))
> +               return AE_BAD_PARAMETER;
> +
> +       mutex_lock(&opregion->lock);
> +
> +       ret = (function == ACPI_READ) ?
> +               get(regmap, reg, bitmask, value) :
> +               update(regmap, reg, bitmask, *value);
> +
> +       mutex_unlock(&opregion->lock);
> +
> +       return ret ? AE_ERROR : AE_OK;
> +}
> +
> +static acpi_status ti_pmic_clk_freq_handler(u32 function,
> +                                           acpi_physical_address address,
> +                                           u32 bits, u64 *value,
> +                                           void *handler_context,
> +                                           void *region_context)
> +{
> +       return ti_pmic_common_handler(function, address, bits, value,
> +                               handler_context, region_context,
> +                               ti_tps68470_pmic_get_clk_freq,
> +                               ti_tps68470_regmap_update_bits,
> +                               (struct ti_pmic_table *) &clk_freq_table,
> +                               ARRAY_SIZE(clk_freq_table));
> +}
> +
> +static acpi_status ti_pmic_clk_handler(u32 function,
> +                                      acpi_physical_address address, u32 bits,
> +                                      u64 *value, void *handler_context,
> +                                      void *region_context)
> +{
> +       return ti_pmic_common_handler(function, address, bits, value,
> +                               handler_context, region_context,
> +                               ti_tps68470_pmic_get_clk,
> +                               ti_tps68470_regmap_update_bits,
> +                               (struct ti_pmic_table *) &clk_table,
> +                                ARRAY_SIZE(clk_table));
> +}
> +
> +static acpi_status ti_pmic_vr_val_handler(u32 function,
> +                                         acpi_physical_address address,
> +                                         u32 bits, u64 *value,
> +                                         void *handler_context,
> +                                         void *region_context)
> +{
> +       return ti_pmic_common_handler(function, address, bits, value,
> +                               handler_context, region_context,
> +                               ti_tps68470_pmic_get_vr_val,
> +                               ti_tps68470_regmap_update_bits,
> +                               (struct ti_pmic_table *) &vr_val_table,
> +                               ARRAY_SIZE(vr_val_table));
> +}
> +
> +static acpi_status ti_pmic_power_handler(u32 function,
> +                                        acpi_physical_address address,
> +                                        u32 bits, u64 *value,
> +                                        void *handler_context,
> +                                        void *region_context)
> +{
> +       if (bits != 32)
> +               return AE_BAD_PARAMETER;
> +
> +       /* set/clear for bit 0, bits 0 and 1 together */
> +       if (function == ACPI_WRITE &&
> +           !(*value == 0 || *value == 1 || *value == 3)) {
> +               return AE_BAD_PARAMETER;
> +       }
> +
> +       return ti_pmic_common_handler(function, address, bits, value,
> +                               handler_context, region_context,
> +                               ti_tps68470_pmic_get_power,
> +                               ti_tps68470_regmap_update_bits,
> +                               (struct ti_pmic_table *) &power_table,
> +                               ARRAY_SIZE(power_table));
> +}
> +
> +static int ti_tps68470_pmic_opregion_probe(struct platform_device *pdev)
> +{
> +       struct tps68470 *pmic = dev_get_drvdata(pdev->dev.parent);
> +       acpi_handle handle = ACPI_HANDLE(pdev->dev.parent);
> +       struct device *dev = &pdev->dev;
> +       struct ti_pmic_opregion *opregion;
> +       acpi_status status;
> +

> +       if (!dev || !pmic->regmap) {
> +               WARN(1, "dev or regmap is NULL\n");
> +               return -EINVAL;
> +       }
> +
> +       if (!handle) {
> +               WARN(1, "acpi handle is NULL\n");
> +               return -ENODEV;
> +       }

I dunno if WARNs make user experience any better.
Besides that I would double check you may have such cases.

> +
> +       opregion = devm_kzalloc(dev, sizeof(*opregion), GFP_KERNEL);
> +       if (!opregion)
> +               return -ENOMEM;
> +
> +       mutex_init(&opregion->lock);
> +       opregion->regmap = pmic->regmap;
> +
> +       status = acpi_install_address_space_handler(handle,
> +                                                   TI_PMIC_POWER_OPREGION_ID,
> +                                                   ti_pmic_power_handler,
> +                                                   NULL, opregion);
> +       if (ACPI_FAILURE(status))
> +               return -ENODEV;
> +
> +       status = acpi_install_address_space_handler(handle,
> +                                                   TI_PMIC_VR_VAL_OPREGION_ID,
> +                                                   ti_pmic_vr_val_handler,
> +                                                   NULL, opregion);
> +       if (ACPI_FAILURE(status))
> +               goto out_remove_power_handler;
> +
> +       status = acpi_install_address_space_handler(handle,
> +                                                   TI_PMIC_CLOCK_OPREGION_ID,
> +                                                   ti_pmic_clk_handler,
> +                                                   NULL, opregion);
> +       if (ACPI_FAILURE(status))
> +               goto out_remove_vr_val_handler;
> +
> +       status = acpi_install_address_space_handler(handle,
> +                                                   TI_PMIC_CLKFREQ_OPREGION_ID,
> +                                                   ti_pmic_clk_freq_handler,
> +                                                   NULL, opregion);
> +       if (ACPI_FAILURE(status))
> +               goto out_remove_clk_handler;
> +
> +       return 0;
> +
> +out_remove_clk_handler:
> +       acpi_remove_address_space_handler(handle, TI_PMIC_CLOCK_OPREGION_ID,
> +                                         ti_pmic_clk_handler);
> +out_remove_vr_val_handler:
> +       acpi_remove_address_space_handler(handle, TI_PMIC_VR_VAL_OPREGION_ID,
> +                                         ti_pmic_vr_val_handler);
> +out_remove_power_handler:
> +       acpi_remove_address_space_handler(handle, TI_PMIC_POWER_OPREGION_ID,
> +                                         ti_pmic_power_handler);
> +       return -ENODEV;
> +}
> +
> +static struct platform_driver ti_tps68470_pmic_opregion_driver = {
> +       .probe = ti_tps68470_pmic_opregion_probe,
> +       .driver = {
> +               .name = "tps68470_pmic_opregion",
> +       },
> +};
> +
> +builtin_platform_driver(ti_tps68470_pmic_opregion_driver)
> --
> 1.9.1
>
Hans de Goede June 6, 2017, 3:21 p.m. UTC | #2
Hi,

On 06/06/2017 04:23 PM, Andy Shevchenko wrote:
> +Cc Hans (that's why didn't delete anything from original mail, just
> adding my comments).
> 
> Hans, if you have few minutes it would be appreciated to glance on the
> below for some issues if any since you did pass quite a good quest
> with other PMIC drivers.

I've gone over this driver, nothing stands out in a bad way to me,
IOW this seems like a normal PMIC OpRegion handler to me.

Regards,

Hans



> 
> On Tue, Jun 6, 2017 at 2:55 PM, Rajmohan Mani <rajmohan.mani@intel.com> wrote:
>> The Kabylake platform coreboot (Chrome OS equivalent of
>> BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
>> These operation regions are to enable/disable voltage
>> regulators, configure voltage regulators, enable/disable
>> clocks and to configure clocks.
>>
>> This config adds ACPI operation region support for TI TPS68470 PMIC.
>> TPS68470 device is an advanced power management unit that powers
>> a Compact Camera Module (CCM), generates clocks for image sensors,
>> drives a dual LED for flash and incorporates two LED drivers for
>> general purpose indicators.
>> This driver enables ACPI operation region support to control voltage
>> regulators and clocks for the TPS68470 PMIC.
>>
>> Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
>> ---
>>   drivers/acpi/Kconfig              |  12 +
>>   drivers/acpi/Makefile             |   2 +
> 
>>   drivers/acpi/pmic/pmic_tps68470.c | 454 ++++++++++++++++++++++++++++++++++++++
> 
> Follow the pattern, please, I suppose
> ti_pmic_tps68470.c
> 
>>   3 files changed, 468 insertions(+)
>>   create mode 100644 drivers/acpi/pmic/pmic_tps68470.c
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 1ce52f8..218d22d 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -535,4 +535,16 @@ if ARM64
>>   source "drivers/acpi/arm64/Kconfig"
>>   endif
>>
>> +config TPS68470_PMIC_OPREGION
>> +       bool "ACPI operation region support for TPS68470 PMIC"
>> +       help
>> +         This config adds ACPI operation region support for TI TPS68470 PMIC.
>> +         TPS68470 device is an advanced power management unit that powers
>> +         a Compact Camera Module (CCM), generates clocks for image sensors,
>> +         drives a dual LED for flash and incorporates two LED drivers for
>> +         general purpose indicators.
>> +         This driver enables ACPI operation region support control voltage
>> +         regulators and clocks.
>> +
> 
>> +
> 
> Extra line, remove.
> 
>>   endif  # ACPI
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index b1aacfc..7113d05 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -106,6 +106,8 @@ obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o
>>
>>   obj-$(CONFIG_ACPI_CONFIGFS)    += acpi_configfs.o
>>
>> +obj-$(CONFIG_TPS68470_PMIC_OPREGION)   += pmic/pmic_tps68470.o
>> +
>>   video-objs                     += acpi_video.o video_detect.o
>>   obj-y                          += dptf/
>>
>> diff --git a/drivers/acpi/pmic/pmic_tps68470.c b/drivers/acpi/pmic/pmic_tps68470.c
>> new file mode 100644
>> index 0000000..b2d608b
>> --- /dev/null
>> +++ b/drivers/acpi/pmic/pmic_tps68470.c
>> @@ -0,0 +1,454 @@
>> +/*
>> + * TI TPS68470 PMIC operation region driver
>> + *
>> + * Copyright (C) 2017 Intel Corporation. All rights reserved.
>> + * Author: Rajmohan Mani <rajmohan.mani@intel.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * Based on drivers/acpi/pmic/intel_pmic* drivers
>> + *
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/mfd/tps68470.h>
>> +#include <linux/init.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +struct ti_pmic_table {
>> +       u32 address;            /* operation region address */
>> +       u32 reg;                /* corresponding register */
>> +       u32 bitmask;            /* bit mask for power, clock */
>> +};
>> +
>> +#define TI_PMIC_POWER_OPREGION_ID              0xB0
>> +#define TI_PMIC_VR_VAL_OPREGION_ID             0xB1
>> +#define TI_PMIC_CLOCK_OPREGION_ID              0xB2
>> +#define TI_PMIC_CLKFREQ_OPREGION_ID            0xB3
>> +
>> +struct ti_pmic_opregion {
>> +       struct mutex lock;
>> +       struct regmap *regmap;
>> +};
>> +
>> +#define S_IO_I2C_EN    (BIT(0) | BIT(1))
>> +
>> +static const struct ti_pmic_table power_table[] = {
>> +       {
>> +               .address = 0x00,
>> +               .reg = TPS68470_REG_S_I2C_CTL,
>> +               .bitmask = S_IO_I2C_EN,
>> +               /* S_I2C_CTL */
>> +       },
>> +       {
>> +               .address = 0x04,
>> +               .reg = TPS68470_REG_VCMCTL,
>> +               .bitmask = BIT(0),
>> +               /* VCMCTL */
>> +       },
>> +       {
>> +               .address = 0x08,
>> +               .reg = TPS68470_REG_VAUX1CTL,
>> +               .bitmask = BIT(0),
>> +               /* VAUX1_CTL */
>> +       },
>> +       {
>> +               .address = 0x0C,
>> +               .reg = TPS68470_REG_VAUX2CTL,
>> +               .bitmask = BIT(0),
>> +               /* VAUX2CTL */
>> +       },
>> +       {
>> +               .address = 0x10,
>> +               .reg = TPS68470_REG_VACTL,
>> +               .bitmask = BIT(0),
>> +               /* VACTL */
>> +       },
>> +       {
>> +               .address = 0x14,
>> +               .reg = TPS68470_REG_VDCTL,
>> +               .bitmask = BIT(0),
>> +               /* VDCTL */
>> +       },
>> +};
>> +
>> +/* Table to set voltage regulator value */
>> +static const struct ti_pmic_table vr_val_table[] = {
>> +       {
>> +               .address = 0x00,
>> +               .reg = TPS68470_REG_VSIOVAL,
>> +               .bitmask = TPS68470_VSIOVAL_IOVOLT_MASK,
>> +               /* TPS68470_REG_VSIOVAL */
>> +       },
>> +       {
>> +               .address = 0x04,
>> +               .reg = TPS68470_REG_VIOVAL,
>> +               .bitmask = TPS68470_VIOVAL_IOVOLT_MASK,
>> +               /* TPS68470_REG_VIOVAL */
>> +       },
>> +       {
>> +               .address = 0x08,
>> +               .reg = TPS68470_REG_VCMVAL,
>> +               .bitmask = TPS68470_VCMVAL_VCVOLT_MASK,
>> +               /* TPS68470_REG_VCMVAL */
>> +       },
>> +       {
>> +               .address = 0x0C,
>> +               .reg = TPS68470_REG_VAUX1VAL,
>> +               .bitmask = TPS68470_VAUX1VAL_AUX1VOLT_MASK,
>> +               /* TPS68470_REG_VAUX1VAL */
>> +       },
>> +       {
>> +               .address = 0x10,
>> +               .reg = TPS68470_REG_VAUX2VAL,
>> +               .bitmask = TPS68470_VAUX2VAL_AUX2VOLT_MASK,
>> +               /* TPS68470_REG_VAUX2VAL */
>> +       },
>> +       {
>> +               .address = 0x14,
>> +               .reg = TPS68470_REG_VAVAL,
>> +               .bitmask = TPS68470_VAVAL_AVOLT_MASK,
>> +               /* TPS68470_REG_VAVAL */
>> +       },
>> +       {
>> +               .address = 0x18,
>> +               .reg = TPS68470_REG_VDVAL,
>> +               .bitmask = TPS68470_VDVAL_DVOLT_MASK,
>> +               /* TPS68470_REG_VDVAL */
>> +       },
>> +};
>> +
>> +/* Table to configure clock frequency */
>> +static const struct ti_pmic_table clk_freq_table[] = {
>> +       {
>> +               .address = 0x00,
>> +               .reg = TPS68470_REG_POSTDIV2,
>> +               .bitmask = BIT(0) | BIT(1),
>> +               /* TPS68470_REG_POSTDIV2 */
>> +       },
>> +       {
>> +               .address = 0x04,
>> +               .reg = TPS68470_REG_BOOSTDIV,
>> +               .bitmask = 0x1F,
>> +               /* TPS68470_REG_BOOSTDIV */
>> +       },
>> +       {
>> +               .address = 0x08,
>> +               .reg = TPS68470_REG_BUCKDIV,
>> +               .bitmask = 0x0F,
>> +               /* TPS68470_REG_BUCKDIV */
>> +       },
>> +       {
>> +               .address = 0x0C,
>> +               .reg = TPS68470_REG_PLLSWR,
>> +               .bitmask = 0x13,
>> +               /* TPS68470_REG_PLLSWR */
>> +       },
>> +       {
>> +               .address = 0x10,
>> +               .reg = TPS68470_REG_XTALDIV,
>> +               .bitmask = 0xFF,
>> +               /* TPS68470_REG_XTALDIV */
>> +       },
>> +       {
>> +               .address = 0x14,
>> +               .reg = TPS68470_REG_PLLDIV,
>> +               .bitmask = 0xFF,
>> +               /* TPS68470_REG_PLLDIV */
>> +       },
>> +       {
>> +               .address = 0x18,
>> +               .reg = TPS68470_REG_POSTDIV,
>> +               .bitmask = 0x83,
>> +               /* TPS68470_REG_POSTDIV */
>> +       },
>> +};
>> +
>> +/* Table to configure and enable clocks */
>> +static const struct ti_pmic_table clk_table[] = {
>> +       {
>> +               .address = 0x00,
>> +               .reg = TPS68470_REG_PLLCTL,
>> +               .bitmask = 0xF5,
>> +               /* TPS68470_REG_PLLCTL */
>> +       },
>> +       {
>> +               .address = 0x04,
>> +               .reg = TPS68470_REG_PLLCTL2,
>> +               .bitmask = BIT(0),
>> +               /* TPS68470_REG_PLLCTL2 */
>> +       },
>> +       {
>> +               .address = 0x08,
>> +               .reg = TPS68470_REG_CLKCFG1,
>> +               .bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
>> +                       TPS68470_CLKCFG1_MODE_B_MASK,
>> +               /* TPS68470_REG_CLKCFG1 */
>> +       },
>> +       {
>> +               .address = 0x0C,
>> +               .reg = TPS68470_REG_CLKCFG2,
>> +               .bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
>> +                       TPS68470_CLKCFG1_MODE_B_MASK,
>> +               /* TPS68470_REG_CLKCFG2 */
>> +       },
>> +};
>> +
>> +static int pmic_get_reg_bit(u64 address, struct ti_pmic_table *table,
>> +                           int count, int *reg, int *bitmask)
>> +{
>> +       u64 i;
>> +
> 
>> +       i = address / 4;
> 
> 
>> +
> 
> Remove this line.
> 
>> +       if (i >= count)
>> +               return -ENOENT;
>> +
>> +       if (!reg || !bitmask)
>> +               return -EINVAL;
>> +
>> +       *reg = table[i].reg;
>> +       *bitmask = table[i].bitmask;
>> +
>> +       return 0;
>> +}
>> +
>> +static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg,
>> +                                      int bitmask, u64 *value)
>> +{
>> +       int data;
>> +
>> +       if (regmap_read(regmap, reg, &data))
>> +               return -EIO;
>> +
>> +       *value = (data & bitmask) ? 1 : 0;
>> +       return 0;
>> +}
>> +
>> +static int ti_tps68470_pmic_get_vr_val(struct regmap *regmap, int reg,
>> +                                      int bitmask, u64 *value)
>> +{
>> +       int data;
>> +
>> +       if (regmap_read(regmap, reg, &data))
>> +               return -EIO;
>> +
>> +       *value = data & bitmask;
>> +       return 0;
>> +}
>> +
>> +static int ti_tps68470_pmic_get_clk(struct regmap *regmap, int reg,
>> +                                      int bitmask, u64 *value)
>> +{
>> +       int data;
>> +
>> +       if (regmap_read(regmap, reg, &data))
>> +               return -EIO;
>> +
>> +       *value = (data & bitmask) ? 1 : 0;
>> +       return 0;
>> +}
>> +
>> +static int ti_tps68470_pmic_get_clk_freq(struct regmap *regmap, int reg,
>> +                                      int bitmask, u64 *value)
>> +{
>> +       int data;
>> +
>> +       if (regmap_read(regmap, reg, &data))
>> +               return -EIO;
>> +
>> +       *value = data & bitmask;
>> +       return 0;
>> +}
>> +
>> +static int ti_tps68470_regmap_update_bits(struct regmap *regmap, int reg,
>> +                                       int bitmask, u64 value)
>> +{
>> +       return regmap_update_bits(regmap, reg, bitmask, value);
>> +}
>> +
>> +static acpi_status ti_pmic_common_handler(u32 function,
>> +                                         acpi_physical_address address,
>> +                                         u32 bits, u64 *value,
>> +                                         void *handler_context,
>> +                                         void *region_context,
>> +                                         int (*get)(struct regmap *,
>> +                                                    int, int, u64 *),
>> +                                         int (*update)(struct regmap *,
>> +                                                       int, int, u64),
>> +                                         struct ti_pmic_table *table,
>> +                                         int table_size)
>> +{
>> +       struct ti_pmic_opregion *opregion = region_context;
>> +       struct regmap *regmap = opregion->regmap;
>> +       int reg, ret, bitmask;
>> +
>> +       if (bits != 32)
>> +               return AE_BAD_PARAMETER;
>> +
>> +       ret = pmic_get_reg_bit(address, table,
>> +                                 table_size, &reg, &bitmask);
>> +       if (ret < 0)
>> +               return AE_BAD_PARAMETER;
>> +
>> +       if (function == ACPI_WRITE && (*value > bitmask))
>> +               return AE_BAD_PARAMETER;
>> +
>> +       mutex_lock(&opregion->lock);
>> +
>> +       ret = (function == ACPI_READ) ?
>> +               get(regmap, reg, bitmask, value) :
>> +               update(regmap, reg, bitmask, *value);
>> +
>> +       mutex_unlock(&opregion->lock);
>> +
>> +       return ret ? AE_ERROR : AE_OK;
>> +}
>> +
>> +static acpi_status ti_pmic_clk_freq_handler(u32 function,
>> +                                           acpi_physical_address address,
>> +                                           u32 bits, u64 *value,
>> +                                           void *handler_context,
>> +                                           void *region_context)
>> +{
>> +       return ti_pmic_common_handler(function, address, bits, value,
>> +                               handler_context, region_context,
>> +                               ti_tps68470_pmic_get_clk_freq,
>> +                               ti_tps68470_regmap_update_bits,
>> +                               (struct ti_pmic_table *) &clk_freq_table,
>> +                               ARRAY_SIZE(clk_freq_table));
>> +}
>> +
>> +static acpi_status ti_pmic_clk_handler(u32 function,
>> +                                      acpi_physical_address address, u32 bits,
>> +                                      u64 *value, void *handler_context,
>> +                                      void *region_context)
>> +{
>> +       return ti_pmic_common_handler(function, address, bits, value,
>> +                               handler_context, region_context,
>> +                               ti_tps68470_pmic_get_clk,
>> +                               ti_tps68470_regmap_update_bits,
>> +                               (struct ti_pmic_table *) &clk_table,
>> +                                ARRAY_SIZE(clk_table));
>> +}
>> +
>> +static acpi_status ti_pmic_vr_val_handler(u32 function,
>> +                                         acpi_physical_address address,
>> +                                         u32 bits, u64 *value,
>> +                                         void *handler_context,
>> +                                         void *region_context)
>> +{
>> +       return ti_pmic_common_handler(function, address, bits, value,
>> +                               handler_context, region_context,
>> +                               ti_tps68470_pmic_get_vr_val,
>> +                               ti_tps68470_regmap_update_bits,
>> +                               (struct ti_pmic_table *) &vr_val_table,
>> +                               ARRAY_SIZE(vr_val_table));
>> +}
>> +
>> +static acpi_status ti_pmic_power_handler(u32 function,
>> +                                        acpi_physical_address address,
>> +                                        u32 bits, u64 *value,
>> +                                        void *handler_context,
>> +                                        void *region_context)
>> +{
>> +       if (bits != 32)
>> +               return AE_BAD_PARAMETER;
>> +
>> +       /* set/clear for bit 0, bits 0 and 1 together */
>> +       if (function == ACPI_WRITE &&
>> +           !(*value == 0 || *value == 1 || *value == 3)) {
>> +               return AE_BAD_PARAMETER;
>> +       }
>> +
>> +       return ti_pmic_common_handler(function, address, bits, value,
>> +                               handler_context, region_context,
>> +                               ti_tps68470_pmic_get_power,
>> +                               ti_tps68470_regmap_update_bits,
>> +                               (struct ti_pmic_table *) &power_table,
>> +                               ARRAY_SIZE(power_table));
>> +}
>> +
>> +static int ti_tps68470_pmic_opregion_probe(struct platform_device *pdev)
>> +{
>> +       struct tps68470 *pmic = dev_get_drvdata(pdev->dev.parent);
>> +       acpi_handle handle = ACPI_HANDLE(pdev->dev.parent);
>> +       struct device *dev = &pdev->dev;
>> +       struct ti_pmic_opregion *opregion;
>> +       acpi_status status;
>> +
> 
>> +       if (!dev || !pmic->regmap) {
>> +               WARN(1, "dev or regmap is NULL\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (!handle) {
>> +               WARN(1, "acpi handle is NULL\n");
>> +               return -ENODEV;
>> +       }
> 
> I dunno if WARNs make user experience any better.
> Besides that I would double check you may have such cases.
> 
>> +
>> +       opregion = devm_kzalloc(dev, sizeof(*opregion), GFP_KERNEL);
>> +       if (!opregion)
>> +               return -ENOMEM;
>> +
>> +       mutex_init(&opregion->lock);
>> +       opregion->regmap = pmic->regmap;
>> +
>> +       status = acpi_install_address_space_handler(handle,
>> +                                                   TI_PMIC_POWER_OPREGION_ID,
>> +                                                   ti_pmic_power_handler,
>> +                                                   NULL, opregion);
>> +       if (ACPI_FAILURE(status))
>> +               return -ENODEV;
>> +
>> +       status = acpi_install_address_space_handler(handle,
>> +                                                   TI_PMIC_VR_VAL_OPREGION_ID,
>> +                                                   ti_pmic_vr_val_handler,
>> +                                                   NULL, opregion);
>> +       if (ACPI_FAILURE(status))
>> +               goto out_remove_power_handler;
>> +
>> +       status = acpi_install_address_space_handler(handle,
>> +                                                   TI_PMIC_CLOCK_OPREGION_ID,
>> +                                                   ti_pmic_clk_handler,
>> +                                                   NULL, opregion);
>> +       if (ACPI_FAILURE(status))
>> +               goto out_remove_vr_val_handler;
>> +
>> +       status = acpi_install_address_space_handler(handle,
>> +                                                   TI_PMIC_CLKFREQ_OPREGION_ID,
>> +                                                   ti_pmic_clk_freq_handler,
>> +                                                   NULL, opregion);
>> +       if (ACPI_FAILURE(status))
>> +               goto out_remove_clk_handler;
>> +
>> +       return 0;
>> +
>> +out_remove_clk_handler:
>> +       acpi_remove_address_space_handler(handle, TI_PMIC_CLOCK_OPREGION_ID,
>> +                                         ti_pmic_clk_handler);
>> +out_remove_vr_val_handler:
>> +       acpi_remove_address_space_handler(handle, TI_PMIC_VR_VAL_OPREGION_ID,
>> +                                         ti_pmic_vr_val_handler);
>> +out_remove_power_handler:
>> +       acpi_remove_address_space_handler(handle, TI_PMIC_POWER_OPREGION_ID,
>> +                                         ti_pmic_power_handler);
>> +       return -ENODEV;
>> +}
>> +
>> +static struct platform_driver ti_tps68470_pmic_opregion_driver = {
>> +       .probe = ti_tps68470_pmic_opregion_probe,
>> +       .driver = {
>> +               .name = "tps68470_pmic_opregion",
>> +       },
>> +};
>> +
>> +builtin_platform_driver(ti_tps68470_pmic_opregion_driver)
>> --
>> 1.9.1
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus June 7, 2017, 12:07 p.m. UTC | #3
Hi Rajmohan,

Thanks for removing the redundant struct definition. A couple more comments
below. Not really necessarily bugs but a few things to clean things up a
bit.

On Tue, Jun 06, 2017 at 04:55:18AM -0700, Rajmohan Mani wrote:
> The Kabylake platform coreboot (Chrome OS equivalent of
> BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
> These operation regions are to enable/disable voltage
> regulators, configure voltage regulators, enable/disable
> clocks and to configure clocks.
> 
> This config adds ACPI operation region support for TI TPS68470 PMIC.
> TPS68470 device is an advanced power management unit that powers
> a Compact Camera Module (CCM), generates clocks for image sensors,
> drives a dual LED for flash and incorporates two LED drivers for
> general purpose indicators.
> This driver enables ACPI operation region support to control voltage
> regulators and clocks for the TPS68470 PMIC.
> 
> Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
> ---
>  drivers/acpi/Kconfig              |  12 +
>  drivers/acpi/Makefile             |   2 +
>  drivers/acpi/pmic/pmic_tps68470.c | 454 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 468 insertions(+)
>  create mode 100644 drivers/acpi/pmic/pmic_tps68470.c
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 1ce52f8..218d22d 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -535,4 +535,16 @@ if ARM64
>  source "drivers/acpi/arm64/Kconfig"
>  endif
>  
> +config TPS68470_PMIC_OPREGION
> +	bool "ACPI operation region support for TPS68470 PMIC"
> +	help
> +	  This config adds ACPI operation region support for TI TPS68470 PMIC.
> +	  TPS68470 device is an advanced power management unit that powers
> +	  a Compact Camera Module (CCM), generates clocks for image sensors,
> +	  drives a dual LED for flash and incorporates two LED drivers for
> +	  general purpose indicators.
> +	  This driver enables ACPI operation region support control voltage
> +	  regulators and clocks.
> +

Extra newline.

> +
>  endif	# ACPI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index b1aacfc..7113d05 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -106,6 +106,8 @@ obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o
>  
>  obj-$(CONFIG_ACPI_CONFIGFS)	+= acpi_configfs.o
>  
> +obj-$(CONFIG_TPS68470_PMIC_OPREGION)	+= pmic/pmic_tps68470.o
> +
>  video-objs			+= acpi_video.o video_detect.o
>  obj-y				+= dptf/
>  
> diff --git a/drivers/acpi/pmic/pmic_tps68470.c b/drivers/acpi/pmic/pmic_tps68470.c
> new file mode 100644
> index 0000000..b2d608b
> --- /dev/null
> +++ b/drivers/acpi/pmic/pmic_tps68470.c
> @@ -0,0 +1,454 @@
> +/*
> + * TI TPS68470 PMIC operation region driver
> + *
> + * Copyright (C) 2017 Intel Corporation. All rights reserved.
> + * Author: Rajmohan Mani <rajmohan.mani@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Based on drivers/acpi/pmic/intel_pmic* drivers
> + *
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/mfd/tps68470.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +struct ti_pmic_table {
> +	u32 address;		/* operation region address */
> +	u32 reg;		/* corresponding register */
> +	u32 bitmask;		/* bit mask for power, clock */
> +};
> +
> +#define TI_PMIC_POWER_OPREGION_ID		0xB0
> +#define TI_PMIC_VR_VAL_OPREGION_ID		0xB1
> +#define TI_PMIC_CLOCK_OPREGION_ID		0xB2
> +#define TI_PMIC_CLKFREQ_OPREGION_ID		0xB3
> +
> +struct ti_pmic_opregion {
> +	struct mutex lock;
> +	struct regmap *regmap;
> +};
> +
> +#define S_IO_I2C_EN	(BIT(0) | BIT(1))
> +
> +static const struct ti_pmic_table power_table[] = {
> +	{
> +		.address = 0x00,
> +		.reg = TPS68470_REG_S_I2C_CTL,
> +		.bitmask = S_IO_I2C_EN,
> +		/* S_I2C_CTL */
> +	},
> +	{
> +		.address = 0x04,
> +		.reg = TPS68470_REG_VCMCTL,
> +		.bitmask = BIT(0),
> +		/* VCMCTL */
> +	},
> +	{
> +		.address = 0x08,
> +		.reg = TPS68470_REG_VAUX1CTL,
> +		.bitmask = BIT(0),
> +		/* VAUX1_CTL */
> +	},
> +	{
> +		.address = 0x0C,
> +		.reg = TPS68470_REG_VAUX2CTL,
> +		.bitmask = BIT(0),
> +		/* VAUX2CTL */
> +	},
> +	{
> +		.address = 0x10,
> +		.reg = TPS68470_REG_VACTL,
> +		.bitmask = BIT(0),
> +		/* VACTL */
> +	},
> +	{
> +		.address = 0x14,
> +		.reg = TPS68470_REG_VDCTL,
> +		.bitmask = BIT(0),
> +		/* VDCTL */
> +	},
> +};
> +
> +/* Table to set voltage regulator value */
> +static const struct ti_pmic_table vr_val_table[] = {
> +	{
> +		.address = 0x00,
> +		.reg = TPS68470_REG_VSIOVAL,
> +		.bitmask = TPS68470_VSIOVAL_IOVOLT_MASK,
> +		/* TPS68470_REG_VSIOVAL */
> +	},
> +	{
> +		.address = 0x04,
> +		.reg = TPS68470_REG_VIOVAL,
> +		.bitmask = TPS68470_VIOVAL_IOVOLT_MASK,
> +		/* TPS68470_REG_VIOVAL */
> +	},
> +	{
> +		.address = 0x08,
> +		.reg = TPS68470_REG_VCMVAL,
> +		.bitmask = TPS68470_VCMVAL_VCVOLT_MASK,
> +		/* TPS68470_REG_VCMVAL */
> +	},
> +	{
> +		.address = 0x0C,
> +		.reg = TPS68470_REG_VAUX1VAL,
> +		.bitmask = TPS68470_VAUX1VAL_AUX1VOLT_MASK,
> +		/* TPS68470_REG_VAUX1VAL */
> +	},
> +	{
> +		.address = 0x10,
> +		.reg = TPS68470_REG_VAUX2VAL,
> +		.bitmask = TPS68470_VAUX2VAL_AUX2VOLT_MASK,
> +		/* TPS68470_REG_VAUX2VAL */
> +	},
> +	{
> +		.address = 0x14,
> +		.reg = TPS68470_REG_VAVAL,
> +		.bitmask = TPS68470_VAVAL_AVOLT_MASK,
> +		/* TPS68470_REG_VAVAL */
> +	},
> +	{
> +		.address = 0x18,
> +		.reg = TPS68470_REG_VDVAL,
> +		.bitmask = TPS68470_VDVAL_DVOLT_MASK,
> +		/* TPS68470_REG_VDVAL */
> +	},
> +};
> +
> +/* Table to configure clock frequency */
> +static const struct ti_pmic_table clk_freq_table[] = {
> +	{
> +		.address = 0x00,
> +		.reg = TPS68470_REG_POSTDIV2,
> +		.bitmask = BIT(0) | BIT(1),
> +		/* TPS68470_REG_POSTDIV2 */
> +	},
> +	{
> +		.address = 0x04,
> +		.reg = TPS68470_REG_BOOSTDIV,
> +		.bitmask = 0x1F,
> +		/* TPS68470_REG_BOOSTDIV */
> +	},
> +	{
> +		.address = 0x08,
> +		.reg = TPS68470_REG_BUCKDIV,
> +		.bitmask = 0x0F,
> +		/* TPS68470_REG_BUCKDIV */
> +	},
> +	{
> +		.address = 0x0C,
> +		.reg = TPS68470_REG_PLLSWR,
> +		.bitmask = 0x13,
> +		/* TPS68470_REG_PLLSWR */
> +	},
> +	{
> +		.address = 0x10,
> +		.reg = TPS68470_REG_XTALDIV,
> +		.bitmask = 0xFF,
> +		/* TPS68470_REG_XTALDIV */
> +	},
> +	{
> +		.address = 0x14,
> +		.reg = TPS68470_REG_PLLDIV,
> +		.bitmask = 0xFF,
> +		/* TPS68470_REG_PLLDIV */
> +	},
> +	{
> +		.address = 0x18,
> +		.reg = TPS68470_REG_POSTDIV,
> +		.bitmask = 0x83,
> +		/* TPS68470_REG_POSTDIV */
> +	},
> +};
> +
> +/* Table to configure and enable clocks */
> +static const struct ti_pmic_table clk_table[] = {
> +	{
> +		.address = 0x00,
> +		.reg = TPS68470_REG_PLLCTL,
> +		.bitmask = 0xF5,
> +		/* TPS68470_REG_PLLCTL */
> +	},
> +	{
> +		.address = 0x04,
> +		.reg = TPS68470_REG_PLLCTL2,
> +		.bitmask = BIT(0),
> +		/* TPS68470_REG_PLLCTL2 */
> +	},
> +	{
> +		.address = 0x08,
> +		.reg = TPS68470_REG_CLKCFG1,
> +		.bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
> +			TPS68470_CLKCFG1_MODE_B_MASK,
> +		/* TPS68470_REG_CLKCFG1 */
> +	},
> +	{
> +		.address = 0x0C,
> +		.reg = TPS68470_REG_CLKCFG2,
> +		.bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
> +			TPS68470_CLKCFG1_MODE_B_MASK,
> +		/* TPS68470_REG_CLKCFG2 */
> +	},
> +};
> +
> +static int pmic_get_reg_bit(u64 address, struct ti_pmic_table *table,
> +			    int count, int *reg, int *bitmask)

unsigned int count?

> +{
> +	u64 i;
> +
> +	i = address / 4;
> +
> +	if (i >= count)
> +		return -ENOENT;
> +
> +	if (!reg || !bitmask)
> +		return -EINVAL;
> +
> +	*reg = table[i].reg;
> +	*bitmask = table[i].bitmask;
> +
> +	return 0;
> +}
> +
> +static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg,
> +				       int bitmask, u64 *value)
> +{
> +	int data;

Shouldn't you use unsigned int here? Same in the functions below.

> +
> +	if (regmap_read(regmap, reg, &data))
> +		return -EIO;
> +
> +	*value = (data & bitmask) ? 1 : 0;
> +	return 0;
> +}
> +
> +static int ti_tps68470_pmic_get_vr_val(struct regmap *regmap, int reg,
> +				       int bitmask, u64 *value)
> +{
> +	int data;
> +
> +	if (regmap_read(regmap, reg, &data))
> +		return -EIO;
> +
> +	*value = data & bitmask;
> +	return 0;
> +}
> +
> +static int ti_tps68470_pmic_get_clk(struct regmap *regmap, int reg,
> +				       int bitmask, u64 *value)
> +{
> +	int data;
> +
> +	if (regmap_read(regmap, reg, &data))
> +		return -EIO;
> +
> +	*value = (data & bitmask) ? 1 : 0;
> +	return 0;
> +}
> +
> +static int ti_tps68470_pmic_get_clk_freq(struct regmap *regmap, int reg,
> +				       int bitmask, u64 *value)
> +{
> +	int data;
> +
> +	if (regmap_read(regmap, reg, &data))
> +		return -EIO;
> +
> +	*value = data & bitmask;
> +	return 0;
> +}
> +
> +static int ti_tps68470_regmap_update_bits(struct regmap *regmap, int reg,
> +					int bitmask, u64 value)
> +{
> +	return regmap_update_bits(regmap, reg, bitmask, value);
> +}
> +
> +static acpi_status ti_pmic_common_handler(u32 function,
> +					  acpi_physical_address address,
> +					  u32 bits, u64 *value,
> +					  void *handler_context,

handler_context is unused.

> +					  void *region_context,
> +					  int (*get)(struct regmap *,
> +						     int, int, u64 *),
> +					  int (*update)(struct regmap *,
> +							int, int, u64),
> +					  struct ti_pmic_table *table,
> +					  int table_size)

unsigned int. (Or size_t or whatever.)

> +{
> +	struct ti_pmic_opregion *opregion = region_context;
> +	struct regmap *regmap = opregion->regmap;
> +	int reg, ret, bitmask;
> +
> +	if (bits != 32)
> +		return AE_BAD_PARAMETER;
> +
> +	ret = pmic_get_reg_bit(address, table,
> +				  table_size, &reg, &bitmask);
> +	if (ret < 0)
> +		return AE_BAD_PARAMETER;
> +
> +	if (function == ACPI_WRITE && (*value > bitmask))

Extra parentheses.

> +		return AE_BAD_PARAMETER;
> +
> +	mutex_lock(&opregion->lock);
> +
> +	ret = (function == ACPI_READ) ?
> +		get(regmap, reg, bitmask, value) :
> +		update(regmap, reg, bitmask, *value);
> +
> +	mutex_unlock(&opregion->lock);
> +
> +	return ret ? AE_ERROR : AE_OK;
> +}
> +
> +static acpi_status ti_pmic_clk_freq_handler(u32 function,
> +					    acpi_physical_address address,
> +					    u32 bits, u64 *value,
> +					    void *handler_context,
> +					    void *region_context)
> +{
> +	return ti_pmic_common_handler(function, address, bits, value,
> +				handler_context, region_context,
> +				ti_tps68470_pmic_get_clk_freq,
> +				ti_tps68470_regmap_update_bits,
> +				(struct ti_pmic_table *) &clk_freq_table,

You shouldn't use an explicit cast here. Instead make the function argument
const as well and you're fine.

> +				ARRAY_SIZE(clk_freq_table));
> +}
> +
> +static acpi_status ti_pmic_clk_handler(u32 function,
> +				       acpi_physical_address address, u32 bits,
> +				       u64 *value, void *handler_context,
> +				       void *region_context)
> +{
> +	return ti_pmic_common_handler(function, address, bits, value,
> +				handler_context, region_context,
> +				ti_tps68470_pmic_get_clk,
> +				ti_tps68470_regmap_update_bits,
> +				(struct ti_pmic_table *) &clk_table,
> +				 ARRAY_SIZE(clk_table));
> +}
> +
> +static acpi_status ti_pmic_vr_val_handler(u32 function,
> +					  acpi_physical_address address,
> +					  u32 bits, u64 *value,
> +					  void *handler_context,
> +					  void *region_context)
> +{
> +	return ti_pmic_common_handler(function, address, bits, value,
> +				handler_context, region_context,
> +				ti_tps68470_pmic_get_vr_val,
> +				ti_tps68470_regmap_update_bits,
> +				(struct ti_pmic_table *) &vr_val_table,
> +				ARRAY_SIZE(vr_val_table));
> +}
> +
> +static acpi_status ti_pmic_power_handler(u32 function,
> +					 acpi_physical_address address,
> +					 u32 bits, u64 *value,
> +					 void *handler_context,
> +					 void *region_context)
> +{
> +	if (bits != 32)
> +		return AE_BAD_PARAMETER;
> +
> +	/* set/clear for bit 0, bits 0 and 1 together */
> +	if (function == ACPI_WRITE &&
> +	    !(*value == 0 || *value == 1 || *value == 3)) {
> +		return AE_BAD_PARAMETER;
> +	}
> +
> +	return ti_pmic_common_handler(function, address, bits, value,
> +				handler_context, region_context,
> +				ti_tps68470_pmic_get_power,
> +				ti_tps68470_regmap_update_bits,
> +				(struct ti_pmic_table *) &power_table,
> +				ARRAY_SIZE(power_table));
> +}
> +
> +static int ti_tps68470_pmic_opregion_probe(struct platform_device *pdev)
> +{
> +	struct tps68470 *pmic = dev_get_drvdata(pdev->dev.parent);
> +	acpi_handle handle = ACPI_HANDLE(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct ti_pmic_opregion *opregion;
> +	acpi_status status;
> +
> +	if (!dev || !pmic->regmap) {
> +		WARN(1, "dev or regmap is NULL\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!handle) {
> +		WARN(1, "acpi handle is NULL\n");
> +		return -ENODEV;
> +	}
> +
> +	opregion = devm_kzalloc(dev, sizeof(*opregion), GFP_KERNEL);
> +	if (!opregion)
> +		return -ENOMEM;
> +
> +	mutex_init(&opregion->lock);
> +	opregion->regmap = pmic->regmap;
> +
> +	status = acpi_install_address_space_handler(handle,
> +						    TI_PMIC_POWER_OPREGION_ID,
> +						    ti_pmic_power_handler,
> +						    NULL, opregion);
> +	if (ACPI_FAILURE(status))

mutex_destroy() after mutex_init() --- please add a label for this.

> +		return -ENODEV;
> +
> +	status = acpi_install_address_space_handler(handle,
> +						    TI_PMIC_VR_VAL_OPREGION_ID,
> +						    ti_pmic_vr_val_handler,
> +						    NULL, opregion);
> +	if (ACPI_FAILURE(status))
> +		goto out_remove_power_handler;
> +
> +	status = acpi_install_address_space_handler(handle,
> +						    TI_PMIC_CLOCK_OPREGION_ID,
> +						    ti_pmic_clk_handler,
> +						    NULL, opregion);
> +	if (ACPI_FAILURE(status))
> +		goto out_remove_vr_val_handler;
> +
> +	status = acpi_install_address_space_handler(handle,
> +						    TI_PMIC_CLKFREQ_OPREGION_ID,
> +						    ti_pmic_clk_freq_handler,
> +						    NULL, opregion);
> +	if (ACPI_FAILURE(status))
> +		goto out_remove_clk_handler;
> +
> +	return 0;
> +
> +out_remove_clk_handler:
> +	acpi_remove_address_space_handler(handle, TI_PMIC_CLOCK_OPREGION_ID,
> +					  ti_pmic_clk_handler);
> +out_remove_vr_val_handler:
> +	acpi_remove_address_space_handler(handle, TI_PMIC_VR_VAL_OPREGION_ID,
> +					  ti_pmic_vr_val_handler);
> +out_remove_power_handler:
> +	acpi_remove_address_space_handler(handle, TI_PMIC_POWER_OPREGION_ID,
> +					  ti_pmic_power_handler);
> +	return -ENODEV;
> +}
> +
> +static struct platform_driver ti_tps68470_pmic_opregion_driver = {
> +	.probe = ti_tps68470_pmic_opregion_probe,
> +	.driver = {
> +		.name = "tps68470_pmic_opregion",
> +	},
> +};
> +
> +builtin_platform_driver(ti_tps68470_pmic_opregion_driver)
Sakari Ailus June 7, 2017, 12:15 p.m. UTC | #4
On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote:
> Follow the pattern, please, I suppose
> ti_pmic_tps68470.c

This pattern is weird. "ti" in front of the file name is redundant, and in
very few places the vendor prefix is used anyway. Especially when the chip
has a proper name --- as this one does.

I assume for the Intel PMICs it could be there for a couple of reasons which
are

1) lack of a clearly unique chip ID and

2) the use of common frameworklet for Intel PMICs.

There are also no other PMIC chips supported currently.

The pmic_tps68470 naming is in line with the GPIO driver (apart from the
dash / underscore difference).
Andy Shevchenko June 7, 2017, 1:37 p.m. UTC | #5
On Wed, Jun 7, 2017 at 3:07 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:

>> +static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg,
>> +                                    int bitmask, u64 *value)
>> +{
>> +     int data;
>
> Shouldn't you use unsigned int here? Same in the functions below.

+1, regmap_read() returns unsigned int.

>> +static acpi_status ti_pmic_common_handler(u32 function,
> +                                       acpi_physical_address address,
> +                                       u32 bits, u64 *value,
> +                                       void *handler_context,

> handler_context is unused.

>> +                                                  int, int, u64 *),
>> +                                       int (*update)(struct regmap *,
>> +                                                     int, int, u64),
>> +                                       struct ti_pmic_table *table,
>> +                                       int table_size)

I would even split this to have separate update() and get() paths
instead of having such a monster of parameters.

>> +static acpi_status ti_pmic_clk_freq_handler(u32 function,
>> +                                         acpi_physical_address address,
>> +                                         u32 bits, u64 *value,
>> +                                         void *handler_context,
>> +                                         void *region_context)
>> +{
>> +     return ti_pmic_common_handler(function, address, bits, value,
>> +                             handler_context, region_context,
>> +                             ti_tps68470_pmic_get_clk_freq,
>> +                             ti_tps68470_regmap_update_bits,
>> +                             (struct ti_pmic_table *) &clk_freq_table,
>
> You shouldn't use an explicit cast here. Instead make the function argument
> const as well and you're fine.

+1.
Andy Shevchenko June 7, 2017, 1:40 p.m. UTC | #6
On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote:
>> Follow the pattern, please, I suppose
>> ti_pmic_tps68470.c
>
> This pattern is weird. "ti" in front of the file name is redundant, and in
> very few places the vendor prefix is used anyway. Especially when the chip
> has a proper name --- as this one does.
>
> I assume for the Intel PMICs it could be there for a couple of reasons which
> are
>
> 1) lack of a clearly unique chip ID and
>
> 2) the use of common frameworklet for Intel PMICs.
>
> There are also no other PMIC chips supported currently.
>
> The pmic_tps68470 naming is in line with the GPIO driver (apart from the
> dash / underscore difference).

Since

% git ls-files *pmic*

returns somewhat interesting results, I would even go further and use

tps68470.c here

and

s/ti_pmic/tps6840/g

inside the file.

Would it work for you?
Sakari Ailus June 7, 2017, 8:06 p.m. UTC | #7
On Wed, Jun 07, 2017 at 04:37:12PM +0300, Andy Shevchenko wrote:
> >> +static acpi_status ti_pmic_common_handler(u32 function,
> > +                                       acpi_physical_address address,
> > +                                       u32 bits, u64 *value,
> > +                                       void *handler_context,
> 
> > handler_context is unused.
> 
> >> +                                                  int, int, u64 *),
> >> +                                       int (*update)(struct regmap *,
> >> +                                                     int, int, u64),
> >> +                                       struct ti_pmic_table *table,
> >> +                                       int table_size)
> 
> I would even split this to have separate update() and get() paths
> instead of having such a monster of parameters.

I'm not really worried about the two callbacks --- you have the compexity,
which is agruably rather manageable, split into a number of caller
functions. I'd rather keep it as-is.
Sakari Ailus June 7, 2017, 8:10 p.m. UTC | #8
Hi Andy,

On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote:
> >> Follow the pattern, please, I suppose
> >> ti_pmic_tps68470.c
> >
> > This pattern is weird. "ti" in front of the file name is redundant, and in
> > very few places the vendor prefix is used anyway. Especially when the chip
> > has a proper name --- as this one does.
> >
> > I assume for the Intel PMICs it could be there for a couple of reasons which
> > are
> >
> > 1) lack of a clearly unique chip ID and
> >
> > 2) the use of common frameworklet for Intel PMICs.
> >
> > There are also no other PMIC chips supported currently.
> >
> > The pmic_tps68470 naming is in line with the GPIO driver (apart from the
> > dash / underscore difference).
> 
> Since
> 
> % git ls-files *pmic*
> 
> returns somewhat interesting results, I would even go further and use
> 
> tps68470.c here
> 
> and
> 
> s/ti_pmic/tps6840/g
> 
> inside the file.
> 
> Would it work for you?

This is still a different driver from the tps68470 driver which is an MFD
driver. For clarity, I'd keep pmic as part of the name (and I'd use
tps68470_pmic_ prefix for internal symbols, too).

As PMICs are typically linked to the kernel (vs. being modules), there's no
issue with the module name. I would suppose few if any PMICs will be
compiled as modules in general.

It's not a big deal though. I'm fine either way.
Andy Shevchenko June 7, 2017, 8:40 p.m. UTC | #9
On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote:
>> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>> > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote:

>> >> Follow the pattern, please, I suppose
>> >> ti_pmic_tps68470.c
>>
>> Would it work for you?
>
> This is still a different driver from the tps68470 driver which is an MFD
> driver. For clarity, I'd keep pmic as part of the name (and I'd use
> tps68470_pmic_ prefix for internal symbols, too).
>
> As PMICs are typically linked to the kernel (vs. being modules), there's no
> issue with the module name. I would suppose few if any PMICs will be
> compiled as modules in general.
>
> It's not a big deal though. I'm fine either way.

Okay, let's agree on  tps68470_pmic for internal prefix and for file name?
Sakari Ailus June 7, 2017, 9:12 p.m. UTC | #10
On Wed, Jun 07, 2017 at 11:40:27PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote:
> >> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> >> > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote:
> 
> >> >> Follow the pattern, please, I suppose
> >> >> ti_pmic_tps68470.c
> >>
> >> Would it work for you?
> >
> > This is still a different driver from the tps68470 driver which is an MFD
> > driver. For clarity, I'd keep pmic as part of the name (and I'd use
> > tps68470_pmic_ prefix for internal symbols, too).
> >
> > As PMICs are typically linked to the kernel (vs. being modules), there's no
> > issue with the module name. I would suppose few if any PMICs will be
> > compiled as modules in general.
> >
> > It's not a big deal though. I'm fine either way.
> 
> Okay, let's agree on  tps68470_pmic for internal prefix and for file name?

Ack. Thanks!
Hans de Goede June 8, 2017, 7:03 a.m. UTC | #11
Hi,

On 07-06-17 22:10, Sakari Ailus wrote:
> Hi Andy,
> 
> On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote:
>> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>>> On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote:
>>>> Follow the pattern, please, I suppose
>>>> ti_pmic_tps68470.c
>>>
>>> This pattern is weird. "ti" in front of the file name is redundant, and in
>>> very few places the vendor prefix is used anyway. Especially when the chip
>>> has a proper name --- as this one does.
>>>
>>> I assume for the Intel PMICs it could be there for a couple of reasons which
>>> are
>>>
>>> 1) lack of a clearly unique chip ID and
>>>
>>> 2) the use of common frameworklet for Intel PMICs.
>>>
>>> There are also no other PMIC chips supported currently.
>>>
>>> The pmic_tps68470 naming is in line with the GPIO driver (apart from the
>>> dash / underscore difference).
>>
>> Since
>>
>> % git ls-files *pmic*
>>
>> returns somewhat interesting results, I would even go further and use
>>
>> tps68470.c here
>>
>> and
>>
>> s/ti_pmic/tps6840/g
>>
>> inside the file.
>>
>> Would it work for you?
> 
> This is still a different driver from the tps68470 driver which is an MFD
> driver. For clarity, I'd keep pmic as part of the name (and I'd use
> tps68470_pmic_ prefix for internal symbols, too).
> 
> As PMICs are typically linked to the kernel (vs. being modules), there's no
> issue with the module name. I would suppose few if any PMICs will be
> compiled as modules in general.

Good point about the OpRegion driver usually being built-in, in my experience
it MUST always be built-in, so the Kconfig option should be a bool. Note this
is useless unless the mfd driver is also a bool (I would advice to go that
route) and the mfd driver's Kconfig should select the right i2c bus driver
to make sure that is built-in too, see for example:

https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=2f91ded5f8f4fdd67d8daae514b0d434c98ab1e0
https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=c5065d8625ebdc164199b99d838ac0636faa7f0b
https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=5f125f1f570568a29edf783fba1ebb606d5c6b24

Which are all recent commits from me dealing with making the mfd driver
built-in / selecting the i2c bus driver.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mani, Rajmohan June 9, 2017, 10:19 p.m. UTC | #12
SGkgQW5keSwNCg0KVGhhbmtzIGZvciB0aGUgcmV2aWV3cyBhbmQgcGF0aWVuY2UuDQoNCj4gLS0t
LS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQW5keSBTaGV2Y2hlbmtvIFttYWlsdG86
YW5keS5zaGV2Y2hlbmtvQGdtYWlsLmNvbV0NCj4gU2VudDogVHVlc2RheSwgSnVuZSAwNiwgMjAx
NyA3OjI0IEFNDQo+IFRvOiBNYW5pLCBSYWptb2hhbiA8cmFqbW9oYW4ubWFuaUBpbnRlbC5jb20+
OyBIYW5zIGRlIEdvZWRlDQo+IDxoZGVnb2VkZUByZWRoYXQuY29tPg0KPiBDYzogbGludXgta2Vy
bmVsQHZnZXIua2VybmVsLm9yZzsgbGludXgtZ3Bpb0B2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LQ0K
PiBhY3BpQHZnZXIua2VybmVsLm9yZzsgTGVlIEpvbmVzIDxsZWUuam9uZXNAbGluYXJvLm9yZz47
IExpbnVzIFdhbGxlaWoNCj4gPGxpbnVzLndhbGxlaWpAbGluYXJvLm9yZz47IEFsZXhhbmRyZSBD
b3VyYm90IDxnbnVyb3VAZ21haWwuY29tPjsgUmFmYWVsIEouDQo+IFd5c29ja2kgPHJqd0Byand5
c29ja2kubmV0PjsgTGVuIEJyb3duIDxsZW5iQGtlcm5lbC5vcmc+DQo+IFN1YmplY3Q6IFJlOiBb
UEFUQ0ggdjEgMy8zXSBBQ1BJIC8gUE1JQzogQWRkIFRJIFBNSUMgVFBTNjg0NzAgb3BlcmF0aW9u
DQo+IHJlZ2lvbiBkcml2ZXINCj4gDQo+ICtDYyBIYW5zICh0aGF0J3Mgd2h5IGRpZG4ndCBkZWxl
dGUgYW55dGhpbmcgZnJvbSBvcmlnaW5hbCBtYWlsLCBqdXN0DQo+IGFkZGluZyBteSBjb21tZW50
cykuDQo+IA0KPiBIYW5zLCBpZiB5b3UgaGF2ZSBmZXcgbWludXRlcyBpdCB3b3VsZCBiZSBhcHBy
ZWNpYXRlZCB0byBnbGFuY2Ugb24gdGhlIGJlbG93DQo+IGZvciBzb21lIGlzc3VlcyBpZiBhbnkg
c2luY2UgeW91IGRpZCBwYXNzIHF1aXRlIGEgZ29vZCBxdWVzdCB3aXRoIG90aGVyIFBNSUMNCj4g
ZHJpdmVycy4NCj4gDQo+IE9uIFR1ZSwgSnVuIDYsIDIwMTcgYXQgMjo1NSBQTSwgUmFqbW9oYW4g
TWFuaSA8cmFqbW9oYW4ubWFuaUBpbnRlbC5jb20+DQo+IHdyb3RlOg0KPiA+IFRoZSBLYWJ5bGFr
ZSBwbGF0Zm9ybSBjb3JlYm9vdCAoQ2hyb21lIE9TIGVxdWl2YWxlbnQgb2YNCj4gPiBCSU9TKSBo
YXMgZGVmaW5lZCA0IG9wZXJhdGlvbiByZWdpb25zIGZvciB0aGUgVEkgVFBTNjg0NzAgUE1JQy4N
Cj4gPiBUaGVzZSBvcGVyYXRpb24gcmVnaW9ucyBhcmUgdG8gZW5hYmxlL2Rpc2FibGUgdm9sdGFn
ZSByZWd1bGF0b3JzLA0KPiA+IGNvbmZpZ3VyZSB2b2x0YWdlIHJlZ3VsYXRvcnMsIGVuYWJsZS9k
aXNhYmxlIGNsb2NrcyBhbmQgdG8gY29uZmlndXJlDQo+ID4gY2xvY2tzLg0KPiA+DQo+ID4gVGhp
cyBjb25maWcgYWRkcyBBQ1BJIG9wZXJhdGlvbiByZWdpb24gc3VwcG9ydCBmb3IgVEkgVFBTNjg0
NzAgUE1JQy4NCj4gPiBUUFM2ODQ3MCBkZXZpY2UgaXMgYW4gYWR2YW5jZWQgcG93ZXIgbWFuYWdl
bWVudCB1bml0IHRoYXQgcG93ZXJzIGENCj4gPiBDb21wYWN0IENhbWVyYSBNb2R1bGUgKENDTSks
IGdlbmVyYXRlcyBjbG9ja3MgZm9yIGltYWdlIHNlbnNvcnMsDQo+ID4gZHJpdmVzIGEgZHVhbCBM
RUQgZm9yIGZsYXNoIGFuZCBpbmNvcnBvcmF0ZXMgdHdvIExFRCBkcml2ZXJzIGZvcg0KPiA+IGdl
bmVyYWwgcHVycG9zZSBpbmRpY2F0b3JzLg0KPiA+IFRoaXMgZHJpdmVyIGVuYWJsZXMgQUNQSSBv
cGVyYXRpb24gcmVnaW9uIHN1cHBvcnQgdG8gY29udHJvbCB2b2x0YWdlDQo+ID4gcmVndWxhdG9y
cyBhbmQgY2xvY2tzIGZvciB0aGUgVFBTNjg0NzAgUE1JQy4NCj4gPg0KPiA+IFNpZ25lZC1vZmYt
Ynk6IFJham1vaGFuIE1hbmkgPHJham1vaGFuLm1hbmlAaW50ZWwuY29tPg0KPiA+IC0tLQ0KPiA+
ICBkcml2ZXJzL2FjcGkvS2NvbmZpZyAgICAgICAgICAgICAgfCAgMTIgKw0KPiA+ICBkcml2ZXJz
L2FjcGkvTWFrZWZpbGUgICAgICAgICAgICAgfCAgIDIgKw0KPiANCj4gPiAgZHJpdmVycy9hY3Bp
L3BtaWMvcG1pY190cHM2ODQ3MC5jIHwgNDU0DQo+ID4gKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysNCj4gDQo+IEZvbGxvdyB0aGUgcGF0dGVybiwgcGxlYXNlLCBJIHN1cHBv
c2UNCj4gdGlfcG1pY190cHM2ODQ3MC5jDQo+IA0KDQpJIHdpbGwgcmVwbHkgdG8gdGhpcywgb24g
dGhlIGxhdGVyIHRocmVhZHMgb24gdGhlIHNhbWUgc3ViamVjdA0KDQo+ID4gIDMgZmlsZXMgY2hh
bmdlZCwgNDY4IGluc2VydGlvbnMoKykNCj4gPiAgY3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMv
YWNwaS9wbWljL3BtaWNfdHBzNjg0NzAuYw0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv
YWNwaS9LY29uZmlnIGIvZHJpdmVycy9hY3BpL0tjb25maWcgaW5kZXgNCj4gPiAxY2U1MmY4Li4y
MThkMjJkIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvYWNwaS9LY29uZmlnDQo+ID4gKysrIGIv
ZHJpdmVycy9hY3BpL0tjb25maWcNCj4gPiBAQCAtNTM1LDQgKzUzNSwxNiBAQCBpZiBBUk02NA0K
PiA+ICBzb3VyY2UgImRyaXZlcnMvYWNwaS9hcm02NC9LY29uZmlnIg0KPiA+ICBlbmRpZg0KPiA+
DQo+ID4gK2NvbmZpZyBUUFM2ODQ3MF9QTUlDX09QUkVHSU9ODQo+ID4gKyAgICAgICBib29sICJB
Q1BJIG9wZXJhdGlvbiByZWdpb24gc3VwcG9ydCBmb3IgVFBTNjg0NzAgUE1JQyINCj4gPiArICAg
ICAgIGhlbHANCj4gPiArICAgICAgICAgVGhpcyBjb25maWcgYWRkcyBBQ1BJIG9wZXJhdGlvbiBy
ZWdpb24gc3VwcG9ydCBmb3IgVEkgVFBTNjg0NzAgUE1JQy4NCj4gPiArICAgICAgICAgVFBTNjg0
NzAgZGV2aWNlIGlzIGFuIGFkdmFuY2VkIHBvd2VyIG1hbmFnZW1lbnQgdW5pdCB0aGF0IHBvd2Vy
cw0KPiA+ICsgICAgICAgICBhIENvbXBhY3QgQ2FtZXJhIE1vZHVsZSAoQ0NNKSwgZ2VuZXJhdGVz
IGNsb2NrcyBmb3IgaW1hZ2Ugc2Vuc29ycywNCj4gPiArICAgICAgICAgZHJpdmVzIGEgZHVhbCBM
RUQgZm9yIGZsYXNoIGFuZCBpbmNvcnBvcmF0ZXMgdHdvIExFRCBkcml2ZXJzIGZvcg0KPiA+ICsg
ICAgICAgICBnZW5lcmFsIHB1cnBvc2UgaW5kaWNhdG9ycy4NCj4gPiArICAgICAgICAgVGhpcyBk
cml2ZXIgZW5hYmxlcyBBQ1BJIG9wZXJhdGlvbiByZWdpb24gc3VwcG9ydCBjb250cm9sIHZvbHRh
Z2UNCj4gPiArICAgICAgICAgcmVndWxhdG9ycyBhbmQgY2xvY2tzLg0KPiA+ICsNCj4gDQo+ID4g
Kw0KPiANCj4gRXh0cmEgbGluZSwgcmVtb3ZlLg0KPiANCg0KQWNrDQoNCj4gPiAgZW5kaWYgICMg
QUNQSQ0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2FjcGkvTWFrZWZpbGUgYi9kcml2ZXJzL2Fj
cGkvTWFrZWZpbGUgaW5kZXgNCj4gPiBiMWFhY2ZjLi43MTEzZDA1IDEwMDY0NA0KPiA+IC0tLSBh
L2RyaXZlcnMvYWNwaS9NYWtlZmlsZQ0KPiA+ICsrKyBiL2RyaXZlcnMvYWNwaS9NYWtlZmlsZQ0K
PiA+IEBAIC0xMDYsNiArMTA2LDggQEAgb2JqLSQoQ09ORklHX0NIVF9XQ19QTUlDX09QUkVHSU9O
KSArPQ0KPiA+IHBtaWMvaW50ZWxfcG1pY19jaHR3Yy5vDQo+ID4NCj4gPiAgb2JqLSQoQ09ORklH
X0FDUElfQ09ORklHRlMpICAgICs9IGFjcGlfY29uZmlnZnMubw0KPiA+DQo+ID4gK29iai0kKENP
TkZJR19UUFM2ODQ3MF9QTUlDX09QUkVHSU9OKSAgICs9IHBtaWMvcG1pY190cHM2ODQ3MC5vDQo+
ID4gKw0KPiA+ICB2aWRlby1vYmpzICAgICAgICAgICAgICAgICAgICAgKz0gYWNwaV92aWRlby5v
IHZpZGVvX2RldGVjdC5vDQo+ID4gIG9iai15ICAgICAgICAgICAgICAgICAgICAgICAgICArPSBk
cHRmLw0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvYWNwaS9wbWljL3BtaWNfdHBzNjg0
NzAuYw0KPiA+IGIvZHJpdmVycy9hY3BpL3BtaWMvcG1pY190cHM2ODQ3MC5jDQo+ID4gbmV3IGZp
bGUgbW9kZSAxMDA2NDQNCj4gPiBpbmRleCAwMDAwMDAwLi5iMmQ2MDhiDQo+ID4gLS0tIC9kZXYv
bnVsbA0KPiA+ICsrKyBiL2RyaXZlcnMvYWNwaS9wbWljL3BtaWNfdHBzNjg0NzAuYw0KPiA+IEBA
IC0wLDAgKzEsNDU0IEBADQo+ID4gKy8qDQo+ID4gKyAqIFRJIFRQUzY4NDcwIFBNSUMgb3BlcmF0
aW9uIHJlZ2lvbiBkcml2ZXINCj4gPiArICoNCj4gPiArICogQ29weXJpZ2h0IChDKSAyMDE3IElu
dGVsIENvcnBvcmF0aW9uLiBBbGwgcmlnaHRzIHJlc2VydmVkLg0KPiA+ICsgKiBBdXRob3I6IFJh
am1vaGFuIE1hbmkgPHJham1vaGFuLm1hbmlAaW50ZWwuY29tPg0KPiA+ICsgKg0KPiA+ICsgKiBU
aGlzIHByb2dyYW0gaXMgZnJlZSBzb2Z0d2FyZTsgeW91IGNhbiByZWRpc3RyaWJ1dGUgaXQgYW5k
L29yDQo+ID4gKyAqIG1vZGlmeSBpdCB1bmRlciB0aGUgdGVybXMgb2YgdGhlIEdOVSBHZW5lcmFs
IFB1YmxpYyBMaWNlbnNlDQo+ID4gK3ZlcnNpb24NCj4gPiArICogMiBhcyBwdWJsaXNoZWQgYnkg
dGhlIEZyZWUgU29mdHdhcmUgRm91bmRhdGlvbi4NCj4gPiArICoNCj4gPiArICogVGhpcyBwcm9n
cmFtIGlzIGRpc3RyaWJ1dGVkIGluIHRoZSBob3BlIHRoYXQgaXQgd2lsbCBiZSB1c2VmdWwsDQo+
ID4gKyAqIGJ1dCBXSVRIT1VUIEFOWSBXQVJSQU5UWTsgd2l0aG91dCBldmVuIHRoZSBpbXBsaWVk
IHdhcnJhbnR5IG9mDQo+ID4gKyAqIE1FUkNIQU5UQUJJTElUWSBvciBGSVRORVNTIEZPUiBBIFBB
UlRJQ1VMQVIgUFVSUE9TRS4gIFNlZSB0aGUNCj4gPiArICogR05VIEdlbmVyYWwgUHVibGljIExp
Y2Vuc2UgZm9yIG1vcmUgZGV0YWlscy4NCj4gPiArICoNCj4gPiArICogQmFzZWQgb24gZHJpdmVy
cy9hY3BpL3BtaWMvaW50ZWxfcG1pYyogZHJpdmVycw0KPiA+ICsgKg0KPiA+ICsgKi8NCj4gPiAr
DQo+ID4gKyNpbmNsdWRlIDxsaW51eC9hY3BpLmg+DQo+ID4gKyNpbmNsdWRlIDxsaW51eC9tZmQv
dHBzNjg0NzAuaD4NCj4gPiArI2luY2x1ZGUgPGxpbnV4L2luaXQuaD4NCj4gPiArI2luY2x1ZGUg
PGxpbnV4L3BsYXRmb3JtX2RldmljZS5oPg0KPiA+ICsjaW5jbHVkZSA8bGludXgvcmVnbWFwLmg+
DQo+ID4gKw0KPiA+ICtzdHJ1Y3QgdGlfcG1pY190YWJsZSB7DQo+ID4gKyAgICAgICB1MzIgYWRk
cmVzczsgICAgICAgICAgICAvKiBvcGVyYXRpb24gcmVnaW9uIGFkZHJlc3MgKi8NCj4gPiArICAg
ICAgIHUzMiByZWc7ICAgICAgICAgICAgICAgIC8qIGNvcnJlc3BvbmRpbmcgcmVnaXN0ZXIgKi8N
Cj4gPiArICAgICAgIHUzMiBiaXRtYXNrOyAgICAgICAgICAgIC8qIGJpdCBtYXNrIGZvciBwb3dl
ciwgY2xvY2sgKi8NCj4gPiArfTsNCj4gPiArDQo+ID4gKyNkZWZpbmUgVElfUE1JQ19QT1dFUl9P
UFJFR0lPTl9JRCAgICAgICAgICAgICAgMHhCMA0KPiA+ICsjZGVmaW5lIFRJX1BNSUNfVlJfVkFM
X09QUkVHSU9OX0lEICAgICAgICAgICAgIDB4QjENCj4gPiArI2RlZmluZSBUSV9QTUlDX0NMT0NL
X09QUkVHSU9OX0lEICAgICAgICAgICAgICAweEIyDQo+ID4gKyNkZWZpbmUgVElfUE1JQ19DTEtG
UkVRX09QUkVHSU9OX0lEICAgICAgICAgICAgMHhCMw0KPiA+ICsNCj4gPiArc3RydWN0IHRpX3Bt
aWNfb3ByZWdpb24gew0KPiA+ICsgICAgICAgc3RydWN0IG11dGV4IGxvY2s7DQo+ID4gKyAgICAg
ICBzdHJ1Y3QgcmVnbWFwICpyZWdtYXA7DQo+ID4gK307DQo+ID4gKw0KPiA+ICsjZGVmaW5lIFNf
SU9fSTJDX0VOICAgIChCSVQoMCkgfCBCSVQoMSkpDQo+ID4gKw0KPiA+ICtzdGF0aWMgY29uc3Qg
c3RydWN0IHRpX3BtaWNfdGFibGUgcG93ZXJfdGFibGVbXSA9IHsNCj4gPiArICAgICAgIHsNCj4g
PiArICAgICAgICAgICAgICAgLmFkZHJlc3MgPSAweDAwLA0KPiA+ICsgICAgICAgICAgICAgICAu
cmVnID0gVFBTNjg0NzBfUkVHX1NfSTJDX0NUTCwNCj4gPiArICAgICAgICAgICAgICAgLmJpdG1h
c2sgPSBTX0lPX0kyQ19FTiwNCj4gPiArICAgICAgICAgICAgICAgLyogU19JMkNfQ1RMICovDQo+
ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAgICAuYWRkcmVz
cyA9IDB4MDQsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9SRUdfVkNNQ1RM
LA0KPiA+ICsgICAgICAgICAgICAgICAuYml0bWFzayA9IEJJVCgwKSwNCj4gPiArICAgICAgICAg
ICAgICAgLyogVkNNQ1RMICovDQo+ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsg
ICAgICAgICAgICAgICAuYWRkcmVzcyA9IDB4MDgsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcg
PSBUUFM2ODQ3MF9SRUdfVkFVWDFDVEwsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0g
QklUKDApLA0KPiA+ICsgICAgICAgICAgICAgICAvKiBWQVVYMV9DVEwgKi8NCj4gPiArICAgICAg
IH0sDQo+ID4gKyAgICAgICB7DQo+ID4gKyAgICAgICAgICAgICAgIC5hZGRyZXNzID0gMHgwQywN
Cj4gPiArICAgICAgICAgICAgICAgLnJlZyA9IFRQUzY4NDcwX1JFR19WQVVYMkNUTCwNCj4gPiAr
ICAgICAgICAgICAgICAgLmJpdG1hc2sgPSBCSVQoMCksDQo+ID4gKyAgICAgICAgICAgICAgIC8q
IFZBVVgyQ1RMICovDQo+ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAg
ICAgICAgICAuYWRkcmVzcyA9IDB4MTAsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2
ODQ3MF9SRUdfVkFDVEwsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gQklUKDApLA0K
PiA+ICsgICAgICAgICAgICAgICAvKiBWQUNUTCAqLw0KPiA+ICsgICAgICAgfSwNCj4gPiArICAg
ICAgIHsNCj4gPiArICAgICAgICAgICAgICAgLmFkZHJlc3MgPSAweDE0LA0KPiA+ICsgICAgICAg
ICAgICAgICAucmVnID0gVFBTNjg0NzBfUkVHX1ZEQ1RMLA0KPiA+ICsgICAgICAgICAgICAgICAu
Yml0bWFzayA9IEJJVCgwKSwNCj4gPiArICAgICAgICAgICAgICAgLyogVkRDVEwgKi8NCj4gPiAr
ICAgICAgIH0sDQo+ID4gK307DQo+ID4gKw0KPiA+ICsvKiBUYWJsZSB0byBzZXQgdm9sdGFnZSBy
ZWd1bGF0b3IgdmFsdWUgKi8gc3RhdGljIGNvbnN0IHN0cnVjdA0KPiA+ICt0aV9wbWljX3RhYmxl
IHZyX3ZhbF90YWJsZVtdID0gew0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAgICAu
YWRkcmVzcyA9IDB4MDAsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9SRUdf
VlNJT1ZBTCwNCj4gPiArICAgICAgICAgICAgICAgLmJpdG1hc2sgPSBUUFM2ODQ3MF9WU0lPVkFM
X0lPVk9MVF9NQVNLLA0KPiA+ICsgICAgICAgICAgICAgICAvKiBUUFM2ODQ3MF9SRUdfVlNJT1ZB
TCAqLw0KPiA+ICsgICAgICAgfSwNCj4gPiArICAgICAgIHsNCj4gPiArICAgICAgICAgICAgICAg
LmFkZHJlc3MgPSAweDA0LA0KPiA+ICsgICAgICAgICAgICAgICAucmVnID0gVFBTNjg0NzBfUkVH
X1ZJT1ZBTCwNCj4gPiArICAgICAgICAgICAgICAgLmJpdG1hc2sgPSBUUFM2ODQ3MF9WSU9WQUxf
SU9WT0xUX01BU0ssDQo+ID4gKyAgICAgICAgICAgICAgIC8qIFRQUzY4NDcwX1JFR19WSU9WQUwg
Ki8NCj4gPiArICAgICAgIH0sDQo+ID4gKyAgICAgICB7DQo+ID4gKyAgICAgICAgICAgICAgIC5h
ZGRyZXNzID0gMHgwOCwNCj4gPiArICAgICAgICAgICAgICAgLnJlZyA9IFRQUzY4NDcwX1JFR19W
Q01WQUwsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gVFBTNjg0NzBfVkNNVkFMX1ZD
Vk9MVF9NQVNLLA0KPiA+ICsgICAgICAgICAgICAgICAvKiBUUFM2ODQ3MF9SRUdfVkNNVkFMICov
DQo+ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAgICAuYWRk
cmVzcyA9IDB4MEMsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9SRUdfVkFV
WDFWQUwsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gVFBTNjg0NzBfVkFVWDFWQUxf
QVVYMVZPTFRfTUFTSywNCj4gPiArICAgICAgICAgICAgICAgLyogVFBTNjg0NzBfUkVHX1ZBVVgx
VkFMICovDQo+ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAg
ICAuYWRkcmVzcyA9IDB4MTAsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9S
RUdfVkFVWDJWQUwsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gVFBTNjg0NzBfVkFV
WDJWQUxfQVVYMlZPTFRfTUFTSywNCj4gPiArICAgICAgICAgICAgICAgLyogVFBTNjg0NzBfUkVH
X1ZBVVgyVkFMICovDQo+ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAg
ICAgICAgICAuYWRkcmVzcyA9IDB4MTQsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2
ODQ3MF9SRUdfVkFWQUwsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gVFBTNjg0NzBf
VkFWQUxfQVZPTFRfTUFTSywNCj4gPiArICAgICAgICAgICAgICAgLyogVFBTNjg0NzBfUkVHX1ZB
VkFMICovDQo+ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAg
ICAuYWRkcmVzcyA9IDB4MTgsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9S
RUdfVkRWQUwsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gVFBTNjg0NzBfVkRWQUxf
RFZPTFRfTUFTSywNCj4gPiArICAgICAgICAgICAgICAgLyogVFBTNjg0NzBfUkVHX1ZEVkFMICov
DQo+ID4gKyAgICAgICB9LA0KPiA+ICt9Ow0KPiA+ICsNCj4gPiArLyogVGFibGUgdG8gY29uZmln
dXJlIGNsb2NrIGZyZXF1ZW5jeSAqLyBzdGF0aWMgY29uc3Qgc3RydWN0DQo+ID4gK3RpX3BtaWNf
dGFibGUgY2xrX2ZyZXFfdGFibGVbXSA9IHsNCj4gPiArICAgICAgIHsNCj4gPiArICAgICAgICAg
ICAgICAgLmFkZHJlc3MgPSAweDAwLA0KPiA+ICsgICAgICAgICAgICAgICAucmVnID0gVFBTNjg0
NzBfUkVHX1BPU1RESVYyLA0KPiA+ICsgICAgICAgICAgICAgICAuYml0bWFzayA9IEJJVCgwKSB8
IEJJVCgxKSwNCj4gPiArICAgICAgICAgICAgICAgLyogVFBTNjg0NzBfUkVHX1BPU1RESVYyICov
DQo+ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAgICAuYWRk
cmVzcyA9IDB4MDQsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9SRUdfQk9P
U1RESVYsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gMHgxRiwNCj4gPiArICAgICAg
ICAgICAgICAgLyogVFBTNjg0NzBfUkVHX0JPT1NURElWICovDQo+ID4gKyAgICAgICB9LA0KPiA+
ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAgICAuYWRkcmVzcyA9IDB4MDgsDQo+ID4gKyAg
ICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9SRUdfQlVDS0RJViwNCj4gPiArICAgICAgICAg
ICAgICAgLmJpdG1hc2sgPSAweDBGLA0KPiA+ICsgICAgICAgICAgICAgICAvKiBUUFM2ODQ3MF9S
RUdfQlVDS0RJViAqLw0KPiA+ICsgICAgICAgfSwNCj4gPiArICAgICAgIHsNCj4gPiArICAgICAg
ICAgICAgICAgLmFkZHJlc3MgPSAweDBDLA0KPiA+ICsgICAgICAgICAgICAgICAucmVnID0gVFBT
Njg0NzBfUkVHX1BMTFNXUiwNCj4gPiArICAgICAgICAgICAgICAgLmJpdG1hc2sgPSAweDEzLA0K
PiA+ICsgICAgICAgICAgICAgICAvKiBUUFM2ODQ3MF9SRUdfUExMU1dSICovDQo+ID4gKyAgICAg
ICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAgICAuYWRkcmVzcyA9IDB4MTAs
DQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9SRUdfWFRBTERJViwNCj4gPiAr
ICAgICAgICAgICAgICAgLmJpdG1hc2sgPSAweEZGLA0KPiA+ICsgICAgICAgICAgICAgICAvKiBU
UFM2ODQ3MF9SRUdfWFRBTERJViAqLw0KPiA+ICsgICAgICAgfSwNCj4gPiArICAgICAgIHsNCj4g
PiArICAgICAgICAgICAgICAgLmFkZHJlc3MgPSAweDE0LA0KPiA+ICsgICAgICAgICAgICAgICAu
cmVnID0gVFBTNjg0NzBfUkVHX1BMTERJViwNCj4gPiArICAgICAgICAgICAgICAgLmJpdG1hc2sg
PSAweEZGLA0KPiA+ICsgICAgICAgICAgICAgICAvKiBUUFM2ODQ3MF9SRUdfUExMRElWICovDQo+
ID4gKyAgICAgICB9LA0KPiA+ICsgICAgICAgew0KPiA+ICsgICAgICAgICAgICAgICAuYWRkcmVz
cyA9IDB4MTgsDQo+ID4gKyAgICAgICAgICAgICAgIC5yZWcgPSBUUFM2ODQ3MF9SRUdfUE9TVERJ
ViwNCj4gPiArICAgICAgICAgICAgICAgLmJpdG1hc2sgPSAweDgzLA0KPiA+ICsgICAgICAgICAg
ICAgICAvKiBUUFM2ODQ3MF9SRUdfUE9TVERJViAqLw0KPiA+ICsgICAgICAgfSwNCj4gPiArfTsN
Cj4gPiArDQo+ID4gKy8qIFRhYmxlIHRvIGNvbmZpZ3VyZSBhbmQgZW5hYmxlIGNsb2NrcyAqLyBz
dGF0aWMgY29uc3Qgc3RydWN0DQo+ID4gK3RpX3BtaWNfdGFibGUgY2xrX3RhYmxlW10gPSB7DQo+
ID4gKyAgICAgICB7DQo+ID4gKyAgICAgICAgICAgICAgIC5hZGRyZXNzID0gMHgwMCwNCj4gPiAr
ICAgICAgICAgICAgICAgLnJlZyA9IFRQUzY4NDcwX1JFR19QTExDVEwsDQo+ID4gKyAgICAgICAg
ICAgICAgIC5iaXRtYXNrID0gMHhGNSwNCj4gPiArICAgICAgICAgICAgICAgLyogVFBTNjg0NzBf
UkVHX1BMTENUTCAqLw0KPiA+ICsgICAgICAgfSwNCj4gPiArICAgICAgIHsNCj4gPiArICAgICAg
ICAgICAgICAgLmFkZHJlc3MgPSAweDA0LA0KPiA+ICsgICAgICAgICAgICAgICAucmVnID0gVFBT
Njg0NzBfUkVHX1BMTENUTDIsDQo+ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gQklUKDAp
LA0KPiA+ICsgICAgICAgICAgICAgICAvKiBUUFM2ODQ3MF9SRUdfUExMQ1RMMiAqLw0KPiA+ICsg
ICAgICAgfSwNCj4gPiArICAgICAgIHsNCj4gPiArICAgICAgICAgICAgICAgLmFkZHJlc3MgPSAw
eDA4LA0KPiA+ICsgICAgICAgICAgICAgICAucmVnID0gVFBTNjg0NzBfUkVHX0NMS0NGRzEsDQo+
ID4gKyAgICAgICAgICAgICAgIC5iaXRtYXNrID0gVFBTNjg0NzBfQ0xLQ0ZHMV9NT0RFX0FfTUFT
SyB8DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgVFBTNjg0NzBfQ0xLQ0ZHMV9NT0RFX0Jf
TUFTSywNCj4gPiArICAgICAgICAgICAgICAgLyogVFBTNjg0NzBfUkVHX0NMS0NGRzEgKi8NCj4g
PiArICAgICAgIH0sDQo+ID4gKyAgICAgICB7DQo+ID4gKyAgICAgICAgICAgICAgIC5hZGRyZXNz
ID0gMHgwQywNCj4gPiArICAgICAgICAgICAgICAgLnJlZyA9IFRQUzY4NDcwX1JFR19DTEtDRkcy
LA0KPiA+ICsgICAgICAgICAgICAgICAuYml0bWFzayA9IFRQUzY4NDcwX0NMS0NGRzFfTU9ERV9B
X01BU0sgfA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgIFRQUzY4NDcwX0NMS0NGRzFfTU9E
RV9CX01BU0ssDQo+ID4gKyAgICAgICAgICAgICAgIC8qIFRQUzY4NDcwX1JFR19DTEtDRkcyICov
DQo+ID4gKyAgICAgICB9LA0KPiA+ICt9Ow0KPiA+ICsNCj4gPiArc3RhdGljIGludCBwbWljX2dl
dF9yZWdfYml0KHU2NCBhZGRyZXNzLCBzdHJ1Y3QgdGlfcG1pY190YWJsZSAqdGFibGUsDQo+ID4g
KyAgICAgICAgICAgICAgICAgICAgICAgICAgIGludCBjb3VudCwgaW50ICpyZWcsIGludCAqYml0
bWFzaykgew0KPiA+ICsgICAgICAgdTY0IGk7DQo+ID4gKw0KPiANCj4gPiArICAgICAgIGkgPSBh
ZGRyZXNzIC8gNDsNCj4gDQo+IA0KPiA+ICsNCj4gDQo+IFJlbW92ZSB0aGlzIGxpbmUuDQo+IA0K
DQpBY2sNCg0KPiA+ICsgICAgICAgaWYgKGkgPj0gY291bnQpDQo+ID4gKyAgICAgICAgICAgICAg
IHJldHVybiAtRU5PRU5UOw0KPiA+ICsNCj4gPiArICAgICAgIGlmICghcmVnIHx8ICFiaXRtYXNr
KQ0KPiA+ICsgICAgICAgICAgICAgICByZXR1cm4gLUVJTlZBTDsNCj4gPiArDQo+ID4gKyAgICAg
ICAqcmVnID0gdGFibGVbaV0ucmVnOw0KPiA+ICsgICAgICAgKmJpdG1hc2sgPSB0YWJsZVtpXS5i
aXRtYXNrOw0KPiA+ICsNCj4gPiArICAgICAgIHJldHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+
ICtzdGF0aWMgaW50IHRpX3RwczY4NDcwX3BtaWNfZ2V0X3Bvd2VyKHN0cnVjdCByZWdtYXAgKnJl
Z21hcCwgaW50IHJlZywNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICBpbnQgYml0bWFzaywgdTY0ICp2YWx1ZSkgew0KPiA+ICsgICAgICAgaW50IGRhdGE7DQo+ID4g
Kw0KPiA+ICsgICAgICAgaWYgKHJlZ21hcF9yZWFkKHJlZ21hcCwgcmVnLCAmZGF0YSkpDQo+ID4g
KyAgICAgICAgICAgICAgIHJldHVybiAtRUlPOw0KPiA+ICsNCj4gPiArICAgICAgICp2YWx1ZSA9
IChkYXRhICYgYml0bWFzaykgPyAxIDogMDsNCj4gPiArICAgICAgIHJldHVybiAwOw0KPiA+ICt9
DQo+ID4gKw0KPiA+ICtzdGF0aWMgaW50IHRpX3RwczY4NDcwX3BtaWNfZ2V0X3ZyX3ZhbChzdHJ1
Y3QgcmVnbWFwICpyZWdtYXAsIGludCByZWcsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgaW50IGJpdG1hc2ssIHU2NCAqdmFsdWUpIHsNCj4gPiArICAgICAgIGlu
dCBkYXRhOw0KPiA+ICsNCj4gPiArICAgICAgIGlmIChyZWdtYXBfcmVhZChyZWdtYXAsIHJlZywg
JmRhdGEpKQ0KPiA+ICsgICAgICAgICAgICAgICByZXR1cm4gLUVJTzsNCj4gPiArDQo+ID4gKyAg
ICAgICAqdmFsdWUgPSBkYXRhICYgYml0bWFzazsNCj4gPiArICAgICAgIHJldHVybiAwOw0KPiA+
ICt9DQo+ID4gKw0KPiA+ICtzdGF0aWMgaW50IHRpX3RwczY4NDcwX3BtaWNfZ2V0X2NsayhzdHJ1
Y3QgcmVnbWFwICpyZWdtYXAsIGludCByZWcsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgaW50IGJpdG1hc2ssIHU2NCAqdmFsdWUpIHsNCj4gPiArICAgICAgIGlu
dCBkYXRhOw0KPiA+ICsNCj4gPiArICAgICAgIGlmIChyZWdtYXBfcmVhZChyZWdtYXAsIHJlZywg
JmRhdGEpKQ0KPiA+ICsgICAgICAgICAgICAgICByZXR1cm4gLUVJTzsNCj4gPiArDQo+ID4gKyAg
ICAgICAqdmFsdWUgPSAoZGF0YSAmIGJpdG1hc2spID8gMSA6IDA7DQo+ID4gKyAgICAgICByZXR1
cm4gMDsNCj4gPiArfQ0KPiA+ICsNCj4gPiArc3RhdGljIGludCB0aV90cHM2ODQ3MF9wbWljX2dl
dF9jbGtfZnJlcShzdHJ1Y3QgcmVnbWFwICpyZWdtYXAsIGludCByZWcsDQo+ID4gKyAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaW50IGJpdG1hc2ssIHU2NCAqdmFsdWUpIHsN
Cj4gPiArICAgICAgIGludCBkYXRhOw0KPiA+ICsNCj4gPiArICAgICAgIGlmIChyZWdtYXBfcmVh
ZChyZWdtYXAsIHJlZywgJmRhdGEpKQ0KPiA+ICsgICAgICAgICAgICAgICByZXR1cm4gLUVJTzsN
Cj4gPiArDQo+ID4gKyAgICAgICAqdmFsdWUgPSBkYXRhICYgYml0bWFzazsNCj4gPiArICAgICAg
IHJldHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtzdGF0aWMgaW50IHRpX3RwczY4NDcwX3Jl
Z21hcF91cGRhdGVfYml0cyhzdHJ1Y3QgcmVnbWFwICpyZWdtYXAsIGludCByZWcsDQo+ID4gKyAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGludCBiaXRtYXNrLCB1NjQgdmFs
dWUpIHsNCj4gPiArICAgICAgIHJldHVybiByZWdtYXBfdXBkYXRlX2JpdHMocmVnbWFwLCByZWcs
IGJpdG1hc2ssIHZhbHVlKTsgfQ0KPiA+ICsNCj4gPiArc3RhdGljIGFjcGlfc3RhdHVzIHRpX3Bt
aWNfY29tbW9uX2hhbmRsZXIodTMyIGZ1bmN0aW9uLA0KPiA+ICsgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgIGFjcGlfcGh5c2ljYWxfYWRkcmVzcyBhZGRyZXNzLA0KPiA+
ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHUzMiBiaXRzLCB1NjQg
KnZhbHVlLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHZv
aWQgKmhhbmRsZXJfY29udGV4dCwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICB2b2lkICpyZWdpb25fY29udGV4dCwNCj4gPiArICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICBpbnQgKCpnZXQpKHN0cnVjdCByZWdtYXAgKiwNCj4gPiAr
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGludCwg
aW50LCB1NjQgKiksDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgaW50ICgqdXBkYXRlKShzdHJ1Y3QgcmVnbWFwICosDQo+ID4gKyAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBpbnQsIGludCwgdTY0KSwNCj4g
PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgdGlfcG1p
Y190YWJsZSAqdGFibGUsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgaW50IHRhYmxlX3NpemUpIHsNCj4gPiArICAgICAgIHN0cnVjdCB0aV9wbWljX29wcmVn
aW9uICpvcHJlZ2lvbiA9IHJlZ2lvbl9jb250ZXh0Ow0KPiA+ICsgICAgICAgc3RydWN0IHJlZ21h
cCAqcmVnbWFwID0gb3ByZWdpb24tPnJlZ21hcDsNCj4gPiArICAgICAgIGludCByZWcsIHJldCwg
Yml0bWFzazsNCj4gPiArDQo+ID4gKyAgICAgICBpZiAoYml0cyAhPSAzMikNCj4gPiArICAgICAg
ICAgICAgICAgcmV0dXJuIEFFX0JBRF9QQVJBTUVURVI7DQo+ID4gKw0KPiA+ICsgICAgICAgcmV0
ID0gcG1pY19nZXRfcmVnX2JpdChhZGRyZXNzLCB0YWJsZSwNCj4gPiArICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgdGFibGVfc2l6ZSwgJnJlZywgJmJpdG1hc2spOw0KPiA+ICsgICAg
ICAgaWYgKHJldCA8IDApDQo+ID4gKyAgICAgICAgICAgICAgIHJldHVybiBBRV9CQURfUEFSQU1F
VEVSOw0KPiA+ICsNCj4gPiArICAgICAgIGlmIChmdW5jdGlvbiA9PSBBQ1BJX1dSSVRFICYmICgq
dmFsdWUgPiBiaXRtYXNrKSkNCj4gPiArICAgICAgICAgICAgICAgcmV0dXJuIEFFX0JBRF9QQVJB
TUVURVI7DQo+ID4gKw0KPiA+ICsgICAgICAgbXV0ZXhfbG9jaygmb3ByZWdpb24tPmxvY2spOw0K
PiA+ICsNCj4gPiArICAgICAgIHJldCA9IChmdW5jdGlvbiA9PSBBQ1BJX1JFQUQpID8NCj4gPiAr
ICAgICAgICAgICAgICAgZ2V0KHJlZ21hcCwgcmVnLCBiaXRtYXNrLCB2YWx1ZSkgOg0KPiA+ICsg
ICAgICAgICAgICAgICB1cGRhdGUocmVnbWFwLCByZWcsIGJpdG1hc2ssICp2YWx1ZSk7DQo+ID4g
Kw0KPiA+ICsgICAgICAgbXV0ZXhfdW5sb2NrKCZvcHJlZ2lvbi0+bG9jayk7DQo+ID4gKw0KPiA+
ICsgICAgICAgcmV0dXJuIHJldCA/IEFFX0VSUk9SIDogQUVfT0s7IH0NCj4gPiArDQo+ID4gK3N0
YXRpYyBhY3BpX3N0YXR1cyB0aV9wbWljX2Nsa19mcmVxX2hhbmRsZXIodTMyIGZ1bmN0aW9uLA0K
PiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgYWNwaV9waHlz
aWNhbF9hZGRyZXNzIGFkZHJlc3MsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICB1MzIgYml0cywgdTY0ICp2YWx1ZSwNCj4gPiArICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgIHZvaWQgKmhhbmRsZXJfY29udGV4dCwNCj4gPiAr
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHZvaWQgKnJlZ2lvbl9j
b250ZXh0KSB7DQo+ID4gKyAgICAgICByZXR1cm4gdGlfcG1pY19jb21tb25faGFuZGxlcihmdW5j
dGlvbiwgYWRkcmVzcywgYml0cywgdmFsdWUsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICBoYW5kbGVyX2NvbnRleHQsIHJlZ2lvbl9jb250ZXh0LA0KPiA+ICsgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgdGlfdHBzNjg0NzBfcG1pY19nZXRfY2xrX2ZyZXEsDQo+ID4g
KyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB0aV90cHM2ODQ3MF9yZWdtYXBfdXBkYXRl
X2JpdHMsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAoc3RydWN0IHRpX3Bt
aWNfdGFibGUgKikgJmNsa19mcmVxX3RhYmxlLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgQVJSQVlfU0laRShjbGtfZnJlcV90YWJsZSkpOyB9DQo+ID4gKw0KPiA+ICtzdGF0
aWMgYWNwaV9zdGF0dXMgdGlfcG1pY19jbGtfaGFuZGxlcih1MzIgZnVuY3Rpb24sDQo+ID4gKyAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgYWNwaV9waHlzaWNhbF9hZGRyZXNz
IGFkZHJlc3MsIHUzMiBiaXRzLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgIHU2NCAqdmFsdWUsIHZvaWQgKmhhbmRsZXJfY29udGV4dCwNCj4gPiArICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICB2b2lkICpyZWdpb25fY29udGV4dCkgew0KPiA+
ICsgICAgICAgcmV0dXJuIHRpX3BtaWNfY29tbW9uX2hhbmRsZXIoZnVuY3Rpb24sIGFkZHJlc3Ms
IGJpdHMsIHZhbHVlLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaGFuZGxl
cl9jb250ZXh0LCByZWdpb25fY29udGV4dCwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgIHRpX3RwczY4NDcwX3BtaWNfZ2V0X2NsaywNCj4gPiArICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgIHRpX3RwczY4NDcwX3JlZ21hcF91cGRhdGVfYml0cywNCj4gPiArICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIChzdHJ1Y3QgdGlfcG1pY190YWJsZSAqKSAmY2xrX3Rh
YmxlLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIEFSUkFZX1NJWkUoY2xr
X3RhYmxlKSk7IH0NCj4gPiArDQo+ID4gK3N0YXRpYyBhY3BpX3N0YXR1cyB0aV9wbWljX3ZyX3Zh
bF9oYW5kbGVyKHUzMiBmdW5jdGlvbiwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICBhY3BpX3BoeXNpY2FsX2FkZHJlc3MgYWRkcmVzcywNCj4gPiArICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB1MzIgYml0cywgdTY0ICp2YWx1ZSwN
Cj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB2b2lkICpoYW5k
bGVyX2NvbnRleHQsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgdm9pZCAqcmVnaW9uX2NvbnRleHQpIHsNCj4gPiArICAgICAgIHJldHVybiB0aV9wbWljX2Nv
bW1vbl9oYW5kbGVyKGZ1bmN0aW9uLCBhZGRyZXNzLCBiaXRzLCB2YWx1ZSwNCj4gPiArICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIGhhbmRsZXJfY29udGV4dCwgcmVnaW9uX2NvbnRleHQs
DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB0aV90cHM2ODQ3MF9wbWljX2dl
dF92cl92YWwsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB0aV90cHM2ODQ3
MF9yZWdtYXBfdXBkYXRlX2JpdHMsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAoc3RydWN0IHRpX3BtaWNfdGFibGUgKikgJnZyX3ZhbF90YWJsZSwNCj4gPiArICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgIEFSUkFZX1NJWkUodnJfdmFsX3RhYmxlKSk7IH0NCj4gPiAr
DQo+ID4gK3N0YXRpYyBhY3BpX3N0YXR1cyB0aV9wbWljX3Bvd2VyX2hhbmRsZXIodTMyIGZ1bmN0
aW9uLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgYWNwaV9w
aHlzaWNhbF9hZGRyZXNzIGFkZHJlc3MsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICB1MzIgYml0cywgdTY0ICp2YWx1ZSwNCj4gPiArICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIHZvaWQgKmhhbmRsZXJfY29udGV4dCwNCj4gPiArICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHZvaWQgKnJlZ2lvbl9jb250ZXh0
KSB7DQo+ID4gKyAgICAgICBpZiAoYml0cyAhPSAzMikNCj4gPiArICAgICAgICAgICAgICAgcmV0
dXJuIEFFX0JBRF9QQVJBTUVURVI7DQo+ID4gKw0KPiA+ICsgICAgICAgLyogc2V0L2NsZWFyIGZv
ciBiaXQgMCwgYml0cyAwIGFuZCAxIHRvZ2V0aGVyICovDQo+ID4gKyAgICAgICBpZiAoZnVuY3Rp
b24gPT0gQUNQSV9XUklURSAmJg0KPiA+ICsgICAgICAgICAgICEoKnZhbHVlID09IDAgfHwgKnZh
bHVlID09IDEgfHwgKnZhbHVlID09IDMpKSB7DQo+ID4gKyAgICAgICAgICAgICAgIHJldHVybiBB
RV9CQURfUEFSQU1FVEVSOw0KPiA+ICsgICAgICAgfQ0KPiA+ICsNCj4gPiArICAgICAgIHJldHVy
biB0aV9wbWljX2NvbW1vbl9oYW5kbGVyKGZ1bmN0aW9uLCBhZGRyZXNzLCBiaXRzLCB2YWx1ZSwN
Cj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGhhbmRsZXJfY29udGV4dCwgcmVn
aW9uX2NvbnRleHQsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB0aV90cHM2
ODQ3MF9wbWljX2dldF9wb3dlciwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
IHRpX3RwczY4NDcwX3JlZ21hcF91cGRhdGVfYml0cywNCj4gPiArICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgIChzdHJ1Y3QgdGlfcG1pY190YWJsZSAqKSAmcG93ZXJfdGFibGUsDQo+ID4g
KyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBBUlJBWV9TSVpFKHBvd2VyX3RhYmxlKSk7
IH0NCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgdGlfdHBzNjg0NzBfcG1pY19vcHJlZ2lvbl9wcm9i
ZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlDQo+ID4gKypwZGV2KSB7DQo+ID4gKyAgICAgICBzdHJ1
Y3QgdHBzNjg0NzAgKnBtaWMgPSBkZXZfZ2V0X2RydmRhdGEocGRldi0+ZGV2LnBhcmVudCk7DQo+
ID4gKyAgICAgICBhY3BpX2hhbmRsZSBoYW5kbGUgPSBBQ1BJX0hBTkRMRShwZGV2LT5kZXYucGFy
ZW50KTsNCj4gPiArICAgICAgIHN0cnVjdCBkZXZpY2UgKmRldiA9ICZwZGV2LT5kZXY7DQo+ID4g
KyAgICAgICBzdHJ1Y3QgdGlfcG1pY19vcHJlZ2lvbiAqb3ByZWdpb247DQo+ID4gKyAgICAgICBh
Y3BpX3N0YXR1cyBzdGF0dXM7DQo+ID4gKw0KPiANCj4gPiArICAgICAgIGlmICghZGV2IHx8ICFw
bWljLT5yZWdtYXApIHsNCj4gPiArICAgICAgICAgICAgICAgV0FSTigxLCAiZGV2IG9yIHJlZ21h
cCBpcyBOVUxMXG4iKTsNCj4gPiArICAgICAgICAgICAgICAgcmV0dXJuIC1FSU5WQUw7DQo+ID4g
KyAgICAgICB9DQo+ID4gKw0KPiA+ICsgICAgICAgaWYgKCFoYW5kbGUpIHsNCj4gPiArICAgICAg
ICAgICAgICAgV0FSTigxLCAiYWNwaSBoYW5kbGUgaXMgTlVMTFxuIik7DQo+ID4gKyAgICAgICAg
ICAgICAgIHJldHVybiAtRU5PREVWOw0KPiA+ICsgICAgICAgfQ0KPiANCj4gSSBkdW5ubyBpZiBX
QVJOcyBtYWtlIHVzZXIgZXhwZXJpZW5jZSBhbnkgYmV0dGVyLg0KPiBCZXNpZGVzIHRoYXQgSSB3
b3VsZCBkb3VibGUgY2hlY2sgeW91IG1heSBoYXZlIHN1Y2ggY2FzZXMuDQo+IA0KDQpBY2sNClNp
bmNlIHRoZSBjYWxsZXIgaXMgZnJvbSB3aXRoaW4gdGhlIHNhbWUgZmlsZSBhbmQgd2Uga25vdyB0
aGUgcGFyYW1ldGVycyB3aWxsIGJlIHNldCBwcm9wZXJseSwgSSBjYW4gY2hhbmdlIHRoaXMgdG8g
ZGV2X3dhcm4oKQ0KDQo+ID4gKw0KPiA+ICsgICAgICAgb3ByZWdpb24gPSBkZXZtX2t6YWxsb2Mo
ZGV2LCBzaXplb2YoKm9wcmVnaW9uKSwgR0ZQX0tFUk5FTCk7DQo+ID4gKyAgICAgICBpZiAoIW9w
cmVnaW9uKQ0KPiA+ICsgICAgICAgICAgICAgICByZXR1cm4gLUVOT01FTTsNCj4gPiArDQo+ID4g
KyAgICAgICBtdXRleF9pbml0KCZvcHJlZ2lvbi0+bG9jayk7DQo+ID4gKyAgICAgICBvcHJlZ2lv
bi0+cmVnbWFwID0gcG1pYy0+cmVnbWFwOw0KPiA+ICsNCj4gPiArICAgICAgIHN0YXR1cyA9IGFj
cGlfaW5zdGFsbF9hZGRyZXNzX3NwYWNlX2hhbmRsZXIoaGFuZGxlLA0KPiA+ICsgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBUSV9QTUlDX1BPV0VSX09Q
UkVHSU9OX0lELA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICB0aV9wbWljX3Bvd2VyX2hhbmRsZXIsDQo+ID4gKyAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIE5VTEwsIG9wcmVnaW9uKTsNCj4gPiAr
ICAgICAgIGlmIChBQ1BJX0ZBSUxVUkUoc3RhdHVzKSkNCj4gPiArICAgICAgICAgICAgICAgcmV0
dXJuIC1FTk9ERVY7DQo+ID4gKw0KPiA+ICsgICAgICAgc3RhdHVzID0gYWNwaV9pbnN0YWxsX2Fk
ZHJlc3Nfc3BhY2VfaGFuZGxlcihoYW5kbGUsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgIFRJX1BNSUNfVlJfVkFMX09QUkVHSU9OX0lELA0K
PiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB0
aV9wbWljX3ZyX3ZhbF9oYW5kbGVyLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICBOVUxMLCBvcHJlZ2lvbik7DQo+ID4gKyAgICAgICBpZiAo
QUNQSV9GQUlMVVJFKHN0YXR1cykpDQo+ID4gKyAgICAgICAgICAgICAgIGdvdG8gb3V0X3JlbW92
ZV9wb3dlcl9oYW5kbGVyOw0KPiA+ICsNCj4gPiArICAgICAgIHN0YXR1cyA9IGFjcGlfaW5zdGFs
bF9hZGRyZXNzX3NwYWNlX2hhbmRsZXIoaGFuZGxlLA0KPiA+ICsgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBUSV9QTUlDX0NMT0NLX09QUkVHSU9OX0lE
LA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICB0aV9wbWljX2Nsa19oYW5kbGVyLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICBOVUxMLCBvcHJlZ2lvbik7DQo+ID4gKyAgICAgICBpZiAo
QUNQSV9GQUlMVVJFKHN0YXR1cykpDQo+ID4gKyAgICAgICAgICAgICAgIGdvdG8gb3V0X3JlbW92
ZV92cl92YWxfaGFuZGxlcjsNCj4gPiArDQo+ID4gKyAgICAgICBzdGF0dXMgPSBhY3BpX2luc3Rh
bGxfYWRkcmVzc19zcGFjZV9oYW5kbGVyKGhhbmRsZSwNCj4gPiArICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgVElfUE1JQ19DTEtGUkVRX09QUkVHSU9O
X0lELA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICB0aV9wbWljX2Nsa19mcmVxX2hhbmRsZXIsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIE5VTEwsIG9wcmVnaW9uKTsNCj4gPiArICAg
ICAgIGlmIChBQ1BJX0ZBSUxVUkUoc3RhdHVzKSkNCj4gPiArICAgICAgICAgICAgICAgZ290byBv
dXRfcmVtb3ZlX2Nsa19oYW5kbGVyOw0KPiA+ICsNCj4gPiArICAgICAgIHJldHVybiAwOw0KPiA+
ICsNCj4gPiArb3V0X3JlbW92ZV9jbGtfaGFuZGxlcjoNCj4gPiArICAgICAgIGFjcGlfcmVtb3Zl
X2FkZHJlc3Nfc3BhY2VfaGFuZGxlcihoYW5kbGUsDQo+IFRJX1BNSUNfQ0xPQ0tfT1BSRUdJT05f
SUQsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdGlfcG1p
Y19jbGtfaGFuZGxlcik7DQo+ID4gK291dF9yZW1vdmVfdnJfdmFsX2hhbmRsZXI6DQo+ID4gKyAg
ICAgICBhY3BpX3JlbW92ZV9hZGRyZXNzX3NwYWNlX2hhbmRsZXIoaGFuZGxlLA0KPiBUSV9QTUlD
X1ZSX1ZBTF9PUFJFR0lPTl9JRCwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICB0aV9wbWljX3ZyX3ZhbF9oYW5kbGVyKTsNCj4gPiArb3V0X3JlbW92ZV9wb3dl
cl9oYW5kbGVyOg0KPiA+ICsgICAgICAgYWNwaV9yZW1vdmVfYWRkcmVzc19zcGFjZV9oYW5kbGVy
KGhhbmRsZSwNCj4gVElfUE1JQ19QT1dFUl9PUFJFR0lPTl9JRCwNCj4gPiArICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICB0aV9wbWljX3Bvd2VyX2hhbmRsZXIpOw0KPiA+
ICsgICAgICAgcmV0dXJuIC1FTk9ERVY7DQo+ID4gK30NCj4gPiArDQo+ID4gK3N0YXRpYyBzdHJ1
Y3QgcGxhdGZvcm1fZHJpdmVyIHRpX3RwczY4NDcwX3BtaWNfb3ByZWdpb25fZHJpdmVyID0gew0K
PiA+ICsgICAgICAgLnByb2JlID0gdGlfdHBzNjg0NzBfcG1pY19vcHJlZ2lvbl9wcm9iZSwNCj4g
PiArICAgICAgIC5kcml2ZXIgPSB7DQo+ID4gKyAgICAgICAgICAgICAgIC5uYW1lID0gInRwczY4
NDcwX3BtaWNfb3ByZWdpb24iLA0KPiA+ICsgICAgICAgfSwNCj4gPiArfTsNCj4gPiArDQo+ID4g
K2J1aWx0aW5fcGxhdGZvcm1fZHJpdmVyKHRpX3RwczY4NDcwX3BtaWNfb3ByZWdpb25fZHJpdmVy
KQ0KPiA+IC0tDQo+ID4gMS45LjENCj4gPg0KPiANCj4gLS0NCj4gV2l0aCBCZXN0IFJlZ2FyZHMs
DQo+IEFuZHkgU2hldmNoZW5rbw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mani, Rajmohan June 9, 2017, 10:20 p.m. UTC | #13
Hi Hans,

> -----Original Message-----

> From: Hans de Goede [mailto:hdegoede@redhat.com]

> Sent: Tuesday, June 06, 2017 8:22 AM

> To: Andy Shevchenko <andy.shevchenko@gmail.com>; Mani, Rajmohan

> <rajmohan.mani@intel.com>

> Cc: linux-kernel@vger.kernel.org; linux-gpio@vger.kernel.org; linux-

> acpi@vger.kernel.org; Lee Jones <lee.jones@linaro.org>; Linus Walleij

> <linus.walleij@linaro.org>; Alexandre Courbot <gnurou@gmail.com>; Rafael J.

> Wysocki <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>

> Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation

> region driver

> 

> Hi,

> 

> On 06/06/2017 04:23 PM, Andy Shevchenko wrote:

> > +Cc Hans (that's why didn't delete anything from original mail, just

> > adding my comments).

> >

> > Hans, if you have few minutes it would be appreciated to glance on the

> > below for some issues if any since you did pass quite a good quest

> > with other PMIC drivers.

> 

> I've gone over this driver, nothing stands out in a bad way to me, IOW this

> seems like a normal PMIC OpRegion handler to me.

> 


Thanks for the reviews and time.
Mani, Rajmohan June 9, 2017, 11:38 p.m. UTC | #14
Hi Sakari, Andy,

> -----Original Message-----
> From: Sakari Ailus [mailto:sakari.ailus@iki.fi]
> Sent: Wednesday, June 07, 2017 2:13 PM
> To: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Mani, Rajmohan <rajmohan.mani@intel.com>; Hans de Goede
> <hdegoede@redhat.com>; linux-kernel@vger.kernel.org; linux-
> gpio@vger.kernel.org; linux-acpi@vger.kernel.org; Lee Jones
> <lee.jones@linaro.org>; Linus Walleij <linus.walleij@linaro.org>; Alexandre
> Courbot <gnurou@gmail.com>; Rafael J. Wysocki <rjw@rjwysocki.net>; Len
> Brown <lenb@kernel.org>
> Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation
> region driver
> 
> On Wed, Jun 07, 2017 at 11:40:27PM +0300, Andy Shevchenko wrote:
> > On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > > On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote:
> > >> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > >> > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote:
> >
> > >> >> Follow the pattern, please, I suppose ti_pmic_tps68470.c
> > >>
> > >> Would it work for you?
> > >
> > > This is still a different driver from the tps68470 driver which is
> > > an MFD driver. For clarity, I'd keep pmic as part of the name (and
> > > I'd use tps68470_pmic_ prefix for internal symbols, too).
> > >
> > > As PMICs are typically linked to the kernel (vs. being modules),
> > > there's no issue with the module name. I would suppose few if any
> > > PMICs will be compiled as modules in general.
> > >
> > > It's not a big deal though. I'm fine either way.
> >
> > Okay, let's agree on  tps68470_pmic for internal prefix and for file name?
> 
> Ack. Thanks!
> 

Ack
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mani, Rajmohan June 9, 2017, 11:47 p.m. UTC | #15
SGkgSGFucywNCg0KPiA+DQo+ID4gQXMgUE1JQ3MgYXJlIHR5cGljYWxseSBsaW5rZWQgdG8gdGhl
IGtlcm5lbCAodnMuIGJlaW5nIG1vZHVsZXMpLA0KPiA+IHRoZXJlJ3Mgbm8gaXNzdWUgd2l0aCB0
aGUgbW9kdWxlIG5hbWUuIEkgd291bGQgc3VwcG9zZSBmZXcgaWYgYW55DQo+ID4gUE1JQ3Mgd2ls
bCBiZSBjb21waWxlZCBhcyBtb2R1bGVzIGluIGdlbmVyYWwuDQo+IA0KPiBHb29kIHBvaW50IGFi
b3V0IHRoZSBPcFJlZ2lvbiBkcml2ZXIgdXN1YWxseSBiZWluZyBidWlsdC1pbiwgaW4gbXkgZXhw
ZXJpZW5jZSBpdA0KPiBNVVNUIGFsd2F5cyBiZSBidWlsdC1pbiwgc28gdGhlIEtjb25maWcgb3B0
aW9uIHNob3VsZCBiZSBhIGJvb2wuIE5vdGUgdGhpcyBpcw0KPiB1c2VsZXNzIHVubGVzcyB0aGUg
bWZkIGRyaXZlciBpcyBhbHNvIGEgYm9vbCAoSSB3b3VsZCBhZHZpY2UgdG8gZ28gdGhhdA0KPiBy
b3V0ZSkgYW5kIHRoZSBtZmQgZHJpdmVyJ3MgS2NvbmZpZyBzaG91bGQgc2VsZWN0IHRoZSByaWdo
dCBpMmMgYnVzIGRyaXZlciB0bw0KPiBtYWtlIHN1cmUgdGhhdCBpcyBidWlsdC1pbiB0b28sIHNl
ZSBmb3IgZXhhbXBsZToNCj4gDQo+IGh0dHBzOi8vZ2l0Lmtlcm5lbC5vcmcvcHViL3NjbS9saW51
eC9rZXJuZWwvZ2l0L2xlZS9tZmQuZ2l0L2NvbW1pdC8/aD1mb3ItDQo+IG1mZC1uZXh0JmlkPTJm
OTFkZWQ1ZjhmNGZkZDY3ZDhkYWFlNTE0YjBkNDM0Yzk4YWIxZTANCj4gaHR0cHM6Ly9naXQua2Vy
bmVsLm9yZy9wdWIvc2NtL2xpbnV4L2tlcm5lbC9naXQvbGVlL21mZC5naXQvY29tbWl0Lz9oPWZv
ci0NCj4gbWZkLW5leHQmaWQ9YzUwNjVkODYyNWViZGMxNjQxOTliOTlkODM4YWMwNjM2ZmFhN2Yw
Yg0KPiBodHRwczovL2dpdC5rZXJuZWwub3JnL3B1Yi9zY20vbGludXgva2VybmVsL2dpdC9sZWUv
bWZkLmdpdC9jb21taXQvP2g9Zm9yLQ0KPiBtZmQtbmV4dCZpZD01ZjEyNWYxZjU3MDU2OGEyOWVk
Zjc4M2ZiYTFlYmI2MDZkNWM2YjI0DQo+IA0KPiBXaGljaCBhcmUgYWxsIHJlY2VudCBjb21taXRz
IGZyb20gbWUgZGVhbGluZyB3aXRoIG1ha2luZyB0aGUgbWZkIGRyaXZlciBidWlsdC0NCj4gaW4g
LyBzZWxlY3RpbmcgdGhlIGkyYyBidXMgZHJpdmVyLg0KPiANCg0KVGhhbmtzIGZvciB0aGVzZSBs
aW5rcy4NCkkgd2lsbCB1cGRhdGUgdGhlIEtjb25maWcgYW5kIGNvbW1pdCBtZXNzYWdlcyB3aXRo
IHJlbGV2YW50IGRlc2NyaXB0aW9uIGFyb3VuZCB0aGlzLg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mani, Rajmohan June 10, 2017, 12:04 a.m. UTC | #16
Hi Sakari,

Thanks for the reviews.

> -----Original Message-----
> From: Sakari Ailus [mailto:sakari.ailus@iki.fi]
> Sent: Wednesday, June 07, 2017 5:08 AM
> To: Mani, Rajmohan <rajmohan.mani@intel.com>
> Cc: linux-kernel@vger.kernel.org; linux-gpio@vger.kernel.org; linux-
> acpi@vger.kernel.org; Lee Jones <lee.jones@linaro.org>; Linus Walleij
> <linus.walleij@linaro.org>; Alexandre Courbot <gnurou@gmail.com>; Rafael J.
> Wysocki <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>
> Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation
> region driver
> 
> Hi Rajmohan,
> 
> Thanks for removing the redundant struct definition. A couple more comments
> below. Not really necessarily bugs but a few things to clean things up a bit.
> 

Ack

> On Tue, Jun 06, 2017 at 04:55:18AM -0700, Rajmohan Mani wrote:
> > The Kabylake platform coreboot (Chrome OS equivalent of
> > BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
> > These operation regions are to enable/disable voltage regulators,
> > configure voltage regulators, enable/disable clocks and to configure
> > clocks.
> >
> > This config adds ACPI operation region support for TI TPS68470 PMIC.
> > TPS68470 device is an advanced power management unit that powers a
> > Compact Camera Module (CCM), generates clocks for image sensors,
> > drives a dual LED for flash and incorporates two LED drivers for
> > general purpose indicators.
> > This driver enables ACPI operation region support to control voltage
> > regulators and clocks for the TPS68470 PMIC.
> >
> > Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
> > ---
> >  drivers/acpi/Kconfig              |  12 +
> >  drivers/acpi/Makefile             |   2 +
> >  drivers/acpi/pmic/pmic_tps68470.c | 454
> > ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 468 insertions(+)
> >  create mode 100644 drivers/acpi/pmic/pmic_tps68470.c
> >
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index
> > 1ce52f8..218d22d 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -535,4 +535,16 @@ if ARM64
> >  source "drivers/acpi/arm64/Kconfig"
> >  endif
> >
> > +config TPS68470_PMIC_OPREGION
> > +	bool "ACPI operation region support for TPS68470 PMIC"
> > +	help
> > +	  This config adds ACPI operation region support for TI TPS68470 PMIC.
> > +	  TPS68470 device is an advanced power management unit that powers
> > +	  a Compact Camera Module (CCM), generates clocks for image sensors,
> > +	  drives a dual LED for flash and incorporates two LED drivers for
> > +	  general purpose indicators.
> > +	  This driver enables ACPI operation region support control voltage
> > +	  regulators and clocks.
> > +
> 
> Extra newline.
> 

Ack

> > +
> >  endif	# ACPI
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index
> > b1aacfc..7113d05 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -106,6 +106,8 @@ obj-$(CONFIG_CHT_WC_PMIC_OPREGION) +=
> > pmic/intel_pmic_chtwc.o
> >
> >  obj-$(CONFIG_ACPI_CONFIGFS)	+= acpi_configfs.o
> >
> > +obj-$(CONFIG_TPS68470_PMIC_OPREGION)	+= pmic/pmic_tps68470.o
> > +
> >  video-objs			+= acpi_video.o video_detect.o
> >  obj-y				+= dptf/
> >
> > diff --git a/drivers/acpi/pmic/pmic_tps68470.c
> > b/drivers/acpi/pmic/pmic_tps68470.c
> > new file mode 100644
> > index 0000000..b2d608b
> > --- /dev/null
> > +++ b/drivers/acpi/pmic/pmic_tps68470.c
> > @@ -0,0 +1,454 @@
> > +/*
> > + * TI TPS68470 PMIC operation region driver
> > + *
> > + * Copyright (C) 2017 Intel Corporation. All rights reserved.
> > + * Author: Rajmohan Mani <rajmohan.mani@intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > +version
> > + * 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * Based on drivers/acpi/pmic/intel_pmic* drivers
> > + *
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/mfd/tps68470.h>
> > +#include <linux/init.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +
> > +struct ti_pmic_table {
> > +	u32 address;		/* operation region address */
> > +	u32 reg;		/* corresponding register */
> > +	u32 bitmask;		/* bit mask for power, clock */
> > +};
> > +
> > +#define TI_PMIC_POWER_OPREGION_ID		0xB0
> > +#define TI_PMIC_VR_VAL_OPREGION_ID		0xB1
> > +#define TI_PMIC_CLOCK_OPREGION_ID		0xB2
> > +#define TI_PMIC_CLKFREQ_OPREGION_ID		0xB3
> > +
> > +struct ti_pmic_opregion {
> > +	struct mutex lock;
> > +	struct regmap *regmap;
> > +};
> > +
> > +#define S_IO_I2C_EN	(BIT(0) | BIT(1))
> > +
> > +static const struct ti_pmic_table power_table[] = {
> > +	{
> > +		.address = 0x00,
> > +		.reg = TPS68470_REG_S_I2C_CTL,
> > +		.bitmask = S_IO_I2C_EN,
> > +		/* S_I2C_CTL */
> > +	},
> > +	{
> > +		.address = 0x04,
> > +		.reg = TPS68470_REG_VCMCTL,
> > +		.bitmask = BIT(0),
> > +		/* VCMCTL */
> > +	},
> > +	{
> > +		.address = 0x08,
> > +		.reg = TPS68470_REG_VAUX1CTL,
> > +		.bitmask = BIT(0),
> > +		/* VAUX1_CTL */
> > +	},
> > +	{
> > +		.address = 0x0C,
> > +		.reg = TPS68470_REG_VAUX2CTL,
> > +		.bitmask = BIT(0),
> > +		/* VAUX2CTL */
> > +	},
> > +	{
> > +		.address = 0x10,
> > +		.reg = TPS68470_REG_VACTL,
> > +		.bitmask = BIT(0),
> > +		/* VACTL */
> > +	},
> > +	{
> > +		.address = 0x14,
> > +		.reg = TPS68470_REG_VDCTL,
> > +		.bitmask = BIT(0),
> > +		/* VDCTL */
> > +	},
> > +};
> > +
> > +/* Table to set voltage regulator value */ static const struct
> > +ti_pmic_table vr_val_table[] = {
> > +	{
> > +		.address = 0x00,
> > +		.reg = TPS68470_REG_VSIOVAL,
> > +		.bitmask = TPS68470_VSIOVAL_IOVOLT_MASK,
> > +		/* TPS68470_REG_VSIOVAL */
> > +	},
> > +	{
> > +		.address = 0x04,
> > +		.reg = TPS68470_REG_VIOVAL,
> > +		.bitmask = TPS68470_VIOVAL_IOVOLT_MASK,
> > +		/* TPS68470_REG_VIOVAL */
> > +	},
> > +	{
> > +		.address = 0x08,
> > +		.reg = TPS68470_REG_VCMVAL,
> > +		.bitmask = TPS68470_VCMVAL_VCVOLT_MASK,
> > +		/* TPS68470_REG_VCMVAL */
> > +	},
> > +	{
> > +		.address = 0x0C,
> > +		.reg = TPS68470_REG_VAUX1VAL,
> > +		.bitmask = TPS68470_VAUX1VAL_AUX1VOLT_MASK,
> > +		/* TPS68470_REG_VAUX1VAL */
> > +	},
> > +	{
> > +		.address = 0x10,
> > +		.reg = TPS68470_REG_VAUX2VAL,
> > +		.bitmask = TPS68470_VAUX2VAL_AUX2VOLT_MASK,
> > +		/* TPS68470_REG_VAUX2VAL */
> > +	},
> > +	{
> > +		.address = 0x14,
> > +		.reg = TPS68470_REG_VAVAL,
> > +		.bitmask = TPS68470_VAVAL_AVOLT_MASK,
> > +		/* TPS68470_REG_VAVAL */
> > +	},
> > +	{
> > +		.address = 0x18,
> > +		.reg = TPS68470_REG_VDVAL,
> > +		.bitmask = TPS68470_VDVAL_DVOLT_MASK,
> > +		/* TPS68470_REG_VDVAL */
> > +	},
> > +};
> > +
> > +/* Table to configure clock frequency */ static const struct
> > +ti_pmic_table clk_freq_table[] = {
> > +	{
> > +		.address = 0x00,
> > +		.reg = TPS68470_REG_POSTDIV2,
> > +		.bitmask = BIT(0) | BIT(1),
> > +		/* TPS68470_REG_POSTDIV2 */
> > +	},
> > +	{
> > +		.address = 0x04,
> > +		.reg = TPS68470_REG_BOOSTDIV,
> > +		.bitmask = 0x1F,
> > +		/* TPS68470_REG_BOOSTDIV */
> > +	},
> > +	{
> > +		.address = 0x08,
> > +		.reg = TPS68470_REG_BUCKDIV,
> > +		.bitmask = 0x0F,
> > +		/* TPS68470_REG_BUCKDIV */
> > +	},
> > +	{
> > +		.address = 0x0C,
> > +		.reg = TPS68470_REG_PLLSWR,
> > +		.bitmask = 0x13,
> > +		/* TPS68470_REG_PLLSWR */
> > +	},
> > +	{
> > +		.address = 0x10,
> > +		.reg = TPS68470_REG_XTALDIV,
> > +		.bitmask = 0xFF,
> > +		/* TPS68470_REG_XTALDIV */
> > +	},
> > +	{
> > +		.address = 0x14,
> > +		.reg = TPS68470_REG_PLLDIV,
> > +		.bitmask = 0xFF,
> > +		/* TPS68470_REG_PLLDIV */
> > +	},
> > +	{
> > +		.address = 0x18,
> > +		.reg = TPS68470_REG_POSTDIV,
> > +		.bitmask = 0x83,
> > +		/* TPS68470_REG_POSTDIV */
> > +	},
> > +};
> > +
> > +/* Table to configure and enable clocks */ static const struct
> > +ti_pmic_table clk_table[] = {
> > +	{
> > +		.address = 0x00,
> > +		.reg = TPS68470_REG_PLLCTL,
> > +		.bitmask = 0xF5,
> > +		/* TPS68470_REG_PLLCTL */
> > +	},
> > +	{
> > +		.address = 0x04,
> > +		.reg = TPS68470_REG_PLLCTL2,
> > +		.bitmask = BIT(0),
> > +		/* TPS68470_REG_PLLCTL2 */
> > +	},
> > +	{
> > +		.address = 0x08,
> > +		.reg = TPS68470_REG_CLKCFG1,
> > +		.bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
> > +			TPS68470_CLKCFG1_MODE_B_MASK,
> > +		/* TPS68470_REG_CLKCFG1 */
> > +	},
> > +	{
> > +		.address = 0x0C,
> > +		.reg = TPS68470_REG_CLKCFG2,
> > +		.bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
> > +			TPS68470_CLKCFG1_MODE_B_MASK,
> > +		/* TPS68470_REG_CLKCFG2 */
> > +	},
> > +};
> > +
> > +static int pmic_get_reg_bit(u64 address, struct ti_pmic_table *table,
> > +			    int count, int *reg, int *bitmask)
> 
> unsigned int count?
> 

Ack
I will also rename the variable to be table_size, so it's more meaningful.

> > +{
> > +	u64 i;
> > +
> > +	i = address / 4;
> > +
> > +	if (i >= count)
> > +		return -ENOENT;
> > +
> > +	if (!reg || !bitmask)
> > +		return -EINVAL;
> > +
> > +	*reg = table[i].reg;
> > +	*bitmask = table[i].bitmask;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg,
> > +				       int bitmask, u64 *value)
> > +{
> > +	int data;
> 
> Shouldn't you use unsigned int here? Same in the functions below.
> 

Ack

> > +
> > +	if (regmap_read(regmap, reg, &data))
> > +		return -EIO;
> > +
> > +	*value = (data & bitmask) ? 1 : 0;
> > +	return 0;
> > +}
> > +
> > +static int ti_tps68470_pmic_get_vr_val(struct regmap *regmap, int reg,
> > +				       int bitmask, u64 *value)
> > +{
> > +	int data;
> > +
> > +	if (regmap_read(regmap, reg, &data))
> > +		return -EIO;
> > +
> > +	*value = data & bitmask;
> > +	return 0;
> > +}
> > +
> > +static int ti_tps68470_pmic_get_clk(struct regmap *regmap, int reg,
> > +				       int bitmask, u64 *value)
> > +{
> > +	int data;
> > +
> > +	if (regmap_read(regmap, reg, &data))
> > +		return -EIO;
> > +
> > +	*value = (data & bitmask) ? 1 : 0;
> > +	return 0;
> > +}
> > +
> > +static int ti_tps68470_pmic_get_clk_freq(struct regmap *regmap, int reg,
> > +				       int bitmask, u64 *value)
> > +{
> > +	int data;
> > +
> > +	if (regmap_read(regmap, reg, &data))
> > +		return -EIO;
> > +
> > +	*value = data & bitmask;
> > +	return 0;
> > +}
> > +
> > +static int ti_tps68470_regmap_update_bits(struct regmap *regmap, int reg,
> > +					int bitmask, u64 value)
> > +{
> > +	return regmap_update_bits(regmap, reg, bitmask, value); }
> > +
> > +static acpi_status ti_pmic_common_handler(u32 function,
> > +					  acpi_physical_address address,
> > +					  u32 bits, u64 *value,
> > +					  void *handler_context,
> 
> handler_context is unused.
> 

Ack

> > +					  void *region_context,
> > +					  int (*get)(struct regmap *,
> > +						     int, int, u64 *),
> > +					  int (*update)(struct regmap *,
> > +							int, int, u64),
> > +					  struct ti_pmic_table *table,
> > +					  int table_size)
> 
> unsigned int. (Or size_t or whatever.)
> 

Ack

> > +{
> > +	struct ti_pmic_opregion *opregion = region_context;
> > +	struct regmap *regmap = opregion->regmap;
> > +	int reg, ret, bitmask;
> > +
> > +	if (bits != 32)
> > +		return AE_BAD_PARAMETER;
> > +
> > +	ret = pmic_get_reg_bit(address, table,
> > +				  table_size, &reg, &bitmask);
> > +	if (ret < 0)
> > +		return AE_BAD_PARAMETER;
> > +
> > +	if (function == ACPI_WRITE && (*value > bitmask))
> 
> Extra parentheses.
> 

Ack

> > +		return AE_BAD_PARAMETER;
> > +
> > +	mutex_lock(&opregion->lock);
> > +
> > +	ret = (function == ACPI_READ) ?
> > +		get(regmap, reg, bitmask, value) :
> > +		update(regmap, reg, bitmask, *value);
> > +
> > +	mutex_unlock(&opregion->lock);
> > +
> > +	return ret ? AE_ERROR : AE_OK;
> > +}
> > +
> > +static acpi_status ti_pmic_clk_freq_handler(u32 function,
> > +					    acpi_physical_address address,
> > +					    u32 bits, u64 *value,
> > +					    void *handler_context,
> > +					    void *region_context)
> > +{
> > +	return ti_pmic_common_handler(function, address, bits, value,
> > +				handler_context, region_context,
> > +				ti_tps68470_pmic_get_clk_freq,
> > +				ti_tps68470_regmap_update_bits,
> > +				(struct ti_pmic_table *) &clk_freq_table,
> 
> You shouldn't use an explicit cast here. Instead make the function argument
> const as well and you're fine.
> 

Ack

> > +				ARRAY_SIZE(clk_freq_table));
> > +}
> > +
> > +static acpi_status ti_pmic_clk_handler(u32 function,
> > +				       acpi_physical_address address, u32 bits,
> > +				       u64 *value, void *handler_context,
> > +				       void *region_context)
> > +{
> > +	return ti_pmic_common_handler(function, address, bits, value,
> > +				handler_context, region_context,
> > +				ti_tps68470_pmic_get_clk,
> > +				ti_tps68470_regmap_update_bits,
> > +				(struct ti_pmic_table *) &clk_table,
> > +				 ARRAY_SIZE(clk_table));
> > +}
> > +
> > +static acpi_status ti_pmic_vr_val_handler(u32 function,
> > +					  acpi_physical_address address,
> > +					  u32 bits, u64 *value,
> > +					  void *handler_context,
> > +					  void *region_context)
> > +{
> > +	return ti_pmic_common_handler(function, address, bits, value,
> > +				handler_context, region_context,
> > +				ti_tps68470_pmic_get_vr_val,
> > +				ti_tps68470_regmap_update_bits,
> > +				(struct ti_pmic_table *) &vr_val_table,
> > +				ARRAY_SIZE(vr_val_table));
> > +}
> > +
> > +static acpi_status ti_pmic_power_handler(u32 function,
> > +					 acpi_physical_address address,
> > +					 u32 bits, u64 *value,
> > +					 void *handler_context,
> > +					 void *region_context)
> > +{
> > +	if (bits != 32)
> > +		return AE_BAD_PARAMETER;
> > +
> > +	/* set/clear for bit 0, bits 0 and 1 together */
> > +	if (function == ACPI_WRITE &&
> > +	    !(*value == 0 || *value == 1 || *value == 3)) {
> > +		return AE_BAD_PARAMETER;
> > +	}
> > +
> > +	return ti_pmic_common_handler(function, address, bits, value,
> > +				handler_context, region_context,
> > +				ti_tps68470_pmic_get_power,
> > +				ti_tps68470_regmap_update_bits,
> > +				(struct ti_pmic_table *) &power_table,
> > +				ARRAY_SIZE(power_table));
> > +}
> > +
> > +static int ti_tps68470_pmic_opregion_probe(struct platform_device
> > +*pdev) {
> > +	struct tps68470 *pmic = dev_get_drvdata(pdev->dev.parent);
> > +	acpi_handle handle = ACPI_HANDLE(pdev->dev.parent);
> > +	struct device *dev = &pdev->dev;
> > +	struct ti_pmic_opregion *opregion;
> > +	acpi_status status;
> > +
> > +	if (!dev || !pmic->regmap) {
> > +		WARN(1, "dev or regmap is NULL\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!handle) {
> > +		WARN(1, "acpi handle is NULL\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	opregion = devm_kzalloc(dev, sizeof(*opregion), GFP_KERNEL);
> > +	if (!opregion)
> > +		return -ENOMEM;
> > +
> > +	mutex_init(&opregion->lock);
> > +	opregion->regmap = pmic->regmap;
> > +
> > +	status = acpi_install_address_space_handler(handle,
> > +
> TI_PMIC_POWER_OPREGION_ID,
> > +						    ti_pmic_power_handler,
> > +						    NULL, opregion);
> > +	if (ACPI_FAILURE(status))
> 
> mutex_destroy() after mutex_init() --- please add a label for this.
> 
> > +		return -ENODEV;
> > +
> > +	status = acpi_install_address_space_handler(handle,
> > +
> TI_PMIC_VR_VAL_OPREGION_ID,
> > +						    ti_pmic_vr_val_handler,
> > +						    NULL, opregion);
> > +	if (ACPI_FAILURE(status))
> > +		goto out_remove_power_handler;
> > +
> > +	status = acpi_install_address_space_handler(handle,
> > +
> TI_PMIC_CLOCK_OPREGION_ID,
> > +						    ti_pmic_clk_handler,
> > +						    NULL, opregion);
> > +	if (ACPI_FAILURE(status))
> > +		goto out_remove_vr_val_handler;
> > +
> > +	status = acpi_install_address_space_handler(handle,
> > +
> TI_PMIC_CLKFREQ_OPREGION_ID,
> > +						    ti_pmic_clk_freq_handler,
> > +						    NULL, opregion);
> > +	if (ACPI_FAILURE(status))
> > +		goto out_remove_clk_handler;
> > +
> > +	return 0;
> > +
> > +out_remove_clk_handler:
> > +	acpi_remove_address_space_handler(handle,
> TI_PMIC_CLOCK_OPREGION_ID,
> > +					  ti_pmic_clk_handler);
> > +out_remove_vr_val_handler:
> > +	acpi_remove_address_space_handler(handle,
> TI_PMIC_VR_VAL_OPREGION_ID,
> > +					  ti_pmic_vr_val_handler);
> > +out_remove_power_handler:
> > +	acpi_remove_address_space_handler(handle,
> TI_PMIC_POWER_OPREGION_ID,
> > +					  ti_pmic_power_handler);
> > +	return -ENODEV;
> > +}
> > +
> > +static struct platform_driver ti_tps68470_pmic_opregion_driver = {
> > +	.probe = ti_tps68470_pmic_opregion_probe,
> > +	.driver = {
> > +		.name = "tps68470_pmic_opregion",
> > +	},
> > +};
> > +
> > +builtin_platform_driver(ti_tps68470_pmic_opregion_driver)
> 
> --
> Kind regards,
> 
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mani, Rajmohan June 10, 2017, 12:07 a.m. UTC | #17
Hi Sakari, Andy,

> Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation
> region driver
> 
> On Wed, Jun 07, 2017 at 04:37:12PM +0300, Andy Shevchenko wrote:
> > >> +static acpi_status ti_pmic_common_handler(u32 function,
> > > +                                       acpi_physical_address address,
> > > +                                       u32 bits, u64 *value,
> > > +                                       void *handler_context,
> >
> > > handler_context is unused.
> >
> > >> +                                                  int, int, u64 *),
> > >> +                                       int (*update)(struct regmap *,
> > >> +                                                     int, int, u64),
> > >> +                                       struct ti_pmic_table *table,
> > >> +                                       int table_size)
> >
> > I would even split this to have separate update() and get() paths
> > instead of having such a monster of parameters.
> 
> I'm not really worried about the two callbacks --- you have the compexity,
> which is agruably rather manageable, split into a number of caller functions. I'd
> rather keep it as-is.
> 

Ack

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mani, Rajmohan June 10, 2017, 12:09 a.m. UTC | #18
SGkgQW5keSwNCg0KVGhhbmtzIGZvciB0aGUgcmV2aWV3cyBhbmQgcGF0aWVuY2UuDQoNCj4gU3Vi
amVjdDogUmU6IFtQQVRDSCB2MSAzLzNdIEFDUEkgLyBQTUlDOiBBZGQgVEkgUE1JQyBUUFM2ODQ3
MCBvcGVyYXRpb24NCj4gcmVnaW9uIGRyaXZlcg0KPiANCj4gT24gV2VkLCBKdW4gNywgMjAxNyBh
dCAzOjA3IFBNLCBTYWthcmkgQWlsdXMgPHNha2FyaS5haWx1c0Bpa2kuZmk+IHdyb3RlOg0KPiAN
Cj4gPj4gK3N0YXRpYyBpbnQgdGlfdHBzNjg0NzBfcG1pY19nZXRfcG93ZXIoc3RydWN0IHJlZ21h
cCAqcmVnbWFwLCBpbnQgcmVnLA0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgaW50IGJpdG1hc2ssIHU2NCAqdmFsdWUpIHsNCj4gPj4gKyAgICAgaW50IGRhdGE7DQo+
ID4NCj4gPiBTaG91bGRuJ3QgeW91IHVzZSB1bnNpZ25lZCBpbnQgaGVyZT8gU2FtZSBpbiB0aGUg
ZnVuY3Rpb25zIGJlbG93Lg0KPiANCj4gKzEsIHJlZ21hcF9yZWFkKCkgcmV0dXJucyB1bnNpZ25l
ZCBpbnQuDQo+IA0KDQpBY2sNCg0KPiA+PiArc3RhdGljIGFjcGlfc3RhdHVzIHRpX3BtaWNfY29t
bW9uX2hhbmRsZXIodTMyIGZ1bmN0aW9uLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICBhY3BpX3BoeXNpY2FsX2FkZHJlc3MgYWRkcmVzcywNCj4gPiArICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdTMyIGJpdHMsIHU2NCAqdmFsdWUsDQo+
ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHZvaWQgKmhhbmRsZXJf
Y29udGV4dCwNCj4gDQo+ID4gaGFuZGxlcl9jb250ZXh0IGlzIHVudXNlZC4NCj4gDQoNCkFjaw0K
DQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
IGludCwgaW50LCB1NjQgKiksDQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICBpbnQgKCp1cGRhdGUpKHN0cnVjdCByZWdtYXAgKiwNCj4gPj4gKyAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaW50LCBpbnQsIHU2NCks
DQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgdGlf
cG1pY190YWJsZSAqdGFibGUsDQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICBpbnQgdGFibGVfc2l6ZSkNCj4gDQo+IEkgd291bGQgZXZlbiBzcGxpdCB0aGlzIHRv
IGhhdmUgc2VwYXJhdGUgdXBkYXRlKCkgYW5kIGdldCgpIHBhdGhzIGluc3RlYWQgb2YNCj4gaGF2
aW5nIHN1Y2ggYSBtb25zdGVyIG9mIHBhcmFtZXRlcnMuDQo+IA0KDQpJIGhhdmUgcmVzcG9uZGVk
IG9uIHRvcCBvZiBTYWthcmkncyByZXNwb25zZS4NCg0KPiA+PiArc3RhdGljIGFjcGlfc3RhdHVz
IHRpX3BtaWNfY2xrX2ZyZXFfaGFuZGxlcih1MzIgZnVuY3Rpb24sDQo+ID4+ICsgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGFjcGlfcGh5c2ljYWxfYWRkcmVzcyBhZGRy
ZXNzLA0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB1MzIg
Yml0cywgdTY0ICp2YWx1ZSwNCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgdm9pZCAqaGFuZGxlcl9jb250ZXh0LA0KPiA+PiArICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICB2b2lkICpyZWdpb25fY29udGV4dCkgew0KPiA+PiArICAg
ICByZXR1cm4gdGlfcG1pY19jb21tb25faGFuZGxlcihmdW5jdGlvbiwgYWRkcmVzcywgYml0cywg
dmFsdWUsDQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGhhbmRsZXJfY29udGV4
dCwgcmVnaW9uX2NvbnRleHQsDQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHRp
X3RwczY4NDcwX3BtaWNfZ2V0X2Nsa19mcmVxLA0KPiA+PiArICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICB0aV90cHM2ODQ3MF9yZWdtYXBfdXBkYXRlX2JpdHMsDQo+ID4+ICsgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgIChzdHJ1Y3QgdGlfcG1pY190YWJsZSAqKQ0KPiA+PiArJmNsa19m
cmVxX3RhYmxlLA0KPiA+DQo+ID4gWW91IHNob3VsZG4ndCB1c2UgYW4gZXhwbGljaXQgY2FzdCBo
ZXJlLiBJbnN0ZWFkIG1ha2UgdGhlIGZ1bmN0aW9uDQo+ID4gYXJndW1lbnQgY29uc3QgYXMgd2Vs
bCBhbmQgeW91J3JlIGZpbmUuDQo+IA0KPiArMS4NCj4gDQoNCkFjaw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mani, Rajmohan June 10, 2017, 12:10 a.m. UTC | #19
Hi Sakari, Andy,

> Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation
> region driver
> 
> On Wed, Jun 07, 2017 at 11:40:27PM +0300, Andy Shevchenko wrote:
> > On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > > On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote:
> > >> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > >> > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote:
> >
> > >> >> Follow the pattern, please, I suppose ti_pmic_tps68470.c
> > >>
> > >> Would it work for you?
> > >
> > > This is still a different driver from the tps68470 driver which is
> > > an MFD driver. For clarity, I'd keep pmic as part of the name (and
> > > I'd use tps68470_pmic_ prefix for internal symbols, too).
> > >
> > > As PMICs are typically linked to the kernel (vs. being modules),
> > > there's no issue with the module name. I would suppose few if any
> > > PMICs will be compiled as modules in general.
> > >
> > > It's not a big deal though. I'm fine either way.
> >
> > Okay, let's agree on  tps68470_pmic for internal prefix and for file name?
> 
> Ack. Thanks!
> 

Ack
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1ce52f8..218d22d 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -535,4 +535,16 @@  if ARM64
 source "drivers/acpi/arm64/Kconfig"
 endif
 
+config TPS68470_PMIC_OPREGION
+	bool "ACPI operation region support for TPS68470 PMIC"
+	help
+	  This config adds ACPI operation region support for TI TPS68470 PMIC.
+	  TPS68470 device is an advanced power management unit that powers
+	  a Compact Camera Module (CCM), generates clocks for image sensors,
+	  drives a dual LED for flash and incorporates two LED drivers for
+	  general purpose indicators.
+	  This driver enables ACPI operation region support control voltage
+	  regulators and clocks.
+
+
 endif	# ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index b1aacfc..7113d05 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -106,6 +106,8 @@  obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o
 
 obj-$(CONFIG_ACPI_CONFIGFS)	+= acpi_configfs.o
 
+obj-$(CONFIG_TPS68470_PMIC_OPREGION)	+= pmic/pmic_tps68470.o
+
 video-objs			+= acpi_video.o video_detect.o
 obj-y				+= dptf/
 
diff --git a/drivers/acpi/pmic/pmic_tps68470.c b/drivers/acpi/pmic/pmic_tps68470.c
new file mode 100644
index 0000000..b2d608b
--- /dev/null
+++ b/drivers/acpi/pmic/pmic_tps68470.c
@@ -0,0 +1,454 @@ 
+/*
+ * TI TPS68470 PMIC operation region driver
+ *
+ * Copyright (C) 2017 Intel Corporation. All rights reserved.
+ * Author: Rajmohan Mani <rajmohan.mani@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Based on drivers/acpi/pmic/intel_pmic* drivers
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/mfd/tps68470.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct ti_pmic_table {
+	u32 address;		/* operation region address */
+	u32 reg;		/* corresponding register */
+	u32 bitmask;		/* bit mask for power, clock */
+};
+
+#define TI_PMIC_POWER_OPREGION_ID		0xB0
+#define TI_PMIC_VR_VAL_OPREGION_ID		0xB1
+#define TI_PMIC_CLOCK_OPREGION_ID		0xB2
+#define TI_PMIC_CLKFREQ_OPREGION_ID		0xB3
+
+struct ti_pmic_opregion {
+	struct mutex lock;
+	struct regmap *regmap;
+};
+
+#define S_IO_I2C_EN	(BIT(0) | BIT(1))
+
+static const struct ti_pmic_table power_table[] = {
+	{
+		.address = 0x00,
+		.reg = TPS68470_REG_S_I2C_CTL,
+		.bitmask = S_IO_I2C_EN,
+		/* S_I2C_CTL */
+	},
+	{
+		.address = 0x04,
+		.reg = TPS68470_REG_VCMCTL,
+		.bitmask = BIT(0),
+		/* VCMCTL */
+	},
+	{
+		.address = 0x08,
+		.reg = TPS68470_REG_VAUX1CTL,
+		.bitmask = BIT(0),
+		/* VAUX1_CTL */
+	},
+	{
+		.address = 0x0C,
+		.reg = TPS68470_REG_VAUX2CTL,
+		.bitmask = BIT(0),
+		/* VAUX2CTL */
+	},
+	{
+		.address = 0x10,
+		.reg = TPS68470_REG_VACTL,
+		.bitmask = BIT(0),
+		/* VACTL */
+	},
+	{
+		.address = 0x14,
+		.reg = TPS68470_REG_VDCTL,
+		.bitmask = BIT(0),
+		/* VDCTL */
+	},
+};
+
+/* Table to set voltage regulator value */
+static const struct ti_pmic_table vr_val_table[] = {
+	{
+		.address = 0x00,
+		.reg = TPS68470_REG_VSIOVAL,
+		.bitmask = TPS68470_VSIOVAL_IOVOLT_MASK,
+		/* TPS68470_REG_VSIOVAL */
+	},
+	{
+		.address = 0x04,
+		.reg = TPS68470_REG_VIOVAL,
+		.bitmask = TPS68470_VIOVAL_IOVOLT_MASK,
+		/* TPS68470_REG_VIOVAL */
+	},
+	{
+		.address = 0x08,
+		.reg = TPS68470_REG_VCMVAL,
+		.bitmask = TPS68470_VCMVAL_VCVOLT_MASK,
+		/* TPS68470_REG_VCMVAL */
+	},
+	{
+		.address = 0x0C,
+		.reg = TPS68470_REG_VAUX1VAL,
+		.bitmask = TPS68470_VAUX1VAL_AUX1VOLT_MASK,
+		/* TPS68470_REG_VAUX1VAL */
+	},
+	{
+		.address = 0x10,
+		.reg = TPS68470_REG_VAUX2VAL,
+		.bitmask = TPS68470_VAUX2VAL_AUX2VOLT_MASK,
+		/* TPS68470_REG_VAUX2VAL */
+	},
+	{
+		.address = 0x14,
+		.reg = TPS68470_REG_VAVAL,
+		.bitmask = TPS68470_VAVAL_AVOLT_MASK,
+		/* TPS68470_REG_VAVAL */
+	},
+	{
+		.address = 0x18,
+		.reg = TPS68470_REG_VDVAL,
+		.bitmask = TPS68470_VDVAL_DVOLT_MASK,
+		/* TPS68470_REG_VDVAL */
+	},
+};
+
+/* Table to configure clock frequency */
+static const struct ti_pmic_table clk_freq_table[] = {
+	{
+		.address = 0x00,
+		.reg = TPS68470_REG_POSTDIV2,
+		.bitmask = BIT(0) | BIT(1),
+		/* TPS68470_REG_POSTDIV2 */
+	},
+	{
+		.address = 0x04,
+		.reg = TPS68470_REG_BOOSTDIV,
+		.bitmask = 0x1F,
+		/* TPS68470_REG_BOOSTDIV */
+	},
+	{
+		.address = 0x08,
+		.reg = TPS68470_REG_BUCKDIV,
+		.bitmask = 0x0F,
+		/* TPS68470_REG_BUCKDIV */
+	},
+	{
+		.address = 0x0C,
+		.reg = TPS68470_REG_PLLSWR,
+		.bitmask = 0x13,
+		/* TPS68470_REG_PLLSWR */
+	},
+	{
+		.address = 0x10,
+		.reg = TPS68470_REG_XTALDIV,
+		.bitmask = 0xFF,
+		/* TPS68470_REG_XTALDIV */
+	},
+	{
+		.address = 0x14,
+		.reg = TPS68470_REG_PLLDIV,
+		.bitmask = 0xFF,
+		/* TPS68470_REG_PLLDIV */
+	},
+	{
+		.address = 0x18,
+		.reg = TPS68470_REG_POSTDIV,
+		.bitmask = 0x83,
+		/* TPS68470_REG_POSTDIV */
+	},
+};
+
+/* Table to configure and enable clocks */
+static const struct ti_pmic_table clk_table[] = {
+	{
+		.address = 0x00,
+		.reg = TPS68470_REG_PLLCTL,
+		.bitmask = 0xF5,
+		/* TPS68470_REG_PLLCTL */
+	},
+	{
+		.address = 0x04,
+		.reg = TPS68470_REG_PLLCTL2,
+		.bitmask = BIT(0),
+		/* TPS68470_REG_PLLCTL2 */
+	},
+	{
+		.address = 0x08,
+		.reg = TPS68470_REG_CLKCFG1,
+		.bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
+			TPS68470_CLKCFG1_MODE_B_MASK,
+		/* TPS68470_REG_CLKCFG1 */
+	},
+	{
+		.address = 0x0C,
+		.reg = TPS68470_REG_CLKCFG2,
+		.bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
+			TPS68470_CLKCFG1_MODE_B_MASK,
+		/* TPS68470_REG_CLKCFG2 */
+	},
+};
+
+static int pmic_get_reg_bit(u64 address, struct ti_pmic_table *table,
+			    int count, int *reg, int *bitmask)
+{
+	u64 i;
+
+	i = address / 4;
+
+	if (i >= count)
+		return -ENOENT;
+
+	if (!reg || !bitmask)
+		return -EINVAL;
+
+	*reg = table[i].reg;
+	*bitmask = table[i].bitmask;
+
+	return 0;
+}
+
+static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg,
+				       int bitmask, u64 *value)
+{
+	int data;
+
+	if (regmap_read(regmap, reg, &data))
+		return -EIO;
+
+	*value = (data & bitmask) ? 1 : 0;
+	return 0;
+}
+
+static int ti_tps68470_pmic_get_vr_val(struct regmap *regmap, int reg,
+				       int bitmask, u64 *value)
+{
+	int data;
+
+	if (regmap_read(regmap, reg, &data))
+		return -EIO;
+
+	*value = data & bitmask;
+	return 0;
+}
+
+static int ti_tps68470_pmic_get_clk(struct regmap *regmap, int reg,
+				       int bitmask, u64 *value)
+{
+	int data;
+
+	if (regmap_read(regmap, reg, &data))
+		return -EIO;
+
+	*value = (data & bitmask) ? 1 : 0;
+	return 0;
+}
+
+static int ti_tps68470_pmic_get_clk_freq(struct regmap *regmap, int reg,
+				       int bitmask, u64 *value)
+{
+	int data;
+
+	if (regmap_read(regmap, reg, &data))
+		return -EIO;
+
+	*value = data & bitmask;
+	return 0;
+}
+
+static int ti_tps68470_regmap_update_bits(struct regmap *regmap, int reg,
+					int bitmask, u64 value)
+{
+	return regmap_update_bits(regmap, reg, bitmask, value);
+}
+
+static acpi_status ti_pmic_common_handler(u32 function,
+					  acpi_physical_address address,
+					  u32 bits, u64 *value,
+					  void *handler_context,
+					  void *region_context,
+					  int (*get)(struct regmap *,
+						     int, int, u64 *),
+					  int (*update)(struct regmap *,
+							int, int, u64),
+					  struct ti_pmic_table *table,
+					  int table_size)
+{
+	struct ti_pmic_opregion *opregion = region_context;
+	struct regmap *regmap = opregion->regmap;
+	int reg, ret, bitmask;
+
+	if (bits != 32)
+		return AE_BAD_PARAMETER;
+
+	ret = pmic_get_reg_bit(address, table,
+				  table_size, &reg, &bitmask);
+	if (ret < 0)
+		return AE_BAD_PARAMETER;
+
+	if (function == ACPI_WRITE && (*value > bitmask))
+		return AE_BAD_PARAMETER;
+
+	mutex_lock(&opregion->lock);
+
+	ret = (function == ACPI_READ) ?
+		get(regmap, reg, bitmask, value) :
+		update(regmap, reg, bitmask, *value);
+
+	mutex_unlock(&opregion->lock);
+
+	return ret ? AE_ERROR : AE_OK;
+}
+
+static acpi_status ti_pmic_clk_freq_handler(u32 function,
+					    acpi_physical_address address,
+					    u32 bits, u64 *value,
+					    void *handler_context,
+					    void *region_context)
+{
+	return ti_pmic_common_handler(function, address, bits, value,
+				handler_context, region_context,
+				ti_tps68470_pmic_get_clk_freq,
+				ti_tps68470_regmap_update_bits,
+				(struct ti_pmic_table *) &clk_freq_table,
+				ARRAY_SIZE(clk_freq_table));
+}
+
+static acpi_status ti_pmic_clk_handler(u32 function,
+				       acpi_physical_address address, u32 bits,
+				       u64 *value, void *handler_context,
+				       void *region_context)
+{
+	return ti_pmic_common_handler(function, address, bits, value,
+				handler_context, region_context,
+				ti_tps68470_pmic_get_clk,
+				ti_tps68470_regmap_update_bits,
+				(struct ti_pmic_table *) &clk_table,
+				 ARRAY_SIZE(clk_table));
+}
+
+static acpi_status ti_pmic_vr_val_handler(u32 function,
+					  acpi_physical_address address,
+					  u32 bits, u64 *value,
+					  void *handler_context,
+					  void *region_context)
+{
+	return ti_pmic_common_handler(function, address, bits, value,
+				handler_context, region_context,
+				ti_tps68470_pmic_get_vr_val,
+				ti_tps68470_regmap_update_bits,
+				(struct ti_pmic_table *) &vr_val_table,
+				ARRAY_SIZE(vr_val_table));
+}
+
+static acpi_status ti_pmic_power_handler(u32 function,
+					 acpi_physical_address address,
+					 u32 bits, u64 *value,
+					 void *handler_context,
+					 void *region_context)
+{
+	if (bits != 32)
+		return AE_BAD_PARAMETER;
+
+	/* set/clear for bit 0, bits 0 and 1 together */
+	if (function == ACPI_WRITE &&
+	    !(*value == 0 || *value == 1 || *value == 3)) {
+		return AE_BAD_PARAMETER;
+	}
+
+	return ti_pmic_common_handler(function, address, bits, value,
+				handler_context, region_context,
+				ti_tps68470_pmic_get_power,
+				ti_tps68470_regmap_update_bits,
+				(struct ti_pmic_table *) &power_table,
+				ARRAY_SIZE(power_table));
+}
+
+static int ti_tps68470_pmic_opregion_probe(struct platform_device *pdev)
+{
+	struct tps68470 *pmic = dev_get_drvdata(pdev->dev.parent);
+	acpi_handle handle = ACPI_HANDLE(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct ti_pmic_opregion *opregion;
+	acpi_status status;
+
+	if (!dev || !pmic->regmap) {
+		WARN(1, "dev or regmap is NULL\n");
+		return -EINVAL;
+	}
+
+	if (!handle) {
+		WARN(1, "acpi handle is NULL\n");
+		return -ENODEV;
+	}
+
+	opregion = devm_kzalloc(dev, sizeof(*opregion), GFP_KERNEL);
+	if (!opregion)
+		return -ENOMEM;
+
+	mutex_init(&opregion->lock);
+	opregion->regmap = pmic->regmap;
+
+	status = acpi_install_address_space_handler(handle,
+						    TI_PMIC_POWER_OPREGION_ID,
+						    ti_pmic_power_handler,
+						    NULL, opregion);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	status = acpi_install_address_space_handler(handle,
+						    TI_PMIC_VR_VAL_OPREGION_ID,
+						    ti_pmic_vr_val_handler,
+						    NULL, opregion);
+	if (ACPI_FAILURE(status))
+		goto out_remove_power_handler;
+
+	status = acpi_install_address_space_handler(handle,
+						    TI_PMIC_CLOCK_OPREGION_ID,
+						    ti_pmic_clk_handler,
+						    NULL, opregion);
+	if (ACPI_FAILURE(status))
+		goto out_remove_vr_val_handler;
+
+	status = acpi_install_address_space_handler(handle,
+						    TI_PMIC_CLKFREQ_OPREGION_ID,
+						    ti_pmic_clk_freq_handler,
+						    NULL, opregion);
+	if (ACPI_FAILURE(status))
+		goto out_remove_clk_handler;
+
+	return 0;
+
+out_remove_clk_handler:
+	acpi_remove_address_space_handler(handle, TI_PMIC_CLOCK_OPREGION_ID,
+					  ti_pmic_clk_handler);
+out_remove_vr_val_handler:
+	acpi_remove_address_space_handler(handle, TI_PMIC_VR_VAL_OPREGION_ID,
+					  ti_pmic_vr_val_handler);
+out_remove_power_handler:
+	acpi_remove_address_space_handler(handle, TI_PMIC_POWER_OPREGION_ID,
+					  ti_pmic_power_handler);
+	return -ENODEV;
+}
+
+static struct platform_driver ti_tps68470_pmic_opregion_driver = {
+	.probe = ti_tps68470_pmic_opregion_probe,
+	.driver = {
+		.name = "tps68470_pmic_opregion",
+	},
+};
+
+builtin_platform_driver(ti_tps68470_pmic_opregion_driver)