2015-08-06 12:46:34

by Matt Fleming

[permalink] [raw]
Subject: [PATCH v4 0/3] iTCO_wdt: Add support for Intel Sunrisepoint

From: Matt Fleming <[email protected]>

Starting with Intel Sunrisepoint (Skylake PCH) the TCO watchdog device
is now on the SMBUS, whereas for previous ICH/PCH it was on the LPC bus.

Because iTCO_wdt devices may now appear on either the LPC bus or the
SMBUS we need to abstract the bus information into an agnostic structure
instead of the existing 'lpc_ich_info' which tightly integrates both the
lpc_ich and iTCO_wdt drivers.

The first patch introduces a platform data structure to handle this and
shuffles the existing code around. The other patches add the
device-specific information to the i2c-i801 and iTCO_wdt drivers.

Patches based against v4.2-rc4, if there's some other tree I should base
this on, please let me know.

Comments welcome!

Changes in v4:

- Added Darren's ACK for PATCH 1
- Explicitly list versions in iTCO_wdt_probe()
- Delete superfluous 'ret' variable in iTCO_wdt_unset_NO_REBOOT_bit()

Changes in v3:

- Added Ack/Review tags to patches

Changes in v2:

- Use lowercase for all new files and data structures
- Allocate platform data with devm_kzalloc()
- Move Kconfig changes to final patch and use 'select', not 'depends'
- Swap strcpy() for strlcpy()
- Fixup use of lpc_ich_info in intel_pmc_ipc which was missed in v1
- Don't use bitops in i2c-i801
- Remove superfluous NULL checks in i2c-i801
- Explicitly list reboot bit versions in no_reboot_bit()
- Use switch/case construst instead of crazy if else
- Fold original fixups from PATCH 4 and 5 into PATCH 1 and 2

Matt Fleming (2):
iTCO_wdt: Expose watchdog properties using platform data
iTCO_wdt: Add support for TCO on Intel Sunrisepoint

Mika Westerberg (1):
i2c: i801: Create iTCO device on newer Intel PCHs

drivers/i2c/busses/i2c-i801.c | 120 ++++++++++++++++++++++++++++++++-
drivers/mfd/lpc_ich.c | 32 ++++++++-
drivers/platform/x86/intel_pmc_ipc.c | 9 ++-
drivers/watchdog/Kconfig | 3 +-
drivers/watchdog/iTCO_wdt.c | 82 +++++++++++++---------
include/linux/mfd/lpc_ich.h | 6 --
include/linux/platform_data/itco_wdt.h | 19 ++++++
7 files changed, 222 insertions(+), 49 deletions(-)
create mode 100644 include/linux/platform_data/itco_wdt.h

--
2.1.0


2015-08-06 12:47:38

by Matt Fleming

[permalink] [raw]
Subject: [PATCH v4 1/3] iTCO_wdt: Expose watchdog properties using platform data

From: Matt Fleming <[email protected]>

Intel Sunrisepoint (Skylake PCH) has the iTCO watchdog accessible across
the SMBus, unlike previous generations of PCH/ICH where it was on the
LPC bus. Because it's on the SMBus, it doesn't make sense to pass around
a 'struct lpc_ich_info', and leaking the type of bus into the iTCO
watchdog driver is kind of backwards anyway.

This change introduces a new 'struct itco_wdt_platform_data' for use
inside the iTCO watchdog driver and by the upcoming Intel Sunrisepoint
code, which neatly avoids having to include lpc_ich headers in the i801
i2c driver.

This change is overdue because lpc_ich_info has already found its way
into other TCO watchdog users, notably the intel_pmc_ipc driver where
the watchdog actually isn't on the LPC bus as far as I can see.

A simple translation layer is provided for converting from the existing
'struct lpc_ich_info' inside the lpc_ich mfd driver.

Cc: Peter Tyser <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Acked-by: Lee Jones <[email protected]>
Cc: Wim Van Sebroeck <[email protected]>
Cc: Zha Qipeng <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Jean Delvare <[email protected]>
Acked-by: Darren Hart <[email protected]> [drivers/x86 refactoring]
Signed-off-by: Matt Fleming <[email protected]>
---
v4:
- Added Darren's ACK

v3:
- Added Lee's ACK

v2:
- Use lowercase "itco" in all new files and data structures.
- Use devm_kzalloc() to allocate platform data, fixing a memory leak
and saving us from having to free the allocation in error paths
- Move stray Kconfig hunk to PATCH 3 that accidentally snuck into v1
- Swap strcpy() for the safer strlcpy()
- Fix lpc_ich_info user in the intel_pmc_ipc driver that was merged for
v4.2.

drivers/mfd/lpc_ich.c | 32 +++++++++++++++++++++++++++++---
drivers/platform/x86/intel_pmc_ipc.c | 9 ++++-----
drivers/watchdog/iTCO_wdt.c | 11 +++++------
include/linux/mfd/lpc_ich.h | 6 ------
include/linux/platform_data/itco_wdt.h | 19 +++++++++++++++++++
5 files changed, 57 insertions(+), 20 deletions(-)
create mode 100644 include/linux/platform_data/itco_wdt.h

diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index 8de34398abc0..c5a9a08b5dfb 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -66,6 +66,7 @@
#include <linux/pci.h>
#include <linux/mfd/core.h>
#include <linux/mfd/lpc_ich.h>
+#include <linux/platform_data/itco_wdt.h>

#define ACPIBASE 0x40
#define ACPIBASE_GPE_OFF 0x28
@@ -835,9 +836,31 @@ static void lpc_ich_enable_pmc_space(struct pci_dev *dev)
priv->actrl_pbase_save = reg_save;
}

-static void lpc_ich_finalize_cell(struct pci_dev *dev, struct mfd_cell *cell)
+static int lpc_ich_finalize_wdt_cell(struct pci_dev *dev)
{
+ struct itco_wdt_platform_data *pdata;
struct lpc_ich_priv *priv = pci_get_drvdata(dev);
+ struct lpc_ich_info *info;
+ struct mfd_cell *cell = &lpc_ich_cells[LPC_WDT];
+
+ pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ info = &lpc_chipset_info[priv->chipset];
+
+ pdata->version = info->iTCO_version;
+ strlcpy(pdata->name, info->name, sizeof(pdata->name));
+
+ cell->platform_data = pdata;
+ cell->pdata_size = sizeof(*pdata);
+ return 0;
+}
+
+static void lpc_ich_finalize_gpio_cell(struct pci_dev *dev)
+{
+ struct lpc_ich_priv *priv = pci_get_drvdata(dev);
+ struct mfd_cell *cell = &lpc_ich_cells[LPC_GPIO];

cell->platform_data = &lpc_chipset_info[priv->chipset];
cell->pdata_size = sizeof(struct lpc_ich_info);
@@ -933,7 +956,7 @@ gpe0_done:
lpc_chipset_info[priv->chipset].use_gpio = ret;
lpc_ich_enable_gpio_space(dev);

- lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_GPIO]);
+ lpc_ich_finalize_gpio_cell(dev);
ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
&lpc_ich_cells[LPC_GPIO], 1, NULL, 0, NULL);

@@ -1007,7 +1030,10 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
res->end = base_addr + ACPIBASE_PMC_END;
}

- lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_WDT]);
+ ret = lpc_ich_finalize_wdt_cell(dev);
+ if (ret)
+ goto wdt_done;
+
ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
&lpc_ich_cells[LPC_WDT], 1, NULL, 0, NULL);

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index 105cfffe82c6..28b2a12bb26d 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -33,7 +33,7 @@
#include <linux/suspend.h>
#include <linux/acpi.h>
#include <asm/intel_pmc_ipc.h>
-#include <linux/mfd/lpc_ich.h>
+#include <linux/platform_data/itco_wdt.h>

