* [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits @ 2013-01-14 2:01 Takahisa Tanaka 2013-01-14 2:01 ` [PATCH 2/2] sp5100_tco: Write back the original value to reserved bits, instead of zero Takahisa Tanaka 2013-01-30 19:19 ` [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits Wim Van Sebroeck 0 siblings, 2 replies; 6+ messages in thread From: Takahisa Tanaka @ 2013-01-14 2:01 UTC (permalink / raw) To: linux-watchdog Cc: Wim Van Sebroeck, Paul Menzel, Arkadiusz Miskiewicz, Bjorn Helgaas, Andrew Morton, Jonathan Nieder, linux-kernel, Florian Mickler, Takahisa Tanaka 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> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] sp5100_tco: Write back the original value to reserved bits, instead of zero 2013-01-14 2:01 [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits Takahisa Tanaka @ 2013-01-14 2:01 ` Takahisa Tanaka 2013-01-30 19:20 ` Wim Van Sebroeck 2013-01-30 19:19 ` [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits Wim Van Sebroeck 1 sibling, 1 reply; 6+ messages in thread From: Takahisa Tanaka @ 2013-01-14 2:01 UTC (permalink / raw) To: linux-watchdog Cc: Wim Van Sebroeck, Paul Menzel, Arkadiusz Miskiewicz, Bjorn Helgaas, Andrew Morton, Jonathan Nieder, linux-kernel, Florian Mickler, Takahisa Tanaka 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> --- 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] 6+ messages in thread
* Re: [PATCH 2/2] sp5100_tco: Write back the original value to reserved bits, instead of zero 2013-01-14 2:01 ` [PATCH 2/2] sp5100_tco: Write back the original value to reserved bits, instead of zero Takahisa Tanaka @ 2013-01-30 19:20 ` Wim Van Sebroeck 2013-01-31 13:43 ` t t 0 siblings, 1 reply; 6+ messages in thread From: Wim Van Sebroeck @ 2013-01-30 19:20 UTC (permalink / raw) To: Takahisa Tanaka Cc: linux-watchdog, Paul Menzel, Arkadiusz Miskiewicz, Bjorn Helgaas, Andrew Morton, Jonathan Nieder, linux-kernel, Florian Mickler Hi, > 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> Added to linux-watchdog-next. Kind regards, Wim. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] sp5100_tco: Write back the original value to reserved bits, instead of zero 2013-01-30 19:20 ` Wim Van Sebroeck @ 2013-01-31 13:43 ` t t 0 siblings, 0 replies; 6+ messages in thread From: t t @ 2013-01-31 13:43 UTC (permalink / raw) To: Wim Van Sebroeck Cc: linux-watchdog, Paul Menzel, Arkadiusz Miskiewicz, Bjorn Helgaas, Andrew Morton, Jonathan Nieder, linux-kernel, Florian Mickler 2013/1/31 Wim Van Sebroeck <wim@iguana.be>: > Hi, > >> 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> > > Added to linux-watchdog-next. Thanks Wim! Kind regards, Takahisa ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits 2013-01-14 2:01 [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits Takahisa Tanaka 2013-01-14 2:01 ` [PATCH 2/2] sp5100_tco: Write back the original value to reserved bits, instead of zero Takahisa Tanaka @ 2013-01-30 19:19 ` Wim Van Sebroeck 2013-02-06 8:46 ` Paul Menzel 1 sibling, 1 reply; 6+ messages in thread From: Wim Van Sebroeck @ 2013-01-30 19:19 UTC (permalink / raw) To: Takahisa Tanaka Cc: linux-watchdog, Paul Menzel, Arkadiusz Miskiewicz, Bjorn Helgaas, Andrew Morton, Jonathan Nieder, linux-kernel, Florian Mickler Hi, > 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> Added to linux-watchdog-next. Kind regards, Wim. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits 2013-01-30 19:19 ` [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits Wim Van Sebroeck @ 2013-02-06 8:46 ` Paul Menzel 0 siblings, 0 replies; 6+ messages in thread From: Paul Menzel @ 2013-02-06 8:46 UTC (permalink / raw) To: Wim Van Sebroeck Cc: Takahisa Tanaka, linux-watchdog, Arkadiusz Miskiewicz, Bjorn Helgaas, Andrew Morton, Jonathan Nieder, linux-kernel, Florian Mickler [-- Attachment #1: Type: text/plain, Size: 1225 bytes --] Dear Wim, Am Mittwoch, den 30.01.2013, 20:19 +0100 schrieb Wim Van Sebroeck: > > 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> > > Added to linux-watchdog-next. Is it allowed to rewrite history of linux-watchdog-next? If yes, could you please add `CC: stable@vger.kernel.org` to both of Takahisa’s patches and also add my Tested-by: Paul Menzel <paulepanter@users.sourceforge.net> ASRock E350M1 (SB800) as commented on the bug report to it? Thanks, Paul [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-06 8:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-14 2:01 [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits Takahisa Tanaka 2013-01-14 2:01 ` [PATCH 2/2] sp5100_tco: Write back the original value to reserved bits, instead of zero Takahisa Tanaka 2013-01-30 19:20 ` Wim Van Sebroeck 2013-01-31 13:43 ` t t 2013-01-30 19:19 ` [PATCH 1/2] sp5100_tco: Fix wrong indirect I/O access for getting value of reserved bits Wim Van Sebroeck 2013-02-06 8:46 ` Paul Menzel
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).