[PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[Anson Huang]

According to datasheet, the isl29018 has "vddd/vdda" power
supply, and isl29023/isl29035 ONLY has "vdd" power supply,
update the power supply name with "vdd" and "vdda" according
to datasheet to cover all devices and avoid confusion.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 Documentation/devicetree/bindings/iio/light/isl29018.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/light/isl29018.txt b/Documentation/devicetree/bindings/iio/light/isl29018.txt
index b9bbde3..36f737d 100644
--- a/Documentation/devicetree/bindings/iio/light/isl29018.txt
+++ b/Documentation/devicetree/bindings/iio/light/isl29018.txt
@@ -15,7 +15,9 @@ Optional properties:
   Refer to interrupt-controller/interrupts.txt for generic interrupt client
   node bindings.
 
-  - vcc-supply: phandle to the regulator that provides power to the sensor.
+  - vdd-supply: phandle to the regulator that provides vdd power to the sensor.
+
+  - vdda-supply: phandle to the regulator that provides vdda power to the sensor.
 
 Example:
 
-- 
2.7.4

[PATCH V3 2/2] iio: light: isl29018: add optional vdd/vdda regulator operation support

[Anson Huang]

The light sensor's power supply could be controlled by regulator
on some platforms, such as i.MX6Q-SABRESD board, the light sensor
isl29023's power supply is controlled by a GPIO fixed regulator,
need to make sure the regulator is enabled before any operation of
sensor, this patch adds optional vdd/vdda regulator operation support.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
ChangeLog since V2:
    - Add defer_probe check if the return value from devm_regulator_get_optional is ERR_PTR(-EPROBE_DEFER);
    - Replace the "vcc" with "vdd" according to datasheet, and add optional "vdda" supply as well since
      isl29018 also has a vdda supply.
---
 drivers/iio/light/isl29018.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
index b45400f..477eb95 100644
--- a/drivers/iio/light/isl29018.c
+++ b/drivers/iio/light/isl29018.c
@@ -23,6 +23,7 @@
 #include <linux/mutex.h>
 #include <linux/delay.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -95,6 +96,8 @@ struct isl29018_chip {
 	struct isl29018_scale	scale;
 	int			prox_scheme;
 	bool			suspended;
+	struct regulator	*vdd_reg;
+	struct regulator	*vdda_reg;
 };
 
 static int isl29018_set_integration_time(struct isl29018_chip *chip,
@@ -735,6 +738,28 @@ static int isl29018_probe(struct i2c_client *client,
 
 	mutex_init(&chip->lock);
 
+	chip->vdd_reg = devm_regulator_get_optional(&client->dev, "vdd");
+	if (!IS_ERR(chip->vdd_reg)) {
+		err = regulator_enable(chip->vdd_reg);
+		if (err) {
+			dev_err(&client->dev, "failed to enable VDD regulator\n");
+			return err;
+		}
+	} else if (chip->vdd_reg == ERR_PTR(-EPROBE_DEFER)) {
+		return -EPROBE_DEFER;
+	}
+
+	chip->vdda_reg = devm_regulator_get_optional(&client->dev, "vdda");
+	if (!IS_ERR(chip->vdda_reg)) {
+		err = regulator_enable(chip->vdda_reg);
+		if (err) {
+			dev_err(&client->dev, "failed to enable VDDA regulator\n");
+			return err;
+		}
+	} else if (chip->vdda_reg == ERR_PTR(-EPROBE_DEFER)) {
+		return -EPROBE_DEFER;
+	}
+
 	chip->type = dev_id;
 	chip->calibscale = 1;
 	chip->ucalibscale = 0;
@@ -768,6 +793,7 @@ static int isl29018_probe(struct i2c_client *client,
 static int isl29018_suspend(struct device *dev)
 {
 	struct isl29018_chip *chip = iio_priv(dev_get_drvdata(dev));
+	int ret;
 
 	mutex_lock(&chip->lock);
 
@@ -777,6 +803,22 @@ static int isl29018_suspend(struct device *dev)
 	 * So we do not have much to do here.
 	 */
 	chip->suspended = true;
+	if (!IS_ERR(chip->vdd_reg)) {
+		ret = regulator_disable(chip->vdd_reg);
+		if (ret) {
+			dev_err(dev, "failed to disable VDD regulator\n");
+			mutex_unlock(&chip->lock);
+			return ret;
+		}
+	}
+	if (!IS_ERR(chip->vdda_reg)) {
+		ret = regulator_disable(chip->vdda_reg);
+		if (ret) {
+			dev_err(dev, "failed to disable VDDA regulator\n");
+			mutex_unlock(&chip->lock);
+			return ret;
+		}
+	}
 
 	mutex_unlock(&chip->lock);
 
@@ -790,6 +832,23 @@ static int isl29018_resume(struct device *dev)
 
 	mutex_lock(&chip->lock);
 
+	if (!IS_ERR(chip->vdd_reg)) {
+		err = regulator_enable(chip->vdd_reg);
+		if (err) {
+			dev_err(dev, "failed to enable VDD regulator\n");
+			mutex_unlock(&chip->lock);
+			return err;
+		}
+	}
+	if (!IS_ERR(chip->vdda_reg)) {
+		err = regulator_enable(chip->vdda_reg);
+		if (err) {
+			dev_err(dev, "failed to enable VDDA regulator\n");
+			mutex_unlock(&chip->lock);
+			return err;
+		}
+	}
+
 	err = isl29018_chip_init(chip);
 	if (!err)
 		chip->suspended = false;
-- 
2.7.4

Re: [PATCH V3 2/2] iio: light: isl29018: add optional vdd/vdda regulator operation support

[Jonathan Cameron]

On Mon, 10 Dec 2018 07:11:23 +0000
Anson Huang <anson.huang@nxp.com> wrote:

> The light sensor's power supply could be controlled by regulator
> on some platforms, such as i.MX6Q-SABRESD board, the light sensor
> isl29023's power supply is controlled by a GPIO fixed regulator,
> need to make sure the regulator is enabled before any operation of
> sensor, this patch adds optional vdd/vdda regulator operation support.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Comments on the other patches apply on error handling and
cleaner pattern for the error handling so I won't review this one.

Thanks,

Jonathan


> ---
> ChangeLog since V2:
>     - Add defer_probe check if the return value from devm_regulator_get_optional is ERR_PTR(-EPROBE_DEFER);
>     - Replace the "vcc" with "vdd" according to datasheet, and add optional "vdda" supply as well since
>       isl29018 also has a vdda supply.
> ---
>  drivers/iio/light/isl29018.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
> index b45400f..477eb95 100644
> --- a/drivers/iio/light/isl29018.c
> +++ b/drivers/iio/light/isl29018.c
> @@ -23,6 +23,7 @@
>  #include <linux/mutex.h>
>  #include <linux/delay.h>
>  #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -95,6 +96,8 @@ struct isl29018_chip {
>  	struct isl29018_scale	scale;
>  	int			prox_scheme;
>  	bool			suspended;
> +	struct regulator	*vdd_reg;
> +	struct regulator	*vdda_reg;
>  };
>  
>  static int isl29018_set_integration_time(struct isl29018_chip *chip,
> @@ -735,6 +738,28 @@ static int isl29018_probe(struct i2c_client *client,
>  
>  	mutex_init(&chip->lock);
>  
> +	chip->vdd_reg = devm_regulator_get_optional(&client->dev, "vdd");
> +	if (!IS_ERR(chip->vdd_reg)) {
> +		err = regulator_enable(chip->vdd_reg);
> +		if (err) {
> +			dev_err(&client->dev, "failed to enable VDD regulator\n");
> +			return err;
> +		}
> +	} else if (chip->vdd_reg == ERR_PTR(-EPROBE_DEFER)) {
> +		return -EPROBE_DEFER;
> +	}
> +
> +	chip->vdda_reg = devm_regulator_get_optional(&client->dev, "vdda");
> +	if (!IS_ERR(chip->vdda_reg)) {
> +		err = regulator_enable(chip->vdda_reg);
> +		if (err) {
> +			dev_err(&client->dev, "failed to enable VDDA regulator\n");
> +			return err;
> +		}
> +	} else if (chip->vdda_reg == ERR_PTR(-EPROBE_DEFER)) {
> +		return -EPROBE_DEFER;
> +	}
> +
>  	chip->type = dev_id;
>  	chip->calibscale = 1;
>  	chip->ucalibscale = 0;
> @@ -768,6 +793,7 @@ static int isl29018_probe(struct i2c_client *client,
>  static int isl29018_suspend(struct device *dev)
>  {
>  	struct isl29018_chip *chip = iio_priv(dev_get_drvdata(dev));
> +	int ret;
>  
>  	mutex_lock(&chip->lock);
>  
> @@ -777,6 +803,22 @@ static int isl29018_suspend(struct device *dev)
>  	 * So we do not have much to do here.
>  	 */
>  	chip->suspended = true;
> +	if (!IS_ERR(chip->vdd_reg)) {
> +		ret = regulator_disable(chip->vdd_reg);
> +		if (ret) {
> +			dev_err(dev, "failed to disable VDD regulator\n");
> +			mutex_unlock(&chip->lock);
> +			return ret;
> +		}
> +	}
> +	if (!IS_ERR(chip->vdda_reg)) {
> +		ret = regulator_disable(chip->vdda_reg);
> +		if (ret) {
> +			dev_err(dev, "failed to disable VDDA regulator\n");
> +			mutex_unlock(&chip->lock);
> +			return ret;
> +		}
> +	}
>  
>  	mutex_unlock(&chip->lock);
>  
> @@ -790,6 +832,23 @@ static int isl29018_resume(struct device *dev)
>  
>  	mutex_lock(&chip->lock);
>  
> +	if (!IS_ERR(chip->vdd_reg)) {
> +		err = regulator_enable(chip->vdd_reg);
> +		if (err) {
> +			dev_err(dev, "failed to enable VDD regulator\n");
> +			mutex_unlock(&chip->lock);
> +			return err;
> +		}
> +	}
> +	if (!IS_ERR(chip->vdda_reg)) {
> +		err = regulator_enable(chip->vdda_reg);
> +		if (err) {
> +			dev_err(dev, "failed to enable VDDA regulator\n");
> +			mutex_unlock(&chip->lock);
> +			return err;
> +		}
> +	}
> +
>  	err = isl29018_chip_init(chip);
>  	if (!err)
>  		chip->suspended = false;

Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[Rob Herring]

On Mon, Dec 10, 2018 at 07:11:19AM +0000, Anson Huang wrote:
> According to datasheet, the isl29018 has "vddd/vdda" power
> supply, and isl29023/isl29035 ONLY has "vdd" power supply,
> update the power supply name with "vdd" and "vdda" according
> to datasheet to cover all devices and avoid confusion.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  Documentation/devicetree/bindings/iio/light/isl29018.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/isl29018.txt b/Documentation/devicetree/bindings/iio/light/isl29018.txt
> index b9bbde3..36f737d 100644
> --- a/Documentation/devicetree/bindings/iio/light/isl29018.txt
> +++ b/Documentation/devicetree/bindings/iio/light/isl29018.txt
> @@ -15,7 +15,9 @@ Optional properties:
>    Refer to interrupt-controller/interrupts.txt for generic interrupt client
>    node bindings.
>  
> -  - vcc-supply: phandle to the regulator that provides power to the sensor.
> +  - vdd-supply: phandle to the regulator that provides vdd power to the sensor.
> +
> +  - vdda-supply: phandle to the regulator that provides vdda power to the sensor.

Is this in use? You can't just change things if it is.

Rob

RE: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[Anson Huang]

Hi, Rob

Best Regards!
Anson Huang

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2018年12月11日 7:24
> To: Anson Huang <anson.huang@nxp.com>
> Cc: jic23@kernel.org; knaack.h@gmx.de; lars@metafoo.de;
> pmeerw@pmeerw.net; mark.rutland@arm.com; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power
> supply name
> 
> On Mon, Dec 10, 2018 at 07:11:19AM +0000, Anson Huang wrote:
> > According to datasheet, the isl29018 has "vddd/vdda" power supply, and
> > isl29023/isl29035 ONLY has "vdd" power supply, update the power supply
> > name with "vdd" and "vdda" according to datasheet to cover all devices
> > and avoid confusion.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/iio/light/isl29018.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > b/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > index b9bbde3..36f737d 100644
> > --- a/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > +++ b/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > @@ -15,7 +15,9 @@ Optional properties:
> >    Refer to interrupt-controller/interrupts.txt for generic interrupt client
> >    node bindings.
> >
> > -  - vcc-supply: phandle to the regulator that provides power to the sensor.
> > +  - vdd-supply: phandle to the regulator that provides vdd power to the
> sensor.
> > +
> > +  - vdda-supply: phandle to the regulator that provides vdda power to the
> sensor.
> 
> Is this in use? You can't just change things if it is.

I did NOT see any "vcc" in folder drivers/iio/light/, so I think it is NOT used at all,
so I take this chance to update it according to datasheet. Thanks.

Anson


> 
> Rob

Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[Rob Herring]

On Mon, Dec 10, 2018 at 7:40 PM Anson Huang <anson.huang@nxp.com> wrote:
>
> Hi, Rob
>
> Best Regards!
> Anson Huang
>
> > -----Original Message-----
> > From: Rob Herring [mailto:robh@kernel.org]
> > Sent: 2018年12月11日 7:24
> > To: Anson Huang <anson.huang@nxp.com>
> > Cc: jic23@kernel.org; knaack.h@gmx.de; lars@metafoo.de;
> > pmeerw@pmeerw.net; mark.rutland@arm.com; linux-iio@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> > <linux-imx@nxp.com>
> > Subject: Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power
> > supply name
> >
> > On Mon, Dec 10, 2018 at 07:11:19AM +0000, Anson Huang wrote:
> > > According to datasheet, the isl29018 has "vddd/vdda" power supply, and
> > > isl29023/isl29035 ONLY has "vdd" power supply, update the power supply
> > > name with "vdd" and "vdda" according to datasheet to cover all devices
> > > and avoid confusion.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/iio/light/isl29018.txt | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > > b/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > > index b9bbde3..36f737d 100644
> > > --- a/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > > +++ b/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > > @@ -15,7 +15,9 @@ Optional properties:
> > >    Refer to interrupt-controller/interrupts.txt for generic interrupt client
> > >    node bindings.
> > >
> > > -  - vcc-supply: phandle to the regulator that provides power to the sensor.
> > > +  - vdd-supply: phandle to the regulator that provides vdd power to the
> > sensor.
> > > +
> > > +  - vdda-supply: phandle to the regulator that provides vdda power to the
> > sensor.
> >
> > Is this in use? You can't just change things if it is.
>
> I did NOT see any "vcc" in folder drivers/iio/light/, so I think it is NOT used at all,
> so I take this chance to update it according to datasheet. Thanks.

arch/arm/boot/dts/exynos5420-peach-pit.dts-629- light-sensor@44 {
arch/arm/boot/dts/exynos5420-peach-pit.dts:630:         compatible =
"isil,isl29018";
arch/arm/boot/dts/exynos5420-peach-pit.dts-631-         reg = <0x44>;
arch/arm/boot/dts/exynos5420-peach-pit.dts-632-         vcc-supply =
<&tps65090_fet5>;
arch/arm/boot/dts/exynos5420-peach-pit.dts-633- };

arch/arm/boot/dts/exynos5800-peach-pi.dts-629-  light-sensor@44 {
arch/arm/boot/dts/exynos5800-peach-pi.dts:630:          compatible =
"isil,isl29018";
arch/arm/boot/dts/exynos5800-peach-pi.dts-631-          reg = <0x44>;
arch/arm/boot/dts/exynos5800-peach-pi.dts-632-          vcc-supply =
<&tps65090_fet5>;
arch/arm/boot/dts/exynos5800-peach-pi.dts-633-  };

The rest of the dts files using this don't have a supply it seems. So
you need permission from the Exynos folks if you want to just drop
this. And also update their dts files.

Rob

RE: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[Anson Huang]

Hi, Fabio
	Obviously, some of the dts files (such as arch/arm/boot/dts/exynos5420-peach-pit.dts) using "vcc" as isl29018's power supply name, they are NOT matched with datasheet, so should we update those dts files with "vdd" as well or just using the "vcc" in isl29018 driver? Which solution is better?

Best Regards!
Anson Huang

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2018年12月11日 23:19
> To: Anson Huang <anson.huang@nxp.com>
> Cc: Jonathan Cameron <jic23@kernel.org>; Hartmut Knaack
> <knaack.h@gmx.de>; Lars-Peter Clausen <lars@metafoo.de>; Peter Meerwald
> <pmeerw@pmeerw.net>; Mark Rutland <mark.rutland@arm.com>;
> linux-iio@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power
> supply name
> 
> On Mon, Dec 10, 2018 at 7:40 PM Anson Huang <anson.huang@nxp.com>
> wrote:
> >
> > Hi, Rob
> >
> > Best Regards!
> > Anson Huang
> >
> > > -----Original Message-----
> > > From: Rob Herring [mailto:robh@kernel.org]
> > > Sent: 2018年12月11日 7:24
> > > To: Anson Huang <anson.huang@nxp.com>
> > > Cc: jic23@kernel.org; knaack.h@gmx.de; lars@metafoo.de;
> > > pmeerw@pmeerw.net; mark.rutland@arm.com; linux-iio@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > dl-linux-imx <linux-imx@nxp.com>
> > > Subject: Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018:
> > > update power supply name
> > >
> > > On Mon, Dec 10, 2018 at 07:11:19AM +0000, Anson Huang wrote:
> > > > According to datasheet, the isl29018 has "vddd/vdda" power supply,
> > > > and
> > > > isl29023/isl29035 ONLY has "vdd" power supply, update the power
> > > > supply name with "vdd" and "vdda" according to datasheet to cover
> > > > all devices and avoid confusion.
> > > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/iio/light/isl29018.txt | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > > > b/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > > > index b9bbde3..36f737d 100644
> > > > --- a/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > > > +++ b/Documentation/devicetree/bindings/iio/light/isl29018.txt
> > > > @@ -15,7 +15,9 @@ Optional properties:
> > > >    Refer to interrupt-controller/interrupts.txt for generic interrupt client
> > > >    node bindings.
> > > >
> > > > -  - vcc-supply: phandle to the regulator that provides power to the
> sensor.
> > > > +  - vdd-supply: phandle to the regulator that provides vdd power
> > > > + to the
> > > sensor.
> > > > +
> > > > +  - vdda-supply: phandle to the regulator that provides vdda
> > > > + power to the
> > > sensor.
> > >
> > > Is this in use? You can't just change things if it is.
> >
> > I did NOT see any "vcc" in folder drivers/iio/light/, so I think it is
> > NOT used at all, so I take this chance to update it according to datasheet.
> Thanks.
> 
> arch/arm/boot/dts/exynos5420-peach-pit.dts-629- light-sensor@44 {
> arch/arm/boot/dts/exynos5420-peach-pit.dts:630:         compatible =
> "isil,isl29018";
> arch/arm/boot/dts/exynos5420-peach-pit.dts-631-         reg = <0x44>;
> arch/arm/boot/dts/exynos5420-peach-pit.dts-632-         vcc-supply =
> <&tps65090_fet5>;
> arch/arm/boot/dts/exynos5420-peach-pit.dts-633- };
> 
> arch/arm/boot/dts/exynos5800-peach-pi.dts-629-  light-sensor@44 {
> arch/arm/boot/dts/exynos5800-peach-pi.dts:630:          compatible =
> "isil,isl29018";
> arch/arm/boot/dts/exynos5800-peach-pi.dts-631-          reg = <0x44>;
> arch/arm/boot/dts/exynos5800-peach-pi.dts-632-          vcc-supply =
> <&tps65090_fet5>;
> arch/arm/boot/dts/exynos5800-peach-pi.dts-633-  };
> 
> The rest of the dts files using this don't have a supply it seems. So you need
> permission from the Exynos folks if you want to just drop this. And also update
> their dts files.
> 
> Rob

Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[Fabio Estevam]

Hi Anson,

On Wed, Dec 12, 2018 at 4:17 AM Anson Huang <anson.huang@nxp.com> wrote:
>
> Hi, Fabio
>         Obviously, some of the dts files (such as arch/arm/boot/dts/exynos5420-peach-pit.dts) using "vcc" as isl29018's power supply name, they are NOT matched with datasheet, so should we update those dts files with "vdd" as well or just using the "vcc" in isl29018 driver? Which solution is better?

We should not break existing dtbs, so in this case just keep the
existing supply name and you could add the new optional vdda supply as
per the datasheet.

Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[Anson Huang]

Hi, Fabio

From Anson's iPhone 6

> 
> We should not break existing dtbs, so in this case just keep the
> existing supply name and you could add the new optional vdda supply as
> per the datasheet.

        Since the datasheet of isl29018 states the vdda and vddd MUST be shorted externally, and the later chips even remove vdda, ONLY has vdd one supply, so I think adding vdda does NOT make enough sense. Looks like the vcc they added is just for prepare, the driver did NOT support regulator operation at that time. So can we just use vcc in driver accordingly? Do we have to use the name “VDD” in datasheet for regulator name?

Anson.

Re: [PATCH V3 1/2] dt-bindings: iio: light: isl29018: update power supply name

[Fabio Estevam]

Hi Anson,

On Wed, Dec 12, 2018 at 10:49 AM Anson Huang <anson.huang@nxp.com> wrote:

>         Since the datasheet of isl29018 states the vdda and vddd MUST be shorted externally, and the later chips even remove vdda, ONLY has vdd one supply, so I think adding vdda does NOT make enough sense. Looks like the vcc they added is just for prepare, the driver did NOT support regulator operation at that time. So can we just use vcc in driver accordingly? Do we have to use the name “VDD” in datasheet for regulator name?

Ok, understood. In this case, I would suggest keeping the existing name.