[PATCH 0/2] Correct iTCO Watchdog for Apollo Lake

[PATCH 0/2] Correct iTCO Watchdog for Apollo Lake

[Yong, Jonathan]

These patches fix the iTCO watchdog for Apollo Lake.
I changed the watchdog memory io to only use 4 bytes rather
the whole region, I'm not sure if that is the correct way.

Let me know if the patches need changes.
Please CC me as I am not subscribed, thanks.

Yong, Jonathan (2):
  watchdog: iTCO-wdt handle 5th variation for Apollo Lake
  x86: Fix Apollo Lake Watchdog address in PMC driver

 drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
 drivers/watchdog/iTCO_wdt.c          |  4 ++++
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.7.3

[PATCH 1/2] watchdog: iTCO-wdt handle 5th variation for Apollo Lake

[Yong, Jonathan]

The Apollo Lake Watchdog has the no_reboot flag in the 4th bit.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/watchdog/iTCO_wdt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 0acc6c5..eccba32 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -150,6 +150,9 @@ static inline u32 no_reboot_bit(void)
 	u32 enable_bit;
 
 	switch (iTCO_wdt_private.iTCO_version) {
+	case 5:
+		enable_bit = 0x00000008;
+		break;
 	case 3:
 		enable_bit = 0x00000010;
 		break;
@@ -512,6 +515,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
 
 	/* Clear out the (probably old) status */
 	switch (iTCO_wdt_private.iTCO_version) {
+	case 5:
 	case 4:
 		outw(0x0008, TCO1_STS);	/* Clear the Time Out Status bit */
 		outw(0x0002, TCO2_STS);	/* Clear SECOND_TO_STS bit */
-- 
2.7.3

Re: [PATCH 1/2] watchdog: iTCO-wdt handle 5th variation for Apollo Lake

[Guenter Roeck]

On 05/17/2016 08:26 PM, Yong, Jonathan wrote:
> The Apollo Lake Watchdog has the no_reboot flag in the 4th bit.
>
> Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/iTCO_wdt.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 0acc6c5..eccba32 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -150,6 +150,9 @@ static inline u32 no_reboot_bit(void)
>   	u32 enable_bit;
>
>   	switch (iTCO_wdt_private.iTCO_version) {
> +	case 5:
> +		enable_bit = 0x00000008;
> +		break;
>   	case 3:
>   		enable_bit = 0x00000010;
>   		break;
> @@ -512,6 +515,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
>
>   	/* Clear out the (probably old) status */
>   	switch (iTCO_wdt_private.iTCO_version) {
> +	case 5:
>   	case 4:
>   		outw(0x0008, TCO1_STS);	/* Clear the Time Out Status bit */
>   		outw(0x0002, TCO2_STS);	/* Clear SECOND_TO_STS bit */
>

[PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Yong, Jonathan]

The TCO I/O base is 40h rather than the usual 30h, and the re_reboot
bit is at ACPIBASE+8.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index 6f497e8..b86e1bc 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -85,7 +85,7 @@
  * platform device and to export resources for those functions.
  */
 #define TCO_DEVICE_NAME			"iTCO_wdt"
-#define SMI_EN_OFFSET			0x30
+#define SMI_EN_OFFSET			0x40
 #define SMI_EN_SIZE			4
 #define TCO_BASE_OFFSET			0x60
 #define TCO_REGS_SIZE			16
@@ -94,6 +94,8 @@
 #define TELEM_SSRAM_SIZE		240
 #define TELEM_PMC_SSRAM_OFFSET		0x1B00
 #define TELEM_PUNIT_SSRAM_OFFSET	0x1A00
+#define TCO_PMC_OFFSET			0x8
+#define TCO_PMC_SIZE			0x4
 
 static const int iTCO_version = 3;
 
@@ -502,7 +504,7 @@ static struct resource tco_res[] = {
 
 static struct itco_wdt_platform_data tco_info = {
 	.name = "Apollo Lake SoC",
-	.version = 3,
+	.version = 5,
 };
 
 #define TELEMETRY_RESOURCE_PUNIT_SSRAM	0
@@ -572,8 +574,8 @@ static int ipc_create_tco_device(void)
 	res->end = res->start + SMI_EN_SIZE - 1;
 
 	res = tco_res + TCO_RESOURCE_GCR_MEM;
-	res->start = ipcdev.gcr_base;
-	res->end = res->start + ipcdev.gcr_size - 1;
+	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
+	res->end = res->start + TCO_PMC_SIZE - 1;
 
 	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
 	if (ret) {
-- 
2.7.3

Re: [PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Guenter Roeck]

On 05/17/2016 08:26 PM, Yong, Jonathan wrote:
> The TCO I/O base is 40h rather than the usual 30h, and the re_reboot
> bit is at ACPIBASE+8.
>

Does this mean that the code never worked, or in other words that it was never tested,
or are there now other chips which used the old definitions which no longer work
after this patch has been applied ?

> Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
> ---
>   drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index 6f497e8..b86e1bc 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -85,7 +85,7 @@
>    * platform device and to export resources for those functions.
>    */
>   #define TCO_DEVICE_NAME			"iTCO_wdt"
> -#define SMI_EN_OFFSET			0x30
> +#define SMI_EN_OFFSET			0x40
>   #define SMI_EN_SIZE			4
>   #define TCO_BASE_OFFSET			0x60
>   #define TCO_REGS_SIZE			16
> @@ -94,6 +94,8 @@
>   #define TELEM_SSRAM_SIZE		240
>   #define TELEM_PMC_SSRAM_OFFSET		0x1B00
>   #define TELEM_PUNIT_SSRAM_OFFSET	0x1A00
> +#define TCO_PMC_OFFSET			0x8
> +#define TCO_PMC_SIZE			0x4
>
>   static const int iTCO_version = 3;
>
> @@ -502,7 +504,7 @@ static struct resource tco_res[] = {
>
>   static struct itco_wdt_platform_data tco_info = {
>   	.name = "Apollo Lake SoC",
> -	.version = 3,
> +	.version = 5,
>   };
>
>   #define TELEMETRY_RESOURCE_PUNIT_SSRAM	0
> @@ -572,8 +574,8 @@ static int ipc_create_tco_device(void)
>   	res->end = res->start + SMI_EN_SIZE - 1;
>
>   	res = tco_res + TCO_RESOURCE_GCR_MEM;
> -	res->start = ipcdev.gcr_base;
> -	res->end = res->start + ipcdev.gcr_size - 1;
> +	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
> +	res->end = res->start + TCO_PMC_SIZE - 1;
>
>   	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
>   	if (ret) {
>

Re: [PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Yong, Jonathan]

On 05/18/2016 12:03, Guenter Roeck wrote:
> On 05/17/2016 08:26 PM, Yong, Jonathan wrote:
>> The TCO I/O base is 40h rather than the usual 30h, and the
>> re_reboot bit is at ACPIBASE+8.
>>
>
> Does this mean that the code never worked, or in other words that it
> was never tested, or are there now other chips which used the old
> definitions which no longer work after this patch has been applied ?

To my knowledge, it has never worked since 30h is claimed by ACPI GPE0_BLK.

With this patch, watchdog ioctl is working properly with 
WDIOC_SETTIMEOUT, WDIOC_GETTIMEOUT and WDIOC_GETTIMELEFT as expected.

Re: [PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Guenter Roeck]

On 05/17/2016 09:48 PM, Yong, Jonathan wrote:
> On 05/18/2016 12:03, Guenter Roeck wrote:
>> On 05/17/2016 08:26 PM, Yong, Jonathan wrote:
>>> The TCO I/O base is 40h rather than the usual 30h, and the
>>> re_reboot bit is at ACPIBASE+8.
>>>
>>
>> Does this mean that the code never worked, or in other words that it
>> was never tested, or are there now other chips which used the old
>> definitions which no longer work after this patch has been applied ?
>
> To my knowledge, it has never worked since 30h is claimed by ACPI GPE0_BLK.
>
Hmm.. Darren will have to take that one. I am not in a position to
determine if the new patch is correct or not, and/or if it causes some
other platform/architecture to fail. I'll Ack the other patch, and
would suggest for Darren to take it as well to keep the two patches together.

Guenter

> With this patch, watchdog ioctl is working properly with WDIOC_SETTIMEOUT, WDIOC_GETTIMEOUT and WDIOC_GETTIMELEFT as expected.
>

Re: [PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Darren Hart]

On Wed, May 18, 2016 at 07:37:00AM -0700, Guenter Roeck wrote:
> On 05/17/2016 09:48 PM, Yong, Jonathan wrote:
> >On 05/18/2016 12:03, Guenter Roeck wrote:
> >>On 05/17/2016 08:26 PM, Yong, Jonathan wrote:
> >>>The TCO I/O base is 40h rather than the usual 30h, and the
> >>>re_reboot bit is at ACPIBASE+8.
> >>>
> >>
> >>Does this mean that the code never worked, or in other words that it
> >>was never tested, or are there now other chips which used the old
> >>definitions which no longer work after this patch has been applied ?
> >
> >To my knowledge, it has never worked since 30h is claimed by ACPI GPE0_BLK.
> >
> Hmm.. Darren will have to take that one. I am not in a position to
> determine if the new patch is correct or not, and/or if it causes some
> other platform/architecture to fail. I'll Ack the other patch, and
> would suggest for Darren to take it as well to keep the two patches together.

I actually have the same exact question :-)

The history of this driver is for Apollo Lake, so the question is if there exist
v3 and v5 APL products, or if we only need to concern ourselves with v5 in the
upstream driver.

Especially given the "I don't know if this is the right way" in the cover
letter, I will need an Acked-by at the very least from Qipeng.

Jonathan, please remember to Cc ALL the maintainers and all the lists listed by
get_maintainers when sending patches:

$ scripts/get_maintainer.pl -f drivers/platform/x86/intel_pmc_ipc.c
Zha Qipeng <qipeng.zha@intel.com> (maintainer:INTEL PMC/P-Unit IPC DRIVER)
Darren Hart <dvhart@infradead.org> (maintainer:X86 PLATFORM DRIVERS)
platform-driver-x86@vger.kernel.org (open list:INTEL PMC/P-Unit IPC DRIVER)
linux-kernel@vger.kernel.org (open list)

In particular, ALL patches MUST cc  LKML.

> 
> Guenter
> 
> >With this patch, watchdog ioctl is working properly with WDIOC_SETTIMEOUT, WDIOC_GETTIMEOUT and WDIOC_GETTIMELEFT as expected.
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Yong, Jonathan]

On 05/25/2016 04:13, Darren Hart wrote:
>
> The history of this driver is for Apollo Lake, so the question is if there exist
> v3 and v5 APL products, or if we only need to concern ourselves with v5 in the
> upstream driver.
>
> Especially given the "I don't know if this is the right way" in the cover
> letter, I will need an Acked-by at the very least from Qipeng.
>

Hi Qipeng,

Do you know the history behind drivers/platform/x86/intel_pmc_ipc.c? 
Were the specs based on earlier prototypes?

Re: [PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Yong, Jonathan]

On 05/25/2016 15:55, Yong, Jonathan wrote:
> On 05/25/2016 04:13, Darren Hart wrote:
>>
>> The history of this driver is for Apollo Lake, so the question is if
>> there exist
>> v3 and v5 APL products, or if we only need to concern ourselves with
>> v5 in the
>> upstream driver.
>>
>> Especially given the "I don't know if this is the right way" in the cover
>> letter, I will need an Acked-by at the very least from Qipeng.
>>
>
> Hi Qipeng,
>
> Do you know the history behind drivers/platform/x86/intel_pmc_ipc.c?
> Were the specs based on earlier prototypes?
>

Qipeng, ping?

Re: [PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Darren Hart]

On Thu, Jun 02, 2016 at 04:28:03PM +0800, Yong, Jonathan wrote:
> On 05/25/2016 15:55, Yong, Jonathan wrote:
> > On 05/25/2016 04:13, Darren Hart wrote:
> > > 
> > > The history of this driver is for Apollo Lake, so the question is if
> > > there exist
> > > v3 and v5 APL products, or if we only need to concern ourselves with
> > > v5 in the
> > > upstream driver.
> > > 
> > > Especially given the "I don't know if this is the right way" in the cover
> > > letter, I will need an Acked-by at the very least from Qipeng.
> > > 
> > 
> > Hi Qipeng,
> > 
> > Do you know the history behind drivers/platform/x86/intel_pmc_ipc.c?
> > Were the specs based on earlier prototypes?
> > 
> 
> Qipeng, ping?
> 

Jonathan, can you follow-up with Qiping (possibly internally) and get back to
the list with a definitive response? Getting these into 4.7-rc would be good so
we don't have to push it back via stable, but we need to be sure it's the right
fix.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Yong, Jonathan]

On 06/08/2016 06:03, Darren Hart wrote:
>>>
>>> Hi Qipeng,
>>>
>>> Do you know the history behind drivers/platform/x86/intel_pmc_ipc.c?
>>> Were the specs based on earlier prototypes?
>>>
>>
>> Qipeng, ping?
>>
>
> Jonathan, can you follow-up with Qiping (possibly internally) and get back to
> the list with a definitive response? Getting these into 4.7-rc would be good so
> we don't have to push it back via stable, but we need to be sure it's the right
> fix.
>

I checked with Qipeng, the 0x30h offset was based on an earlier internal 
prototype, 0x40h should be OK for Apollo Lake.

[PATCH v2 0/2] Correct iTCO Watchdog for Apollo Lake

[Yong, Jonathan]

These patches fix the iTCO watchdog for Apollo Lake.
I changed the watchdog memory io to only use 4 bytes rather
the whole region, I'm not sure if that is the correct way.

The previous 0x30h offset in intel_pmc_ipc.c was for based
on the earlier BXT-M platform. Apollo Lake has it at 0x40h.

Let me know if the patches need changes.
Please CC me as I am not subscribed, thanks.

Changes since v1:
	* Watchdog NO_REBOOT bit off-by-one corrected.

Yong, Jonathan (2):
  watchdog: iTCO-wdt handle 5th variation for Apollo Lake
  x86: Fix Apollo Lake Watchdog address in PMC driver

 drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
 drivers/watchdog/iTCO_wdt.c          |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

-- 
2.7.3

[PATCH v2 1/2] watchdog: iTCO-wdt handle 5th variation for Apollo Lake

[Yong, Jonathan]

The Apollo Lake Watchdog has the no_reboot flag in the 4th bit.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/watchdog/iTCO_wdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 0acc6c5..54cab18 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -150,6 +150,7 @@ static inline u32 no_reboot_bit(void)
 	u32 enable_bit;
 
 	switch (iTCO_wdt_private.iTCO_version) {
+	case 5:
 	case 3:
 		enable_bit = 0x00000010;
 		break;
@@ -512,6 +513,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
 
 	/* Clear out the (probably old) status */
 	switch (iTCO_wdt_private.iTCO_version) {
+	case 5:
 	case 4:
 		outw(0x0008, TCO1_STS);	/* Clear the Time Out Status bit */
 		outw(0x0002, TCO2_STS);	/* Clear SECOND_TO_STS bit */
-- 
2.7.3

Re: [PATCH v2 1/2] watchdog: iTCO-wdt handle 5th variation for Apollo Lake

[Guenter Roeck]

On 06/16/2016 05:33 PM, Yong, Jonathan wrote:
> The Apollo Lake Watchdog has the no_reboot flag in the 4th bit.
>
> Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/iTCO_wdt.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 0acc6c5..54cab18 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -150,6 +150,7 @@ static inline u32 no_reboot_bit(void)
>   	u32 enable_bit;
>
>   	switch (iTCO_wdt_private.iTCO_version) {
> +	case 5:
>   	case 3:
>   		enable_bit = 0x00000010;
>   		break;
> @@ -512,6 +513,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
>
>   	/* Clear out the (probably old) status */
>   	switch (iTCO_wdt_private.iTCO_version) {
> +	case 5:
>   	case 4:
>   		outw(0x0008, TCO1_STS);	/* Clear the Time Out Status bit */
>   		outw(0x0002, TCO2_STS);	/* Clear SECOND_TO_STS bit */
>

[PATCH v2 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Yong, Jonathan]

The TCO I/O base is 40h rather than the usual 30h, and the re_reboot
bit is at ACPIBASE+8.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index 6f497e8..b86e1bc 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -85,7 +85,7 @@
  * platform device and to export resources for those functions.
  */
 #define TCO_DEVICE_NAME			"iTCO_wdt"
-#define SMI_EN_OFFSET			0x30
+#define SMI_EN_OFFSET			0x40
 #define SMI_EN_SIZE			4
 #define TCO_BASE_OFFSET			0x60
 #define TCO_REGS_SIZE			16
@@ -94,6 +94,8 @@
 #define TELEM_SSRAM_SIZE		240
 #define TELEM_PMC_SSRAM_OFFSET		0x1B00
 #define TELEM_PUNIT_SSRAM_OFFSET	0x1A00
+#define TCO_PMC_OFFSET			0x8
+#define TCO_PMC_SIZE			0x4
 
 static const int iTCO_version = 3;
 
@@ -502,7 +504,7 @@ static struct resource tco_res[] = {
 
 static struct itco_wdt_platform_data tco_info = {
 	.name = "Apollo Lake SoC",
-	.version = 3,
+	.version = 5,
 };
 
 #define TELEMETRY_RESOURCE_PUNIT_SSRAM	0
@@ -572,8 +574,8 @@ static int ipc_create_tco_device(void)
 	res->end = res->start + SMI_EN_SIZE - 1;
 
 	res = tco_res + TCO_RESOURCE_GCR_MEM;
-	res->start = ipcdev.gcr_base;
-	res->end = res->start + ipcdev.gcr_size - 1;
+	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
+	res->end = res->start + TCO_PMC_SIZE - 1;
 
 	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
 	if (ret) {
-- 
2.7.3

Re: [PATCH v2 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Guenter Roeck]

On 06/16/2016 05:33 PM, Yong, Jonathan wrote:
> The TCO I/O base is 40h rather than the usual 30h, and the re_reboot
> bit is at ACPIBASE+8.
>
> Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index 6f497e8..b86e1bc 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -85,7 +85,7 @@
>    * platform device and to export resources for those functions.
>    */
>   #define TCO_DEVICE_NAME			"iTCO_wdt"
> -#define SMI_EN_OFFSET			0x30
> +#define SMI_EN_OFFSET			0x40
>   #define SMI_EN_SIZE			4
>   #define TCO_BASE_OFFSET			0x60
>   #define TCO_REGS_SIZE			16
> @@ -94,6 +94,8 @@
>   #define TELEM_SSRAM_SIZE		240
>   #define TELEM_PMC_SSRAM_OFFSET		0x1B00
>   #define TELEM_PUNIT_SSRAM_OFFSET	0x1A00
> +#define TCO_PMC_OFFSET			0x8
> +#define TCO_PMC_SIZE			0x4
>
>   static const int iTCO_version = 3;
>
> @@ -502,7 +504,7 @@ static struct resource tco_res[] = {
>
>   static struct itco_wdt_platform_data tco_info = {
>   	.name = "Apollo Lake SoC",
> -	.version = 3,
> +	.version = 5,
>   };
>
>   #define TELEMETRY_RESOURCE_PUNIT_SSRAM	0
> @@ -572,8 +574,8 @@ static int ipc_create_tco_device(void)
>   	res->end = res->start + SMI_EN_SIZE - 1;
>
>   	res = tco_res + TCO_RESOURCE_GCR_MEM;
> -	res->start = ipcdev.gcr_base;
> -	res->end = res->start + ipcdev.gcr_size - 1;
> +	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
> +	res->end = res->start + TCO_PMC_SIZE - 1;
>
>   	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
>   	if (ret) {
>

Re: [PATCH v2 0/2] Correct iTCO Watchdog for Apollo Lake

[Wim Van Sebroeck]

Hi Jonathan,

> These patches fix the iTCO watchdog for Apollo Lake.
> I changed the watchdog memory io to only use 4 bytes rather
> the whole region, I'm not sure if that is the correct way.
> 
> The previous 0x30h offset in intel_pmc_ipc.c was for based
> on the earlier BXT-M platform. Apollo Lake has it at 0x40h.
> 
> Let me know if the patches need changes.
> Please CC me as I am not subscribed, thanks.
> 
> Changes since v1:
> 	* Watchdog NO_REBOOT bit off-by-one corrected.
> 
> Yong, Jonathan (2):
>   watchdog: iTCO-wdt handle 5th variation for Apollo Lake
>   x86: Fix Apollo Lake Watchdog address in PMC driver
> 
>  drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
>  drivers/watchdog/iTCO_wdt.c          |  2 ++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> -- 
> 2.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Both patches have been added to linux-watchdog-next.

Kind regards,
Wim.

Re: [PATCH v2 0/2] Correct iTCO Watchdog for Apollo Lake

[Yong, Jonathan]

On 07/18/2016 04:16, Wim Van Sebroeck wrote:
> 
> Both patches have been added to linux-watchdog-next.
> 

Thanks!

[PATCH v2 0/2] Correct iTCO Watchdog for Apollo Lake

[Yong, Jonathan]

These patches fix the iTCO watchdog for Apollo Lake.
I changed the watchdog memory io to only use 4 bytes rather
the whole region, I'm not sure if that is the correct way.

The previous 0x30h offset in intel_pmc_ipc.c was for based
on the earlier BXT-M platform. Apollo Lake has it at 0x40h.

Let me know if the patches need changes.
Please CC me as I am not subscribed, thanks.

* Resent, typo in linux-kernel email address

Changes since v1:
	* Watchdog NO_REBOOT bit off-by-one corrected.

Yong, Jonathan (2):
  watchdog: iTCO-wdt handle 5th variation for Apollo Lake
  x86: Fix Apollo Lake Watchdog address in PMC driver

 drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
 drivers/watchdog/iTCO_wdt.c          |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

-- 
2.7.3

[PATCH v2 1/2] watchdog: iTCO-wdt handle 5th variation for Apollo Lake

[Yong, Jonathan]

The Apollo Lake Watchdog has the no_reboot flag in the 4th bit.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/watchdog/iTCO_wdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 0acc6c5..54cab18 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -150,6 +150,7 @@ static inline u32 no_reboot_bit(void)
 	u32 enable_bit;
 
 	switch (iTCO_wdt_private.iTCO_version) {
+	case 5:
 	case 3:
 		enable_bit = 0x00000010;
 		break;
@@ -512,6 +513,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
 
 	/* Clear out the (probably old) status */
 	switch (iTCO_wdt_private.iTCO_version) {
+	case 5:
 	case 4:
 		outw(0x0008, TCO1_STS);	/* Clear the Time Out Status bit */
 		outw(0x0002, TCO2_STS);	/* Clear SECOND_TO_STS bit */
-- 
2.7.3

[PATCH v2 2/2] x86: Fix Apollo Lake Watchdog address in PMC driver

[Yong, Jonathan]

The TCO I/O base is 40h rather than the usual 30h, and the re_reboot
bit is at ACPIBASE+8.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index 6f497e8..b86e1bc 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -85,7 +85,7 @@
  * platform device and to export resources for those functions.
  */
 #define TCO_DEVICE_NAME			"iTCO_wdt"
-#define SMI_EN_OFFSET			0x30
+#define SMI_EN_OFFSET			0x40
 #define SMI_EN_SIZE			4
 #define TCO_BASE_OFFSET			0x60
 #define TCO_REGS_SIZE			16
@@ -94,6 +94,8 @@
 #define TELEM_SSRAM_SIZE		240
 #define TELEM_PMC_SSRAM_OFFSET		0x1B00
 #define TELEM_PUNIT_SSRAM_OFFSET	0x1A00
+#define TCO_PMC_OFFSET			0x8
+#define TCO_PMC_SIZE			0x4
 
 static const int iTCO_version = 3;
 
@@ -502,7 +504,7 @@ static struct resource tco_res[] = {
 
 static struct itco_wdt_platform_data tco_info = {
 	.name = "Apollo Lake SoC",
-	.version = 3,
+	.version = 5,
 };
 
 #define TELEMETRY_RESOURCE_PUNIT_SSRAM	0
@@ -572,8 +574,8 @@ static int ipc_create_tco_device(void)
 	res->end = res->start + SMI_EN_SIZE - 1;
 
 	res = tco_res + TCO_RESOURCE_GCR_MEM;
-	res->start = ipcdev.gcr_base;
-	res->end = res->start + ipcdev.gcr_size - 1;
+	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
+	res->end = res->start + TCO_PMC_SIZE - 1;
 
 	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
 	if (ret) {
-- 
2.7.3

Re: [PATCH v2 0/2] Correct iTCO Watchdog for Apollo Lake

[Yong, Jonathan]

On 06/17/2016 08:36, Yong, Jonathan wrote:
> These patches fix the iTCO watchdog for Apollo Lake.
> I changed the watchdog memory io to only use 4 bytes rather
> the whole region, I'm not sure if that is the correct way.
>
> The previous 0x30h offset in intel_pmc_ipc.c was for based
> on the earlier BXT-M platform. Apollo Lake has it at 0x40h.
>
> Let me know if the patches need changes.
> Please CC me as I am not subscribed, thanks.
>
> * Resent, typo in linux-kernel email address
>
> Changes since v1:
> 	* Watchdog NO_REBOOT bit off-by-one corrected.
>
> Yong, Jonathan (2):
>    watchdog: iTCO-wdt handle 5th variation for Apollo Lake
>    x86: Fix Apollo Lake Watchdog address in PMC driver
>
>   drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
>   drivers/watchdog/iTCO_wdt.c          |  2 ++
>   2 files changed, 8 insertions(+), 4 deletions(-)
>

Ping.

Re: [PATCH v2 0/2] Correct iTCO Watchdog for Apollo Lake

[Guenter Roeck]

On 06/21/2016 09:53 PM, Yong, Jonathan wrote:
> On 06/17/2016 08:36, Yong, Jonathan wrote:
>> These patches fix the iTCO watchdog for Apollo Lake.
>> I changed the watchdog memory io to only use 4 bytes rather
>> the whole region, I'm not sure if that is the correct way.
>>
>> The previous 0x30h offset in intel_pmc_ipc.c was for based
>> on the earlier BXT-M platform. Apollo Lake has it at 0x40h.
>>
>> Let me know if the patches need changes.
>> Please CC me as I am not subscribed, thanks.
>>
>> * Resent, typo in linux-kernel email address
>>
>> Changes since v1:
>>     * Watchdog NO_REBOOT bit off-by-one corrected.
>>
>> Yong, Jonathan (2):
>>    watchdog: iTCO-wdt handle 5th variation for Apollo Lake
>>    x86: Fix Apollo Lake Watchdog address in PMC driver
>>
>>   drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
>>   drivers/watchdog/iTCO_wdt.c          |  2 ++
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>
> Ping.
>
>
Waiting for an Ack from Darren.

Guenter

Re: [PATCH v2 0/2] Correct iTCO Watchdog for Apollo Lake

[Darren Hart]

On Tue, Jun 21, 2016 at 11:01:01PM -0700, Guenter Roeck wrote:
> On 06/21/2016 09:53 PM, Yong, Jonathan wrote:
> > On 06/17/2016 08:36, Yong, Jonathan wrote:
> > > These patches fix the iTCO watchdog for Apollo Lake.
> > > I changed the watchdog memory io to only use 4 bytes rather
> > > the whole region, I'm not sure if that is the correct way.
> > > 
> > > The previous 0x30h offset in intel_pmc_ipc.c was for based
> > > on the earlier BXT-M platform. Apollo Lake has it at 0x40h.
> > > 
> > > Let me know if the patches need changes.
> > > Please CC me as I am not subscribed, thanks.
> > > 
> > > * Resent, typo in linux-kernel email address
> > > 
> > > Changes since v1:
> > >     * Watchdog NO_REBOOT bit off-by-one corrected.
> > > 
> > > Yong, Jonathan (2):
> > >    watchdog: iTCO-wdt handle 5th variation for Apollo Lake
> > >    x86: Fix Apollo Lake Watchdog address in PMC driver
> > > 
> > >   drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
> > >   drivers/watchdog/iTCO_wdt.c          |  2 ++
> > >   2 files changed, 8 insertions(+), 4 deletions(-)
> > > 
> > 
> > Ping.
> > 
> > 
> Waiting for an Ack from Darren.

Jonathan verified that the change will not break existing external platforms.
I'm happy with it from that perspective.

Guenter, will you take both patches together?

Reviewed-by: Darren Hart <dvhart@linux.intel.com>


-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH v2 0/2] Correct iTCO Watchdog for Apollo Lake

[Guenter Roeck]

On 06/22/2016 10:02 PM, Darren Hart wrote:
> On Tue, Jun 21, 2016 at 11:01:01PM -0700, Guenter Roeck wrote:
>> On 06/21/2016 09:53 PM, Yong, Jonathan wrote:
>>> On 06/17/2016 08:36, Yong, Jonathan wrote:
>>>> These patches fix the iTCO watchdog for Apollo Lake.
>>>> I changed the watchdog memory io to only use 4 bytes rather
>>>> the whole region, I'm not sure if that is the correct way.
>>>>
>>>> The previous 0x30h offset in intel_pmc_ipc.c was for based
>>>> on the earlier BXT-M platform. Apollo Lake has it at 0x40h.
>>>>
>>>> Let me know if the patches need changes.
>>>> Please CC me as I am not subscribed, thanks.
>>>>
>>>> * Resent, typo in linux-kernel email address
>>>>
>>>> Changes since v1:
>>>>      * Watchdog NO_REBOOT bit off-by-one corrected.
>>>>
>>>> Yong, Jonathan (2):
>>>>     watchdog: iTCO-wdt handle 5th variation for Apollo Lake
>>>>     x86: Fix Apollo Lake Watchdog address in PMC driver
>>>>
>>>>    drivers/platform/x86/intel_pmc_ipc.c | 10 ++++++----
>>>>    drivers/watchdog/iTCO_wdt.c          |  2 ++
>>>>    2 files changed, 8 insertions(+), 4 deletions(-)
>>>>
>>>
>>> Ping.
>>>
>>>
>> Waiting for an Ack from Darren.
>
> Jonathan verified that the change will not break existing external platforms.
> I'm happy with it from that perspective.
>
> Guenter, will you take both patches together?
>

Yes, I'll add them to my queue of patches to send to Wim.

Thanks,
Guenter

> Reviewed-by: Darren Hart <dvhart@linux.intel.com>
>
>