/*
* IPC registers
@@ -473,9 +473,9 @@ static struct resource tco_res[] = {
},
};

-static struct lpc_ich_info tco_info = {
+static struct itco_wdt_platform_data tco_info = {
.name = "Apollo Lake SoC",
- .iTCO_version = 3,
+ .version = 3,
};

static int ipc_create_punit_device(void)
@@ -552,8 +552,7 @@ static int ipc_create_tco_device(void)
goto err;
}

- ret = platform_device_add_data(pdev, &tco_info,
- sizeof(struct lpc_ich_info));
+ ret = platform_device_add_data(pdev, &tco_info, sizeof(tco_info));
if (ret) {
dev_err(ipcdev.dev, "Failed to add tco platform data\n");
goto err;
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 3c3fd417ddeb..a94401b2deca 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -66,8 +66,7 @@
#include <linux/spinlock.h> /* For spin_lock/spin_unlock/... */
#include <linux/uaccess.h> /* For copy_to_user/put_user/... */
#include <linux/io.h> /* For inb/outb/... */
-#include <linux/mfd/core.h>
-#include <linux/mfd/lpc_ich.h>
+#include <linux/platform_data/itco_wdt.h>

#include "iTCO_vendor.h"

@@ -418,9 +417,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
{
int ret = -ENODEV;
unsigned long val32;
- struct lpc_ich_info *ich_info = dev_get_platdata(&dev->dev);
+ struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);

- if (!ich_info)
+ if (!pdata)
goto out;

spin_lock_init(&iTCO_wdt_private.io_lock);
@@ -435,7 +434,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
if (!iTCO_wdt_private.smi_res)
goto out;

- iTCO_wdt_private.iTCO_version = ich_info->iTCO_version;
+ iTCO_wdt_private.iTCO_version = pdata->version;
iTCO_wdt_private.dev = dev;
iTCO_wdt_private.pdev = to_pci_dev(dev->dev.parent);

@@ -501,7 +500,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
}

pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
- ich_info->name, ich_info->iTCO_version, (u64)TCOBASE);
+ pdata->name, pdata->version, (u64)TCOBASE);

/* Clear out the (probably old) status */
if (iTCO_wdt_private.iTCO_version == 3) {
diff --git a/include/linux/mfd/lpc_ich.h b/include/linux/mfd/lpc_ich.h
index 8feac782fa83..2b300b44f994 100644
--- a/include/linux/mfd/lpc_ich.h
+++ b/include/linux/mfd/lpc_ich.h
@@ -20,12 +20,6 @@
#ifndef LPC_ICH_H
#define LPC_ICH_H

-/* Watchdog resources */
-#define ICH_RES_IO_TCO 0
-#define ICH_RES_IO_SMI 1
-#define ICH_RES_MEM_OFF 2
-#define ICH_RES_MEM_GCS_PMC 0
-
/* GPIO resources */
#define ICH_RES_GPIO 0
#define ICH_RES_GPE0 1
diff --git a/include/linux/platform_data/itco_wdt.h b/include/linux/platform_data/itco_wdt.h
new file mode 100644
index 000000000000..f16542c77ff7
--- /dev/null
+++ b/include/linux/platform_data/itco_wdt.h
@@ -0,0 +1,19 @@
+/*
+ * Platform data for the Intel TCO Watchdog
+ */
+
+#ifndef _ITCO_WDT_H_
+#define _ITCO_WDT_H_
+
+/* Watchdog resources */
+#define ICH_RES_IO_TCO 0
+#define ICH_RES_IO_SMI 1
+#define ICH_RES_MEM_OFF 2
+#define ICH_RES_MEM_GCS_PMC 0
+
+struct itco_wdt_platform_data {
+ char name[32];
+ unsigned int version;
+};
+
+#endif /* _ITCO_WDT_H_ */
--
2.1.0

2015-08-06 12:46:55

by Matt Fleming

[permalink] [raw]
Subject: [PATCH v4 2/3] i2c: i801: Create iTCO device on newer Intel PCHs

From: Mika Westerberg <[email protected]>

Starting from Intel Sunrisepoint (Skylake PCH) the iTCO watchdog resources
have been moved to reside under the i801 SMBus host controller whereas
previously they were under the LPC device.

In order to support the iTCO watchdog on newer PCHs we need to create the
platform device here in the SMBus driver and pass all known resources using
platform data.

Cc: Jean Delvare <[email protected]>
Cc: Wolfram Sang <[email protected]>
Cc: <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
Signed-off-by: Mika Westerberg <[email protected]>
Signed-off-by: Matt Fleming <[email protected]>
---

v4:
- No changes

v3:
- Added Guenter's Review tag

v2:
- Don't use bitops but same scheme used already
- Delete superfluous NULL-checks for platform_device_unregister()

drivers/i2c/busses/i2c-i801.c | 120 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 119 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5ecbb3fdc27e..eaef9bc9d88c 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -88,12 +88,13 @@
#include <linux/slab.h>
#include <linux/wait.h>
#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/itco_wdt.h>

#if (defined CONFIG_I2C_MUX_GPIO || defined CONFIG_I2C_MUX_GPIO_MODULE) && \
defined CONFIG_DMI
#include <linux/gpio.h>
#include <linux/i2c-mux-gpio.h>
-#include <linux/platform_device.h>
#endif

/* I801 SMBus address offsets */
@@ -113,6 +114,16 @@
#define SMBPCICTL 0x004
#define SMBPCISTS 0x006
#define SMBHSTCFG 0x040
+#define TCOBASE 0x050
+#define TCOCTL 0x054
+
+#define ACPIBASE 0x040
+#define ACPIBASE_SMI_OFF 0x030
+#define ACPICTRL 0x044
+#define ACPICTRL_EN 0x080
+
+#define SBREG_BAR 0x10
+#define SBREG_SMBCTRL 0xc6000c

/* Host status bits for SMBPCISTS */
#define SMBPCISTS_INTS 0x08
@@ -125,6 +136,9 @@
#define SMBHSTCFG_SMB_SMI_EN 2
#define SMBHSTCFG_I2C_EN 4

+/* TCO configuration bits for TCOCTL */
+#define TCOCTL_EN 0x0100
+
/* Auxiliary control register bits, ICH4+ only */
#define SMBAUXCTL_CRC 1
#define SMBAUXCTL_E32B 2
@@ -221,6 +235,7 @@ struct i801_priv {
const struct i801_mux_config *mux_drvdata;
struct platform_device *mux_pdev;
#endif
+ struct platform_device *tco_pdev;
};

#define FEATURE_SMBUS_PEC (1 << 0)
@@ -230,6 +245,7 @@ struct i801_priv {
#define FEATURE_IRQ (1 << 4)
/* Not really a feature, but it's convenient to handle it as such */
#define FEATURE_IDF (1 << 15)
+#define FEATURE_TCO (1 << 16)

static const char *i801_feature_names[] = {
"SMBus PEC",
@@ -1132,6 +1148,95 @@ static inline unsigned int i801_get_adapter_class(struct i801_priv *priv)
}
#endif

+static const struct itco_wdt_platform_data tco_platform_data = {
+ .name = "Intel PCH",
+ .version = 4,
+};
+
+static DEFINE_SPINLOCK(p2sb_spinlock);
+
+static void i801_add_tco(struct i801_priv *priv)
+{
+ struct pci_dev *pci_dev = priv->pci_dev;
+ struct resource tco_res[3], *res;
+ struct platform_device *pdev;
+ unsigned int devfn;
+ u32 tco_base, tco_ctl;
+ u32 base_addr, ctrl_val;
+ u64 base64_addr;
+
+ if (!(priv->features & FEATURE_TCO))
+ return;
+
+ pci_read_config_dword(pci_dev, TCOBASE, &tco_base);
+ pci_read_config_dword(pci_dev, TCOCTL, &tco_ctl);
+ if (!(tco_ctl & TCOCTL_EN))
+ return;
+
+ memset(tco_res, 0, sizeof(tco_res));
+
+ res = &tco_res[ICH_RES_IO_TCO];
+ res->start = tco_base & ~1;
+ res->end = res->start + 32 - 1;
+ res->flags = IORESOURCE_IO;
+
+ /*
+ * Power Management registers.
+ */
+ devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 2);
+ pci_bus_read_config_dword(pci_dev->bus, devfn, ACPIBASE, &base_addr);
+
+ res = &tco_res[ICH_RES_IO_SMI];
+ res->start = (base_addr & ~1) + ACPIBASE_SMI_OFF;
+ res->end = res->start + 3;
+ res->flags = IORESOURCE_IO;
+
+ /*
+ * Enable the ACPI I/O space.
+ */
+ pci_bus_read_config_dword(pci_dev->bus, devfn, ACPICTRL, &ctrl_val);
+ ctrl_val |= ACPICTRL_EN;
+ pci_bus_write_config_dword(pci_dev->bus, devfn, ACPICTRL, ctrl_val);
+
+ /*
+ * We must access the NO_REBOOT bit over the Primary to Sideband
+ * bridge (P2SB). The BIOS prevents the P2SB device from being
+ * enumerated by the PCI subsystem, so we need to unhide/hide it
+ * to lookup the P2SB BAR.
+ */
+ spin_lock(&p2sb_spinlock);
+
+ devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 1);
+
+ /* Unhide the P2SB device */
+ pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, 0x0);
+
+ pci_bus_read_config_dword(pci_dev->bus, devfn, SBREG_BAR, &base_addr);
+ base64_addr = base_addr & 0xfffffff0;
+
+ pci_bus_read_config_dword(pci_dev->bus, devfn, SBREG_BAR + 0x4, &base_addr);
+ base64_addr |= (u64)base_addr << 32;
+
+ /* Hide the P2SB device */
+ pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, 0x1);
+ spin_unlock(&p2sb_spinlock);
+
+ res = &tco_res[ICH_RES_MEM_OFF];
+ res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL;
+ res->end = res->start + 3;
+ res->flags = IORESOURCE_MEM;
+
+ pdev = platform_device_register_resndata(&pci_dev->dev, "iTCO_wdt", -1,
+ tco_res, 3, &tco_platform_data,
+ sizeof(tco_platform_data));
+ if (IS_ERR(pdev)) {
+ dev_warn(&pci_dev->dev, "failed to create iTCO device\n");
+ return;
+ }
+
+ priv->tco_pdev = pdev;
+}
+
static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
{
unsigned char temp;
@@ -1149,6 +1254,15 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)

