Hi Rudraksha, this is a clear improvement! I like the directions this is going.
Mark Brown needs to look at how you're using regulators here. On Wed, Mar 18, 2026 at 7:34 PM Rudraksha Gupta via B4 Relay <[email protected]> wrote: > From: Rudraksha Gupta <[email protected]> > > Extend the RT8515 driver to support flash ICs that use only a single > GPIO for both flash and torch modes (no separate ENT pin), with an > optional vin regulator that gates power to the flash IC. > > When vin-supply is provided, the driver enables the regulator before > activating the LED and disables it when turning off. > > Make ent-gpios optional and validate at probe time that exactly one of > ent-gpios or vin-supply is provided. When ent-gpios is absent, the > driver uses enf-gpios for both flash and torch brightness control. > > Signed-off-by: Rudraksha Gupta <[email protected]> (...) > struct regulator *reg; > + bool reg_enabled; I think you can actually ask a regulator if it is enabled already, regulator_is_enabled(). Also as long as you enable and disable it the same number of times it also contains an internal reference count, so this is often enough. > + if (rt->reg && rt->reg_enabled) { > + regulator_disable(rt->reg); > + rt->reg_enabled = false; > + } I don't think you need to NULL-check rt-reg but not sure. Can you use regulator_is_enabled() instead of the bool local? > + /* Enable the vin regulator if needed */ > + if (rt->reg && !rt->reg_enabled) { Dito > + /* Enable the vin regulator if needed */ > + if (rt->reg && !rt->reg_enabled) { > + ret = regulator_enable(rt->reg); Dito > if (state) { > + /* Enable the vin regulator if needed */ > + if (rt->reg && !rt->reg_enabled) { Dito. > + /* Optional VIN supply (e.g. GPIO-controlled fixed regulator) */ > + rt->reg = devm_regulator_get_optional(dev, "vin"); > + if (IS_ERR(rt->reg)) { > + if (PTR_ERR(rt->reg) == -ENODEV) > + rt->reg = NULL; I think the regulator callbacks are able to deal transparently with "regulators" that are -ENODEV, can you just let it pass? Anyway Mark knows what to do here. > + /* Exactly one of ENT or VIN must be provided */ > + if (!rt->enable_torch == !rt->reg) > + return dev_err_probe(dev, -EINVAL, > + "exactly one of ent-gpios or vin-supply > is required\n"); Please drop this check. I think there can be systems using the torch but also define the regulator. Yours, Linus Walleij
