* [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support
@ 2013-02-14 16:07 Joseph Salisbury
2013-02-15 7:32 ` Wim Van Sebroeck
0 siblings, 1 reply; 9+ messages in thread
From: Joseph Salisbury @ 2013-02-14 16:07 UTC (permalink / raw)
To: mc74hc00; +Cc: wim, linux-watchdog, LKML, Kernel Team
Hi Takahisa,
A bug was opened against the Ubuntu kernel[0]. It was found that
reverting the following commit resolved this bug:
commit 740fbddf5c3f9ad8b23c5d917ba1cc7e376a5104
Author: Takahisa Tanaka <mc74hc00@gmail.com>
Date: Sun Dec 2 14:33:18 2012 +0900
watchdog: sp5100_tco: Add SB8x0 chipset support
The regression was introduced as of v3.8-rc1.
I see that you are the author of this patch, so I wanted to run this by
you. I was thinking of requesting a revert for v3.8, but I wanted to
get your feedback first.
Thanks,
Joe
[0] http://pad.lv/1116835
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support 2013-02-14 16:07 [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support Joseph Salisbury @ 2013-02-15 7:32 ` Wim Van Sebroeck 2013-02-15 14:54 ` Joseph Salisbury 0 siblings, 1 reply; 9+ messages in thread From: Wim Van Sebroeck @ 2013-02-15 7:32 UTC (permalink / raw) To: Joseph Salisbury; +Cc: mc74hc00, linux-watchdog, LKML, Kernel Team [-- Attachment #1: Type: text/plain, Size: 802 bytes --] Hi Joseph, > A bug was opened against the Ubuntu kernel[0]. It was found that > reverting the following commit resolved this bug: > > commit 740fbddf5c3f9ad8b23c5d917ba1cc7e376a5104 > Author: Takahisa Tanaka <mc74hc00@gmail.com> > Date: Sun Dec 2 14:33:18 2012 +0900 > > watchdog: sp5100_tco: Add SB8x0 chipset support > > > The regression was introduced as of v3.8-rc1. > > I see that you are the author of this patch, so I wanted to run this by > you. I was thinking of requesting a revert for v3.8, but I wanted to > get your feedback first. Please check out the attached patches first (They are allready in linux-next). Other references: Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 Kind regards, Wim. [-- Attachment #2: sp5100_tco-fix-indirect_IO.mbox --] [-- Type: text/plain, Size: 4098 bytes --] >From mc74hc00@gmail.com Mon Jan 14 03:02:20 2013 Return-path: <mc74hc00@gmail.com> Envelope-to: wim@infomag.iguana.be Delivery-date: Mon, 14 Jan 2013 03:02:20 +0100 Received: from ylaen.iguana.be ([178.21.116.169]) by spo001.leaseweb.com with esmtp (Exim 4.50) id 1TuZNU-0006Zi-0P for wim@infomag.iguana.be; Mon, 14 Jan 2013 03:02:20 +0100 Received: from mail-pb0-f45.google.com (mail-pb0-f45.google.com [209.85.160.45]) by ylaen.iguana.be (Postfix) with ESMTP id 939FC174278 for <wim@iguana.be>; Mon, 14 Jan 2013 03:02:19 +0100 (CET) Received: by mail-pb0-f45.google.com with SMTP id mc8so1875722pbc.32 for <wim@iguana.be>; Sun, 13 Jan 2013 18:02:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer; bh=Al12X3CF48UZXAYSA0AbzpLWDndmh59rkg9CMqG3eKs=; b=BN3M1HJsUrmYnOe3KiT2w9jZTOWHevYNdIbLumjQWul8MbPVI1WDie98dgoG21C18P BuaHoo82fmCZBViD0qyuQz0RI0vySln5olsfpBsCKJVWTFDSAAg/kysHmcqOVp6sWpDI VPSe22vIVgxiNtqK0BdwGLjyvhQJ7Cgnv/3CrIa4h1HUfP8hKMwy/9V6SCe4VRpt7bOZ ZqTxpMfQ95pesNTKJuZEx2vqL5LM2WMVqUhneVR1dG4ceFTbOZ3xF57waoUEszGFlrmq 0owzHTSi1BVF1wlxEfYXPuSie/gS2Yl9AGqHoaXm3DN3aSXwXbqnIK5VdUi/3Cir7CUK zj3Q== X-Received: by 10.66.83.165 with SMTP id r5mr229452746pay.3.1358128938114; Sun, 13 Jan 2013 18:02:18 -0800 (PST) Received: from localhost (p7204-ipngn2902marunouchi.tokyo.ocn.ne.jp. [180.47.244.204]) by mx.google.com with ESMTPS id qh4sm7226051pbb.9.2013.01.13.18.02.14 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sun, 13 Jan 2013 18:02:16 -0800 (PST) From: Takahisa Tanaka <mc74hc00@gmail.com> To: linux-watchdog@vger.kernel.org Cc: Wim Van Sebroeck <wim@iguana.be>, Paul Menzel <paulepanter@users.sourceforge.net>, Arkadiusz Miskiewicz <arekm@maven.pl>, Bjorn Helgaas <bhelgaas@google.com>, Andrew Morton <akpm@linux-foundation.org>, Jonathan Nieder <jrnieder@gmail.com>, linux-kernel@vger.kernel.org, Florian Mickler <florian@mickler.org>, Takahisa Tanaka <mc74hc00@gmail.com> Subject: [PATCH 1/2] watchdog: sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits Date: Mon, 14 Jan 2013 11:01:57 +0900 Message-Id: <1358128918-4415-1-git-send-email-mc74hc00@gmail.com> X-Mailer: git-send-email 1.7.11.7 Content-Length: 1656 Lines: 42 In case of SB800 or later chipset and re-programming MMIO address(*), sp5100_tco module may read incorrect value of reserved bit, because the module reads a value from an incorrect I/O address. However, this bug doesn't cause a problem, because when re-programming MMIO address, by chance the module writes zero (this is BIOS's default value) to the low three bits of register. * In most cases, PC with SB8x0 or later chipset doesn't need to re-programming MMIO address, because such PC can enable AcpiMmio and can use 0xfed80b00 for watchdog register base address. This patch fixes this bug. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 Signed-off-by: Takahisa Tanaka <mc74hc00@gmail.com> Signed-off-by: Wim Van Sebroeck <wim@iguana.be> --- drivers/watchdog/sp5100_tco.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c index 2b0e000..5dfe86e 100644 --- a/drivers/watchdog/sp5100_tco.c +++ b/drivers/watchdog/sp5100_tco.c @@ -500,14 +500,15 @@ static unsigned char sp5100_tco_setupdevice(void) /* Restore to the low three bits, if chipset is SB8x0(or later) */ if (sp5100_tco_pci->revision >= 0x40) { u8 reserved_bit; - reserved_bit = inb(base_addr) & 0x7; + outb(base_addr+0, index_reg); + reserved_bit = inb(data_reg) & 0x7; val |= (u32)reserved_bit; } /* Re-programming the watchdog timer base address */ outb(base_addr+0, index_reg); /* Low three bits of BASE are reserved */ - outb((val >> 0) & 0xf8, data_reg); + outb((val >> 0) & 0xff, data_reg); outb(base_addr+1, index_reg); outb((val >> 8) & 0xff, data_reg); outb(base_addr+2, index_reg); -- 1.7.11.7 [-- Attachment #3: sp5100_tco-fix-write.mbox --] [-- Type: text/plain, Size: 4886 bytes --] >From mc74hc00@gmail.com Mon Jan 14 03:02:31 2013 Return-path: <mc74hc00@gmail.com> Envelope-to: wim@infomag.iguana.be Delivery-date: Mon, 14 Jan 2013 03:02:31 +0100 Received: from ylaen.iguana.be ([178.21.116.169]) by spo001.leaseweb.com with esmtp (Exim 4.50) id 1TuZNf-0006Zu-2q for wim@infomag.iguana.be; Mon, 14 Jan 2013 03:02:31 +0100 Received: from mail-da0-f51.google.com (mail-da0-f51.google.com [209.85.210.51]) by ylaen.iguana.be (Postfix) with ESMTP id AEAA3174278 for <wim@iguana.be>; Mon, 14 Jan 2013 03:02:30 +0100 (CET) Received: by mail-da0-f51.google.com with SMTP id i30so1585773dad.10 for <wim@iguana.be>; Sun, 13 Jan 2013 18:02:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=xr96DHGo9AJimFBvpkiDW9J9bvC1hncA+l9AGfPz2kI=; b=jZJA2gZXsnoyhJbUNH0wc16xsOEDDa0p3aqqoOF+QkZ69Tj4qKpV+MObEisJwEqJlH FQS2N0o7PIVwMoD4bxB3es1Du10awk91rAqTdq1FPxVzsOfmfxoLbXH78pbK9ZNt2BTn f7Cq0kjpK5IAC+rnTz5HLC8X/gC4AbzLNmmhk/7+BNUfbXTPgbwvYMkJ14DOaZBzbzQC ZnaU17gsZMecQBjraKjmisAVG6DwXq4amD3n5xZLDc/yqJ//DsUhmd7kCbpXBbyazBLn yX4i+278KwYIkYR3DiOIv48SBN24qrXo+WeGhMsR3JIrDKpHhjs1Nqd38+YSF5bcDdsY p8cQ== X-Received: by 10.66.52.102 with SMTP id s6mr228245938pao.6.1358128949874; Sun, 13 Jan 2013 18:02:29 -0800 (PST) Received: from localhost (p7204-ipngn2902marunouchi.tokyo.ocn.ne.jp. [180.47.244.204]) by mx.google.com with ESMTPS id ug6sm7227632pbc.4.2013.01.13.18.02.26 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sun, 13 Jan 2013 18:02:28 -0800 (PST) From: Takahisa Tanaka <mc74hc00@gmail.com> To: linux-watchdog@vger.kernel.org Cc: Wim Van Sebroeck <wim@iguana.be>, Paul Menzel <paulepanter@users.sourceforge.net>, Arkadiusz Miskiewicz <arekm@maven.pl>, Bjorn Helgaas <bhelgaas@google.com>, Andrew Morton <akpm@linux-foundation.org>, Jonathan Nieder <jrnieder@gmail.com>, linux-kernel@vger.kernel.org, Florian Mickler <florian@mickler.org>, Takahisa Tanaka <mc74hc00@gmail.com> Subject: [PATCH 2/2] watchdog: sp5100_tco: Write back the original value to reserved bits, instead of zero Date: Mon, 14 Jan 2013 11:01:58 +0900 Message-Id: <1358128918-4415-2-git-send-email-mc74hc00@gmail.com> X-Mailer: git-send-email 1.7.11.7 In-Reply-To: <1358128918-4415-1-git-send-email-mc74hc00@gmail.com> References: <1358128918-4415-1-git-send-email-mc74hc00@gmail.com> Content-Length: 2279 Lines: 69 In case of SP5100 or SB7x0 chipsets, the sp5100_tco module writes zero to reserved bits. The module, however, shouldn't depend on specific default value, and should perform a read-merge-write operation for the reserved bits. This patch makes the sp5100_tco module perform a read-merge-write operation on all the chipset (sp5100, sb7x0, sb8x0 or later). Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 Signed-off-by: Takahisa Tanaka <mc74hc00@gmail.com> Signed-off-by: Wim Van Sebroeck <wim@iguana.be> --- drivers/watchdog/sp5100_tco.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c index 5dfe86e..e3b8f75 100644 --- a/drivers/watchdog/sp5100_tco.c +++ b/drivers/watchdog/sp5100_tco.c @@ -361,7 +361,7 @@ static unsigned char sp5100_tco_setupdevice(void) { struct pci_dev *dev = NULL; const char *dev_name = NULL; - u32 val; + u32 val, tmp_val; u32 index_reg, data_reg, base_addr; /* Match the PCI device */ @@ -497,31 +497,19 @@ static unsigned char sp5100_tco_setupdevice(void) pr_debug("Got 0x%04x from resource tree\n", val); } - /* Restore to the low three bits, if chipset is SB8x0(or later) */ - if (sp5100_tco_pci->revision >= 0x40) { - u8 reserved_bit; - outb(base_addr+0, index_reg); - reserved_bit = inb(data_reg) & 0x7; - val |= (u32)reserved_bit; - } + /* Restore to the low three bits */ + outb(base_addr+0, index_reg); + tmp_val = val | (inb(data_reg) & 0x7); /* Re-programming the watchdog timer base address */ outb(base_addr+0, index_reg); - /* Low three bits of BASE are reserved */ - outb((val >> 0) & 0xff, data_reg); + outb((tmp_val >> 0) & 0xff, data_reg); outb(base_addr+1, index_reg); - outb((val >> 8) & 0xff, data_reg); + outb((tmp_val >> 8) & 0xff, data_reg); outb(base_addr+2, index_reg); - outb((val >> 16) & 0xff, data_reg); + outb((tmp_val >> 16) & 0xff, data_reg); outb(base_addr+3, index_reg); - outb((val >> 24) & 0xff, data_reg); - - /* - * Clear unnecessary the low three bits, - * if chipset is SB8x0(or later) - */ - if (sp5100_tco_pci->revision >= 0x40) - val &= ~0x7; + outb((tmp_val >> 24) & 0xff, data_reg); if (!request_mem_region_exclusive(val, SP5100_WDT_MEM_MAP_SIZE, dev_name)) { -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support 2013-02-15 7:32 ` Wim Van Sebroeck @ 2013-02-15 14:54 ` Joseph Salisbury 2013-02-17 9:44 ` Tanaka Takahisa 0 siblings, 1 reply; 9+ messages in thread From: Joseph Salisbury @ 2013-02-15 14:54 UTC (permalink / raw) To: Wim Van Sebroeck; +Cc: mc74hc00, linux-watchdog, LKML, Kernel Team On 02/15/2013 02:32 AM, Wim Van Sebroeck wrote: > Hi Joseph, > >> A bug was opened against the Ubuntu kernel[0]. It was found that >> reverting the following commit resolved this bug: >> >> commit 740fbddf5c3f9ad8b23c5d917ba1cc7e376a5104 >> Author: Takahisa Tanaka <mc74hc00@gmail.com> >> Date: Sun Dec 2 14:33:18 2012 +0900 >> >> watchdog: sp5100_tco: Add SB8x0 chipset support >> >> >> The regression was introduced as of v3.8-rc1. >> >> I see that you are the author of this patch, so I wanted to run this by >> you. I was thinking of requesting a revert for v3.8, but I wanted to >> get your feedback first. > Please check out the attached patches first (They are allready in linux-next). Thanks for the feedback, Wim. I'll let you know if the patches resolve this bug. > > Other references: > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 > > Kind regards, > Wim. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support 2013-02-15 14:54 ` Joseph Salisbury @ 2013-02-17 9:44 ` Tanaka Takahisa 2013-02-18 3:13 ` Joseph Salisbury 0 siblings, 1 reply; 9+ messages in thread From: Tanaka Takahisa @ 2013-02-17 9:44 UTC (permalink / raw) To: Joseph Salisbury; +Cc: Wim Van Sebroeck, linux-watchdog, LKML, Kernel Team Hi Joseph, 2013/2/15 Joseph Salisbury <joseph.salisbury@canonical.com>: > On 02/15/2013 02:32 AM, Wim Van Sebroeck wrote: >> >> Hi Joseph, >> >>> A bug was opened against the Ubuntu kernel[0]. It was found that >>> reverting the following commit resolved this bug: >>> >>> commit 740fbddf5c3f9ad8b23c5d917ba1cc7e376a5104 >>> Author: Takahisa Tanaka <mc74hc00@gmail.com> >>> Date: Sun Dec 2 14:33:18 2012 +0900 >>> >>> watchdog: sp5100_tco: Add SB8x0 chipset support >>> >>> >>> The regression was introduced as of v3.8-rc1. >>> >>> I see that you are the author of this patch, so I wanted to run this by >>> you. I was thinking of requesting a revert for v3.8, but I wanted to >>> get your feedback first. >> >> Please check out the attached patches first (They are allready in >> linux-next). > > Thanks for the feedback, Wim. I'll let you know if the patches resolve this > bug. > > >> >> Other references: >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 >> >> Kind regards, >> Wim. Thank you for your information, and sorry for the late reply. I think that I will get rid of code that re-programming the chipset registers, if my patch set doesn't resolve the problem. I'm waiting for your reply at the moment. Regards, Takahisa ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support 2013-02-17 9:44 ` Tanaka Takahisa @ 2013-02-18 3:13 ` Joseph Salisbury 2013-02-18 7:26 ` Joseph Salisbury 0 siblings, 1 reply; 9+ messages in thread From: Joseph Salisbury @ 2013-02-18 3:13 UTC (permalink / raw) To: Tanaka Takahisa; +Cc: Wim Van Sebroeck, linux-watchdog, LKML, Kernel Team On 02/17/2013 04:44 AM, Tanaka Takahisa wrote: > Hi Joseph, > > 2013/2/15 Joseph Salisbury <joseph.salisbury@canonical.com>: >> On 02/15/2013 02:32 AM, Wim Van Sebroeck wrote: >>> Hi Joseph, >>> >>>> A bug was opened against the Ubuntu kernel[0]. It was found that >>>> reverting the following commit resolved this bug: >>>> >>>> commit 740fbddf5c3f9ad8b23c5d917ba1cc7e376a5104 >>>> Author: Takahisa Tanaka <mc74hc00@gmail.com> >>>> Date: Sun Dec 2 14:33:18 2012 +0900 >>>> >>>> watchdog: sp5100_tco: Add SB8x0 chipset support >>>> >>>> >>>> The regression was introduced as of v3.8-rc1. >>>> >>>> I see that you are the author of this patch, so I wanted to run this by >>>> you. I was thinking of requesting a revert for v3.8, but I wanted to >>>> get your feedback first. >>> Please check out the attached patches first (They are allready in >>> linux-next). >> Thanks for the feedback, Wim. I'll let you know if the patches resolve this >> bug. >> >> >>> Other references: >>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 >>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 >>> >>> Kind regards, >>> Wim. > Thank you for your information, and sorry for the late reply. > > I think that I will get rid of code that re-programming the chipset > registers, if my patch set doesn't resolve the problem. > I'm waiting for your reply at the moment. > > > Regards, > Takahisa Hi Takahisa, The bug report tested a kernel with the patches. However, he reports the kernel panic still occurs[0]. Thanks, Joe [0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1116835 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support 2013-02-18 3:13 ` Joseph Salisbury @ 2013-02-18 7:26 ` Joseph Salisbury 2013-02-18 17:14 ` Tanaka Takahisa 0 siblings, 1 reply; 9+ messages in thread From: Joseph Salisbury @ 2013-02-18 7:26 UTC (permalink / raw) To: Joseph Salisbury Cc: Tanaka Takahisa, Wim Van Sebroeck, linux-watchdog, LKML, Kernel Team I've requested a digital image or screen capture of the panic. Is there any additional debug information you thing would be helpful? Thanks, Joe On Sun, Feb 17, 2013 at 10:13 PM, Joseph Salisbury <joseph.salisbury@canonical.com> wrote: > On 02/17/2013 04:44 AM, Tanaka Takahisa wrote: >> >> Hi Joseph, >> >> 2013/2/15 Joseph Salisbury <joseph.salisbury@canonical.com>: >>> >>> On 02/15/2013 02:32 AM, Wim Van Sebroeck wrote: >>>> >>>> Hi Joseph, >>>> >>>>> A bug was opened against the Ubuntu kernel[0]. It was found that >>>>> reverting the following commit resolved this bug: >>>>> >>>>> commit 740fbddf5c3f9ad8b23c5d917ba1cc7e376a5104 >>>>> Author: Takahisa Tanaka <mc74hc00@gmail.com> >>>>> Date: Sun Dec 2 14:33:18 2012 +0900 >>>>> >>>>> watchdog: sp5100_tco: Add SB8x0 chipset support >>>>> >>>>> >>>>> The regression was introduced as of v3.8-rc1. >>>>> >>>>> I see that you are the author of this patch, so I wanted to run this by >>>>> you. I was thinking of requesting a revert for v3.8, but I wanted to >>>>> get your feedback first. >>>> >>>> Please check out the attached patches first (They are allready in >>>> linux-next). >>> >>> Thanks for the feedback, Wim. I'll let you know if the patches resolve >>> this >>> bug. >>> >>> >>>> Other references: >>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 >>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176 >>>> >>>> Kind regards, >>>> Wim. >> >> Thank you for your information, and sorry for the late reply. >> >> I think that I will get rid of code that re-programming the chipset >> registers, if my patch set doesn't resolve the problem. >> I'm waiting for your reply at the moment. >> >> >> Regards, >> Takahisa > > Hi Takahisa, > > The bug report tested a kernel with the patches. However, he reports the > kernel panic still occurs[0]. > > Thanks, > > Joe > > [0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1116835 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support 2013-02-18 7:26 ` Joseph Salisbury @ 2013-02-18 17:14 ` Tanaka Takahisa 2013-02-22 0:04 ` Joseph Salisbury 0 siblings, 1 reply; 9+ messages in thread From: Tanaka Takahisa @ 2013-02-18 17:14 UTC (permalink / raw) To: Joseph Salisbury Cc: Joseph Salisbury, Wim Van Sebroeck, linux-watchdog, LKML, Kernel Team [-- Attachment #1: Type: text/plain, Size: 1182 bytes --] Hi Joseph, Thanks a lot for your help. >The bug report tested a kernel with the patches. However, he reports the kernel panic still occurs[0]. I understood. I guess this problem conflicts with the watchdog MMIO address (*) written in SB700 chipset and other resource. So, I made a patch which gets rid of the code considered to cause the problem. If this patch is applied, although the SP5100 and SB7x0 chipsets can't use watchdog function any longer, I'm sure attached patch resolve the problem. The SB800 or later chipsets can be used as before. (*)This address is obtained from allocate_resource() function. I'm sorry to trouble you, but Would you confirm whether attached patch solves a problem? If there is no problem in this patch, I will submit this patch to a linux-watchdog community. > I've requested a digital image or screen capture of the panic. Is > there any additional debug information you thing would be helpful? I'm interested in the I/O resource of PC in which the problem has occurred, So, I want the result of 'cat /proc/iomem'. When 'cat /proc/iomem' is performed, I don't care about whether sp5100_tco driver is loaded. Thanks in advance. Takahisa [-- Attachment #2: 0001-sp5100_tco-Remove-code-that-may-cause-a-boot-failure.patch --] [-- Type: application/octet-stream, Size: 8588 bytes --] From 7660d82d93eb39beeedaf2e8c28d9bee8e1f3bd7 Mon Sep 17 00:00:00 2001 From: Takahisa Tanaka <mc74hc00@gmail.com> Date: Mon, 18 Feb 2013 23:47:49 +0900 Subject: [PATCH] sp5100_tco: Remove code that may cause a boot failure Signed-off-by: Takahisa Tanaka <mc74hc00@gmail.com> --- drivers/watchdog/sp5100_tco.c | 140 ++++-------------------------------------- drivers/watchdog/sp5100_tco.h | 2 +- 2 files changed, 14 insertions(+), 128 deletions(-) diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c index e3b8f75..eaf396b 100644 --- a/drivers/watchdog/sp5100_tco.c +++ b/drivers/watchdog/sp5100_tco.c @@ -40,13 +40,12 @@ #include "sp5100_tco.h" /* Module and version information */ -#define TCO_VERSION "0.03" +#define TCO_VERSION "0.05" #define TCO_MODULE_NAME "SP5100 TCO timer" #define TCO_DRIVER_NAME TCO_MODULE_NAME ", v" TCO_VERSION /* internal variables */ static u32 tcobase_phys; -static u32 resbase_phys; static u32 tco_wdt_fired; static void __iomem *tcobase; static unsigned int pm_iobase; @@ -54,10 +53,6 @@ static DEFINE_SPINLOCK(tco_lock); /* Guards the hardware */ static unsigned long timer_alive; static char tco_expect_close; static struct pci_dev *sp5100_tco_pci; -static struct resource wdt_res = { - .name = "Watchdog Timer", - .flags = IORESOURCE_MEM, -}; /* the watchdog platform device */ static struct platform_device *sp5100_tco_platform_device; @@ -75,12 +70,6 @@ module_param(nowayout, bool, 0); MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started." " (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); -static unsigned int force_addr; -module_param(force_addr, uint, 0); -MODULE_PARM_DESC(force_addr, "Force the use of specified MMIO address." - " ONLY USE THIS PARAMETER IF YOU REALLY KNOW" - " WHAT YOU ARE DOING (default=none)"); - /* * Some TCO specific functions */ @@ -176,39 +165,6 @@ static void tco_timer_enable(void) } } -static void tco_timer_disable(void) -{ - int val; - - if (sp5100_tco_pci->revision >= 0x40) { - /* For SB800 or later */ - /* Enable watchdog decode bit and Disable watchdog timer */ - outb(SB800_PM_WATCHDOG_CONTROL, SB800_IO_PM_INDEX_REG); - val = inb(SB800_IO_PM_DATA_REG); - val |= SB800_PCI_WATCHDOG_DECODE_EN; - val |= SB800_PM_WATCHDOG_DISABLE; - outb(val, SB800_IO_PM_DATA_REG); - } else { - /* For SP5100 or SB7x0 */ - /* Enable watchdog decode bit */ - pci_read_config_dword(sp5100_tco_pci, - SP5100_PCI_WATCHDOG_MISC_REG, - &val); - - val |= SP5100_PCI_WATCHDOG_DECODE_EN; - - pci_write_config_dword(sp5100_tco_pci, - SP5100_PCI_WATCHDOG_MISC_REG, - val); - - /* Disable Watchdog timer */ - outb(SP5100_PM_WATCHDOG_CONTROL, SP5100_IO_PM_INDEX_REG); - val = inb(SP5100_IO_PM_DATA_REG); - val |= SP5100_PM_WATCHDOG_DISABLE; - outb(val, SP5100_IO_PM_DATA_REG); - } -} - /* * /dev/watchdog handling */ @@ -361,7 +317,7 @@ static unsigned char sp5100_tco_setupdevice(void) { struct pci_dev *dev = NULL; const char *dev_name = NULL; - u32 val, tmp_val; + u32 val; u32 index_reg, data_reg, base_addr; /* Match the PCI device */ @@ -395,7 +351,7 @@ static unsigned char sp5100_tco_setupdevice(void) /* Request the IO ports used by this driver */ pm_iobase = SP5100_IO_PM_INDEX_REG; if (!request_region(pm_iobase, SP5100_PM_IOPORTS_SIZE, dev_name)) { - pr_err("I/O address 0x%04x already in use\n", pm_iobase); + pr_info("I/O address 0x%04x already in use\n", pm_iobase); goto exit; } @@ -412,14 +368,14 @@ static unsigned char sp5100_tco_setupdevice(void) /* Low three bits of BASE are reserved */ val = val << 8 | (inb(data_reg) & 0xf8); - pr_debug("Got 0x%04x from indirect I/O\n", val); + pr_debug("Got 0x%08x from indirect I/O\n", val); /* Check MMIO address conflict */ if (request_mem_region_exclusive(val, SP5100_WDT_MEM_MAP_SIZE, dev_name)) goto setup_wdt; else - pr_debug("MMIO address 0x%04x already in use\n", val); + pr_info("MMIO address 0x%08x already in use\n", val); /* * Secondly, Find the watchdog timer MMIO address @@ -451,71 +407,16 @@ static unsigned char sp5100_tco_setupdevice(void) /* Check MMIO address conflict */ if (request_mem_region_exclusive(val, SP5100_WDT_MEM_MAP_SIZE, dev_name)) { - pr_debug("Got 0x%04x from SBResource_MMIO register\n", + pr_debug("Got 0x%08x from SBResource_MMIO register\n", val); goto setup_wdt; } else - pr_debug("MMIO address 0x%04x already in use\n", val); + pr_info("MMIO address 0x%08x already in use\n", val); } else - pr_debug("SBResource_MMIO is disabled(0x%04x)\n", val); + pr_info("SBResource_MMIO is disabled (0x%08x)\n", val); - /* - * Lastly re-programming the watchdog timer MMIO address, - * This method is a last resort... - * - * Before re-programming, to ensure that the watchdog timer - * is disabled, disable the watchdog timer. - */ - tco_timer_disable(); - - if (force_addr) { - /* - * Force the use of watchdog timer MMIO address, and aligned to - * 8byte boundary. - */ - force_addr &= ~0x7; - val = force_addr; - - pr_info("Force the use of 0x%04x as MMIO address\n", val); - } else { - /* - * Get empty slot into the resource tree for watchdog timer. - */ - if (allocate_resource(&iomem_resource, - &wdt_res, - SP5100_WDT_MEM_MAP_SIZE, - 0xf0000000, - 0xfffffff8, - 0x8, - NULL, - NULL)) { - pr_err("MMIO allocation failed\n"); - goto unreg_region; - } - - val = resbase_phys = wdt_res.start; - pr_debug("Got 0x%04x from resource tree\n", val); - } - - /* Restore to the low three bits */ - outb(base_addr+0, index_reg); - tmp_val = val | (inb(data_reg) & 0x7); - - /* Re-programming the watchdog timer base address */ - outb(base_addr+0, index_reg); - outb((tmp_val >> 0) & 0xff, data_reg); - outb(base_addr+1, index_reg); - outb((tmp_val >> 8) & 0xff, data_reg); - outb(base_addr+2, index_reg); - outb((tmp_val >> 16) & 0xff, data_reg); - outb(base_addr+3, index_reg); - outb((tmp_val >> 24) & 0xff, data_reg); - - if (!request_mem_region_exclusive(val, SP5100_WDT_MEM_MAP_SIZE, - dev_name)) { - pr_err("MMIO address 0x%04x already in use\n", val); - goto unreg_resource; - } + pr_notice("Can't find MMIO address, giving up.\n"); + goto unreg_region; setup_wdt: tcobase_phys = val; @@ -526,7 +427,7 @@ setup_wdt: goto unreg_mem_region; } - pr_info("Using 0x%04x for watchdog MMIO address\n", val); + pr_info("Using 0x%08x for watchdog MMIO address\n", val); /* Setup the watchdog timer */ tco_timer_enable(); @@ -555,9 +456,6 @@ setup_wdt: unreg_mem_region: release_mem_region(tcobase_phys, SP5100_WDT_MEM_MAP_SIZE); -unreg_resource: - if (resbase_phys) - release_resource(&wdt_res); unreg_region: release_region(pm_iobase, SP5100_PM_IOPORTS_SIZE); exit: @@ -567,7 +465,6 @@ exit: static int sp5100_tco_init(struct platform_device *dev) { int ret; - char addr_str[16]; /* * Check whether or not the hardware watchdog is there. If found, then @@ -599,23 +496,14 @@ static int sp5100_tco_init(struct platform_device *dev) clear_bit(0, &timer_alive); /* Show module parameters */ - if (force_addr == tcobase_phys) - /* The force_addr is vaild */ - sprintf(addr_str, "0x%04x", force_addr); - else - strcpy(addr_str, "none"); - - pr_info("initialized (0x%p). heartbeat=%d sec (nowayout=%d, " - "force_addr=%s)\n", - tcobase, heartbeat, nowayout, addr_str); + pr_info("initialized (0x%p). heartbeat=%d sec (nowayout=%d)\n", + tcobase, heartbeat, nowayout); return 0; exit: iounmap(tcobase); release_mem_region(tcobase_phys, SP5100_WDT_MEM_MAP_SIZE); - if (resbase_phys) - release_resource(&wdt_res); release_region(pm_iobase, SP5100_PM_IOPORTS_SIZE); return ret; } @@ -630,8 +518,6 @@ static void sp5100_tco_cleanup(void) misc_deregister(&sp5100_tco_miscdev); iounmap(tcobase); release_mem_region(tcobase_phys, SP5100_WDT_MEM_MAP_SIZE); - if (resbase_phys) - release_resource(&wdt_res); release_region(pm_iobase, SP5100_PM_IOPORTS_SIZE); } diff --git a/drivers/watchdog/sp5100_tco.h b/drivers/watchdog/sp5100_tco.h index 71594a0..2b28c00 100644 --- a/drivers/watchdog/sp5100_tco.h +++ b/drivers/watchdog/sp5100_tco.h @@ -57,7 +57,7 @@ #define SB800_PM_WATCHDOG_DISABLE (1 << 2) #define SB800_PM_WATCHDOG_SECOND_RES (3 << 0) #define SB800_ACPI_MMIO_DECODE_EN (1 << 0) -#define SB800_ACPI_MMIO_SEL (1 << 2) +#define SB800_ACPI_MMIO_SEL (1 << 1) #define SB800_PM_WDT_MMIO_OFFSET 0xB00 -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support 2013-02-18 17:14 ` Tanaka Takahisa @ 2013-02-22 0:04 ` Joseph Salisbury 2013-02-23 6:14 ` Tanaka Takahisa 0 siblings, 1 reply; 9+ messages in thread From: Joseph Salisbury @ 2013-02-22 0:04 UTC (permalink / raw) To: Tanaka Takahisa Cc: Joseph Salisbury, Wim Van Sebroeck, linux-watchdog, LKML, Kernel Team On 02/18/2013 12:14 PM, Tanaka Takahisa wrote: > Hi Joseph, > > Thanks a lot for your help. > >> The bug report tested a kernel with the patches. However, he reports the kernel panic still occurs[0]. > I understood. I guess this problem conflicts with the watchdog MMIO > address (*) written in SB700 chipset and other resource. So, I made a > patch which gets rid of the code considered to cause the problem. If > this patch is applied, although the SP5100 and SB7x0 chipsets can't > use watchdog function any longer, I'm sure attached patch resolve the > problem. The SB800 or later chipsets can be used as before. > (*)This address is obtained from allocate_resource() function. > > I'm sorry to trouble you, but Would you confirm whether attached patch > solves a problem? > If there is no problem in this patch, I will submit this patch to a > linux-watchdog community. This patch did resolve the issue. The bug reporter states the panic no longer happens and the system boots. Thanks so much for your assistance. > > >> I've requested a digital image or screen capture of the panic. Is >> there any additional debug information you thing would be helpful? > I'm interested in the I/O resource of PC in which the problem has > occurred, So, I want the result of 'cat /proc/iomem'. When 'cat > /proc/iomem' is performed, I don't care about whether sp5100_tco > driver is loaded. The I/O data can be seen at: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1116835/+attachment/3540738/+files/iomem.txt > > > Thanks in advance. > Takahisa ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support 2013-02-22 0:04 ` Joseph Salisbury @ 2013-02-23 6:14 ` Tanaka Takahisa 0 siblings, 0 replies; 9+ messages in thread From: Tanaka Takahisa @ 2013-02-23 6:14 UTC (permalink / raw) To: Joseph Salisbury Cc: Joseph Salisbury, Wim Van Sebroeck, linux-watchdog, LKML, Kernel Team Hi Joseph, Thank you for testing! I will submit this patch to the linux-watchdog community after adding commit log to patch. 2013/2/22 Joseph Salisbury <joseph.salisbury@canonical.com>: > The I/O data can be seen at: > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1116835/+attachment/3540738/+files/iomem.txt In the case of SP5100 and SB7x0 chipset, the sp5100_tco driver overwrites a free resource I/O memory address obtained by allocate_resource() to the MMIO address registers for watchdog timer. In the case of M3A78-CM, the sp5100_tco driver was using 0xfed45000 as a MMIO address. Since 0xfed45000 is the free I/O memory resource address, this is the expected behavior. [ 18.852540] sp5100_tco: Using 0xfed45000 for watchdog MMIO address However, Rewriting the MMIO address registers for the watchdog timer must have generated the problem. I think that the problem has occurred with the chipset or the BIOS layer. So, It's difficult for me to investigate the problem, and the problem is critical. Thus, I decided to delete the concerned codes. Regards, Takahisa ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-02-23 6:14 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-02-14 16:07 [v3.8 Regression] watchdog: sp5100_tco: Add SB8x0 chipset support Joseph Salisbury 2013-02-15 7:32 ` Wim Van Sebroeck 2013-02-15 14:54 ` Joseph Salisbury 2013-02-17 9:44 ` Tanaka Takahisa 2013-02-18 3:13 ` Joseph Salisbury 2013-02-18 7:26 ` Joseph Salisbury 2013-02-18 17:14 ` Tanaka Takahisa 2013-02-22 0:04 ` Joseph Salisbury 2013-02-23 6:14 ` Tanaka Takahisa
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).