* [PATCH] lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver
@ 2012-08-14 15:56 Feng Tang
2012-08-22 19:55 ` Wim Van Sebroeck
2012-08-23 15:28 ` Guenter Roeck
0 siblings, 2 replies; 6+ messages in thread
From: Feng Tang @ 2012-08-14 15:56 UTC (permalink / raw)
To: linux-acpi, linux-kernel
Cc: Feng Tang, Aaron Sierra, Wim Van Sebroeck, Len Brown, Bob Moore
There are many reports (including 2 of my machines) that iTCO_wdt watchdog
driver fails to be initialized in 3.5 kernel with error message like:
[ 5.265175] ACPI Warning: 0x00001060-0x0000107f SystemIO conflicts with Region \_SB_.PCI0.LPCB.TCOI 1 (20120320/utaddress-251)
[ 5.265192] ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver
[ 5.265206] lpc_ich: Resource conflict(s) found affecting iTCO_wdt
The root cause the iTCO_wdt driver in 3.4 probes the HW IO resource from
LPC's PCI config space, while in 3.5 kernel it relies on lpc_ich driver
for the probe, which adds a new acpi_check_resource_conflict() check, and
give up the probe if there is any conflict with ACPI.
Fix it by removing all the checks for iTCO_wdt to keep the same behavior as
3.4 kernel.
https://bugzilla.kernel.org/show_bug.cgi?id=44991
Actually the same check could be removed for the gpio-ich in lpc_ich.c,
but I'm not sure if it will cause problems.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Aaron Sierra <asierra@xes-inc.com>
Cc: Wim Van Sebroeck <wim@iguana.be>
Cc: Len Brown <len.brown@intel.com>
Cc: Bob Moore <robert.moore@intel.com>
---
drivers/mfd/lpc_ich.c | 20 +-------------------
1 files changed, 1 insertions(+), 19 deletions(-)
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index 027cc8f..a05fdfc 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -765,7 +765,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
u32 base_addr_cfg;
u32 base_addr;
int ret;
- bool acpi_conflict = false;
struct resource *res;
/* Setup power management base register */
@@ -780,20 +779,11 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
res = wdt_io_res(ICH_RES_IO_TCO);
res->start = base_addr + ACPIBASE_TCO_OFF;
res->end = base_addr + ACPIBASE_TCO_END;
- ret = acpi_check_resource_conflict(res);
- if (ret) {
- acpi_conflict = true;
- goto wdt_done;
- }
res = wdt_io_res(ICH_RES_IO_SMI);
res->start = base_addr + ACPIBASE_SMI_OFF;
res->end = base_addr + ACPIBASE_SMI_END;
- ret = acpi_check_resource_conflict(res);
- if (ret) {
- acpi_conflict = true;
- goto wdt_done;
- }
+
lpc_ich_enable_acpi_space(dev);
/*
@@ -813,11 +803,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
res = wdt_mem_res(ICH_RES_MEM_GCS);
res->start = base_addr + ACPIBASE_GCS_OFF;
res->end = base_addr + ACPIBASE_GCS_END;
- ret = acpi_check_resource_conflict(res);
- if (ret) {
- acpi_conflict = true;
- goto wdt_done;
- }
}
lpc_ich_finalize_cell(&lpc_ich_cells[LPC_WDT], id);
@@ -825,9 +810,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
1, NULL, 0);
wdt_done:
- if (acpi_conflict)
- pr_warn("Resource conflict(s) found affecting %s\n",
- lpc_ich_cells[LPC_WDT].name);
return ret;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver
2012-08-14 15:56 [PATCH] lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver Feng Tang
@ 2012-08-22 19:55 ` Wim Van Sebroeck
2012-08-22 21:55 ` Matthew Garrett
2012-08-23 15:28 ` Guenter Roeck
1 sibling, 1 reply; 6+ messages in thread
From: Wim Van Sebroeck @ 2012-08-22 19:55 UTC (permalink / raw)
To: Len Brown, Feng Tang
Cc: linux-acpi, linux-kernel, Aaron Sierra, Bob Moore, Samuel Ortiz,
Guenter Roeck
Hi All,
Cc-ing Samuel and Guenter also.
> There are many reports (including 2 of my machines) that iTCO_wdt watchdog
> driver fails to be initialized in 3.5 kernel with error message like:
>
> [ 5.265175] ACPI Warning: 0x00001060-0x0000107f SystemIO conflicts with Region \_SB_.PCI0.LPCB.TCOI 1 (20120320/utaddress-251)
> [ 5.265192] ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver
> [ 5.265206] lpc_ich: Resource conflict(s) found affecting iTCO_wdt
>
> The root cause the iTCO_wdt driver in 3.4 probes the HW IO resource from
> LPC's PCI config space, while in 3.5 kernel it relies on lpc_ich driver
> for the probe, which adds a new acpi_check_resource_conflict() check, and
> give up the probe if there is any conflict with ACPI.
>
> Fix it by removing all the checks for iTCO_wdt to keep the same behavior as
> 3.4 kernel.
> https://bugzilla.kernel.org/show_bug.cgi?id=44991
>
> Actually the same check could be removed for the gpio-ich in lpc_ich.c,
> but I'm not sure if it will cause problems.
>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> Cc: Aaron Sierra <asierra@xes-inc.com>
> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Bob Moore <robert.moore@intel.com>
> ---
> drivers/mfd/lpc_ich.c | 20 +-------------------
> 1 files changed, 1 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> index 027cc8f..a05fdfc 100644
> --- a/drivers/mfd/lpc_ich.c
> +++ b/drivers/mfd/lpc_ich.c
> @@ -765,7 +765,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
> u32 base_addr_cfg;
> u32 base_addr;
> int ret;
> - bool acpi_conflict = false;
> struct resource *res;
>
> /* Setup power management base register */
> @@ -780,20 +779,11 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
> res = wdt_io_res(ICH_RES_IO_TCO);
> res->start = base_addr + ACPIBASE_TCO_OFF;
> res->end = base_addr + ACPIBASE_TCO_END;
> - ret = acpi_check_resource_conflict(res);
> - if (ret) {
> - acpi_conflict = true;
> - goto wdt_done;
> - }
>
> res = wdt_io_res(ICH_RES_IO_SMI);
> res->start = base_addr + ACPIBASE_SMI_OFF;
> res->end = base_addr + ACPIBASE_SMI_END;
> - ret = acpi_check_resource_conflict(res);
> - if (ret) {
> - acpi_conflict = true;
> - goto wdt_done;
> - }
> +
> lpc_ich_enable_acpi_space(dev);
>
> /*
> @@ -813,11 +803,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
> res = wdt_mem_res(ICH_RES_MEM_GCS);
> res->start = base_addr + ACPIBASE_GCS_OFF;
> res->end = base_addr + ACPIBASE_GCS_END;
> - ret = acpi_check_resource_conflict(res);
> - if (ret) {
> - acpi_conflict = true;
> - goto wdt_done;
> - }
> }
>
> lpc_ich_finalize_cell(&lpc_ich_cells[LPC_WDT], id);
> @@ -825,9 +810,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
> 1, NULL, 0);
>
> wdt_done:
> - if (acpi_conflict)
> - pr_warn("Resource conflict(s) found affecting %s\n",
> - lpc_ich_cells[LPC_WDT].name);
> return ret;
> }
>
Hi Len,
Any idea why the acpi_check_resource_conflict() check gives a conflict?
Thanks in advance,
Wim.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver
2012-08-22 19:55 ` Wim Van Sebroeck
@ 2012-08-22 21:55 ` Matthew Garrett
2012-08-23 5:08 ` Feng Tang
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Garrett @ 2012-08-22 21:55 UTC (permalink / raw)
To: Wim Van Sebroeck
Cc: Len Brown, Feng Tang, linux-acpi, linux-kernel, Aaron Sierra,
Bob Moore, Samuel Ortiz, Guenter Roeck
On Wed, Aug 22, 2012 at 09:55:12PM +0200, Wim Van Sebroeck wrote:
> Any idea why the acpi_check_resource_conflict() check gives a conflict?
Because the resource range is declared in ACPI and we assume that that
means the firmware wants to scribble on it. We'd need the output of
acpidump to work out whether that's safe or not.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver
2012-08-22 21:55 ` Matthew Garrett
@ 2012-08-23 5:08 ` Feng Tang
2012-08-23 17:13 ` Samuel Ortiz
0 siblings, 1 reply; 6+ messages in thread
From: Feng Tang @ 2012-08-23 5:08 UTC (permalink / raw)
To: Matthew Garrett
Cc: Wim Van Sebroeck, Len Brown, linux-acpi, linux-kernel,
Aaron Sierra, Bob Moore, Samuel Ortiz, Guenter Roeck, rui.zhang
On Wed, 22 Aug 2012 22:55:43 +0100
Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Wed, Aug 22, 2012 at 09:55:12PM +0200, Wim Van Sebroeck wrote:
>
> > Any idea why the acpi_check_resource_conflict() check gives a conflict?
>
> Because the resource range is declared in ACPI and we assume that that
> means the firmware wants to scribble on it. We'd need the output of
> acpidump to work out whether that's safe or not.
Good point, I checked the conflict for iTCO_wdt, the conflict exists on
almost all the machines I have.
According to ICH (7/8/9 etc)spec, the TCO watchdog has a 32 bytes long IO
space resource, and the bit 9 of TCO1_STS register is "DMISCI_STS", which
indicates whether a SCI happens, and will be cleared by writing 1
to it. Most of DSDT table will claim a TCO op region only for one bit:
"DMISCI_STS" , as some method may need to access that bit.
I think there is some risk, but it's quite safe as the DMISCI_STS bit has
nothing to do with TCO driver itself, and TCO driver never access it, also
this TCO driver has been there for years, and this resource conflict also
exists for many generations hardware.
Thanks,
Feng
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver
2012-08-14 15:56 [PATCH] lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver Feng Tang
2012-08-22 19:55 ` Wim Van Sebroeck
@ 2012-08-23 15:28 ` Guenter Roeck
1 sibling, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2012-08-23 15:28 UTC (permalink / raw)
To: Feng Tang
Cc: linux-acpi, linux-kernel, Aaron Sierra, Wim Van Sebroeck,
Len Brown, Bob Moore
On Tue, Aug 14, 2012 at 03:56:12PM -0000, Feng Tang wrote:
> There are many reports (including 2 of my machines) that iTCO_wdt watchdog
> driver fails to be initialized in 3.5 kernel with error message like:
>
> [ 5.265175] ACPI Warning: 0x00001060-0x0000107f SystemIO conflicts with Region \_SB_.PCI0.LPCB.TCOI 1 (20120320/utaddress-251)
> [ 5.265192] ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver
> [ 5.265206] lpc_ich: Resource conflict(s) found affecting iTCO_wdt
>
> The root cause the iTCO_wdt driver in 3.4 probes the HW IO resource from
> LPC's PCI config space, while in 3.5 kernel it relies on lpc_ich driver
> for the probe, which adds a new acpi_check_resource_conflict() check, and
> give up the probe if there is any conflict with ACPI.
>
> Fix it by removing all the checks for iTCO_wdt to keep the same behavior as
> 3.4 kernel.
> https://bugzilla.kernel.org/show_bug.cgi?id=44991
>
> Actually the same check could be removed for the gpio-ich in lpc_ich.c,
> but I'm not sure if it will cause problems.
>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> Cc: Aaron Sierra <asierra@xes-inc.com>
> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Bob Moore <robert.moore@intel.com>
>
Kind of unfortunate to have the ACPI conflict, but I don't have an idea
for a better fix.
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/mfd/lpc_ich.c | 20 +-------------------
> 1 files changed, 1 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> index 027cc8f..a05fdfc 100644
> --- a/drivers/mfd/lpc_ich.c
> +++ b/drivers/mfd/lpc_ich.c
> @@ -765,7 +765,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
> u32 base_addr_cfg;
> u32 base_addr;
> int ret;
> - bool acpi_conflict = false;
> struct resource *res;
>
> /* Setup power management base register */
> @@ -780,20 +779,11 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
> res = wdt_io_res(ICH_RES_IO_TCO);
> res->start = base_addr + ACPIBASE_TCO_OFF;
> res->end = base_addr + ACPIBASE_TCO_END;
> - ret = acpi_check_resource_conflict(res);
> - if (ret) {
> - acpi_conflict = true;
> - goto wdt_done;
> - }
>
> res = wdt_io_res(ICH_RES_IO_SMI);
> res->start = base_addr + ACPIBASE_SMI_OFF;
> res->end = base_addr + ACPIBASE_SMI_END;
> - ret = acpi_check_resource_conflict(res);
> - if (ret) {
> - acpi_conflict = true;
> - goto wdt_done;
> - }
> +
> lpc_ich_enable_acpi_space(dev);
>
> /*
> @@ -813,11 +803,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
> res = wdt_mem_res(ICH_RES_MEM_GCS);
> res->start = base_addr + ACPIBASE_GCS_OFF;
> res->end = base_addr + ACPIBASE_GCS_END;
> - ret = acpi_check_resource_conflict(res);
> - if (ret) {
> - acpi_conflict = true;
> - goto wdt_done;
> - }
> }
>
> lpc_ich_finalize_cell(&lpc_ich_cells[LPC_WDT], id);
> @@ -825,9 +810,6 @@ static int __devinit lpc_ich_init_wdt(struct pci_dev *dev,
> 1, NULL, 0);
>
> wdt_done:
> - if (acpi_conflict)
> - pr_warn("Resource conflict(s) found affecting %s\n",
> - lpc_ich_cells[LPC_WDT].name);
> return ret;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver
2012-08-23 5:08 ` Feng Tang
@ 2012-08-23 17:13 ` Samuel Ortiz
0 siblings, 0 replies; 6+ messages in thread
From: Samuel Ortiz @ 2012-08-23 17:13 UTC (permalink / raw)
To: Feng Tang
Cc: Matthew Garrett, Wim Van Sebroeck, Len Brown, linux-acpi,
linux-kernel, Aaron Sierra, Bob Moore, Guenter Roeck, rui.zhang
Hi Feng,
On Thu, Aug 23, 2012 at 01:08:14PM +0800, Feng Tang wrote:
> On Wed, 22 Aug 2012 22:55:43 +0100
> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>
> > On Wed, Aug 22, 2012 at 09:55:12PM +0200, Wim Van Sebroeck wrote:
> >
> > > Any idea why the acpi_check_resource_conflict() check gives a conflict?
> >
> > Because the resource range is declared in ACPI and we assume that that
> > means the firmware wants to scribble on it. We'd need the output of
> > acpidump to work out whether that's safe or not.
>
> Good point, I checked the conflict for iTCO_wdt, the conflict exists on
> almost all the machines I have.
>
> According to ICH (7/8/9 etc)spec, the TCO watchdog has a 32 bytes long IO
> space resource, and the bit 9 of TCO1_STS register is "DMISCI_STS", which
> indicates whether a SCI happens, and will be cleared by writing 1
> to it. Most of DSDT table will claim a TCO op region only for one bit:
> "DMISCI_STS" , as some method may need to access that bit.
>
> I think there is some risk, but it's quite safe as the DMISCI_STS bit has
> nothing to do with TCO driver itself, and TCO driver never access it, also
> this TCO driver has been there for years, and this resource conflict also
> exists for many generations hardware.
Makes sense to me.
I'm queueing this one to my for-linus branch, I'll send a pull request soon.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-08-23 17:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-14 15:56 [PATCH] lpc_ich: Fix a 3.5 kernel regression for iTCO_wdt driver Feng Tang
2012-08-22 19:55 ` Wim Van Sebroeck
2012-08-22 21:55 ` Matthew Garrett
2012-08-23 5:08 ` Feng Tang
2012-08-23 17:13 ` Samuel Ortiz
2012-08-23 15:28 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).