priv->pci_dev = dev;
switch (dev->device) {
+ case PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS:
+ case PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS:
+ priv->features |= FEATURE_I2C_BLOCK_READ;
+ priv->features |= FEATURE_IRQ;
+ priv->features |= FEATURE_SMBUS_PEC;
+ priv->features |= FEATURE_BLOCK_BUFFER;
+ priv->features |= FEATURE_TCO;
+ break;
+
case PCI_DEVICE_ID_INTEL_PATSBURG_SMBUS_IDF0:
case PCI_DEVICE_ID_INTEL_PATSBURG_SMBUS_IDF1:
case PCI_DEVICE_ID_INTEL_PATSBURG_SMBUS_IDF2:
@@ -1265,6 +1379,8 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
dev_info(&dev->dev, "SMBus using %s\n",
priv->features & FEATURE_IRQ ? "PCI interrupt" : "polling");

+ i801_add_tco(priv);
+
/* set up the sysfs linkage to our parent device */
priv->adapter.dev.parent = &dev->dev;

@@ -1296,6 +1412,8 @@ static void i801_remove(struct pci_dev *dev)
i2c_del_adapter(&priv->adapter);
pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);

+ platform_device_unregister(priv->tco_pdev);
+
/*
* do not call pci_disable_device(dev) since it can cause hard hangs on
* some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)
--
2.1.0

2015-08-06 12:46:40

by Matt Fleming

[permalink] [raw]
Subject: [PATCH v4 3/3] iTCO_wdt: Add support for TCO on Intel Sunrisepoint

From: Matt Fleming <[email protected]>

The revision of the watchdog hardware in Sunrisepoint necessitates a new
"version" inside the TCO watchdog driver because some of the register
layouts have changed.

Also update the Kconfig entry to select both the LPC and SMBus drivers
since the TCO device is on the SMBus in Sunrisepoint.

Cc: Wim Van Sebroeck <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
Cc: Jean Delvare <[email protected]>
Signed-off-by: Matt Fleming <[email protected]>
---

v4:
- Explicitly list versions in iTCO_wdt_probe()
- Delete the now superfluous 'ret' variable in iTCO_wdt_unset_NO_REBOOT_bit()

v3:
- Added Guenter's Review tag

v2:
- Explicitly list TCO versions and their "no reboot" bits in the switch
statement in no_reboot_bit()
- Use a switch/case statement when clearing status bits instead of
convoluted if/else branching
- Select both of the bus drivers instead of using depends

drivers/watchdog/Kconfig | 3 +-
drivers/watchdog/iTCO_wdt.c | 71 ++++++++++++++++++++++++++++-----------------
2 files changed, 46 insertions(+), 28 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 241fafde42cb..55c4b5b0a317 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -797,7 +797,8 @@ config ITCO_WDT
tristate "Intel TCO Timer/Watchdog"
depends on (X86 || IA64) && PCI
select WATCHDOG_CORE
- select LPC_ICH
+ select LPC_ICH if !EXPERT
+ select I2C_I801 if !EXPERT
---help---
Hardware driver for the intel TCO timer based watchdog devices.
These drivers are included in the Intel 82801 I/O Controller
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index a94401b2deca..0acc6c5f729d 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -145,59 +145,67 @@ static inline unsigned int ticks_to_seconds(int ticks)
return iTCO_wdt_private.iTCO_version == 3 ? ticks : (ticks * 6) / 10;
}

+static inline u32 no_reboot_bit(void)
+{
+ u32 enable_bit;
+
+ switch (iTCO_wdt_private.iTCO_version) {
+ case 3:
+ enable_bit = 0x00000010;
+ break;
+ case 2:
+ enable_bit = 0x00000020;
+ break;
+ case 4:
+ case 1:
+ default:
+ enable_bit = 0x00000002;
+ break;
+ }
+
+ return enable_bit;
+}
+
static void iTCO_wdt_set_NO_REBOOT_bit(void)
{
u32 val32;

/* Set the NO_REBOOT bit: this disables reboots */
- if (iTCO_wdt_private.iTCO_version == 3) {
- val32 = readl(iTCO_wdt_private.gcs_pmc);
- val32 |= 0x00000010;
- writel(val32, iTCO_wdt_private.gcs_pmc);
- } else if (iTCO_wdt_private.iTCO_version == 2) {
+ if (iTCO_wdt_private.iTCO_version >= 2) {
val32 = readl(iTCO_wdt_private.gcs_pmc);
- val32 |= 0x00000020;
+ val32 |= no_reboot_bit();
writel(val32, iTCO_wdt_private.gcs_pmc);
} else if (iTCO_wdt_private.iTCO_version == 1) {
pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
- val32 |= 0x00000002;
+ val32 |= no_reboot_bit();
pci_write_config_dword(iTCO_wdt_private.pdev, 0xd4, val32);
}
}

