Re: [v2 3/4] regulator: qcom: Add labibb driver

2020-05-27 Thread Mark Brown
On Wed, May 27, 2020 at 10:01:27PM +0530, Sumit Semwal wrote:
> On Thu, 14 May 2020 at 16:57, Sumit Semwal  wrote:

> > > If this is useful factor it out into a helper or the core, other devices
> > > also have status bits saying if the regulator is enabled.  It looks like
> > > this may be mainly trying to open code something like enable_time, with
> > > possibly some issues where the time taken to enable varies a lot.

> > Makes sense; I am not terribly familiar with the regulator core and
> > helpers, so let me look and refactor accordingly.

> Does something like this make sense, or did I misunderstand your
> suggestion completely? I'll send the updated patches accordingly.

I guess.


signature.asc
Description: PGP signature


Re: [v2 3/4] regulator: qcom: Add labibb driver

2020-05-27 Thread Sumit Semwal
Hello Mark,

On Thu, 14 May 2020 at 16:57, Sumit Semwal  wrote:
>
> Hello Mark,
>
> Thank you for your review comments!
> On Mon, 11 May 2020 at 16:09, Mark Brown  wrote:
> >
> > On Sat, May 09, 2020 at 02:11:59AM +0530, Sumit Semwal wrote:
> >
> > > + ret = regmap_bulk_read(reg->regmap, reg->base +
> > > +REG_LABIBB_STATUS1, , 1);
> > > + if (ret < 0) {
> > > + dev_err(reg->dev, "Read register failed ret = %d\n", ret);
> > > + return ret;
> > > + }
> >
> > Why a bulk read of a single register?
> Right, will change.
> >
> > > +static int _check_enabled_with_retries(struct regulator_dev *rdev,
> > > + int retries, int enabled)
> > > +{
> >
> > This is not retrying, this is polling to see if the regulator actually
> > enabled.
> Yes, will update accordingly.
>
> >
> > > +static int qcom_labibb_regulator_enable(struct regulator_dev *rdev)
> > > +{
> >
> > > + ret = _check_enabled_with_retries(rdev, retries, 1);
> > > + if (ret < 0) {
> > > + dev_err(reg->dev, "retries exhausted: enable %s 
> > > regulator\n",
> > > + reg->desc.name);
> > > + return ret;
> > > + }
> >
> > If this is useful factor it out into a helper or the core, other devices
> > also have status bits saying if the regulator is enabled.  It looks like
> > this may be mainly trying to open code something like enable_time, with
> > possibly some issues where the time taken to enable varies a lot.
> >
> Makes sense; I am not terribly familiar with the regulator core and
> helpers, so let me look and refactor accordingly.
Does something like this make sense, or did I misunderstand your
suggestion completely? I'll send the updated patches accordingly.

--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2353,6 +2353,7 @@ static int _regulator_do_enable(struct
regulator_dev *rdev)
+   /* If max_time_poll_enabled is set for the regulator,
+* Poll upto max_time_poll_enabled time to see if the regulator
+* actually got enabled.
+* For each iteration, wait for the enable_time delay calculated
+* above already.
+* If the regulator isn't enabled after max_time_poll_enabled has
+* expired, return -REG_ENABLED_CHECK_FAILED.
+*/
+   if (rdev->desc->max_time_poll_enabled) {
+   unsigned int remaining_time_to_poll =
rdev->desc->max_time_poll_enabled;
+
+   while (remaining_time_to_poll > 0) {
+   /* We've already waited for enable_time above;
+* so we can start with immediate check of the
+* status of the regulator.
+*/
+   if (rdev->desc->ops->is_enabled(rdev))
+   break;
+
+   _regulator_enable_delay(delay);
+   remaining_time_to_poll -= delay;
+   }
+
+   if (remaining_time_to_poll <= 0) {
+   rdev_err(rdev, "Enabled check failed.\n");
+   return -REG_ENABLED_CHECK_FAILED;
+   }
+   }
+

Since atleast in my use case, the delay is really enable_time (time
before we could check the status register), we could reuse the
already-calculated 'delay' based on enable_time?
>


Best,
Sumit.


Re: [v2 3/4] regulator: qcom: Add labibb driver

2020-05-14 Thread Sumit Semwal
Hello Bjorn,

Thanks very much for the review.

On Tue, 12 May 2020 at 07:46, Bjorn Andersson
 wrote:
>
> On Fri 08 May 13:41 PDT 2020, Sumit Semwal wrote:
>
> > From: Nisha Kumari 
> >
> > Qualcomm platforms have LAB(LCD AMOLED Boost)/IBB(Inverting Buck Boost)
> > Regulators, labibb for short, which are used as power supply for
>
> lowercase Regulators
ok.
>
> > LCD Mode displays.
> >
> > This patch adds labibb regulator driver for pmi8998 pmic, found on
>
> Uppercase PMIC
>
ok.
> > SDM845 platforms.
> >
> > Signed-off-by: Nisha Kumari 
> > Signed-off-by: Sumit Semwal 
> >
> > --
> > v2: sumits: reworked the driver for more common code, and addressed
> > review comments from v1. This includes merging regulator_ops into
> > one, and allowing for future labibb variations.
> > ---
> >  drivers/regulator/Kconfig |  10 +
> >  drivers/regulator/Makefile|   1 +
> >  drivers/regulator/qcom-labibb-regulator.c | 288 ++
> >  3 files changed, 299 insertions(+)
> >  create mode 100644 drivers/regulator/qcom-labibb-regulator.c
> >
> > diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> > index f4b72cb098ef..58704a9fd05d 100644
> > --- a/drivers/regulator/Kconfig
> > +++ b/drivers/regulator/Kconfig
> > @@ -1167,5 +1167,15 @@ config REGULATOR_WM8994
> > This driver provides support for the voltage regulators on the
> > WM8994 CODEC.
> >
> > +config REGULATOR_QCOM_LABIBB
> > + tristate "QCOM LAB/IBB regulator support"
> > + depends on SPMI || COMPILE_TEST
> > + help
> > +   This driver supports Qualcomm's LAB/IBB regulators present on the
> > +   Qualcomm's PMIC chip pmi8998. QCOM LAB and IBB are SPMI
> > +   based PMIC implementations. LAB can be used as positive
> > +   boost regulator and IBB can be used as a negative boost regulator
> > +   for LCD display panel.
> > +
> >  endif
> >
> > diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> > index 6610ee001d9a..5b313786c0e8 100644
> > --- a/drivers/regulator/Makefile
> > +++ b/drivers/regulator/Makefile
> > @@ -87,6 +87,7 @@ obj-$(CONFIG_REGULATOR_MT6323)  += mt6323-regulator.o
> >  obj-$(CONFIG_REGULATOR_MT6358)   += mt6358-regulator.o
> >  obj-$(CONFIG_REGULATOR_MT6380)   += mt6380-regulator.o
> >  obj-$(CONFIG_REGULATOR_MT6397)   += mt6397-regulator.o
> > +obj-$(CONFIG_REGULATOR_QCOM_LABIBB) += qcom-labibb-regulator.o
> >  obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
> >  obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
> >  obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
> > diff --git a/drivers/regulator/qcom-labibb-regulator.c 
> > b/drivers/regulator/qcom-labibb-regulator.c
> > new file mode 100644
> > index ..a9dc7c060375
> > --- /dev/null
> > +++ b/drivers/regulator/qcom-labibb-regulator.c
> > @@ -0,0 +1,288 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2019, The Linux Foundation. All rights reserved.
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define REG_PERPH_TYPE  0x04
> > +#define QCOM_LAB_TYPE0x24
> > +#define QCOM_IBB_TYPE0x20
> > +
> > +#define REG_LABIBB_STATUS1   0x08
> > +#define REG_LABIBB_ENABLE_CTL0x46
> > +#define LABIBB_STATUS1_VREG_OK_BIT   BIT(7)
> > +#define LABIBB_CONTROL_ENABLEBIT(7)
> > +
> > +#define LAB_ENABLE_CTL_MASK  BIT(7)
> > +#define IBB_ENABLE_CTL_MASK  (BIT(7) | BIT(6))
> > +
> > +#define POWER_DELAY  8000
> > +
> > +struct labibb_regulator {
> > + struct regulator_desc   desc;
> > + struct device   *dev;
> > + struct regmap   *regmap;
> > + struct regulator_dev*rdev;
> > + u16 base;
> > + u8  type;
> > +};
> > +
> > +struct qcom_labibb {
>
> You pretty much use this as a local variable within probe, and then you
> use labibb_regulator in runtime. Perhaps you can just drop it?
>
Yes, you're right. It's probably a leftover from the re-design. I will drop it.
> > + struct device   *dev;
> > + struct regmap   *regmap;
> > + struct labibb_regulator lab;
> > + struct labibb_regulator ibb;
> > +};
> > +
> > +struct labibb_regulator_data {
> > + u16 base;
> > + const char  *name;
> > + const char  *irq_name;
> > + u8  type;
> > +};
> > +
> > +static int qcom_labibb_regulator_is_enabled(struct regulator_dev *rdev)
> > +{
> > + int ret;
> > + u8 val;
> > + struct labibb_regulator *reg = rdev_get_drvdata(rdev);
> > +
> > + ret = regmap_bulk_read(reg->regmap, reg->base +
> 

Re: [v2 3/4] regulator: qcom: Add labibb driver

2020-05-14 Thread Sumit Semwal
Hello Mark,

Thank you for your review comments!
On Mon, 11 May 2020 at 16:09, Mark Brown  wrote:
>
> On Sat, May 09, 2020 at 02:11:59AM +0530, Sumit Semwal wrote:
>
> > + ret = regmap_bulk_read(reg->regmap, reg->base +
> > +REG_LABIBB_STATUS1, , 1);
> > + if (ret < 0) {
> > + dev_err(reg->dev, "Read register failed ret = %d\n", ret);
> > + return ret;
> > + }
>
> Why a bulk read of a single register?
Right, will change.
>
> > +static int _check_enabled_with_retries(struct regulator_dev *rdev,
> > + int retries, int enabled)
> > +{
>
> This is not retrying, this is polling to see if the regulator actually
> enabled.
Yes, will update accordingly.

>
> > +static int qcom_labibb_regulator_enable(struct regulator_dev *rdev)
> > +{
>
> > + ret = _check_enabled_with_retries(rdev, retries, 1);
> > + if (ret < 0) {
> > + dev_err(reg->dev, "retries exhausted: enable %s regulator\n",
> > + reg->desc.name);
> > + return ret;
> > + }
>
> If this is useful factor it out into a helper or the core, other devices
> also have status bits saying if the regulator is enabled.  It looks like
> this may be mainly trying to open code something like enable_time, with
> possibly some issues where the time taken to enable varies a lot.
>
Makes sense; I am not terribly familiar with the regulator core and
helpers, so let me look and refactor accordingly.

> > + if (ret)
> > + return 0;
> > +
> > +
> > + dev_err(reg->dev, "Can't enable %s\n", reg->desc.name);
> > + return -EINVAL;
>
> Return the actual error code (the logic here is quite convoluted).
Will try to simplify.
>
> > + ret = regulator_disable_regmap(rdev);
> > +
> > + if (ret < 0) {
>
> You have lots of blank lines between operations and checking their
> return codes?
>
will correct that.
> > + ret = _check_enabled_with_retries(rdev, retries, 0);
> > + if (ret < 0) {
> > + dev_err(reg->dev, "retries exhausted: disable %s regulator\n",
> > + reg->desc.name);
> > + return ret;
> > + }
>
> Similarly to the enable path, but is this one about off_on_delay rather
> than enable_time?
Got it. Let me look deeper.
>
> > + if (reg_data->type == QCOM_LAB_TYPE) {
> > + reg = >lab;
> > + reg->desc.enable_mask = LAB_ENABLE_CTL_MASK;
> > + } else {
> > + reg = >ibb;
> > + reg->desc.enable_mask = IBB_ENABLE_CTL_MASK;
> > + }
>
> Write a switch statement so this is extensible.
I can change over to switch, though in the current set of downstream
code I've seen, it doesn't look that it would get extended. But I
guess there isn't any harm in moving over to switch. Will do.

Best,
Sumit.


Re: [v2 3/4] regulator: qcom: Add labibb driver

2020-05-12 Thread Mark Brown
On Mon, May 11, 2020 at 07:15:09PM -0700, Bjorn Andersson wrote:
> On Fri 08 May 13:41 PDT 2020, Sumit Semwal wrote:

> > +   int ret;
> > +   struct labibb_regulator *reg = rdev_get_drvdata(rdev);
> > +
> > +   while (retries--) {

> Mark's suggestion of extending _regulator_enable_delay() to support
> polling is_enable() seems reasonable.

> The only complication I can see is that code path currently doesn't have
> any expectations of the regulator not being operational at the end -
> this seems to offer that possibility. So some care needs to be taken
> there.

Are we expecting that to happen in normal operation?  Generally this is
a pretty serious problem.  In any caser if we're adding checks of status
we'd need an error return if the status doesn't show the regulator is on
after some reasonable time.


signature.asc
Description: PGP signature


Re: [v2 3/4] regulator: qcom: Add labibb driver

2020-05-11 Thread Bjorn Andersson
On Fri 08 May 13:41 PDT 2020, Sumit Semwal wrote:

> From: Nisha Kumari 
> 
> Qualcomm platforms have LAB(LCD AMOLED Boost)/IBB(Inverting Buck Boost)
> Regulators, labibb for short, which are used as power supply for

lowercase Regulators

> LCD Mode displays.
> 
> This patch adds labibb regulator driver for pmi8998 pmic, found on

Uppercase PMIC

> SDM845 platforms.
> 
> Signed-off-by: Nisha Kumari 
> Signed-off-by: Sumit Semwal 
> 
> --
> v2: sumits: reworked the driver for more common code, and addressed
> review comments from v1. This includes merging regulator_ops into
> one, and allowing for future labibb variations.
> ---
>  drivers/regulator/Kconfig |  10 +
>  drivers/regulator/Makefile|   1 +
>  drivers/regulator/qcom-labibb-regulator.c | 288 ++
>  3 files changed, 299 insertions(+)
>  create mode 100644 drivers/regulator/qcom-labibb-regulator.c
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index f4b72cb098ef..58704a9fd05d 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1167,5 +1167,15 @@ config REGULATOR_WM8994
> This driver provides support for the voltage regulators on the
> WM8994 CODEC.
>  
> +config REGULATOR_QCOM_LABIBB
> + tristate "QCOM LAB/IBB regulator support"
> + depends on SPMI || COMPILE_TEST
> + help
> +   This driver supports Qualcomm's LAB/IBB regulators present on the
> +   Qualcomm's PMIC chip pmi8998. QCOM LAB and IBB are SPMI
> +   based PMIC implementations. LAB can be used as positive
> +   boost regulator and IBB can be used as a negative boost regulator
> +   for LCD display panel.
> +
>  endif
>  
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 6610ee001d9a..5b313786c0e8 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -87,6 +87,7 @@ obj-$(CONFIG_REGULATOR_MT6323)  += mt6323-regulator.o
>  obj-$(CONFIG_REGULATOR_MT6358)   += mt6358-regulator.o
>  obj-$(CONFIG_REGULATOR_MT6380)   += mt6380-regulator.o
>  obj-$(CONFIG_REGULATOR_MT6397)   += mt6397-regulator.o
> +obj-$(CONFIG_REGULATOR_QCOM_LABIBB) += qcom-labibb-regulator.o
>  obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
>  obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
>  obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
> diff --git a/drivers/regulator/qcom-labibb-regulator.c 
> b/drivers/regulator/qcom-labibb-regulator.c
> new file mode 100644
> index ..a9dc7c060375
> --- /dev/null
> +++ b/drivers/regulator/qcom-labibb-regulator.c
> @@ -0,0 +1,288 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019, The Linux Foundation. All rights reserved.
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define REG_PERPH_TYPE  0x04
> +#define QCOM_LAB_TYPE0x24
> +#define QCOM_IBB_TYPE0x20
> +
> +#define REG_LABIBB_STATUS1   0x08
> +#define REG_LABIBB_ENABLE_CTL0x46
> +#define LABIBB_STATUS1_VREG_OK_BIT   BIT(7)
> +#define LABIBB_CONTROL_ENABLEBIT(7)
> +
> +#define LAB_ENABLE_CTL_MASK  BIT(7)
> +#define IBB_ENABLE_CTL_MASK  (BIT(7) | BIT(6))
> +
> +#define POWER_DELAY  8000
> +
> +struct labibb_regulator {
> + struct regulator_desc   desc;
> + struct device   *dev;
> + struct regmap   *regmap;
> + struct regulator_dev*rdev;
> + u16 base;
> + u8  type;
> +};
> +
> +struct qcom_labibb {

You pretty much use this as a local variable within probe, and then you
use labibb_regulator in runtime. Perhaps you can just drop it?

> + struct device   *dev;
> + struct regmap   *regmap;
> + struct labibb_regulator lab;
> + struct labibb_regulator ibb;
> +};
> +
> +struct labibb_regulator_data {
> + u16 base;
> + const char  *name;
> + const char  *irq_name;
> + u8  type;
> +};
> +
> +static int qcom_labibb_regulator_is_enabled(struct regulator_dev *rdev)
> +{
> + int ret;
> + u8 val;
> + struct labibb_regulator *reg = rdev_get_drvdata(rdev);
> +
> + ret = regmap_bulk_read(reg->regmap, reg->base +
> +REG_LABIBB_STATUS1, , 1);
> + if (ret < 0) {
> + dev_err(reg->dev, "Read register failed ret = %d\n", ret);
> + return ret;
> + }
> +
> + if (val & LABIBB_STATUS1_VREG_OK_BIT)
> + return 1;
> + else
> + return 0;

return !!(val & LABIBB_STATUS1_VREG_OK_BIT);

> +}
> +
> +static int _check_enabled_with_retries(struct regulator_dev 

Re: [v2 3/4] regulator: qcom: Add labibb driver

2020-05-11 Thread Mark Brown
On Sat, May 09, 2020 at 02:11:59AM +0530, Sumit Semwal wrote:

> + ret = regmap_bulk_read(reg->regmap, reg->base +
> +REG_LABIBB_STATUS1, , 1);
> + if (ret < 0) {
> + dev_err(reg->dev, "Read register failed ret = %d\n", ret);
> + return ret;
> + }

Why a bulk read of a single register?

> +static int _check_enabled_with_retries(struct regulator_dev *rdev,
> + int retries, int enabled)
> +{

This is not retrying, this is polling to see if the regulator actually
enabled.

> +static int qcom_labibb_regulator_enable(struct regulator_dev *rdev)
> +{

> + ret = _check_enabled_with_retries(rdev, retries, 1);
> + if (ret < 0) {
> + dev_err(reg->dev, "retries exhausted: enable %s regulator\n",
> + reg->desc.name);
> + return ret;
> + }

If this is useful factor it out into a helper or the core, other devices
also have status bits saying if the regulator is enabled.  It looks like
this may be mainly trying to open code something like enable_time, with
possibly some issues where the time taken to enable varies a lot.

> + if (ret)
> + return 0;
> +
> +
> + dev_err(reg->dev, "Can't enable %s\n", reg->desc.name);
> + return -EINVAL;

Return the actual error code (the logic here is quite convoluted).

> + ret = regulator_disable_regmap(rdev);
> +
> + if (ret < 0) {

You have lots of blank lines between operations and checking their
return codes?

> + ret = _check_enabled_with_retries(rdev, retries, 0);
> + if (ret < 0) {
> + dev_err(reg->dev, "retries exhausted: disable %s regulator\n",
> + reg->desc.name);
> + return ret;
> + }

Similarly to the enable path, but is this one about off_on_delay rather
than enable_time?

> + if (reg_data->type == QCOM_LAB_TYPE) {
> + reg = >lab;
> + reg->desc.enable_mask = LAB_ENABLE_CTL_MASK;
> + } else {
> + reg = >ibb;
> + reg->desc.enable_mask = IBB_ENABLE_CTL_MASK;
> + }

Write a switch statement so this is extensible.


signature.asc
Description: PGP signature


[v2 3/4] regulator: qcom: Add labibb driver

2020-05-08 Thread Sumit Semwal
From: Nisha Kumari 

Qualcomm platforms have LAB(LCD AMOLED Boost)/IBB(Inverting Buck Boost)
Regulators, labibb for short, which are used as power supply for
LCD Mode displays.

This patch adds labibb regulator driver for pmi8998 pmic, found on
SDM845 platforms.

Signed-off-by: Nisha Kumari 
Signed-off-by: Sumit Semwal 

--
v2: sumits: reworked the driver for more common code, and addressed
review comments from v1. This includes merging regulator_ops into
one, and allowing for future labibb variations.
---
 drivers/regulator/Kconfig |  10 +
 drivers/regulator/Makefile|   1 +
 drivers/regulator/qcom-labibb-regulator.c | 288 ++
 3 files changed, 299 insertions(+)
 create mode 100644 drivers/regulator/qcom-labibb-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index f4b72cb098ef..58704a9fd05d 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1167,5 +1167,15 @@ config REGULATOR_WM8994
  This driver provides support for the voltage regulators on the
  WM8994 CODEC.
 
+config REGULATOR_QCOM_LABIBB
+   tristate "QCOM LAB/IBB regulator support"
+   depends on SPMI || COMPILE_TEST
+   help
+ This driver supports Qualcomm's LAB/IBB regulators present on the
+ Qualcomm's PMIC chip pmi8998. QCOM LAB and IBB are SPMI
+ based PMIC implementations. LAB can be used as positive
+ boost regulator and IBB can be used as a negative boost regulator
+ for LCD display panel.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6610ee001d9a..5b313786c0e8 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -87,6 +87,7 @@ obj-$(CONFIG_REGULATOR_MT6323)+= mt6323-regulator.o
 obj-$(CONFIG_REGULATOR_MT6358) += mt6358-regulator.o
 obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_LABIBB) += qcom-labibb-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
diff --git a/drivers/regulator/qcom-labibb-regulator.c 
b/drivers/regulator/qcom-labibb-regulator.c
new file mode 100644
index ..a9dc7c060375
--- /dev/null
+++ b/drivers/regulator/qcom-labibb-regulator.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019, The Linux Foundation. All rights reserved.
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define REG_PERPH_TYPE  0x04
+#define QCOM_LAB_TYPE  0x24
+#define QCOM_IBB_TYPE  0x20
+
+#define REG_LABIBB_STATUS1 0x08
+#define REG_LABIBB_ENABLE_CTL  0x46
+#define LABIBB_STATUS1_VREG_OK_BIT BIT(7)
+#define LABIBB_CONTROL_ENABLE  BIT(7)
+
+#define LAB_ENABLE_CTL_MASKBIT(7)
+#define IBB_ENABLE_CTL_MASK(BIT(7) | BIT(6))
+
+#define POWER_DELAY8000
+
+struct labibb_regulator {
+   struct regulator_desc   desc;
+   struct device   *dev;
+   struct regmap   *regmap;
+   struct regulator_dev*rdev;
+   u16 base;
+   u8  type;
+};
+
+struct qcom_labibb {
+   struct device   *dev;
+   struct regmap   *regmap;
+   struct labibb_regulator lab;
+   struct labibb_regulator ibb;
+};
+
+struct labibb_regulator_data {
+   u16 base;
+   const char  *name;
+   const char  *irq_name;
+   u8  type;
+};
+
+static int qcom_labibb_regulator_is_enabled(struct regulator_dev *rdev)
+{
+   int ret;
+   u8 val;
+   struct labibb_regulator *reg = rdev_get_drvdata(rdev);
+
+   ret = regmap_bulk_read(reg->regmap, reg->base +
+  REG_LABIBB_STATUS1, , 1);
+   if (ret < 0) {
+   dev_err(reg->dev, "Read register failed ret = %d\n", ret);
+   return ret;
+   }
+
+   if (val & LABIBB_STATUS1_VREG_OK_BIT)
+   return 1;
+   else
+   return 0;
+}
+
+static int _check_enabled_with_retries(struct regulator_dev *rdev,
+   int retries, int enabled)
+{
+   int ret;
+   struct labibb_regulator *reg = rdev_get_drvdata(rdev);
+
+   while (retries--) {
+   /* Wait for a small period before checking REG_LABIBB_STATUS1 */
+   usleep_range(POWER_DELAY, POWER_DELAY + 200);
+
+   ret = qcom_labibb_regulator_is_enabled(rdev);
+
+   if (ret < 0) {
+   dev_err(reg->dev, "Can't read %s regulator status\n",
+