Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
* wdat_wdt: access width inconsistency
@ 2020-02-10 10:16 Jean Delvare
  2020-02-10 11:23 ` Mika Westerberg
  0 siblings, 1 reply; 20+ messages in thread
From: Jean Delvare @ 2020-02-10 10:16 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Len Brown, linux-acpi, Wim Van Sebroeck, Guenter Roeck,
	linux-watchdog, Mika Westerberg, Tom Abraham

Hi all,

I'm still working on my customer issue where the wdat_wdt driver
reboots the server instantly as soon as the watchdog daemon is started.
I looked at all the upstream fixes and we already have all relevant
ones in our kernel so I start suspecting either a driver bug or a BIOS
issue.

While reading the driver code I noticed one suspect thing related to
the register access width, which I'd like a second opinion on.

Both acpi_watchdog.c and wdat_wdt.c contain code like:

	res.end = res.start + gas->access_width - 1;

This suggests that gas->access_width is expected to be 4 in case of a
32-bit register. However in wdat_wdt_read/wdat_wdt_write we have:

	switch (gas->access_width) {
	(...)
	case 3:
		*value = ioread32(instr->reg);

This looks inconsistent to me.

My reading of the ACPI specification suggests that 3 is the right value
for 32-bit registers. If so, then shouldn't the resource's end be set
to:

	res.end = res.start + (1 << (gas->access_width - 1)) - 1;

?

Thanks,
-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, back to index

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 10:16 wdat_wdt: access width inconsistency Jean Delvare
2020-02-10 11:23 ` Mika Westerberg
2020-02-11 13:11   ` Jean Delvare
2020-02-11 13:59     ` Mika Westerberg
2020-02-11 16:25       ` Jean Delvare
2020-02-11 16:37         ` Mika Westerberg
2020-02-11 17:03           ` Jean Delvare
2020-02-12 11:05             ` [PATCH 1/3] ACPICA: Introduce ACPI_ACCESS_BIT_WIDTH() macro Mika Westerberg
2020-02-12 11:05               ` [PATCH 2/3] ACPI / watchdog: Fix gas->access_width usage Mika Westerberg
2020-02-12 11:56                 ` Jean Delvare
2020-02-12 12:10                   ` Mika Westerberg
2020-02-12 11:05               ` [PATCH 3/3] ACPI / watchdog: Set default timeout in probe Mika Westerberg
2020-02-12 12:07                 ` Jean Delvare
2020-02-12 12:13                   ` Mika Westerberg
2020-02-12 11:52               ` [PATCH 1/3] ACPICA: Introduce ACPI_ACCESS_BIT_WIDTH() macro Jean Delvare
2020-02-12 12:08                 ` Mika Westerberg
2020-02-11 16:45         ` wdat_wdt: access width inconsistency Guenter Roeck
2020-02-12 10:30   ` Jean Delvare
2020-02-12 10:47     ` Mika Westerberg
2020-02-12 11:05       ` Jean Delvare

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org
	public-inbox-index linux-watchdog

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git