On Fri, Oct 22, 2021 at 10:39 AM Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> On Fri, Oct 22, 2021 at 10:06:58AM -0400, Jim Quinlan wrote:
>
> > +enum {
> > + TURN_OFF, /* Turn regulators off, unless an EP is wakeup-capable */
> > + TURN_OFF_ALWAYS, /* Turn regulators off, no exceptions */
> > + TURN_ON, /* Turn regulators on, unless pcie->ep_wakeup_capable */
> > +};
> > +
> > +static int brcm_set_regulators(struct brcm_pcie *pcie, int how)
> > +{
> > + struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
>
> I can't help but think this would be easier to follow as multiple
> functions, there is very little code sharing between the different
> paths especially the on and off paths.
Agree; I just wanted to make less changes to struct pci_host_bridge. Will fix.
>
> > if (pcie->num_supplies) {
> > - (void)brcm_set_regulators(pcie, false);
> > + (void)brcm_set_regulators(pcie, TURN_OFF_ALWAYS);
>
> I should've mentioned this on the earlier path but it's not normal Linux
> style to cast return values to void and looks worrying.
Got it.
Thanks,
JimQ
