ACK/Cmnt: [SRU] [Bionic/Unstable] [PATCH v2 0/4] Enable AMD PCIe MP2 for AMDI0011
diff mbox series

Message ID ed5020a8-8fa8-ef43-f6b1-41bc8f0729bd@canonical.com
State New
Headers show
Series
  • ACK/Cmnt: [SRU] [Bionic/Unstable] [PATCH v2 0/4] Enable AMD PCIe MP2 for AMDI0011
Related show

Commit Message

Stefan Bader Aug. 30, 2018, 9:23 a.m. UTC
On 19.07.2018 12:19, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/1773940
> 
> [Impact]
> Touchpad doesn't work on Latitude 5495.
> 
> [Test]
> I can confirm the driver from AMD works.
> With patch 2/3, no issues found so far.
> 
> [Fix]
> The touchpad connects to AMDI0011, which doesn't have any driver until
> now.
> 
> The original plan is to use an upstream version for this driver, but the
> review process took longer than anticipated, and the driver is really in
> need as the Latitude 5495 is about to hit the market.  I'll replace this
> patch set with upstream version once it's in mainline.
> 
> v2: Fix build for architectures don't support ACPI.
> 
> [Regression Potential]
> Low. It's a new driver, shouldn't affect any other device.
> 
> Kai-Heng Feng (3):
>   UBUNTU: SAUCE: i2c:amd move out pointer in union i2c_event_base
>   UBUNTU: SAUCE: i2c:amd Depends on ACPI
>   UBUNTU: [Config] i2c: CONFIG_I2C_AMD_MP2=y
> 
> Nehal-bakulchandra Shah (1):
>   UBUNTU: SAUCE: i2c:amd I2C Driver based on PCI Interface for upcoming
>     platform
> 
>  debian.master/config/config.common.ubuntu |   1 +
>  drivers/i2c/busses/Kconfig                |  10 +
>  drivers/i2c/busses/Makefile               |   2 +
>  drivers/i2c/busses/i2c-amd-pci-mp2.c      | 625 ++++++++++++++++++++++
>  drivers/i2c/busses/i2c-amd-pci-mp2.h      | 253 +++++++++
>  drivers/i2c/busses/i2c-amd-platdrv.c      | 335 ++++++++++++
>  6 files changed, 1226 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
>  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
>  create mode 100644 drivers/i2c/busses/i2c-amd-platdrv.c
> 
Ok, there is probably no choice but to build the driver into the kernel. But at
least I would suggest to limit this to x86 kernels. So replacing patch 4/4 with
the attached version. How does that sound (I am also adding the update to the
annotations file)?

-Stefan

Acked-by: Stefan Bader <stefan.bader@canonical.com>

Comments

