Re: [PATCH] sp5100_tco: Add SB8x0 chipset support

From: Paul Menzel
Date: Sun Aug 12 2012 - 10:03:49 EST

Next message: richard -rw- weinberger: "Re: fuse: implement i_op->atomic_open() prevents writes to ntfs-3g filesystem"
Previous message: Arthur Marsh: "fuse: implement i_op->atomic_open() prevents writes to ntfs-3g filesystem"
In reply to: Takahisa Tanaka: "[PATCH] sp5100_tco: Add SB8x0 chipset support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Am Sonntag, den 12.08.2012, 20:57 +0900 schrieb Takahisa Tanaka:
> The current sp5100_tco driver only supports SP5100/SB7x0 chipset, doesn't
> support SB8x0 chipset, because current sp5100_tco driver doesn't know that the
> offset address for watchdog timer was changed from SB8x0 chipset.
>
> The offset address of SP5100 and SB7x0 chipsets are as follows, quote from the
> AMD SB700/710/750 Register Reference Guide(Page 164) and the AMD SP5100
> Register Reference Guide(Page 166).
>
> WatchDogTimerControl 69h
> WatchDogTimerBase0 6Ch
> WatchDogTimerBase1 6Dh
> WatchDogTimerBase2 6Eh
> WatchDogTimerBase3 6Fh
>
> In contrast, the offset address of SB8x0 chipset is as follows, quote from
> AMD SB800-Series Southbridges Register Reference Guide(Page 147).
>
> WatchDogTimerEn 48h
> WatchDogTimerConfig 4Ch
>
> So, In the case of SB8x0 chipset, sp5100_tco reads meaningless MMIO
> address(for example, 0xbafe00) from wrong offset address, and the following
> message is logged.
>
> SP5100 TCO timer: mmio address 0xbafe00 already in use
>
> With this patch, sp5100_tco driver supports SB8x0 chipset, and can avoid
> iomem resource conflict. The processing of this patch is as follows.
>
> Step 1) Attempt to get the watchdog base address from indirect I/O(0xCD6
> and 0xCD7).
> - Go to the step 7 if obtained address hasn't conflicted with other
> resource. But, currently, the address(0xfec000f0) conflicts with the
> IOAPIC MMIO address, and the following message is logged.
>
> SP5100 TCO timer: mmio address 0xfec000f0 already in use
>
> 0xfec000f0 is recommended by AMD BIOS Developer's Guide. So, go to the
> next step.
>
> Step 2) Attempt to get the SBResource_MMIO base address from AcpiMmioEN(for
> SB8x0, PM_Reg:24h) or SBResource_MMIO(SP5100/SB7x0, PCI_Reg:9Ch)
> register.
> - Go to the step 7 if these register has enabled by BIOS, and obtained
> address hasn't conflicted with other resource.
> - If above condition isn't true, go to the next step.
>
> Step 3) Attempt to get the free MMIO address from allocate_resource().
> - Go to the step 7 if these register has enabled by BIOS, and obtained
> address hasn't conflicted with other resource.
> - Driver initialization has failed if obtained address has conflicted
> with other resource, and no 'force_addr' parameter is specified.
>
> Step 4) Use the specified address If 'force_addr' parameter is specified.
> - allocate_resource() function may fail, when the PCI bridge device occupies
> iomem resource from 0xf0000000 to 0xffffffff. To handle such a case,
> I added 'force_addr' parameter to sp5100_tco driver. With 'force_addr'
> parameter, sp5100_tco driver directly can assign MMIO address for watchdog
> timer from free iomem region. Note that It's dangerous to specify wrong
> address in the 'force_addr' parameter.
>
> Example of force_addr parameter use
> # cat /proc/iomem
> ...snip...
> fec00000-fec003ff : IOAPIC 0
> <--- free MMIO region
> fec10000-fec1001f : pnp 00:0b
> fec20000-fec203ff : IOAPIC 1
> ...snip...
> # cat /etc/modprobe.d/sp5100_tco.conf
> options sp5100_tco force_addr=0xfec00800
> # modprobe sp5100_tco
> # cat /proc/iomem
> ...snip...
> fec00000-fec003ff : IOAPIC 0
> fec00800-fec00807 : SP5100 TCO <--- watchdog timer MMIO address
> fec10000-fec1001f : pnp 00:0b
> fec20000-fec203ff : IOAPIC 1
> ...snip...
> #
>
> - Driver initialization has failed if specified address has conflicted
> with other resource.
>
> Step 5) Disable the watchdog timer
> - To rewrite the watchdog timer register of the chipset, absolutely
> guarantee that the watchdog timer is disabled.
>
> Step 6) Re-program the watchdog timer MMIO address to chipset.
> - Re-program the obtained MMIO address in Step 3 or Step 4 to chipset via
> indirect I/O(0xCD6 and 0xCD7).
>
> Step 7) Enable and setup the watchdog timer
>
> This patch has worked fine on my test environment(ASUS M4A89GTD-PRO/USB3 and
> DL165G7). therefore I believe that it's no problem to re-program the MMIO
> address for watchdog timer to chipset during disabled watchdog. However,
> I'm not sure about it, because I don't know much about chipset programming.
>
> So, any comments will be welcome.
>
> Tested-by: Paul Menzel <paulepanter@xxxxxxxxxxxxxxxxxxxxx>

