Re: [PATCH v2 2/6] iio: light: stk3310: Implement vdd supply and power it off during suspend
On Wed, Apr 24, 2024 at 02:16:06AM +0300, Andy Shevchenko wrote:
> On Wed, Apr 24, 2024 at 1:41 AM Aren Moynihan wrote:
> >
> > From: Ondrej Jirman
> >
> > VDD power input can be used to completely power off the chip during
> > system suspend. Do so if available.
>
> ...
>
> > ret = stk3310_init(indio_dev);
> > if (ret < 0)
> > - return ret;
> > + goto err_vdd_disable;
>
> This is wrong. You will have the regulator being disabled _before_
> IRQ. Note, that the original code likely has a bug which sets states
> before disabling IRQ and removing a handler.
Oh! now I see the issue you were talking about last time around. I
expect that means the irq shouldn't be managed with devres, so it can be
the first thing freed in the remove function (I haven't checked the docs
to see if there's an easier way yet).
I'll add a patch to fix the order of the handling of the irq (both this and
the issue Ondřej brought up).
> Side note, you may make the driver neater with help of
>
> struct device *dev = >dev;
>
> defined in this patch.
Good point, it's minor, but it should be a net improvement.
> ...
>
> > static int stk3310_suspend(struct device *dev)
> > {
> > struct stk3310_data *data;
>
> > data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>
> Side note: This may be updated (in a separate change) to use
> dev_get_drvdata() directly.
>
> Jonathan, do we have something like iio_priv_from_drvdata(struct
> device *dev)? Seems many drivers may utilise it.
>
At this rate I'm going to need to split off a separate style / code
cleanup series so I don't keep introducing dumb bugs while rebasing this
one.
Thank you for your time
- Aren
[PATCH v2 3/6] iio: light: stk3310: Manage LED power supply
The stk3310 and stk3310 chips have an input for power to the infrared
LED. Add support for managing it's state.
Signed-off-by: Aren Moynihan
---
drivers/iio/light/stk3310.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index a0547eeca3e3..ee1ac95dbc0e 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -120,6 +120,7 @@ struct stk3310_data {
struct regmap_field *reg_flag_psint;
struct regmap_field *reg_flag_nf;
struct regulator *vdd_reg;
+ struct regulator *led_reg;
};
static const struct iio_event_spec stk3310_events[] = {
@@ -614,6 +615,10 @@ static int stk3310_probe(struct i2c_client *client)
if (IS_ERR(data->vdd_reg))
return dev_err_probe(>dev, ret, "get regulator vdd
failed\n");
+ data->led_reg = devm_regulator_get(>dev, "leda");
+ if (IS_ERR(data->led_reg))
+ return dev_err_probe(>dev, ret, "get regulator led
failed\n");
+
ret = stk3310_regmap_init(data);
if (ret < 0)
return ret;
@@ -629,12 +634,18 @@ static int stk3310_probe(struct i2c_client *client)
return dev_err_probe(>dev, ret,
"regulator vdd enable failed\n");
+ ret = regulator_enable(data->led_reg);
+ if (ret) {
+ dev_err_probe(>dev, ret, "regulator led enable
failed\n");
+ goto err_vdd_disable;
+ }
+
/* we need a short delay to allow the chip time to power on */
fsleep(1000);
ret = stk3310_init(indio_dev);
if (ret < 0)
- goto err_vdd_disable;
+ goto err_led_disable;
if (client->irq > 0) {
ret = devm_request_threaded_irq(>dev, client->irq,
@@ -660,6 +671,8 @@ static int stk3310_probe(struct i2c_client *client)
err_standby:
stk3310_set_state(data, STK3310_STATE_STANDBY);
+err_led_disable:
+ regulator_disable(data->led_reg);
err_vdd_disable:
regulator_disable(data->vdd_reg);
return ret;
@@ -672,6 +685,7 @@ static void stk3310_remove(struct i2c_client *client)
iio_device_unregister(indio_dev);
stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
+ regulator_disable(data->led_reg);
regulator_disable(data->vdd_reg);
}
@@ -687,6 +701,7 @@ static int stk3310_suspend(struct device *dev)
return ret;
regcache_mark_dirty(data->regmap);
+ regulator_disable(data->led_reg);
regulator_disable(data->vdd_reg);
return 0;
@@ -706,6 +721,12 @@ static int stk3310_resume(struct device *dev)
return ret;
}
+ ret = regulator_enable(data->led_reg);
+ if (ret) {
+ dev_err(dev, "Failed to re-enable regulator led\n");
+ return ret;
+ }
+
fsleep(1000);
ret = regcache_sync(data->regmap);
--
2.44.0
[PATCH v2 2/6] iio: light: stk3310: Implement vdd supply and power it off during suspend
From: Ondrej Jirman
VDD power input can be used to completely power off the chip during
system suspend. Do so if available.
Signed-off-by: Ondrej Jirman
Signed-off-by: Aren Moynihan
---
Notes:
Changes in v2:
- always enable / disable regulators and rely on a dummy regulator if
one isn't specified
- replace usleep_range with fsleep
- reorder includes so iio headers are last
- add missing error handling to resume
drivers/iio/light/stk3310.c | 49 ++---
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index 7b71ad71d78d..a0547eeca3e3 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -13,6 +13,8 @@
#include
#include
#include
+#include
+
#include
#include
#include
@@ -117,6 +119,7 @@ struct stk3310_data {
struct regmap_field *reg_int_ps;
struct regmap_field *reg_flag_psint;
struct regmap_field *reg_flag_nf;
+ struct regulator *vdd_reg;
};
static const struct iio_event_spec stk3310_events[] = {
@@ -607,6 +610,10 @@ static int stk3310_probe(struct i2c_client *client)
mutex_init(>lock);
+ data->vdd_reg = devm_regulator_get(>dev, "vdd");
+ if (IS_ERR(data->vdd_reg))
+ return dev_err_probe(>dev, ret, "get regulator vdd
failed\n");
+
ret = stk3310_regmap_init(data);
if (ret < 0)
return ret;
@@ -617,9 +624,17 @@ static int stk3310_probe(struct i2c_client *client)
indio_dev->channels = stk3310_channels;
indio_dev->num_channels = ARRAY_SIZE(stk3310_channels);
+ ret = regulator_enable(data->vdd_reg);
+ if (ret)
+ return dev_err_probe(>dev, ret,
+"regulator vdd enable failed\n");
+
+ /* we need a short delay to allow the chip time to power on */
+ fsleep(1000);
+
ret = stk3310_init(indio_dev);
if (ret < 0)
- return ret;
+ goto err_vdd_disable;
if (client->irq > 0) {
ret = devm_request_threaded_irq(>dev, client->irq,
@@ -645,32 +660,60 @@ static int stk3310_probe(struct i2c_client *client)
err_standby:
stk3310_set_state(data, STK3310_STATE_STANDBY);
+err_vdd_disable:
+ regulator_disable(data->vdd_reg);
return ret;
}
static void stk3310_remove(struct i2c_client *client)
{
struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct stk3310_data *data = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
+ regulator_disable(data->vdd_reg);
}
static int stk3310_suspend(struct device *dev)
{
struct stk3310_data *data;
+ int ret;
data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
- return stk3310_set_state(data, STK3310_STATE_STANDBY);
+ ret = stk3310_set_state(data, STK3310_STATE_STANDBY);
+ if (ret)
+ return ret;
+
+ regcache_mark_dirty(data->regmap);
+ regulator_disable(data->vdd_reg);
+
+ return 0;
}
static int stk3310_resume(struct device *dev)
{
- u8 state = 0;
struct stk3310_data *data;
+ u8 state = 0;
+ int ret;
data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+
+ ret = regulator_enable(data->vdd_reg);
+ if (ret) {
+ dev_err(dev, "Failed to re-enable regulator vdd\n");
+ return ret;
+ }
+
+ fsleep(1000);
+
+ ret = regcache_sync(data->regmap);
+ if (ret) {
+ dev_err(dev, "Failed to restore registers: %d\n", ret);
+ return ret;
+ }
+
if (data->ps_enabled)
state |= STK3310_STATE_EN_PS;
if (data->als_enabled)
--
2.44.0
[PATCH v2 6/6] arm64: dts: allwinner: pinephone: Add power supplies to stk3311
From: Ondrej Jirman
This makes the driver disable the supply during sleep.
Signed-off-by: Ondrej Jirman
Signed-off-by: Aren Moynihan
---
Notes:
Changes in v2:
- add leda-supply
arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index b5a232209f2b..51ab1db95f81 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -250,6 +250,8 @@ light-sensor@48 {
reg = <0x48>;
interrupt-parent = <>;
interrupts = <1 0 IRQ_TYPE_EDGE_FALLING>; /* PB0 */
+ vdd-supply = <_ldo_io0>;
+ leda-supply = <_dldo1>;
};
/* Accelerometer/gyroscope */
--
2.44.0
[PATCH v2 5/6] iio: light: stk3310: log error if reading the chip id fails
If the chip isn't powered, this call is likely to return an error. Without a log here the driver will silently fail to probe. Common errors are ENXIO (when the chip isn't powered) and ETIMEDOUT (when the i2c bus isn't powered). Signed-off-by: Aren Moynihan --- Notes: Changes in v2: - use dev_err_probe drivers/iio/light/stk3310.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c index c56c6298d292..6ee6f145a6d5 100644 --- a/drivers/iio/light/stk3310.c +++ b/drivers/iio/light/stk3310.c @@ -475,7 +475,7 @@ static int stk3310_init(struct iio_dev *indio_dev) ret = regmap_read(data->regmap, STK3310_REG_ID, ); if (ret < 0) - return ret; + return dev_err_probe(>dev, ret, "failed to read chip id\n"); if (chipid != STK3310_CHIP_ID_VAL && chipid != STK3311_CHIP_ID_VAL && -- 2.44.0
[PATCH v2 1/6] dt-bindings: iio: light: stk33xx: add vdd and leda regulators
stk3310 and stk3311 are typically connected to power supplies for the chip (vdd) and the infrared LED (leda). Add properties so we can power these up / down appropriately. Signed-off-by: Aren Moynihan --- Notes: Changes in v2: - add leda-supply - add supplies to examples Documentation/devicetree/bindings/iio/light/stk33xx.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml index f6e22dc9814a..43ead524cecb 100644 --- a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml +++ b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml @@ -29,6 +29,8 @@ properties: interrupts: maxItems: 1 + vdd-supply: true + leda-supply: true proximity-near-level: true required: @@ -52,6 +54,8 @@ examples: proximity-near-level = <25>; interrupt-parent = <>; interrupts = <5 IRQ_TYPE_LEVEL_LOW>; +vdd-supply = <_regulator>; +leda-supply = <_regulator>; }; }; ... -- 2.44.0
[PATCH v2 0/6] iio: light: stk3310: support powering off during suspend
In the Pine64 PinePhone, the stk3310 chip is powered by a regulator that is disabled at system boot and can be shut off during suspend. To ensure that the chip properly initializes, both after boot and suspend, we need to manage this regulator. Additionally if the chip is shut off in suspend, we need to make sure that it gets reinitialized with the same parameters after resume. Major changes in v2: - Add handling of the IR LED. I was hesitant to include this as it is the same as pull-up regulator for the i2c bus on the hardware I have, so I can't test it well. I think leaving it out is more likely to cause issues than including it. - Convert stk3310 to use dev_err_probe for errors. - Always enable / disable regulators and rely on dummy devices if they're not specified. - more listed in individual patches Aren Moynihan (4): dt-bindings: iio: light: stk33xx: add vdd and leda regulators iio: light: stk3310: Manage LED power supply iio: light: stk3310: use dev_err_probe where possible iio: light: stk3310: log error if reading the chip id fails Ondrej Jirman (2): iio: light: stk3310: Implement vdd supply and power it off during suspend arm64: dts: allwinner: pinephone: Add power supplies to stk3311 .../bindings/iio/light/stk33xx.yaml | 4 + .../dts/allwinner/sun50i-a64-pinephone.dtsi | 2 + drivers/iio/light/stk3310.c | 116 +- 3 files changed, 94 insertions(+), 28 deletions(-) -- 2.44.0
[PATCH v2 4/6] iio: light: stk3310: use dev_err_probe where possible
Using dev_err_probe instead of dev_err and return makes the errors
easier to understand by including the error name, and saves a little
code.
Signed-off-by: Aren Moynihan
---
Notes:
Added in v2
drivers/iio/light/stk3310.c | 44 +
1 file changed, 20 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index ee1ac95dbc0e..c56c6298d292 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -60,10 +60,10 @@
data->reg_##name = \
devm_regmap_field_alloc(>dev, regmap, \
stk3310_reg_field_##name); \
- if (IS_ERR(data->reg_##name)) { \
- dev_err(>dev, "reg field alloc failed.\n"); \
- return PTR_ERR(data->reg_##name); \
- } \
+ if (IS_ERR(data->reg_##name)) \
+ return dev_err_probe(>dev, \
+ PTR_ERR(data->reg_##name), \
+ "reg field alloc failed.\n"); \
} while (0)
static const struct reg_field stk3310_reg_field_state =
@@ -480,22 +480,20 @@ static int stk3310_init(struct iio_dev *indio_dev)
if (chipid != STK3310_CHIP_ID_VAL &&
chipid != STK3311_CHIP_ID_VAL &&
chipid != STK3311X_CHIP_ID_VAL &&
- chipid != STK3335_CHIP_ID_VAL) {
- dev_err(>dev, "invalid chip id: 0x%x\n", chipid);
- return -ENODEV;
- }
+ chipid != STK3335_CHIP_ID_VAL)
+ return dev_err_probe(>dev, -ENODEV,
+"invalid chip id: 0x%x\n", chipid);
state = STK3310_STATE_EN_ALS | STK3310_STATE_EN_PS;
ret = stk3310_set_state(data, state);
- if (ret < 0) {
- dev_err(>dev, "failed to enable sensor");
- return ret;
- }
+ if (ret < 0)
+ return dev_err_probe(>dev, ret, "failed to enable
sensor\n");
/* Enable PS interrupts */
ret = regmap_field_write(data->reg_int_ps, STK3310_PSINT_EN);
if (ret < 0)
- dev_err(>dev, "failed to enable interrupts!\n");
+ return dev_err_probe(>dev, ret,
+"failed to enable interrupts!\n");
return ret;
}
@@ -530,10 +528,10 @@ static int stk3310_regmap_init(struct stk3310_data *data)
client = data->client;
regmap = devm_regmap_init_i2c(client, _regmap_config);
- if (IS_ERR(regmap)) {
- dev_err(>dev, "regmap initialization failed.\n");
- return PTR_ERR(regmap);
- }
+ if (IS_ERR(regmap))
+ return dev_err_probe(>dev, PTR_ERR(regmap),
+"regmap initialization failed.\n");
+
data->regmap = regmap;
STK3310_REGFIELD(state);
@@ -597,10 +595,8 @@ static int stk3310_probe(struct i2c_client *client)
struct stk3310_data *data;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*data));
- if (!indio_dev) {
- dev_err(>dev, "iio allocation failed!\n");
- return -ENOMEM;
- }
+ if (!indio_dev)
+ return dev_err_probe(>dev, -ENOMEM, "iio allocation
failed!\n");
data = iio_priv(indio_dev);
data->client = client;
@@ -655,15 +651,15 @@ static int stk3310_probe(struct i2c_client *client)
IRQF_ONESHOT,
STK3310_EVENT, indio_dev);
if (ret < 0) {
- dev_err(>dev, "request irq %d failed\n",
- client->irq);
+ dev_err_probe(>dev, ret, "request irq %d
failed\n",
+ client->irq);
goto err_standby;
}
}
ret = iio_device_register(indio_dev);
if (ret < 0) {
- dev_err(>dev, "device_register failed\n");
+ dev_err_probe(>dev, ret, "device_register failed\n");
goto err_standby;
}
--
2.44.0
Re: [PATCH 2/4] iio: light: stk3310: Implement vdd supply and power it off during suspend
On Thu, Apr 18, 2024 at 06:56:09PM +0300, Andy Shevchenko wrote: > On Thu, Apr 18, 2024 at 6:06 PM Aren wrote: > > On Mon, Apr 15, 2024 at 05:04:53PM +0300, Andy Shevchenko wrote: > > > On Sun, Apr 14, 2024 at 8:57 PM Aren Moynihan > > > wrote: > > ... > > > > > stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY); > > > > + if (data->vdd_reg) > > > > + regulator_disable(data->vdd_reg); > > > > > > I forgot to check the order of freeing resources, be sure you have no > > > devm_*() releases happening before this call. > > > > If I understand what you're saying, this should be fine. The driver just > > uses devm to clean up acquired resources after remove is called. Or am I > > missing something and resources could be freed before calling > > stk3310_remove? > > I'm not objecting to that. The point here is that the resources should > be freed in the reversed order. devm-allocated resources are deferred > to be freed after the explicit driver ->remove() callback. At the end > it should not interleave with each other, i.o.w. it should be > probe: devm followed by non-devm > remove: non-devm only. I think what you're describing is already the case, with the exception of parts of the probe function not changed in this patch mixing acquiring resources through devm with configuring the device. I hope I'm not being dense, thanks for the clarification - Aren
Re: [PATCH 2/4] iio: light: stk3310: Implement vdd supply and power it off during suspend
On Mon, Apr 15, 2024 at 05:04:53PM +0300, Andy Shevchenko wrote:
> On Sun, Apr 14, 2024 at 8:57 PM Aren Moynihan wrote:
> >
> > From: Ondrej Jirman
> >
> > VDD power input can be used to completely power off the chip during
> > system suspend. Do so if available.
>
> ...
>
> > #include
> > #include
> > #include
>
> > +#include
>
> Move it to be ordered and add a blank line to separate iio/*.h group.
>
> ...
>
> > + data->vdd_reg = devm_regulator_get_optional(>dev, "vdd");
> > + if (IS_ERR(data->vdd_reg)) {
> > + ret = PTR_ERR(data->vdd_reg);
> > + if (ret == -ENODEV)
> > + data->vdd_reg = NULL;
>
> > + else
>
> Redundant 'else' when you follow the pattern "check for error condition
> first".
>
> > + return dev_err_probe(>dev, ret,
> > +"get regulator vdd failed\n");
> > + }
>
> ...
>
> > + if (data->vdd_reg) {
> > + ret = regulator_enable(data->vdd_reg);
> > + if (ret)
> > + return dev_err_probe(>dev, ret,
> > +"regulator vdd enable
> > failed\n");
> > +
> > + usleep_range(1000, 2000);
>
> fsleep()
>
> > + }
>
> ...
>
> > stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
> > + if (data->vdd_reg)
> > + regulator_disable(data->vdd_reg);
>
> I forgot to check the order of freeing resources, be sure you have no
> devm_*() releases happening before this call.
If I understand what you're saying, this should be fine. The driver just
uses devm to clean up acquired resources after remove is called. Or am I
missing something and resources could be freed before calling
stk3310_remove?
> ...
>
> > + usleep_range(1000, 2000);
>
> fsleep()
Everything else makes sense, I'll include those in v2 along with a patch
to switch stk3310_init to dev_err_probe.
Thanks for taking the time to review
- Aren
[PATCH 4/4] arm64: dts: allwinner: pinephone: Add power supply to stk3311
From: Ondrej Jirman
This makes the driver disable the supply during sleep.
Signed-off-by: Ondrej Jirman
Signed-off-by: Aren Moynihan
---
arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index b5a232209f2b..e87bc21db316 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -250,6 +250,7 @@ light-sensor@48 {
reg = <0x48>;
interrupt-parent = <>;
interrupts = <1 0 IRQ_TYPE_EDGE_FALLING>; /* PB0 */
+ vdd-supply = <_ldo_io0>;
};
/* Accelerometer/gyroscope */
--
2.44.0
[PATCH 3/4] iio: light: stk3310: log error if reading the chip id fails
If the chip isn't powered, this call is likely to return an error.
Without a log here the driver will silently fail to probe. Common errors
are ENXIO (when the chip isn't powered) and ETIMEDOUT (when the i2c bus
isn't powered).
Signed-off-by: Aren Moynihan
---
drivers/iio/light/stk3310.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index bfa090538df7..c0954a63a143 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -472,8 +472,10 @@ static int stk3310_init(struct iio_dev *indio_dev)
struct i2c_client *client = data->client;
ret = regmap_read(data->regmap, STK3310_REG_ID, );
- if (ret < 0)
+ if (ret < 0) {
+ dev_err(>dev, "failed to read chip id: %d", ret);
return ret;
+ }
if (chipid != STK3310_CHIP_ID_VAL &&
chipid != STK3311_CHIP_ID_VAL &&
--
2.44.0
[PATCH 2/4] iio: light: stk3310: Implement vdd supply and power it off during suspend
From: Ondrej Jirman
VDD power input can be used to completely power off the chip during
system suspend. Do so if available.
Signed-off-by: Ondrej Jirman
Signed-off-by: Aren Moynihan
---
drivers/iio/light/stk3310.c | 56 +++--
1 file changed, 53 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index 7b71ad71d78d..bfa090538df7 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#define STK3310_REG_STATE 0x00
#define STK3310_REG_PSCTRL 0x01
@@ -117,6 +118,7 @@ struct stk3310_data {
struct regmap_field *reg_int_ps;
struct regmap_field *reg_flag_psint;
struct regmap_field *reg_flag_nf;
+ struct regulator *vdd_reg;
};
static const struct iio_event_spec stk3310_events[] = {
@@ -607,6 +609,16 @@ static int stk3310_probe(struct i2c_client *client)
mutex_init(>lock);
+ data->vdd_reg = devm_regulator_get_optional(>dev, "vdd");
+ if (IS_ERR(data->vdd_reg)) {
+ ret = PTR_ERR(data->vdd_reg);
+ if (ret == -ENODEV)
+ data->vdd_reg = NULL;
+ else
+ return dev_err_probe(>dev, ret,
+"get regulator vdd failed\n");
+ }
+
ret = stk3310_regmap_init(data);
if (ret < 0)
return ret;
@@ -617,9 +629,18 @@ static int stk3310_probe(struct i2c_client *client)
indio_dev->channels = stk3310_channels;
indio_dev->num_channels = ARRAY_SIZE(stk3310_channels);
+ if (data->vdd_reg) {
+ ret = regulator_enable(data->vdd_reg);
+ if (ret)
+ return dev_err_probe(>dev, ret,
+"regulator vdd enable failed\n");
+
+ usleep_range(1000, 2000);
+ }
+
ret = stk3310_init(indio_dev);
if (ret < 0)
- return ret;
+ goto err_vdd_disable;
if (client->irq > 0) {
ret = devm_request_threaded_irq(>dev, client->irq,
@@ -645,32 +666,61 @@ static int stk3310_probe(struct i2c_client *client)
err_standby:
stk3310_set_state(data, STK3310_STATE_STANDBY);
+err_vdd_disable:
+ if (data->vdd_reg)
+ regulator_disable(data->vdd_reg);
return ret;
}
static void stk3310_remove(struct i2c_client *client)
{
struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct stk3310_data *data = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
+ if (data->vdd_reg)
+ regulator_disable(data->vdd_reg);
}
static int stk3310_suspend(struct device *dev)
{
struct stk3310_data *data;
+ int ret;
data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
- return stk3310_set_state(data, STK3310_STATE_STANDBY);
+ ret = stk3310_set_state(data, STK3310_STATE_STANDBY);
+ if (ret)
+ return ret;
+
+ if (data->vdd_reg) {
+ regcache_mark_dirty(data->regmap);
+ regulator_disable(data->vdd_reg);
+ }
+
+ return 0;
}
static int stk3310_resume(struct device *dev)
{
- u8 state = 0;
struct stk3310_data *data;
+ u8 state = 0;
+ int ret;
data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+
+ if (data->vdd_reg) {
+ ret = regulator_enable(data->vdd_reg);
+ if (ret) {
+ dev_err(dev, "Failed to re-enable regulator vdd\n");
+ return ret;
+ }
+
+ usleep_range(1000, 2000);
+ regcache_sync(data->regmap);
+ }
+
if (data->ps_enabled)
state |= STK3310_STATE_EN_PS;
if (data->als_enabled)
--
2.44.0
[PATCH 1/4] dt-bindings: iio: light: stk33xx: add regulator for vdd supply
Signed-off-by: Aren Moynihan --- Documentation/devicetree/bindings/iio/light/stk33xx.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml index f6e22dc9814a..db35e239d4a8 100644 --- a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml +++ b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml @@ -29,6 +29,7 @@ properties: interrupts: maxItems: 1 + vdd-supply: true proximity-near-level: true required: -- 2.44.0
[PATCH 0/4] iio: light: stk3310: support powering off during suspend
In the Pine64 PinePhone, the stk3310 chip is powered by a regulator that is disabled at system boot and can be shut off during suspend. To ensure that the chip properly initializes, both after boot and suspend, we need to manage this regulator. Additionally if the chip is shut off in suspend, we need to make sure that it gets reinitialized with the same parameters after resume. Aren Moynihan (2): dt-bindings: iio: light: stk33xx: add regulator for vdd supply iio: light: stk3310: log error if reading the chip id fails Ondrej Jirman (2): iio: light: stk3310: Implement vdd supply and power it off during suspend arm64: dts: allwinner: pinephone: Add power supply to stk3311 .../bindings/iio/light/stk33xx.yaml | 1 + .../dts/allwinner/sun50i-a64-pinephone.dtsi | 1 + drivers/iio/light/stk3310.c | 60 +-- 3 files changed, 58 insertions(+), 4 deletions(-) -- 2.44.0