Colin King Aug. 30, 2018, 9:28 a.m. UTC | #1
On 30/08/18 10:23, Stefan Bader wrote:
> On 19.07.2018 12:19, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1773940
>>
>> [Impact]
>> Touchpad doesn't work on Latitude 5495.
>>
>> [Test]
>> I can confirm the driver from AMD works.
>> With patch 2/3, no issues found so far.
>>
>> [Fix]
>> The touchpad connects to AMDI0011, which doesn't have any driver until
>> now.
>>
>> The original plan is to use an upstream version for this driver, but the
>> review process took longer than anticipated, and the driver is really in
>> need as the Latitude 5495 is about to hit the market.  I'll replace this
>> patch set with upstream version once it's in mainline.
>>
>> v2: Fix build for architectures don't support ACPI.
>>
>> [Regression Potential]
>> Low. It's a new driver, shouldn't affect any other device.
>>
>> Kai-Heng Feng (3):
>>   UBUNTU: SAUCE: i2c:amd move out pointer in union i2c_event_base
>>   UBUNTU: SAUCE: i2c:amd Depends on ACPI
>>   UBUNTU: [Config] i2c: CONFIG_I2C_AMD_MP2=y
>>
>> Nehal-bakulchandra Shah (1):
>>   UBUNTU: SAUCE: i2c:amd I2C Driver based on PCI Interface for upcoming
>>     platform
>>
>>  debian.master/config/config.common.ubuntu |   1 +
>>  drivers/i2c/busses/Kconfig                |  10 +
>>  drivers/i2c/busses/Makefile               |   2 +
>>  drivers/i2c/busses/i2c-amd-pci-mp2.c      | 625 ++++++++++++++++++++++
>>  drivers/i2c/busses/i2c-amd-pci-mp2.h      | 253 +++++++++
>>  drivers/i2c/busses/i2c-amd-platdrv.c      | 335 ++++++++++++
>>  6 files changed, 1226 insertions(+)
>>  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
>>  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
>>  create mode 100644 drivers/i2c/busses/i2c-amd-platdrv.c
>>
> Ok, there is probably no choice but to build the driver into the kernel. But at
> least I would suggest to limit this to x86 kernels. So replacing patch 4/4 with
> the attached version. How does that sound (I am also adding the update to the
> annotations file)?
> 
> -Stefan
> 
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
> 
>
I think Stefan's proposal for x86 amd64 and i386 only kernels for this
fix is the best way forward since we know ACPI is implicit for this
arches. And so.. for the patches + update of the annotations file:

Acked-by: Colin Ian King <colin.king@canonical.com>
Kai-Heng Feng Aug. 30, 2018, 9:43 a.m. UTC | #2
at 17:23, Stefan Bader <stefan.bader@canonical.com> wrote:

> On 19.07.2018 12:19, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1773940
>>
>> [Impact]
>> Touchpad doesn't work on Latitude 5495.
>>
>> [Test]
>> I can confirm the driver from AMD works.
>> With patch 2/3, no issues found so far.
>>
>> [Fix]
>> The touchpad connects to AMDI0011, which doesn't have any driver until
>> now.
>>
>> The original plan is to use an upstream version for this driver, but the
>> review process took longer than anticipated, and the driver is really in
>> need as the Latitude 5495 is about to hit the market.  I'll replace this
>> patch set with upstream version once it's in mainline.
>>
>> v2: Fix build for architectures don't support ACPI.
>>
>> [Regression Potential]
>> Low. It's a new driver, shouldn't affect any other device.
>>
>> Kai-Heng Feng (3):
>>   UBUNTU: SAUCE: i2c:amd move out pointer in union i2c_event_base
>>   UBUNTU: SAUCE: i2c:amd Depends on ACPI
>>   UBUNTU: [Config] i2c: CONFIG_I2C_AMD_MP2=y
>>
>> Nehal-bakulchandra Shah (1):
>>   UBUNTU: SAUCE: i2c:amd I2C Driver based on PCI Interface for upcoming
>>     platform
>>
>>  debian.master/config/config.common.ubuntu |   1 +
>>  drivers/i2c/busses/Kconfig                |  10 +
>>  drivers/i2c/busses/Makefile               |   2 +
>>  drivers/i2c/busses/i2c-amd-pci-mp2.c      | 625 ++++++++++++++++++++++
>>  drivers/i2c/busses/i2c-amd-pci-mp2.h      | 253 +++++++++
>>  drivers/i2c/busses/i2c-amd-platdrv.c      | 335 ++++++++++++
>>  6 files changed, 1226 insertions(+)
>>  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
>>  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
>>  create mode 100644 drivers/i2c/busses/i2c-amd-platdrv.c
> Ok, there is probably no choice but to build the driver into the kernel.  
> But at
> least I would suggest to limit this to x86 kernels. So replacing patch  
> 4/4 with
> the attached version. How does that sound (I am also adding the update to  
> the
> annotations file)?

This makes sense and it's probably the best approach in the interim.

Thanks!

Kai-Heng

>
> -Stefan
>
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
> <0001-UBUNTU-Config-i2c-CONFIG_I2C_AMD_MP2-y-on-x86.patch>

