* [PATCH 1/2] watchdog: sp5100: Fix definition of EFCH_PM_DECODEEN3 @ 2020-09-10 16:31 Guenter Roeck 2020-09-10 16:31 ` [PATCH 2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2020-09-10 16:31 UTC (permalink / raw) To: Wim Van Sebroeck; +Cc: linux-watchdog, linux-kernel, Guenter Roeck, Jan Kiszka EFCH_PM_DECODEEN3 is supposed to access DECODEEN register bits 24..31, in other words the register at byte offset 3. Cc: Jan Kiszka <jan.kiszka@siemens.com> Fixes: 887d2ec51e34b ("watchdog: sp5100_tco: Add support for recent FCH versions") Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/watchdog/sp5100_tco.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/sp5100_tco.h b/drivers/watchdog/sp5100_tco.h index 87eaf357ae01..adf015aa4126 100644 --- a/drivers/watchdog/sp5100_tco.h +++ b/drivers/watchdog/sp5100_tco.h @@ -70,7 +70,7 @@ #define EFCH_PM_DECODEEN_WDT_TMREN BIT(7) -#define EFCH_PM_DECODEEN3 0x00 +#define EFCH_PM_DECODEEN3 0x03 #define EFCH_PM_DECODEEN_SECOND_RES GENMASK(1, 0) #define EFCH_PM_WATCHDOG_DISABLE ((u8)GENMASK(3, 2)) -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled 2020-09-10 16:31 [PATCH 1/2] watchdog: sp5100: Fix definition of EFCH_PM_DECODEEN3 Guenter Roeck @ 2020-09-10 16:31 ` Guenter Roeck 2020-09-10 16:34 ` Jan Kiszka 0 siblings, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2020-09-10 16:31 UTC (permalink / raw) To: Wim Van Sebroeck; +Cc: linux-watchdog, linux-kernel, Guenter Roeck, Jan Kiszka On Family 17h (Ryzen) devices, the WatchdogTmrEn bit of PmDecodeEn not only enables watchdog memory decoding at 0xfeb00000, it also enables the watchdog hardware itself. Use this information to enable the watchdog if it is not already enabled. Cc: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/watchdog/sp5100_tco.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c index 85e9664318c9..a730ecbf78cd 100644 --- a/drivers/watchdog/sp5100_tco.c +++ b/drivers/watchdog/sp5100_tco.c @@ -17,6 +17,12 @@ * AMD Publication 51192 "AMD Bolton FCH Register Reference Guide" * AMD Publication 52740 "BIOS and Kernel Developer’s Guide (BKDG) * for AMD Family 16h Models 30h-3Fh Processors" + * AMD Publication 55570-B1-PUB "Processor Programming Reference (PPR) + * for AMD Family 17h Model 18h, Revision B1 + * Processors (PUB) + * AMD Publication 55772-A1-PUB "Processor Programming Reference (PPR) + * for AMD Family 17h Model 20h, Revision A1 + * Processors (PUB) */ /* @@ -241,6 +247,18 @@ static int sp5100_tco_setupdevice(struct device *dev, break; case efch: dev_name = SB800_DEVNAME; + /* + * On Family 17h devices, the EFCH_PM_DECODEEN_WDT_TMREN bit of + * EFCH_PM_DECODEEN not only enables the EFCH_PM_WDT_ADDR memory + * region, it also enables the watchdog itself. + */ + if (boot_cpu_data.x86 == 0x17) { + val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); + if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) { + sp5100_tco_update_pm_reg8(EFCH_PM_DECODEEN, 0xff, + EFCH_PM_DECODEEN_WDT_TMREN); + } + } val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); if (val & EFCH_PM_DECODEEN_WDT_TMREN) mmio_addr = EFCH_PM_WDT_ADDR; -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled 2020-09-10 16:31 ` [PATCH 2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled Guenter Roeck @ 2020-09-10 16:34 ` Jan Kiszka 2020-09-10 16:53 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: Jan Kiszka @ 2020-09-10 16:34 UTC (permalink / raw) To: Guenter Roeck, Wim Van Sebroeck; +Cc: linux-watchdog, linux-kernel On 10.09.20 18:31, Guenter Roeck wrote: > On Family 17h (Ryzen) devices, the WatchdogTmrEn bit of PmDecodeEn not only > enables watchdog memory decoding at 0xfeb00000, it also enables the > watchdog hardware itself. Use this information to enable the watchdog if > it is not already enabled. > > Cc: Jan Kiszka <jan.kiszka@siemens.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/sp5100_tco.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c > index 85e9664318c9..a730ecbf78cd 100644 > --- a/drivers/watchdog/sp5100_tco.c > +++ b/drivers/watchdog/sp5100_tco.c > @@ -17,6 +17,12 @@ > * AMD Publication 51192 "AMD Bolton FCH Register Reference Guide" > * AMD Publication 52740 "BIOS and Kernel Developer’s Guide (BKDG) > * for AMD Family 16h Models 30h-3Fh Processors" > + * AMD Publication 55570-B1-PUB "Processor Programming Reference (PPR) > + * for AMD Family 17h Model 18h, Revision B1 > + * Processors (PUB) > + * AMD Publication 55772-A1-PUB "Processor Programming Reference (PPR) > + * for AMD Family 17h Model 20h, Revision A1 > + * Processors (PUB) > */ > > /* > @@ -241,6 +247,18 @@ static int sp5100_tco_setupdevice(struct device *dev, > break; > case efch: > dev_name = SB800_DEVNAME; > + /* > + * On Family 17h devices, the EFCH_PM_DECODEEN_WDT_TMREN bit of > + * EFCH_PM_DECODEEN not only enables the EFCH_PM_WDT_ADDR memory > + * region, it also enables the watchdog itself. > + */ > + if (boot_cpu_data.x86 == 0x17) { > + val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); > + if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) { > + sp5100_tco_update_pm_reg8(EFCH_PM_DECODEEN, 0xff, > + EFCH_PM_DECODEEN_WDT_TMREN); > + } > + } > val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); > if (val & EFCH_PM_DECODEEN_WDT_TMREN) > mmio_addr = EFCH_PM_WDT_ADDR; > Won't that bring us EFCH_PM_WDT_ADDR as address, rather than EFCH_PM_ACPI_MMIO_ADDR which worked in my case? Or is one an alias of the other. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled 2020-09-10 16:34 ` Jan Kiszka @ 2020-09-10 16:53 ` Guenter Roeck 2020-09-10 16:55 ` Jan Kiszka 0 siblings, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2020-09-10 16:53 UTC (permalink / raw) To: Jan Kiszka, Wim Van Sebroeck; +Cc: linux-watchdog, linux-kernel Hi Jan, On 9/10/20 9:34 AM, Jan Kiszka wrote: > On 10.09.20 18:31, Guenter Roeck wrote: >> On Family 17h (Ryzen) devices, the WatchdogTmrEn bit of PmDecodeEn not only >> enables watchdog memory decoding at 0xfeb00000, it also enables the >> watchdog hardware itself. Use this information to enable the watchdog if >> it is not already enabled. >> >> Cc: Jan Kiszka <jan.kiszka@siemens.com> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> drivers/watchdog/sp5100_tco.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c >> index 85e9664318c9..a730ecbf78cd 100644 >> --- a/drivers/watchdog/sp5100_tco.c >> +++ b/drivers/watchdog/sp5100_tco.c >> @@ -17,6 +17,12 @@ >> * AMD Publication 51192 "AMD Bolton FCH Register Reference Guide" >> * AMD Publication 52740 "BIOS and Kernel Developer’s Guide (BKDG) >> * for AMD Family 16h Models 30h-3Fh Processors" >> + * AMD Publication 55570-B1-PUB "Processor Programming Reference (PPR) >> + * for AMD Family 17h Model 18h, Revision B1 >> + * Processors (PUB) >> + * AMD Publication 55772-A1-PUB "Processor Programming Reference (PPR) >> + * for AMD Family 17h Model 20h, Revision A1 >> + * Processors (PUB) >> */ >> >> /* >> @@ -241,6 +247,18 @@ static int sp5100_tco_setupdevice(struct device *dev, >> break; >> case efch: >> dev_name = SB800_DEVNAME; >> + /* >> + * On Family 17h devices, the EFCH_PM_DECODEEN_WDT_TMREN bit of >> + * EFCH_PM_DECODEEN not only enables the EFCH_PM_WDT_ADDR memory >> + * region, it also enables the watchdog itself. >> + */ >> + if (boot_cpu_data.x86 == 0x17) { >> + val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); >> + if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) { >> + sp5100_tco_update_pm_reg8(EFCH_PM_DECODEEN, 0xff, >> + EFCH_PM_DECODEEN_WDT_TMREN); >> + } >> + } >> val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); >> if (val & EFCH_PM_DECODEEN_WDT_TMREN) >> mmio_addr = EFCH_PM_WDT_ADDR; >> > > Won't that bring us EFCH_PM_WDT_ADDR as address, rather than > EFCH_PM_ACPI_MMIO_ADDR which worked in my case? Or is one an alias of > the other. > Yes, it does use EFCH_PM_WDT_ADDR. EFCH_PM_ACPI_MMIO_ADDR works as well, but is meant to be a fallback. Both point to the watchdog memory space. Thanks, Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled 2020-09-10 16:53 ` Guenter Roeck @ 2020-09-10 16:55 ` Jan Kiszka 2020-09-11 5:32 ` Jan Kiszka 0 siblings, 1 reply; 6+ messages in thread From: Jan Kiszka @ 2020-09-10 16:55 UTC (permalink / raw) To: Guenter Roeck, Wim Van Sebroeck; +Cc: linux-watchdog, linux-kernel On 10.09.20 18:53, Guenter Roeck wrote: > Hi Jan, > > On 9/10/20 9:34 AM, Jan Kiszka wrote: >> On 10.09.20 18:31, Guenter Roeck wrote: >>> On Family 17h (Ryzen) devices, the WatchdogTmrEn bit of PmDecodeEn not only >>> enables watchdog memory decoding at 0xfeb00000, it also enables the >>> watchdog hardware itself. Use this information to enable the watchdog if >>> it is not already enabled. >>> >>> Cc: Jan Kiszka <jan.kiszka@siemens.com> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >>> --- >>> drivers/watchdog/sp5100_tco.c | 18 ++++++++++++++++++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c >>> index 85e9664318c9..a730ecbf78cd 100644 >>> --- a/drivers/watchdog/sp5100_tco.c >>> +++ b/drivers/watchdog/sp5100_tco.c >>> @@ -17,6 +17,12 @@ >>> * AMD Publication 51192 "AMD Bolton FCH Register Reference Guide" >>> * AMD Publication 52740 "BIOS and Kernel Developer’s Guide (BKDG) >>> * for AMD Family 16h Models 30h-3Fh Processors" >>> + * AMD Publication 55570-B1-PUB "Processor Programming Reference (PPR) >>> + * for AMD Family 17h Model 18h, Revision B1 >>> + * Processors (PUB) >>> + * AMD Publication 55772-A1-PUB "Processor Programming Reference (PPR) >>> + * for AMD Family 17h Model 20h, Revision A1 >>> + * Processors (PUB) >>> */ >>> >>> /* >>> @@ -241,6 +247,18 @@ static int sp5100_tco_setupdevice(struct device *dev, >>> break; >>> case efch: >>> dev_name = SB800_DEVNAME; >>> + /* >>> + * On Family 17h devices, the EFCH_PM_DECODEEN_WDT_TMREN bit of >>> + * EFCH_PM_DECODEEN not only enables the EFCH_PM_WDT_ADDR memory >>> + * region, it also enables the watchdog itself. >>> + */ >>> + if (boot_cpu_data.x86 == 0x17) { >>> + val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); >>> + if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) { >>> + sp5100_tco_update_pm_reg8(EFCH_PM_DECODEEN, 0xff, >>> + EFCH_PM_DECODEEN_WDT_TMREN); >>> + } >>> + } >>> val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); >>> if (val & EFCH_PM_DECODEEN_WDT_TMREN) >>> mmio_addr = EFCH_PM_WDT_ADDR; >>> >> >> Won't that bring us EFCH_PM_WDT_ADDR as address, rather than >> EFCH_PM_ACPI_MMIO_ADDR which worked in my case? Or is one an alias of >> the other. >> > > Yes, it does use EFCH_PM_WDT_ADDR. EFCH_PM_ACPI_MMIO_ADDR works as well, > but is meant to be a fallback. Both point to the watchdog memory space. > OK, will test, possibly only on the weekend, and confirm this also on my board. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled 2020-09-10 16:55 ` Jan Kiszka @ 2020-09-11 5:32 ` Jan Kiszka 0 siblings, 0 replies; 6+ messages in thread From: Jan Kiszka @ 2020-09-11 5:32 UTC (permalink / raw) To: Guenter Roeck, Wim Van Sebroeck; +Cc: linux-watchdog, linux-kernel On 10.09.20 18:55, Jan Kiszka wrote: > On 10.09.20 18:53, Guenter Roeck wrote: >> Hi Jan, >> >> On 9/10/20 9:34 AM, Jan Kiszka wrote: >>> On 10.09.20 18:31, Guenter Roeck wrote: >>>> On Family 17h (Ryzen) devices, the WatchdogTmrEn bit of PmDecodeEn not only >>>> enables watchdog memory decoding at 0xfeb00000, it also enables the >>>> watchdog hardware itself. Use this information to enable the watchdog if >>>> it is not already enabled. >>>> >>>> Cc: Jan Kiszka <jan.kiszka@siemens.com> >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >>>> --- >>>> drivers/watchdog/sp5100_tco.c | 18 ++++++++++++++++++ >>>> 1 file changed, 18 insertions(+) >>>> >>>> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c >>>> index 85e9664318c9..a730ecbf78cd 100644 >>>> --- a/drivers/watchdog/sp5100_tco.c >>>> +++ b/drivers/watchdog/sp5100_tco.c >>>> @@ -17,6 +17,12 @@ >>>> * AMD Publication 51192 "AMD Bolton FCH Register Reference Guide" >>>> * AMD Publication 52740 "BIOS and Kernel Developer’s Guide (BKDG) >>>> * for AMD Family 16h Models 30h-3Fh Processors" >>>> + * AMD Publication 55570-B1-PUB "Processor Programming Reference (PPR) >>>> + * for AMD Family 17h Model 18h, Revision B1 >>>> + * Processors (PUB) >>>> + * AMD Publication 55772-A1-PUB "Processor Programming Reference (PPR) >>>> + * for AMD Family 17h Model 20h, Revision A1 >>>> + * Processors (PUB) >>>> */ >>>> >>>> /* >>>> @@ -241,6 +247,18 @@ static int sp5100_tco_setupdevice(struct device *dev, >>>> break; >>>> case efch: >>>> dev_name = SB800_DEVNAME; >>>> + /* >>>> + * On Family 17h devices, the EFCH_PM_DECODEEN_WDT_TMREN bit of >>>> + * EFCH_PM_DECODEEN not only enables the EFCH_PM_WDT_ADDR memory >>>> + * region, it also enables the watchdog itself. >>>> + */ >>>> + if (boot_cpu_data.x86 == 0x17) { >>>> + val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); >>>> + if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) { >>>> + sp5100_tco_update_pm_reg8(EFCH_PM_DECODEEN, 0xff, >>>> + EFCH_PM_DECODEEN_WDT_TMREN); >>>> + } >>>> + } >>>> val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN); >>>> if (val & EFCH_PM_DECODEEN_WDT_TMREN) >>>> mmio_addr = EFCH_PM_WDT_ADDR; >>>> >>> >>> Won't that bring us EFCH_PM_WDT_ADDR as address, rather than >>> EFCH_PM_ACPI_MMIO_ADDR which worked in my case? Or is one an alias of >>> the other. >>> >> >> Yes, it does use EFCH_PM_WDT_ADDR. EFCH_PM_ACPI_MMIO_ADDR works as well, >> but is meant to be a fallback. Both point to the watchdog memory space. >> > > OK, will test, possibly only on the weekend, and confirm this also on my > board. > > Jan > Both patches now Tested-by: Jan Kiszka <jan.kiszka@siemens.com> Thanks, Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-11 5:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-10 16:31 [PATCH 1/2] watchdog: sp5100: Fix definition of EFCH_PM_DECODEEN3 Guenter Roeck 2020-09-10 16:31 ` [PATCH 2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled Guenter Roeck 2020-09-10 16:34 ` Jan Kiszka 2020-09-10 16:53 ` Guenter Roeck 2020-09-10 16:55 ` Jan Kiszka 2020-09-11 5:32 ` Jan Kiszka
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).