| Message ID | 20180320135410.11627-2-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi Andy, On Tue, 20 Mar 2018 15:54:09 +0200, Andy Shevchenko wrote: > dmi_get_bios_year() may return 0 when it is incapable to parse > the BIOS date string. Previously this has been checked in > pci_acpi_crs_quirks(). > > Update the code to restore old behaviour. When fixing a regression, a good practice is to tag the fix commit as such: Fixes: 69c42d493db4 ("x86/pci: Simplify code by using the new dmi_get_bios_year() helper") > Reported-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > arch/x86/pci/acpi.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index 00e60de30328..5559dcaddd5e 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -140,7 +140,9 @@ static const struct dmi_system_id pci_crs_quirks[] __initconst = { > > void __init pci_acpi_crs_quirks(void) > { > - if ((dmi_get_bios_year() < 2008) && (iomem_resource.end <= 0xffffffff)) > + int year = dmi_get_bios_year(); > + > + if (year >= 0 && year < 2008 && iomem_resource.end <= 0xffffffff) > pci_use_crs = false; > > dmi_check_system(pci_crs_quirks); This works with the non-inline version of dmi_get_bios_year() introduced by patch 2/2 of this series. However the inline version of this function would return 0 on error, which makes the condition above always true. The patches need to be applied in the opposite order, or the test above should be changed to "year > 0". Reviewed-by: Jean Delvare <jdelvare@suse.de>
On Tue, 2018-03-20 at 16:32 +0100, Jean Delvare wrote: > Hi Andy, > > On Tue, 20 Mar 2018 15:54:09 +0200, Andy Shevchenko wrote: > > dmi_get_bios_year() may return 0 when it is incapable to parse > > the BIOS date string. Previously this has been checked in > > pci_acpi_crs_quirks(). > > > > Update the code to restore old behaviour. > > When fixing a regression, a good practice is to tag the fix commit as > such: > > Fixes: 69c42d493db4 ("x86/pci: Simplify code by using the new > dmi_get_bios_year() helper") Someone (perhaps Ingo) told me that Fixes doesn't make much sense if the fixing commit is not yet in vanilla. Since it's supposed to go via tip tree I would like to hear Ingo's opinion before adding this line to the patch. > > Reported-by: Jean Delvare <jdelvare@suse.de> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > arch/x86/pci/acpi.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > > index 00e60de30328..5559dcaddd5e 100644 > > --- a/arch/x86/pci/acpi.c > > +++ b/arch/x86/pci/acpi.c > > @@ -140,7 +140,9 @@ static const struct dmi_system_id > > pci_crs_quirks[] __initconst = { > > > > void __init pci_acpi_crs_quirks(void) > > { > > - if ((dmi_get_bios_year() < 2008) && (iomem_resource.end <= > > 0xffffffff)) > > + int year = dmi_get_bios_year(); > > + > > + if (year >= 0 && year < 2008 && iomem_resource.end <= > > 0xffffffff) > > pci_use_crs = false; > > > > dmi_check_system(pci_crs_quirks); > > This works with the non-inline version of dmi_get_bios_year() > introduced by patch 2/2 of this series. However the inline version of > this function would return 0 on error, which makes the condition above > always true. > > The patches need to be applied in the opposite order, or the test > above > should be changed to "year > 0". Err... Yes, I use to have them in opposite order, but at some point I decided to reverse and forgot about the condition. So, you are right, it needs to be applied in reversed order. > > Reviewed-by: Jean Delvare <jdelvare@suse.de> >
On Tue, 20 Mar 2018, Andy Shevchenko wrote: > On Tue, 2018-03-20 at 16:32 +0100, Jean Delvare wrote: > > Hi Andy, > > > > On Tue, 20 Mar 2018 15:54:09 +0200, Andy Shevchenko wrote: > > > dmi_get_bios_year() may return 0 when it is incapable to parse > > > the BIOS date string. Previously this has been checked in > > > pci_acpi_crs_quirks(). > > > > > > Update the code to restore old behaviour. > > > > When fixing a regression, a good practice is to tag the fix commit as > > such: > > > > Fixes: 69c42d493db4 ("x86/pci: Simplify code by using the new > > dmi_get_bios_year() helper") > > Someone (perhaps Ingo) told me that Fixes doesn't make much sense if the > fixing commit is not yet in vanilla. No. If the commit is already queued for the next merge window then a fixes tag is appropriate. Thanks, tglx
On Tuesday, March 20, 2018 2:54:09 PM CET Andy Shevchenko wrote: > dmi_get_bios_year() may return 0 when it is incapable to parse > the BIOS date string. Previously this has been checked in > pci_acpi_crs_quirks(). > > Update the code to restore old behaviour. > > Reported-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > arch/x86/pci/acpi.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index 00e60de30328..5559dcaddd5e 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -140,7 +140,9 @@ static const struct dmi_system_id pci_crs_quirks[] __initconst = { > > void __init pci_acpi_crs_quirks(void) > { > - if ((dmi_get_bios_year() < 2008) && (iomem_resource.end <= 0xffffffff)) > + int year = dmi_get_bios_year(); > + > + if (year >= 0 && year < 2008 && iomem_resource.end <= 0xffffffff) > pci_use_crs = false; > > dmi_check_system(pci_crs_quirks); > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
On Tue, 2018-03-20 at 17:47 +0100, Thomas Gleixner wrote: > On Tue, 20 Mar 2018, Andy Shevchenko wrote: > > On Tue, 2018-03-20 at 16:32 +0100, Jean Delvare wrote: > > > When fixing a regression, a good practice is to tag the fix commit > > > as > > > such: > > > > > > Fixes: 69c42d493db4 ("x86/pci: Simplify code by using the new > > > dmi_get_bios_year() helper") > > > > Someone (perhaps Ingo) told me that Fixes doesn't make much sense if > > the > > fixing commit is not yet in vanilla. > > No. If the commit is already queued for the next merge window then a > fixes > tag is appropriate. Thank you, Thomas, for clarification. Will do this in v4.
On Tue, 20 Mar 2018 17:47:41 +0100 (CET), Thomas Gleixner wrote: > On Tue, 20 Mar 2018, Andy Shevchenko wrote: > > On Tue, 2018-03-20 at 16:32 +0100, Jean Delvare wrote: > > > When fixing a regression, a good practice is to tag the fix commit as > > > such: > > > > > > Fixes: 69c42d493db4 ("x86/pci: Simplify code by using the new > > > dmi_get_bios_year() helper") > > > > Someone (perhaps Ingo) told me that Fixes doesn't make much sense if the > > fixing commit is not yet in vanilla. > > No. If the commit is already queued for the next merge window then a fixes > tag is appropriate. +1 Even if it won't be used for stable branches, it's still valuable for distribution kernel maintainers to find out the fixes to patches they backport. Thanks,
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index 00e60de30328..5559dcaddd5e 100644 --- a/arch/x86/pci/acpi.c +++ b/arch/x86/pci/acpi.c @@ -140,7 +140,9 @@ static const struct dmi_system_id pci_crs_quirks[] __initconst = { void __init pci_acpi_crs_quirks(void) { - if ((dmi_get_bios_year() < 2008) && (iomem_resource.end <= 0xffffffff)) + int year = dmi_get_bios_year(); + + if (year >= 0 && year < 2008 && iomem_resource.end <= 0xffffffff) pci_use_crs = false; dmi_check_system(pci_crs_quirks);
dmi_get_bios_year() may return 0 when it is incapable to parse the BIOS date string. Previously this has been checked in pci_acpi_crs_quirks(). Update the code to restore old behaviour. Reported-by: Jean Delvare <jdelvare@suse.de> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- arch/x86/pci/acpi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)