| Message ID | 58f2ff7b90233fad3d7ae2e9d66d5192e2c1ac01.1645437439.git.sylv@sylv.io (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add supply into PWBUS_REGULATOR macro | expand |
On Mon, Feb 21, 2022 at 12:09:56PM +0100, Marcello Sylvester Bauer wrote: > Add regulator supply into PWBUS_REGULATOR macro. This makes it optional > to define a vin-supply in DT. Not defining a supply will add a dummy > regulator supply instead and only cause the following debug output: > > ``` > Looking up vin-supply property in node [...] failed > ``` > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> Applied to hwmon-next. That should give it some time to mature, and we can pull or modify it if it causes any problems. Thanks, Guenter > --- > drivers/hwmon/pmbus/pmbus.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h > index e0aa8aa46d8c..38f049d68d32 100644 > --- a/drivers/hwmon/pmbus/pmbus.h > +++ b/drivers/hwmon/pmbus/pmbus.h > @@ -464,6 +464,7 @@ extern const struct regulator_ops pmbus_regulator_ops; > #define PMBUS_REGULATOR(_name, _id) \ > [_id] = { \ > .name = (_name # _id), \ > + .supply_name = "vin", \ > .id = (_id), \ > .of_match = of_match_ptr(_name # _id), \ > .regulators_node = of_match_ptr("regulators"), \
On Tue, Feb 22, 2022 at 08:51:04AM PST, Guenter Roeck wrote: >On Mon, Feb 21, 2022 at 12:09:56PM +0100, Marcello Sylvester Bauer wrote: >> Add regulator supply into PWBUS_REGULATOR macro. This makes it optional >> to define a vin-supply in DT. Not defining a supply will add a dummy >> regulator supply instead and only cause the following debug output: >> >> ``` >> Looking up vin-supply property in node [...] failed >> ``` >> >> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> > >Applied to hwmon-next. That should give it some time to mature, >and we can pull or modify it if it causes any problems. > Wish I'd caught this sooner, but unfortunately I've just discovered that this does in fact cause breakage on my systems -- having regulator-dummy set as a supply on my PMBus regulators (instead of having them as their own top-level regulators without an upstream supply) leads to enable-count underflow errors when disabling them: # echo 0 > /sys/bus/platform/devices/efuse01/state [ 906.094477] regulator-dummy: Underflow of regulator enable count [ 906.100563] Failed to disable vout: -EINVAL [ 136.992676] reg-userspace-consumer efuse01: Failed to configure state: -22 A simple revert solves the problem for me, but since I'm honestly a little unclear on the intent of the patch itself I'm not sure what a revert might break and hence I don't know if that's necessarily the right fix. Marcello (or others), any thoughts? Thanks, Zev
On Fri, Nov 04, 2022 at 03:30:30PM -0700, Zev Weiss wrote: > On Tue, Feb 22, 2022 at 08:51:04AM PST, Guenter Roeck wrote: > > On Mon, Feb 21, 2022 at 12:09:56PM +0100, Marcello Sylvester Bauer wrote: > > > Add regulator supply into PWBUS_REGULATOR macro. This makes it optional > > > to define a vin-supply in DT. Not defining a supply will add a dummy > > > regulator supply instead and only cause the following debug output: > > > > > > ``` > > > Looking up vin-supply property in node [...] failed > > > ``` > > > > > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> > > > > Applied to hwmon-next. That should give it some time to mature, > > and we can pull or modify it if it causes any problems. > > > > Wish I'd caught this sooner, but unfortunately I've just discovered that > this does in fact cause breakage on my systems -- having regulator-dummy set > as a supply on my PMBus regulators (instead of having them as their own > top-level regulators without an upstream supply) leads to enable-count > underflow errors when disabling them: > > # echo 0 > /sys/bus/platform/devices/efuse01/state > [ 906.094477] regulator-dummy: Underflow of regulator enable count > [ 906.100563] Failed to disable vout: -EINVAL > [ 136.992676] reg-userspace-consumer efuse01: Failed to configure state: -22 > > A simple revert solves the problem for me, but since I'm honestly a little > unclear on the intent of the patch itself I'm not sure what a revert might > break and hence I don't know if that's necessarily the right fix. Marcello > (or others), any thoughts? Revert now, ask questions later. I'll send a patch. Guenter > > > Thanks, > Zev >
On 11/5/22 00:42, Guenter Roeck wrote: > On Fri, Nov 04, 2022 at 03:30:30PM -0700, Zev Weiss wrote: >> On Tue, Feb 22, 2022 at 08:51:04AM PST, Guenter Roeck wrote: >>> On Mon, Feb 21, 2022 at 12:09:56PM +0100, Marcello Sylvester Bauer wrote: >>>> Add regulator supply into PWBUS_REGULATOR macro. This makes it optional >>>> to define a vin-supply in DT. Not defining a supply will add a dummy >>>> regulator supply instead and only cause the following debug output: >>>> >>>> ``` >>>> Looking up vin-supply property in node [...] failed >>>> ``` >>>> >>>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> >>> Applied to hwmon-next. That should give it some time to mature, >>> and we can pull or modify it if it causes any problems. >>> >> Wish I'd caught this sooner, but unfortunately I've just discovered that >> this does in fact cause breakage on my systems -- having regulator-dummy set >> as a supply on my PMBus regulators (instead of having them as their own >> top-level regulators without an upstream supply) leads to enable-count >> underflow errors when disabling them: >> >> # echo 0 > /sys/bus/platform/devices/efuse01/state >> [ 906.094477] regulator-dummy: Underflow of regulator enable count >> [ 906.100563] Failed to disable vout: -EINVAL >> [ 136.992676] reg-userspace-consumer efuse01: Failed to configure state: -22 >> >> A simple revert solves the problem for me, but since I'm honestly a little >> unclear on the intent of the patch itself I'm not sure what a revert might >> break and hence I don't know if that's necessarily the right fix. Marcello >> (or others), any thoughts? Oh, my bad. I thought this makes it optional to add a supply without having a negative effect. Reverting this patch makes sense, but I'm not sure how else to integrate this. Thanks, Marcello > Revert now, ask questions later. I'll send a patch. > > Guenter > >> >> Thanks, >> Zev >>
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h index e0aa8aa46d8c..38f049d68d32 100644 --- a/drivers/hwmon/pmbus/pmbus.h +++ b/drivers/hwmon/pmbus/pmbus.h @@ -464,6 +464,7 @@ extern const struct regulator_ops pmbus_regulator_ops; #define PMBUS_REGULATOR(_name, _id) \ [_id] = { \ .name = (_name # _id), \ + .supply_name = "vin", \ .id = (_id), \ .of_match = of_match_ptr(_name # _id), \ .regulators_node = of_match_ptr("regulators"), \
Add regulator supply into PWBUS_REGULATOR macro. This makes it optional to define a vin-supply in DT. Not defining a supply will add a dummy regulator supply instead and only cause the following debug output: ``` Looking up vin-supply property in node [...] failed ``` Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> --- drivers/hwmon/pmbus/pmbus.h | 1 + 1 file changed, 1 insertion(+)