static int iTCO_wdt_unset_NO_REBOOT_bit(void)
{
- int ret = 0;
- u32 val32;
+ u32 enable_bit = no_reboot_bit();
+ u32 val32 = 0;

/* Unset the NO_REBOOT bit: this enables reboots */
- if (iTCO_wdt_private.iTCO_version == 3) {
- val32 = readl(iTCO_wdt_private.gcs_pmc);
- val32 &= 0xffffffef;
- writel(val32, iTCO_wdt_private.gcs_pmc);
-
- val32 = readl(iTCO_wdt_private.gcs_pmc);
- if (val32 & 0x00000010)
- ret = -EIO;
- } else if (iTCO_wdt_private.iTCO_version == 2) {
+ if (iTCO_wdt_private.iTCO_version >= 2) {
val32 = readl(iTCO_wdt_private.gcs_pmc);
- val32 &= 0xffffffdf;
+ val32 &= ~enable_bit;
writel(val32, iTCO_wdt_private.gcs_pmc);

val32 = readl(iTCO_wdt_private.gcs_pmc);
- if (val32 & 0x00000020)
- ret = -EIO;
} else if (iTCO_wdt_private.iTCO_version == 1) {
pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
- val32 &= 0xfffffffd;
+ val32 &= ~enable_bit;
pci_write_config_dword(iTCO_wdt_private.pdev, 0xd4, val32);

pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
- if (val32 & 0x00000002)
- ret = -EIO;
}

- return ret; /* returns: 0 = OK, -EIO = Error */
+ if (val32 & enable_bit)
+ return -EIO;
+
+ return 0;
}

static int iTCO_wdt_start(struct watchdog_device *wd_dev)
@@ -503,12 +511,21 @@ static int iTCO_wdt_probe(struct platform_device *dev)
pdata->name, pdata->version, (u64)TCOBASE);

/* Clear out the (probably old) status */
- if (iTCO_wdt_private.iTCO_version == 3) {
+ switch (iTCO_wdt_private.iTCO_version) {
+ case 4:
+ outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
+ outw(0x0002, TCO2_STS); /* Clear SECOND_TO_STS bit */
+ break;
+ case 3:
outl(0x20008, TCO1_STS);
- } else {
+ break;
+ case 2:
+ case 1:
+ default:
outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
outw(0x0002, TCO2_STS); /* Clear SECOND_TO_STS bit */
outw(0x0004, TCO2_STS); /* Clear BOOT_STS bit */
+ break;
}

iTCO_wdt_watchdog_dev.bootstatus = 0;
--
2.1.0

2015-08-07 15:30:49

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] iTCO_wdt: Expose watchdog properties using platform data

On 08/06/2015 05:46 AM, Matt Fleming wrote:
> From: Matt Fleming <[email protected]>
>
> Intel Sunrisepoint (Skylake PCH) has the iTCO watchdog accessible across
> the SMBus, unlike previous generations of PCH/ICH where it was on the
> LPC bus. Because it's on the SMBus, it doesn't make sense to pass around
> a 'struct lpc_ich_info', and leaking the type of bus into the iTCO
> watchdog driver is kind of backwards anyway.
>
> This change introduces a new 'struct itco_wdt_platform_data' for use
> inside the iTCO watchdog driver and by the upcoming Intel Sunrisepoint
> code, which neatly avoids having to include lpc_ich headers in the i801
> i2c driver.
>
> This change is overdue because lpc_ich_info has already found its way
> into other TCO watchdog users, notably the intel_pmc_ipc driver where
> the watchdog actually isn't on the LPC bus as far as I can see.
>
> A simple translation layer is provided for converting from the existing
> 'struct lpc_ich_info' inside the lpc_ich mfd driver.
>
> Cc: Peter Tyser <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Acked-by: Lee Jones <[email protected]>
> Cc: Wim Van Sebroeck <[email protected]>
> Cc: Zha Qipeng <[email protected]>
> Cc: Guenter Roeck <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

> Cc: Jean Delvare <[email protected]>
> Acked-by: Darren Hart <[email protected]> [drivers/x86 refactoring]
> Signed-off-by: Matt Fleming <[email protected]>
> ---
> v4:
> - Added Darren's ACK
>
> v3:
> - Added Lee's ACK
>
> v2:
> - Use lowercase "itco" in all new files and data structures.
> - Use devm_kzalloc() to allocate platform data, fixing a memory leak
> and saving us from having to free the allocation in error paths
> - Move stray Kconfig hunk to PATCH 3 that accidentally snuck into v1
> - Swap strcpy() for the safer strlcpy()
> - Fix lpc_ich_info user in the intel_pmc_ipc driver that was merged for
> v4.2.
>
> drivers/mfd/lpc_ich.c | 32 +++++++++++++++++++++++++++++---
> drivers/platform/x86/intel_pmc_ipc.c | 9 ++++-----
> drivers/watchdog/iTCO_wdt.c | 11 +++++------
> include/linux/mfd/lpc_ich.h | 6 ------
> include/linux/platform_data/itco_wdt.h | 19 +++++++++++++++++++
> 5 files changed, 57 insertions(+), 20 deletions(-)
> create mode 100644 include/linux/platform_data/itco_wdt.h
>
> diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> index 8de34398abc0..c5a9a08b5dfb 100644
> --- a/drivers/mfd/lpc_ich.c
> +++ b/drivers/mfd/lpc_ich.c
> @@ -66,6 +66,7 @@
> #include <linux/pci.h>
> #include <linux/mfd/core.h>
> #include <linux/mfd/lpc_ich.h>
> +#include <linux/platform_data/itco_wdt.h>
>
> #define ACPIBASE 0x40
> #define ACPIBASE_GPE_OFF 0x28
> @@ -835,9 +836,31 @@ static void lpc_ich_enable_pmc_space(struct pci_dev *dev)
> priv->actrl_pbase_save = reg_save;
> }
>
> -static void lpc_ich_finalize_cell(struct pci_dev *dev, struct mfd_cell *cell)
> +static int lpc_ich_finalize_wdt_cell(struct pci_dev *dev)
> {
> + struct itco_wdt_platform_data *pdata;
> struct lpc_ich_priv *priv = pci_get_drvdata(dev);
> + struct lpc_ich_info *info;
> + struct mfd_cell *cell = &lpc_ich_cells[LPC_WDT];
> +
> + pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + info = &lpc_chipset_info[priv->chipset];
> +
> + pdata->version = info->iTCO_version;
> + strlcpy(pdata->name, info->name, sizeof(pdata->name));
> +
> + cell->platform_data = pdata;
> + cell->pdata_size = sizeof(*pdata);
> + return 0;
> +}
> +
> +static void lpc_ich_finalize_gpio_cell(struct pci_dev *dev)
> +{
> + struct lpc_ich_priv *priv = pci_get_drvdata(dev);
> + struct mfd_cell *cell = &lpc_ich_cells[LPC_GPIO];
>
> cell->platform_data = &lpc_chipset_info[priv->chipset];
> cell->pdata_size = sizeof(struct lpc_ich_info);
> @@ -933,7 +956,7 @@ gpe0_done:
> lpc_chipset_info[priv->chipset].use_gpio = ret;
> lpc_ich_enable_gpio_space(dev);
>
> - lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_GPIO]);
> + lpc_ich_finalize_gpio_cell(dev);
> ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> &lpc_ich_cells[LPC_GPIO], 1, NULL, 0, NULL);
>
> @@ -1007,7 +1030,10 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
> res->end = base_addr + ACPIBASE_PMC_END;
> }
>
> - lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_WDT]);
> + ret = lpc_ich_finalize_wdt_cell(dev);
> + if (ret)
> + goto wdt_done;
> +
> ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> &lpc_ich_cells[LPC_WDT], 1, NULL, 0, NULL);
>
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index 105cfffe82c6..28b2a12bb26d 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -33,7 +33,7 @@
> #include <linux/suspend.h>
> #include <linux/acpi.h>
> #include <asm/intel_pmc_ipc.h>
> -#include <linux/mfd/lpc_ich.h>
> +#include <linux/platform_data/itco_wdt.h>
>
> /*
> * IPC registers
> @@ -473,9 +473,9 @@ static struct resource tco_res[] = {
> },
> };
>
> -static struct lpc_ich_info tco_info = {
> +static struct itco_wdt_platform_data tco_info = {
> .name = "Apollo Lake SoC",
> - .iTCO_version = 3,
> + .version = 3,
> };
>
> static int ipc_create_punit_device(void)
> @@ -552,8 +552,7 @@ static int ipc_create_tco_device(void)
> goto err;
> }
>
> - ret = platform_device_add_data(pdev, &tco_info,
> - sizeof(struct lpc_ich_info));
> + ret = platform_device_add_data(pdev, &tco_info, sizeof(tco_info));
> if (ret) {
> dev_err(ipcdev.dev, "Failed to add tco platform data\n");
> goto err;
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 3c3fd417ddeb..a94401b2deca 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -66,8 +66,7 @@
> #include <linux/spinlock.h> /* For spin_lock/spin_unlock/... */
> #include <linux/uaccess.h> /* For copy_to_user/put_user/... */
> #include <linux/io.h> /* For inb/outb/... */
> -#include <linux/mfd/core.h>
> -#include <linux/mfd/lpc_ich.h>
> +#include <linux/platform_data/itco_wdt.h>
>
> #include "iTCO_vendor.h"
>
> @@ -418,9 +417,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> {
> int ret = -ENODEV;
> unsigned long val32;
> - struct lpc_ich_info *ich_info = dev_get_platdata(&dev->dev);
> + struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
>
> - if (!ich_info)
> + if (!pdata)
> goto out;
>
> spin_lock_init(&iTCO_wdt_private.io_lock);
> @@ -435,7 +434,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> if (!iTCO_wdt_private.smi_res)
> goto out;
>
> - iTCO_wdt_private.iTCO_version = ich_info->iTCO_version;
> + iTCO_wdt_private.iTCO_version = pdata->version;
> iTCO_wdt_private.dev = dev;
> iTCO_wdt_private.pdev = to_pci_dev(dev->dev.parent);
>
> @@ -501,7 +500,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> }
>
> pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
> - ich_info->name, ich_info->iTCO_version, (u64)TCOBASE);
> + pdata->name, pdata->version, (u64)TCOBASE);
>
> /* Clear out the (probably old) status */
> if (iTCO_wdt_private.iTCO_version == 3) {
> diff --git a/include/linux/mfd/lpc_ich.h b/include/linux/mfd/lpc_ich.h
> index 8feac782fa83..2b300b44f994 100644
> --- a/include/linux/mfd/lpc_ich.h
> +++ b/include/linux/mfd/lpc_ich.h
> @@ -20,12 +20,6 @@
> #ifndef LPC_ICH_H
> #define LPC_ICH_H
>
> -/* Watchdog resources */
> -#define ICH_RES_IO_TCO 0
> -#define ICH_RES_IO_SMI 1
> -#define ICH_RES_MEM_OFF 2
> -#define ICH_RES_MEM_GCS_PMC 0
> -
> /* GPIO resources */
> #define ICH_RES_GPIO 0
> #define ICH_RES_GPE0 1
> diff --git a/include/linux/platform_data/itco_wdt.h b/include/linux/platform_data/itco_wdt.h
> new file mode 100644
> index 000000000000..f16542c77ff7
> --- /dev/null
> +++ b/include/linux/platform_data/itco_wdt.h
> @@ -0,0 +1,19 @@
> +/*
> + * Platform data for the Intel TCO Watchdog
> + */
> +
> +#ifndef _ITCO_WDT_H_
> +#define _ITCO_WDT_H_
> +
> +/* Watchdog resources */
> +#define ICH_RES_IO_TCO 0
> +#define ICH_RES_IO_SMI 1
> +#define ICH_RES_MEM_OFF 2
> +#define ICH_RES_MEM_GCS_PMC 0
> +
> +struct itco_wdt_platform_data {
> + char name[32];
> + unsigned int version;
> +};
> +
> +#endif /* _ITCO_WDT_H_ */
>

2015-08-07 15:55:49

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] i2c: i801: Create iTCO device on newer Intel PCHs

