From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752940AbcBJQxW (ORCPT ); Wed, 10 Feb 2016 11:53:22 -0500 Received: from mail-wm0-f50.google.com ([74.125.82.50]:33161 "EHLO mail-wm0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750757AbcBJQxS (ORCPT ); Wed, 10 Feb 2016 11:53:18 -0500 Date: Wed, 10 Feb 2016 16:53:12 +0000 From: Lee Jones To: Andy Shevchenko Cc: "Rafael J . Wysocki" , Greg Kroah-Hartman , Jarkko Nikula , linux-i2c@vger.kernel.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, Mika Westerberg , Kevin Fenzi , Arnd Bergmann , Wolfram Sang Subject: Re: [PATCH v2 14/16] mfd: intel-lpss: Pass SDA hold time to I2C host controller driver Message-ID: <20160210165312.GO3782@x1> References: <1448896304-87928-1-git-send-email-andriy.shevchenko@linux.intel.com> <1448896304-87928-15-git-send-email-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1448896304-87928-15-git-send-email-andriy.shevchenko@linux.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 30 Nov 2015, Andy Shevchenko wrote: > From: Mika Westerberg > > Intel Skylake the LPSS I2C pad circuit has internal delays that require > programming non-zero SDA hold time for the I2C host controller. If this is > not done communication to slave devices may fail with arbitration lost > errors like the one seen below taken from Lenovo Yoga 900: > > i2c_hid i2c-SYNA2B29:00: Fetching the HID descriptor > i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=20 00 > i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration > > To fix this we follow what the Windows driver is doing and pass the default > SDA hold time of 230 ns to all Intel Skylake host controllers. This still > allows the platform to override these values by passing special ACPI > methods SSCN and FMCN. > > Reported-by: Kevin Fenzi > Signed-off-by: Mika Westerberg > Signed-off-by: Andy Shevchenko > --- > drivers/mfd/intel-lpss-acpi.c | 19 +++++++++++++++++-- > drivers/mfd/intel-lpss-pci.c | 31 +++++++++++++++++++++++-------- > 2 files changed, 40 insertions(+), 10 deletions(-) Seems fine in principle. Acked-by: Lee Jones > diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c > index b6fd904..06f00d6 100644 > --- a/drivers/mfd/intel-lpss-acpi.c > +++ b/drivers/mfd/intel-lpss-acpi.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > > #include "intel-lpss.h" > > @@ -25,6 +26,20 @@ static const struct intel_lpss_platform_info spt_info = { > .clk_rate = 120000000, > }; > > +static struct property_entry spt_i2c_properties[] = { > + PROPERTY_ENTRY_U32("i2c-sda-hold-time-ns", 230), > + { }, > +}; > + > +static struct property_set spt_i2c_pset = { > + .properties = spt_i2c_properties, > +}; > + > +static const struct intel_lpss_platform_info spt_i2c_info = { > + .clk_rate = 120000000, > + .pset = &spt_i2c_pset, > +}; > + > static const struct intel_lpss_platform_info bxt_info = { > .clk_rate = 100000000, > }; > @@ -35,8 +50,8 @@ static const struct intel_lpss_platform_info bxt_i2c_info = { > > static const struct acpi_device_id intel_lpss_acpi_ids[] = { > /* SPT */ > - { "INT3446", (kernel_ulong_t)&spt_info }, > - { "INT3447", (kernel_ulong_t)&spt_info }, > + { "INT3446", (kernel_ulong_t)&spt_i2c_info }, > + { "INT3447", (kernel_ulong_t)&spt_i2c_info }, > /* BXT */ > { "80860AAC", (kernel_ulong_t)&bxt_i2c_info }, > { "80860ABC", (kernel_ulong_t)&bxt_info }, > diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c > index 5bfdfcc..a677480 100644 > --- a/drivers/mfd/intel-lpss-pci.c > +++ b/drivers/mfd/intel-lpss-pci.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include "intel-lpss.h" > > @@ -65,6 +66,20 @@ static const struct intel_lpss_platform_info spt_info = { > .clk_rate = 120000000, > }; > > +static struct property_entry spt_i2c_properties[] = { > + PROPERTY_ENTRY_U32("i2c-sda-hold-time-ns", 230), > + { }, > +}; > + > +static struct property_set spt_i2c_pset = { > + .properties = spt_i2c_properties, > +}; > + > +static const struct intel_lpss_platform_info spt_i2c_info = { > + .clk_rate = 120000000, > + .pset = &spt_i2c_pset, > +}; > + > static const struct intel_lpss_platform_info spt_uart_info = { > .clk_rate = 120000000, > .clk_con_id = "baudclk", > @@ -121,20 +136,20 @@ static const struct pci_device_id intel_lpss_pci_ids[] = { > { PCI_VDEVICE(INTEL, 0x9d28), (kernel_ulong_t)&spt_uart_info }, > { PCI_VDEVICE(INTEL, 0x9d29), (kernel_ulong_t)&spt_info }, > { PCI_VDEVICE(INTEL, 0x9d2a), (kernel_ulong_t)&spt_info }, > - { PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_info }, > - { PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_info }, > - { PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_info }, > - { PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_info }, > - { PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_info }, > - { PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_info }, > + { PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_i2c_info }, > + { PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_i2c_info }, > + { PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_i2c_info }, > + { PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_i2c_info }, > + { PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_i2c_info }, > + { PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_i2c_info }, > { PCI_VDEVICE(INTEL, 0x9d66), (kernel_ulong_t)&spt_uart_info }, > /* SPT-H */ > { PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info }, > { PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info }, > { PCI_VDEVICE(INTEL, 0xa129), (kernel_ulong_t)&spt_info }, > { PCI_VDEVICE(INTEL, 0xa12a), (kernel_ulong_t)&spt_info }, > - { PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_info }, > - { PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_info }, > + { PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_i2c_info }, > + { PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_i2c_info }, > { PCI_VDEVICE(INTEL, 0xa166), (kernel_ulong_t)&spt_uart_info }, > { } > }; -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog