diff mbox series

i2c: i801: fix DNV's SMBCTRL register offset

Message ID 20180903053705.9844-1-felipe.balbi@linux.intel.com
State Superseded
Headers show
Series i2c: i801: fix DNV's SMBCTRL register offset | expand

Commit Message

Felipe Balbi Sept. 3, 2018, 5:37 a.m. UTC
DNV's iTCO is slightly different with SMBCTRL sitting at a differnt
offset when compared to all other devices. Let's fix so that we can
properly use iTCO watchdog.

Fixes: 84d7f2ebd70d ("i2c: i801: Add support for Intel DNV")
Cc: <stable@vger.kernel.org> # v4.4+
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
---
 drivers/i2c/busses/i2c-i801.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jean Delvare Sept. 3, 2018, 7:09 a.m. UTC | #1
Hi Felipe,

On Mon,  3 Sep 2018 08:37:05 +0300, Felipe Balbi wrote:
> DNV's iTCO is slightly different with SMBCTRL sitting at a differnt
> offset when compared to all other devices. Let's fix so that we can
> properly use iTCO watchdog.
> 
> Fixes: 84d7f2ebd70d ("i2c: i801: Add support for Intel DNV")
> Cc: <stable@vger.kernel.org> # v4.4+
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-i801.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 941c223f6491..390bf253b6ea 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1400,6 +1400,11 @@ static void i801_add_tco(struct i801_priv *priv)
>  
>  	res = &tco_res[ICH_RES_MEM_OFF];
>  	res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL;
> +
> +	/* DNV device has SMBCTRL at 0xcf000c */
> +	if (pci_dev->device == PCI_DEVICE_ID_INTEL_DNV_SMBUS)
> +		res->start += 0x90000;
> +
>  	res->end = res->start + 3;
>  	res->flags = IORESOURCE_MEM;
>  

Thanks for catching this but this is not the way I want it to be fixed.
Applying an arbitrary offset like that is pretty obscure and fragile
too. The value of SMBCTRL for DNV should instead appear explicitly in
the code. Something like that:

#define SBREG_SMBCTRL		0xc6000c
#define SBREG_SMBCTRL_DNV	0xcf000c

	if (pci_dev->device == PCI_DEVICE_ID_INTEL_DNV_SMBUS)
		res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL_DNV;
	else
		res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL;

Alternatively you can add a member to struct i801_priv to store the
register address in i801_probe(), and use that in i801_add_tco(). The
above defines could even go away then. Both approaches are fine with me.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 941c223f6491..390bf253b6ea 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1400,6 +1400,11 @@  static void i801_add_tco(struct i801_priv *priv)
 
 	res = &tco_res[ICH_RES_MEM_OFF];
 	res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL;
+
+	/* DNV device has SMBCTRL at 0xcf000c */
+	if (pci_dev->device == PCI_DEVICE_ID_INTEL_DNV_SMBUS)
+		res->start += 0x90000;
+
 	res->end = res->start + 3;
 	res->flags = IORESOURCE_MEM;