Re: [PATCH v5 5/6] PCI: brcmstb: Do not turn off regulators if EP can wake up

[Jim Quinlan]

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