ASRock A780FullHD <http://www.asrock.com/mb/overview.asp?model=a780fullhd>

> CC: stable@xxxxxxxxxx

I am sorry. This should have been <stable@xxxxxxxxxxxxxxx>.

> Signed-off-by: Takahisa Tanaka <mc74hc00@xxxxxxxxx>

After review from the maintainers, please also add

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43176

when sending `PATCH v2` (`--subject-prefix` [1]).

[1] http://wireless.kernel.org/en/developers/Documentation/git-guide#Annotating_new_revision

> ---
> drivers/watchdog/sp5100_tco.c | 291 ++++++++++++++++++++++++++++++++++++------
> drivers/watchdog/sp5100_tco.h | 46 +++++--
> 2 files changed, 286 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
> index ae5e82c..36e917f 100644
> --- a/drivers/watchdog/sp5100_tco.c
> +++ b/drivers/watchdog/sp5100_tco.c
> @@ -13,7 +13,9 @@
> * as published by the Free Software Foundation; either version
> * 2 of the License, or (at your option) any later version.
> *
> - * See AMD Publication 43009 "AMD SB700/710/750 Register Reference Guide"
> + * See AMD Publication 43009 "AMD SB700/710/750 Register Reference Guide",
> + * AMD Publication 45482 "AMD SB800-Series Sourthbridges Register

Sou*thbridges

> + * Reference Guide"
> */
>
> /*
> @@ -38,18 +40,23 @@
> #include "sp5100_tco.h"
>
> /* Module and version information */
> -#define TCO_VERSION "0.01"
> +#define TCO_VERSION "0.03"
> #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 void __iomem *tcobase;
> static unsigned int pm_iobase;
> 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;
> @@ -67,6 +74,12 @@ 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, "
> + "default is disabled. DON'T USE THIS PARAMETER ONLY "
> + "IF YOU REALLY KNOW WHAT YOU ARE DOING");

ONLY USE THIS PARAMETER IF YOU REALLY KNOW WHAT YOU ARE DOING!

[â]

Thanks,

Paul

Attachment: signature.asc
Description: This is a digitally signed message part

Next message: richard -rw- weinberger: "Re: fuse: implement i_op->atomic_open() prevents writes to ntfs-3g filesystem"
Previous message: Arthur Marsh: "fuse: implement i_op->atomic_open() prevents writes to ntfs-3g filesystem"
In reply to: Takahisa Tanaka: "[PATCH] sp5100_tco: Add SB8x0 chipset support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]