On Fri, Apr 08, 2016 at 08:07:23AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Apr 08, 2016 at 01:36:28PM +0300, Mika Westerberg wrote:
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index f0640b7a1c42..aafc1b093bc8 100644
> > --- a/drivers/usb/host/xhci-pci.c
> > +++ b/drivers/usb/host/xhci-pci.c
> > @@ -379,7 +379,7 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
> > * need to have the registers polled during D3, so avoid D3cold.
> > */
> > if (xhci->quirks & XHCI_COMP_MODE_QUIRK)
> > - pdev->no_d3cold = true;
> > + pci_enable_d3cold(pdev, false);
> >
> > if (xhci->quirks & XHCI_PME_STUCK_QUIRK)
> > xhci_pme_quirk(hcd);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 004b8133417d..d8801587b4ca 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -296,6 +296,7 @@ struct pci_dev {
> > unsigned int d2_support:1; /* Low power state D2 is supported */
> > unsigned int no_d1d2:1; /* D1 and D2 are forbidden */
> > unsigned int no_d3cold:1; /* D3cold is forbidden */
> > + unsigned int bridge_d3:1; /* Allow D3 for bridge */
> > unsigned int d3cold_allowed:1; /* D3cold is allowed by user */
> > unsigned int mmio_always_on:1; /* disallow turning off io/mem
> > decoding during bar sizing */
> > @@ -1085,6 +1086,7 @@ int pci_back_from_sleep(struct pci_dev *dev);
> > bool pci_dev_run_wake(struct pci_dev *dev);
> > bool pci_check_pme_status(struct pci_dev *dev);
> > void pci_pme_wakeup_bus(struct pci_bus *bus);
> > +void pci_enable_d3cold(struct pci_dev *dev, bool enable);
>
> That's an ackward api, as is seen in the above use in the xhci driver
> (enable false?)
>
> Why not just make 2 functions:
> pci_dc3cold_enable(struct pci_dev *dev)
> pci_dc3cold_disable(struct pci_dev *dev)
> which makes it obvious what is going on.
>
> Whenever you add a bool to a function, it requires the developer to go
> look up the implementation to see what it means and how to use it, to
> check if the usage is correct. Just make it obvious in the name itself
> so everyone knows exactly what is going on.
>
> Same thing goes for pci_bridge_pm_update(), even with the documentation
> I'm not quite sure what the bool flag is for. Please split this into
> two calls as well.
Will, do. Thanks for the excellent explanation :)
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