On Thu, Aug 06, 2015 at 01:46:25PM +0100, Matt Fleming wrote:
> From: Mika Westerberg <[email protected]>
>
> Starting from Intel Sunrisepoint (Skylake PCH) the iTCO watchdog resources
> have been moved to reside under the i801 SMBus host controller whereas
> previously they were under the LPC device.
>
> In order to support the iTCO watchdog on newer PCHs we need to create the
> platform device here in the SMBus driver and pass all known resources using
> platform data.
>
> Cc: Jean Delvare <[email protected]>
> Cc: Wolfram Sang <[email protected]>
> Cc: <[email protected]>
> Reviewed-by: Guenter Roeck <[email protected]>
> Signed-off-by: Mika Westerberg <[email protected]>
> Signed-off-by: Matt Fleming <[email protected]>

Dunno which tree this should go through (probably MFD or watchdog), but
doesn't look like I2C, so:

Acked-by: Wolfram Sang <[email protected]>


Attachments:
(No filename) (924.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-07 16:27:14

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] i2c: i801: Create iTCO device on newer Intel PCHs

On 08/07/2015 08:55 AM, Wolfram Sang wrote:
> On Thu, Aug 06, 2015 at 01:46:25PM +0100, Matt Fleming wrote:
>> From: Mika Westerberg <[email protected]>
>>
>> Starting from Intel Sunrisepoint (Skylake PCH) the iTCO watchdog resources
>> have been moved to reside under the i801 SMBus host controller whereas
>> previously they were under the LPC device.
>>
>> In order to support the iTCO watchdog on newer PCHs we need to create the
>> platform device here in the SMBus driver and pass all known resources using
>> platform data.
>>
>> Cc: Jean Delvare <[email protected]>
>> Cc: Wolfram Sang <[email protected]>
>> Cc: <[email protected]>
>> Reviewed-by: Guenter Roeck <[email protected]>
>> Signed-off-by: Mika Westerberg <[email protected]>
>> Signed-off-by: Matt Fleming <[email protected]>
>
> Dunno which tree this should go through (probably MFD or watchdog), but
> doesn't look like I2C, so:
>

Probably watchdog ?

Guenter

2015-08-11 14:14:51

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] iTCO_wdt: Expose watchdog properties using platform data

On Thu, 06 Aug 2015, Matt Fleming wrote:

> From: Matt Fleming <[email protected]>
>
> Intel Sunrisepoint (Skylake PCH) has the iTCO watchdog accessible across
> the SMBus, unlike previous generations of PCH/ICH where it was on the
> LPC bus. Because it's on the SMBus, it doesn't make sense to pass around
> a 'struct lpc_ich_info', and leaking the type of bus into the iTCO
> watchdog driver is kind of backwards anyway.
>
> This change introduces a new 'struct itco_wdt_platform_data' for use
> inside the iTCO watchdog driver and by the upcoming Intel Sunrisepoint
> code, which neatly avoids having to include lpc_ich headers in the i801
> i2c driver.
>
> This change is overdue because lpc_ich_info has already found its way
> into other TCO watchdog users, notably the intel_pmc_ipc driver where
> the watchdog actually isn't on the LPC bus as far as I can see.
>
> A simple translation layer is provided for converting from the existing
> 'struct lpc_ich_info' inside the lpc_ich mfd driver.
>
> Cc: Peter Tyser <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Acked-by: Lee Jones <[email protected]>
> Cc: Wim Van Sebroeck <[email protected]>
> Cc: Zha Qipeng <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Jean Delvare <[email protected]>
> Acked-by: Darren Hart <[email protected]> [drivers/x86 refactoring]
> Signed-off-by: Matt Fleming <[email protected]>
> ---
> v4:
> - Added Darren's ACK
>
> v3:
> - Added Lee's ACK
>
> v2:
> - Use lowercase "itco" in all new files and data structures.
> - Use devm_kzalloc() to allocate platform data, fixing a memory leak
> and saving us from having to free the allocation in error paths
> - Move stray Kconfig hunk to PATCH 3 that accidentally snuck into v1
> - Swap strcpy() for the safer strlcpy()
> - Fix lpc_ich_info user in the intel_pmc_ipc driver that was merged for
> v4.2.
>
> drivers/mfd/lpc_ich.c | 32 +++++++++++++++++++++++++++++---
> drivers/platform/x86/intel_pmc_ipc.c | 9 ++++-----
> drivers/watchdog/iTCO_wdt.c | 11 +++++------
> include/linux/mfd/lpc_ich.h | 6 ------
> include/linux/platform_data/itco_wdt.h | 19 +++++++++++++++++++
> 5 files changed, 57 insertions(+), 20 deletions(-)
> create mode 100644 include/linux/platform_data/itco_wdt.h

