On Mon, Feb 19, 2024 at 6:50 PM Mark Brown <broonie@xxxxxxxxxx> wrote: > > On Fri, Feb 16, 2024 at 09:32:11PM +0100, Bartosz Golaszewski wrote: > > > +static struct pci_pwrctl_wcn7850_vreg pci_pwrctl_wcn7850_vregs[] = { > > + { > > + .name = "vdd", > > + .load_uA = 16000, > > + }, > > I know a bunch of the QC stuff includes these load numbers but are they > actually doing anything constructive? It keeps coming up that they're > causing a bunch of work and it's not clear that they have any great > effect on modern systems. > Yes, we have what is called a high-power mode and a low-power mode in regulators and these values are used to determine which one to use. > > +static int pci_pwrctl_wcn7850_power_on(struct pci_pwrctl_wcn7850_ctx *ctx) > > +{ > > + int ret; > > + > > + ret = regulator_bulk_enable(ctx->pdata->num_vregs, ctx->regs); > > + if (ret) > > + return ret; > > + > > + ret = clk_prepare_enable(ctx->clk); > > + if (ret) > > + return ret; > > This won't disable the regulators on error. Indeed. Thanks for catching this. Bart