| Message ID | 20180222125923.57385-4-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Please use PCI: Re-use ... to match the prevailing drivers/pci style. On Thu, Feb 22, 2018 at 02:59:23PM +0200, Andy Shevchenko wrote: > ...instead of open coding its functionality. Same comment about making the changelog complete, independent of the subject. > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> Let me know if you want me to take the series. Otherwise I'll assume it goes elsewhere. > --- > drivers/pci/pci.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index f6a4dd10d9b0..ae654e21439d 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -2258,8 +2258,6 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev) > */ > bool pci_bridge_d3_possible(struct pci_dev *bridge) > { > - unsigned int year; > - > if (!pci_is_pcie(bridge)) > return false; > > @@ -2287,10 +2285,8 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) > * It should be safe to put PCIe ports from 2015 or newer > * to D3. > */ > - if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) && > - year >= 2015) { > + if (dmi_get_bios_year() >= 2015) > return true; > - } > break; > } > > -- > 2.16.1 >
On Fri, 2018-02-23 at 15:40 -0600, Bjorn Helgaas wrote: > Please use > > PCI: Re-use ... > > to match the prevailing drivers/pci style. Noted. (Same for x86/PCI part) > On Thu, Feb 22, 2018 at 02:59:23PM +0200, Andy Shevchenko wrote: > > ...instead of open coding its functionality. > > Same comment about making the changelog complete, independent of the > subject. Any suggestion how it would look like? (Same question for previous comment) > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> Thanks! > Let me know if you want me to take the series. Otherwise I'll assume > it > goes elsewhere. Ingo took them into x86/platform.
On Sun, Feb 25, 2018 at 03:27:04PM +0200, Andy Shevchenko wrote: > On Fri, 2018-02-23 at 15:40 -0600, Bjorn Helgaas wrote: > > On Thu, Feb 22, 2018 at 02:59:23PM +0200, Andy Shevchenko wrote: > > > ...instead of open coding its functionality. > > > > Same comment about making the changelog complete, independent of the > > subject. > > Any suggestion how it would look like? (Same question for previous > comment) PCI: Re-use new dmi_get_bios_year() helper Use new dmi_get_bios_year() helper instead of open-coding its functionality. The usual document structure is something like: TITLE This abstract contains a summary of the entire document, in a few paragraphs of complete sentences. Where "TITLE" makes sense all by itself, even without reading the body, and "Body" is a complete statement that also makes sense all by itself without having to read "TITLE" first. Granted, it's trivial, but following the convention improves readability slightly because it fits the reader's expectations. When the body is "...instead of open coding its functionality", it's a bit of a hiccup because I have to start over and look back up to the title to re-read the thing as a whole. Bjorn
On Thu, 22 Feb 2018 14:59:23 +0200, Andy Shevchenko wrote: > ...instead of open coding its functionality. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pci/pci.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index f6a4dd10d9b0..ae654e21439d 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -2258,8 +2258,6 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev) > */ > bool pci_bridge_d3_possible(struct pci_dev *bridge) > { > - unsigned int year; > - > if (!pci_is_pcie(bridge)) > return false; > > @@ -2287,10 +2285,8 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) > * It should be safe to put PCIe ports from 2015 or newer > * to D3. > */ > - if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) && > - year >= 2015) { > + if (dmi_get_bios_year() >= 2015) > return true; > - } > break; > } > Reviewed-by: Jean Delvare <jdelvare@suse.de>
On Mon, 2018-02-26 at 12:19 -0600, Bjorn Helgaas wrote: > On Sun, Feb 25, 2018 at 03:27:04PM +0200, Andy Shevchenko wrote: > > On Fri, 2018-02-23 at 15:40 -0600, Bjorn Helgaas wrote: > > > On Thu, Feb 22, 2018 at 02:59:23PM +0200, Andy Shevchenko wrote: > > > > ...instead of open coding its functionality. > > > > > > Same comment about making the changelog complete, independent of > > > the > > > subject. > > > > Any suggestion how it would look like? (Same question for previous > > comment) > > PCI: Re-use new dmi_get_bios_year() helper > > Use new dmi_get_bios_year() helper instead of open-coding its > functionality. > > The usual document structure is something like: > > TITLE > > This abstract contains a summary of the entire document, in a few > paragraphs of complete sentences. > > Where "TITLE" makes sense all by itself, even without reading the > body, and "Body" is a complete statement that also makes sense all by > itself without having to read "TITLE" first. > Thank you for a hint! > Granted, it's trivial, but following the convention improves > readability slightly because it fits the reader's expectations. > When the body is "...instead of open coding its functionality", it's a > bit of a hiccup because I have to start over and look back up to the > title to re-read the thing as a whole. OK, I got your point, though I don't like duplication in the subject and body.
On Wed, Feb 28, 2018 at 12:12:22PM +0200, Andy Shevchenko wrote: > On Mon, 2018-02-26 at 12:19 -0600, Bjorn Helgaas wrote: > > On Sun, Feb 25, 2018 at 03:27:04PM +0200, Andy Shevchenko wrote: > > > On Fri, 2018-02-23 at 15:40 -0600, Bjorn Helgaas wrote: > > > > On Thu, Feb 22, 2018 at 02:59:23PM +0200, Andy Shevchenko wrote: > > > > > ...instead of open coding its functionality. > > > > > > > > Same comment about making the changelog complete, independent of > > > > the > > > > subject. > > > > > > Any suggestion how it would look like? (Same question for previous > > > comment) > > > > PCI: Re-use new dmi_get_bios_year() helper > > > > Use new dmi_get_bios_year() helper instead of open-coding its > > functionality. > > > > The usual document structure is something like: > > > > TITLE > > > > This abstract contains a summary of the entire document, in a few > > paragraphs of complete sentences. > > > > Where "TITLE" makes sense all by itself, even without reading the > > body, and "Body" is a complete statement that also makes sense all by > > itself without having to read "TITLE" first. > > > > Thank you for a hint! > > > Granted, it's trivial, but following the convention improves > > readability slightly because it fits the reader's expectations. > > > When the body is "...instead of open coding its functionality", it's a > > bit of a hiccup because I have to start over and look back up to the > > title to re-read the thing as a whole. > > OK, I got your point, though I don't like duplication in the subject and > body. Ah, I see. I think of the subject and the body as serving two distinct purposes, so for me there's no issue even if they happen to contain exactly the same text. Bjorn
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index f6a4dd10d9b0..ae654e21439d 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2258,8 +2258,6 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev) */ bool pci_bridge_d3_possible(struct pci_dev *bridge) { - unsigned int year; - if (!pci_is_pcie(bridge)) return false; @@ -2287,10 +2285,8 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) * It should be safe to put PCIe ports from 2015 or newer * to D3. */ - if (dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL) && - year >= 2015) { + if (dmi_get_bios_year() >= 2015) return true; - } break; }
...instead of open coding its functionality. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pci/pci.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)