Applied, thanks.

> diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> index 8de34398abc0..c5a9a08b5dfb 100644
> --- a/drivers/mfd/lpc_ich.c
> +++ b/drivers/mfd/lpc_ich.c
> @@ -66,6 +66,7 @@
> #include <linux/pci.h>
> #include <linux/mfd/core.h>
> #include <linux/mfd/lpc_ich.h>
> +#include <linux/platform_data/itco_wdt.h>
>
> #define ACPIBASE 0x40
> #define ACPIBASE_GPE_OFF 0x28
> @@ -835,9 +836,31 @@ static void lpc_ich_enable_pmc_space(struct pci_dev *dev)
> priv->actrl_pbase_save = reg_save;
> }
>
> -static void lpc_ich_finalize_cell(struct pci_dev *dev, struct mfd_cell *cell)
> +static int lpc_ich_finalize_wdt_cell(struct pci_dev *dev)
> {
> + struct itco_wdt_platform_data *pdata;
> struct lpc_ich_priv *priv = pci_get_drvdata(dev);
> + struct lpc_ich_info *info;
> + struct mfd_cell *cell = &lpc_ich_cells[LPC_WDT];
> +
> + pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + info = &lpc_chipset_info[priv->chipset];
> +
> + pdata->version = info->iTCO_version;
> + strlcpy(pdata->name, info->name, sizeof(pdata->name));
> +
> + cell->platform_data = pdata;
> + cell->pdata_size = sizeof(*pdata);
> + return 0;
> +}
> +
> +static void lpc_ich_finalize_gpio_cell(struct pci_dev *dev)
> +{
> + struct lpc_ich_priv *priv = pci_get_drvdata(dev);
> + struct mfd_cell *cell = &lpc_ich_cells[LPC_GPIO];
>
> cell->platform_data = &lpc_chipset_info[priv->chipset];
> cell->pdata_size = sizeof(struct lpc_ich_info);
> @@ -933,7 +956,7 @@ gpe0_done:
> lpc_chipset_info[priv->chipset].use_gpio = ret;
> lpc_ich_enable_gpio_space(dev);
>
> - lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_GPIO]);
> + lpc_ich_finalize_gpio_cell(dev);
> ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> &lpc_ich_cells[LPC_GPIO], 1, NULL, 0, NULL);
>
> @@ -1007,7 +1030,10 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
> res->end = base_addr + ACPIBASE_PMC_END;
> }
>
> - lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_WDT]);
> + ret = lpc_ich_finalize_wdt_cell(dev);
> + if (ret)
> + goto wdt_done;
> +
> ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> &lpc_ich_cells[LPC_WDT], 1, NULL, 0, NULL);
>
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index 105cfffe82c6..28b2a12bb26d 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -33,7 +33,7 @@
> #include <linux/suspend.h>
> #include <linux/acpi.h>
> #include <asm/intel_pmc_ipc.h>
> -#include <linux/mfd/lpc_ich.h>
> +#include <linux/platform_data/itco_wdt.h>
>
> /*
> * IPC registers
> @@ -473,9 +473,9 @@ static struct resource tco_res[] = {
> },
> };
>
> -static struct lpc_ich_info tco_info = {
> +static struct itco_wdt_platform_data tco_info = {
> .name = "Apollo Lake SoC",
> - .iTCO_version = 3,
> + .version = 3,
> };
>
> static int ipc_create_punit_device(void)
> @@ -552,8 +552,7 @@ static int ipc_create_tco_device(void)
> goto err;
> }
>
> - ret = platform_device_add_data(pdev, &tco_info,
> - sizeof(struct lpc_ich_info));
> + ret = platform_device_add_data(pdev, &tco_info, sizeof(tco_info));
> if (ret) {
> dev_err(ipcdev.dev, "Failed to add tco platform data\n");
> goto err;
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 3c3fd417ddeb..a94401b2deca 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -66,8 +66,7 @@
> #include <linux/spinlock.h> /* For spin_lock/spin_unlock/... */
> #include <linux/uaccess.h> /* For copy_to_user/put_user/... */
> #include <linux/io.h> /* For inb/outb/... */
> -#include <linux/mfd/core.h>
> -#include <linux/mfd/lpc_ich.h>
> +#include <linux/platform_data/itco_wdt.h>
>
> #include "iTCO_vendor.h"
>
> @@ -418,9 +417,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> {
> int ret = -ENODEV;
> unsigned long val32;
> - struct lpc_ich_info *ich_info = dev_get_platdata(&dev->dev);
> + struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
>
> - if (!ich_info)
> + if (!pdata)
> goto out;
>
> spin_lock_init(&iTCO_wdt_private.io_lock);
> @@ -435,7 +434,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> if (!iTCO_wdt_private.smi_res)
> goto out;
>
> - iTCO_wdt_private.iTCO_version = ich_info->iTCO_version;
> + iTCO_wdt_private.iTCO_version = pdata->version;
> iTCO_wdt_private.dev = dev;
> iTCO_wdt_private.pdev = to_pci_dev(dev->dev.parent);
>
> @@ -501,7 +500,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> }
>
> pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
> - ich_info->name, ich_info->iTCO_version, (u64)TCOBASE);
> + pdata->name, pdata->version, (u64)TCOBASE);
>
> /* Clear out the (probably old) status */
> if (iTCO_wdt_private.iTCO_version == 3) {
> diff --git a/include/linux/mfd/lpc_ich.h b/include/linux/mfd/lpc_ich.h
> index 8feac782fa83..2b300b44f994 100644
> --- a/include/linux/mfd/lpc_ich.h
> +++ b/include/linux/mfd/lpc_ich.h
> @@ -20,12 +20,6 @@
> #ifndef LPC_ICH_H
> #define LPC_ICH_H
>
> -/* Watchdog resources */
> -#define ICH_RES_IO_TCO 0
> -#define ICH_RES_IO_SMI 1
> -#define ICH_RES_MEM_OFF 2
> -#define ICH_RES_MEM_GCS_PMC 0
> -
> /* GPIO resources */
> #define ICH_RES_GPIO 0
> #define ICH_RES_GPE0 1
> diff --git a/include/linux/platform_data/itco_wdt.h b/include/linux/platform_data/itco_wdt.h
> new file mode 100644
> index 000000000000..f16542c77ff7
> --- /dev/null
> +++ b/include/linux/platform_data/itco_wdt.h
> @@ -0,0 +1,19 @@
> +/*
> + * Platform data for the Intel TCO Watchdog
> + */
> +
> +#ifndef _ITCO_WDT_H_
> +#define _ITCO_WDT_H_
> +
> +/* Watchdog resources */
> +#define ICH_RES_IO_TCO 0
> +#define ICH_RES_IO_SMI 1
> +#define ICH_RES_MEM_OFF 2
> +#define ICH_RES_MEM_GCS_PMC 0
> +
> +struct itco_wdt_platform_data {
> + char name[32];
> + unsigned int version;
> +};
> +
> +#endif /* _ITCO_WDT_H_ */

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2015-08-11 14:15:31

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] i2c: i801: Create iTCO device on newer Intel PCHs

On Thu, 06 Aug 2015, Matt Fleming wrote:

> From: Mika Westerberg <[email protected]>
>
> Starting from Intel Sunrisepoint (Skylake PCH) the iTCO watchdog resources
> have been moved to reside under the i801 SMBus host controller whereas
> previously they were under the LPC device.
>
> In order to support the iTCO watchdog on newer PCHs we need to create the
> platform device here in the SMBus driver and pass all known resources using
> platform data.
>
> Cc: Jean Delvare <[email protected]>
> Cc: Wolfram Sang <[email protected]>
> Cc: <[email protected]>
> Reviewed-by: Guenter Roeck <[email protected]>
> Signed-off-by: Mika Westerberg <[email protected]>
> Signed-off-by: Matt Fleming <[email protected]>
> ---
>
> v4:
> - No changes
>
> v3:
> - Added Guenter's Review tag
>
> v2:
> - Don't use bitops but same scheme used already
> - Delete superfluous NULL-checks for platform_device_unregister()
>
> drivers/i2c/busses/i2c-i801.c | 120 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 119 insertions(+), 1 deletion(-)