Patch
diff mbox series

From fe6885022f95065fe1393a7cd69e78e243450d0a Mon Sep 17 00:00:00 2001
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
Date: Thu, 19 Jul 2018 12:19:00 +0200
Subject: [PATCH] UBUNTU: [Config] i2c: CONFIG_I2C_AMD_MP2=y on x86

Build the new driver into x86 based kernels.

BugLink: https://bugs.launchpad.net/bugs/1773940

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 debian.master/config/amd64/config.common.amd64 | 1 +
 debian.master/config/annotations               | 1 +
 debian.master/config/arm64/config.common.arm64 | 1 +
 debian.master/config/i386/config.common.i386   | 1 +
 4 files changed, 4 insertions(+)

diff --git a/debian.master/config/amd64/config.common.amd64 b/debian.master/config/amd64/config.common.amd64
index c380997..06bd75a 100644
--- a/debian.master/config/amd64/config.common.amd64
+++ b/debian.master/config/amd64/config.common.amd64
@@ -159,6 +159,7 @@  CONFIG_HYPERV_TSCPAGE=y
 # CONFIG_HZ_100 is not set
 CONFIG_I2C=y
 CONFIG_I2C_ALGOBIT=m
+CONFIG_I2C_AMD_MP2=y
 # CONFIG_I2C_EMEV2 is not set
 # CONFIG_I2C_SLAVE is not set
 CONFIG_I6300ESB_WDT=m
diff --git a/debian.master/config/annotations b/debian.master/config/annotations
index 5d8144b..4e24cfa 100644
--- a/debian.master/config/annotations
+++ b/debian.master/config/annotations
@@ -2455,6 +2455,7 @@  CONFIG_I2C_CHARDEV                              note<LP:1417032>
 
 # Menu: Device Drivers >> I2C support >> I2C support >> I2C Algorithms
 CONFIG_I2C_ALGOBIT                              policy<{'amd64': 'm', 'arm64': 'm', 'armhf': 'm', 'i386': 'm', 'ppc64el': 'm'}>
+CONFIG_I2C_AMD_MP2				policy<{'amd64': 'y', 'arm64': 'n', 'armhf': 'n', 'i386': 'y', 'ppc64el': 'n'}>
 CONFIG_I2C_ALGOPCA                              policy<{'amd64': 'm', 'arm64': 'm', 'armhf': 'm', 'i386': 'm', 'ppc64el': 'm'}>
 
 # Menu: Device Drivers >> I2C support >> I2C support >> I2C Hardware Bus support
diff --git a/debian.master/config/arm64/config.common.arm64 b/debian.master/config/arm64/config.common.arm64
index df2eb44..67f8bae 100644
--- a/debian.master/config/arm64/config.common.arm64
+++ b/debian.master/config/arm64/config.common.arm64
@@ -165,6 +165,7 @@  CONFIG_HZ=250
 # CONFIG_HZ_1000 is not set
 CONFIG_HZ_250=y
 CONFIG_I2C=y
+# CONFIG_I2C_AMD_MP2 is not set
 # CONFIG_I2C_EMEV2 is not set
 CONFIG_I2C_IMX=m
 CONFIG_I2C_SLAVE=y
diff --git a/debian.master/config/i386/config.common.i386 b/debian.master/config/i386/config.common.i386
index 85d45bd..1a87045 100644
--- a/debian.master/config/i386/config.common.i386
+++ b/debian.master/config/i386/config.common.i386
@@ -155,6 +155,7 @@  CONFIG_HW_RANDOM_TIMERIOMEM=m
 # CONFIG_HZ_100 is not set
 CONFIG_I2C=y
 CONFIG_I2C_ALGOBIT=m
+CONFIG_I2C_AMD_MP2=y
 # CONFIG_I2C_EMEV2 is not set
 # CONFIG_I2C_SLAVE is not set
 CONFIG_I6300ESB_WDT=m
-- 
2.7.4