Applied, thanks.

> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 5ecbb3fdc27e..eaef9bc9d88c 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -88,12 +88,13 @@
> #include <linux/slab.h>
> #include <linux/wait.h>
> #include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/itco_wdt.h>
>
> #if (defined CONFIG_I2C_MUX_GPIO || defined CONFIG_I2C_MUX_GPIO_MODULE) && \
> defined CONFIG_DMI
> #include <linux/gpio.h>
> #include <linux/i2c-mux-gpio.h>
> -#include <linux/platform_device.h>
> #endif
>
> /* I801 SMBus address offsets */
> @@ -113,6 +114,16 @@
> #define SMBPCICTL 0x004
> #define SMBPCISTS 0x006
> #define SMBHSTCFG 0x040
> +#define TCOBASE 0x050
> +#define TCOCTL 0x054
> +
> +#define ACPIBASE 0x040
> +#define ACPIBASE_SMI_OFF 0x030
> +#define ACPICTRL 0x044
> +#define ACPICTRL_EN 0x080
> +
> +#define SBREG_BAR 0x10
> +#define SBREG_SMBCTRL 0xc6000c
>
> /* Host status bits for SMBPCISTS */
> #define SMBPCISTS_INTS 0x08
> @@ -125,6 +136,9 @@
> #define SMBHSTCFG_SMB_SMI_EN 2
> #define SMBHSTCFG_I2C_EN 4
>
> +/* TCO configuration bits for TCOCTL */
> +#define TCOCTL_EN 0x0100
> +
> /* Auxiliary control register bits, ICH4+ only */
> #define SMBAUXCTL_CRC 1
> #define SMBAUXCTL_E32B 2
> @@ -221,6 +235,7 @@ struct i801_priv {
> const struct i801_mux_config *mux_drvdata;
> struct platform_device *mux_pdev;
> #endif
> + struct platform_device *tco_pdev;
> };
>
> #define FEATURE_SMBUS_PEC (1 << 0)
> @@ -230,6 +245,7 @@ struct i801_priv {
> #define FEATURE_IRQ (1 << 4)
> /* Not really a feature, but it's convenient to handle it as such */
> #define FEATURE_IDF (1 << 15)
> +#define FEATURE_TCO (1 << 16)
>
> static const char *i801_feature_names[] = {
> "SMBus PEC",
> @@ -1132,6 +1148,95 @@ static inline unsigned int i801_get_adapter_class(struct i801_priv *priv)
> }
> #endif
>
> +static const struct itco_wdt_platform_data tco_platform_data = {
> + .name = "Intel PCH",
> + .version = 4,
> +};
> +
> +static DEFINE_SPINLOCK(p2sb_spinlock);
> +
> +static void i801_add_tco(struct i801_priv *priv)
> +{
> + struct pci_dev *pci_dev = priv->pci_dev;
> + struct resource tco_res[3], *res;
> + struct platform_device *pdev;
> + unsigned int devfn;
> + u32 tco_base, tco_ctl;
> + u32 base_addr, ctrl_val;
> + u64 base64_addr;
> +
> + if (!(priv->features & FEATURE_TCO))
> + return;
> +
> + pci_read_config_dword(pci_dev, TCOBASE, &tco_base);
> + pci_read_config_dword(pci_dev, TCOCTL, &tco_ctl);
> + if (!(tco_ctl & TCOCTL_EN))
> + return;
> +
> + memset(tco_res, 0, sizeof(tco_res));
> +
> + res = &tco_res[ICH_RES_IO_TCO];
> + res->start = tco_base & ~1;
> + res->end = res->start + 32 - 1;
> + res->flags = IORESOURCE_IO;
> +
> + /*
> + * Power Management registers.
> + */
> + devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 2);
> + pci_bus_read_config_dword(pci_dev->bus, devfn, ACPIBASE, &base_addr);
> +
> + res = &tco_res[ICH_RES_IO_SMI];
> + res->start = (base_addr & ~1) + ACPIBASE_SMI_OFF;
> + res->end = res->start + 3;
> + res->flags = IORESOURCE_IO;
> +
> + /*
> + * Enable the ACPI I/O space.
> + */
> + pci_bus_read_config_dword(pci_dev->bus, devfn, ACPICTRL, &ctrl_val);
> + ctrl_val |= ACPICTRL_EN;
> + pci_bus_write_config_dword(pci_dev->bus, devfn, ACPICTRL, ctrl_val);
> +
> + /*
> + * We must access the NO_REBOOT bit over the Primary to Sideband
> + * bridge (P2SB). The BIOS prevents the P2SB device from being
> + * enumerated by the PCI subsystem, so we need to unhide/hide it
> + * to lookup the P2SB BAR.
> + */
> + spin_lock(&p2sb_spinlock);
> +
> + devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 1);
> +
> + /* Unhide the P2SB device */
> + pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, 0x0);
> +
> + pci_bus_read_config_dword(pci_dev->bus, devfn, SBREG_BAR, &base_addr);
> + base64_addr = base_addr & 0xfffffff0;
> +
> + pci_bus_read_config_dword(pci_dev->bus, devfn, SBREG_BAR + 0x4, &base_addr);
> + base64_addr |= (u64)base_addr << 32;
> +
> + /* Hide the P2SB device */
> + pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, 0x1);
> + spin_unlock(&p2sb_spinlock);
> +
> + res = &tco_res[ICH_RES_MEM_OFF];
> + res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL;
> + res->end = res->start + 3;
> + res->flags = IORESOURCE_MEM;
> +
> + pdev = platform_device_register_resndata(&pci_dev->dev, "iTCO_wdt", -1,
> + tco_res, 3, &tco_platform_data,
> + sizeof(tco_platform_data));
> + if (IS_ERR(pdev)) {
> + dev_warn(&pci_dev->dev, "failed to create iTCO device\n");
> + return;
> + }
> +
> + priv->tco_pdev = pdev;
> +}
> +
> static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
> {
> unsigned char temp;
> @@ -1149,6 +1254,15 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
>
> priv->pci_dev = dev;
> switch (dev->device) {
> + case PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS:
> + case PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS:
> + priv->features |= FEATURE_I2C_BLOCK_READ;
> + priv->features |= FEATURE_IRQ;
> + priv->features |= FEATURE_SMBUS_PEC;
> + priv->features |= FEATURE_BLOCK_BUFFER;
> + priv->features |= FEATURE_TCO;
> + break;
> +
> case PCI_DEVICE_ID_INTEL_PATSBURG_SMBUS_IDF0:
> case PCI_DEVICE_ID_INTEL_PATSBURG_SMBUS_IDF1:
> case PCI_DEVICE_ID_INTEL_PATSBURG_SMBUS_IDF2:
> @@ -1265,6 +1379,8 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
> dev_info(&dev->dev, "SMBus using %s\n",
> priv->features & FEATURE_IRQ ? "PCI interrupt" : "polling");
>
> + i801_add_tco(priv);
> +
> /* set up the sysfs linkage to our parent device */
> priv->adapter.dev.parent = &dev->dev;
>
> @@ -1296,6 +1412,8 @@ static void i801_remove(struct pci_dev *dev)
> i2c_del_adapter(&priv->adapter);
> pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
>
> + platform_device_unregister(priv->tco_pdev);
> +
> /*
> * do not call pci_disable_device(dev) since it can cause hard hangs on
> * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2015-08-11 14:15:24

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] iTCO_wdt: Add support for TCO on Intel Sunrisepoint

On Thu, 06 Aug 2015, Matt Fleming wrote:

> From: Matt Fleming <[email protected]>
>
> The revision of the watchdog hardware in Sunrisepoint necessitates a new
> "version" inside the TCO watchdog driver because some of the register
> layouts have changed.
>
> Also update the Kconfig entry to select both the LPC and SMBus drivers
> since the TCO device is on the SMBus in Sunrisepoint.
>
> Cc: Wim Van Sebroeck <[email protected]>
> Reviewed-by: Guenter Roeck <[email protected]>
> Cc: Jean Delvare <[email protected]>
> Signed-off-by: Matt Fleming <[email protected]>
> ---
>
> v4:
> - Explicitly list versions in iTCO_wdt_probe()
> - Delete the now superfluous 'ret' variable in iTCO_wdt_unset_NO_REBOOT_bit()
>
> v3:
> - Added Guenter's Review tag
>
> v2:
> - Explicitly list TCO versions and their "no reboot" bits in the switch
> statement in no_reboot_bit()
> - Use a switch/case statement when clearing status bits instead of
> convoluted if/else branching
> - Select both of the bus drivers instead of using depends
>
> drivers/watchdog/Kconfig | 3 +-
> drivers/watchdog/iTCO_wdt.c | 71 ++++++++++++++++++++++++++++-----------------
> 2 files changed, 46 insertions(+), 28 deletions(-)

Applied, thanks.

> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 241fafde42cb..55c4b5b0a317 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -797,7 +797,8 @@ config ITCO_WDT
> tristate "Intel TCO Timer/Watchdog"
> depends on (X86 || IA64) && PCI
> select WATCHDOG_CORE
> - select LPC_ICH
> + select LPC_ICH if !EXPERT
> + select I2C_I801 if !EXPERT
> ---help---
> Hardware driver for the intel TCO timer based watchdog devices.
> These drivers are included in the Intel 82801 I/O Controller
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index a94401b2deca..0acc6c5f729d 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -145,59 +145,67 @@ static inline unsigned int ticks_to_seconds(int ticks)
> return iTCO_wdt_private.iTCO_version == 3 ? ticks : (ticks * 6) / 10;
> }
>
> +static inline u32 no_reboot_bit(void)
> +{
> + u32 enable_bit;
> +
> + switch (iTCO_wdt_private.iTCO_version) {
> + case 3:
> + enable_bit = 0x00000010;
> + break;
> + case 2:
> + enable_bit = 0x00000020;
> + break;
> + case 4:
> + case 1:
> + default:
> + enable_bit = 0x00000002;
> + break;
> + }
> +
> + return enable_bit;
> +}
> +
> static void iTCO_wdt_set_NO_REBOOT_bit(void)
> {
> u32 val32;
>
> /* Set the NO_REBOOT bit: this disables reboots */
> - if (iTCO_wdt_private.iTCO_version == 3) {
> - val32 = readl(iTCO_wdt_private.gcs_pmc);
> - val32 |= 0x00000010;
> - writel(val32, iTCO_wdt_private.gcs_pmc);
> - } else if (iTCO_wdt_private.iTCO_version == 2) {
> + if (iTCO_wdt_private.iTCO_version >= 2) {
> val32 = readl(iTCO_wdt_private.gcs_pmc);
> - val32 |= 0x00000020;
> + val32 |= no_reboot_bit();
> writel(val32, iTCO_wdt_private.gcs_pmc);
> } else if (iTCO_wdt_private.iTCO_version == 1) {
> pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
> - val32 |= 0x00000002;
> + val32 |= no_reboot_bit();
> pci_write_config_dword(iTCO_wdt_private.pdev, 0xd4, val32);
> }
> }
>
> static int iTCO_wdt_unset_NO_REBOOT_bit(void)
> {
> - int ret = 0;
> - u32 val32;
> + u32 enable_bit = no_reboot_bit();
> + u32 val32 = 0;
>
> /* Unset the NO_REBOOT bit: this enables reboots */
> - if (iTCO_wdt_private.iTCO_version == 3) {
> - val32 = readl(iTCO_wdt_private.gcs_pmc);
> - val32 &= 0xffffffef;
> - writel(val32, iTCO_wdt_private.gcs_pmc);
> -
> - val32 = readl(iTCO_wdt_private.gcs_pmc);
> - if (val32 & 0x00000010)
> - ret = -EIO;
> - } else if (iTCO_wdt_private.iTCO_version == 2) {
> + if (iTCO_wdt_private.iTCO_version >= 2) {
> val32 = readl(iTCO_wdt_private.gcs_pmc);
> - val32 &= 0xffffffdf;
> + val32 &= ~enable_bit;
> writel(val32, iTCO_wdt_private.gcs_pmc);
>
> val32 = readl(iTCO_wdt_private.gcs_pmc);
> - if (val32 & 0x00000020)
> - ret = -EIO;
> } else if (iTCO_wdt_private.iTCO_version == 1) {
> pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
> - val32 &= 0xfffffffd;
> + val32 &= ~enable_bit;
> pci_write_config_dword(iTCO_wdt_private.pdev, 0xd4, val32);
>
> pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
> - if (val32 & 0x00000002)
> - ret = -EIO;
> }
>
> - return ret; /* returns: 0 = OK, -EIO = Error */
> + if (val32 & enable_bit)
> + return -EIO;
> +
> + return 0;
> }
>
> static int iTCO_wdt_start(struct watchdog_device *wd_dev)
> @@ -503,12 +511,21 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> pdata->name, pdata->version, (u64)TCOBASE);
>
> /* Clear out the (probably old) status */
> - if (iTCO_wdt_private.iTCO_version == 3) {
> + switch (iTCO_wdt_private.iTCO_version) {
> + case 4:
> + outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
> + outw(0x0002, TCO2_STS); /* Clear SECOND_TO_STS bit */
> + break;
> + case 3:
> outl(0x20008, TCO1_STS);
> - } else {
> + break;
> + case 2:
> + case 1:
> + default:
> outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
> outw(0x0002, TCO2_STS); /* Clear SECOND_TO_STS bit */
> outw(0x0004, TCO2_STS); /* Clear BOOT_STS bit */
> + break;
> }
>
> iTCO_wdt_watchdog_dev.bootstatus = 0;

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2015-08-12 08:26:43

by Lee Jones

[permalink] [raw]
Subject: [GIT PULL] Immutable branch between MFD, I2C, X86 and Watchdog due for v4.3

Enjoy!

The following changes since commit bc0195aad0daa2ad5b0d76cce22b167bc3435590:

Linux 4.2-rc2 (2015-07-12 15:10:30 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-i2c-x86-watchdog-v4.3

for you to fetch changes up to 2a7a0e9bf7b32e838d873226808ab8a6c00148f7:

watchdog: iTCO_wdt: Add support for TCO on Intel Sunrisepoint (2015-08-11 15:03:52 +0100)

----------------------------------------------------------------
Immutable branch between MFD, I2C, X86 and Watchdog due for v4.3

----------------------------------------------------------------
Matt Fleming (2):
mfd: watchdog: iTCO_wdt: Expose watchdog properties using platform data
watchdog: iTCO_wdt: Add support for TCO on Intel Sunrisepoint

Mika Westerberg (1):
i2c: i801: Create iTCO device on newer Intel PCHs

drivers/i2c/busses/i2c-i801.c | 120 ++++++++++++++++++++++++++++++++-
drivers/mfd/lpc_ich.c | 32 ++++++++-
drivers/platform/x86/intel_pmc_ipc.c | 9 ++-
drivers/watchdog/Kconfig | 3 +-
drivers/watchdog/iTCO_wdt.c | 82 +++++++++++++---------
include/linux/mfd/lpc_ich.h | 6 --
include/linux/platform_data/itco_wdt.h | 19 ++++++
7 files changed, 222 insertions(+), 49 deletions(-)
create mode 100644 include/linux/platform_data/itco_wdt.h

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog