[SRU] [B/C/D] [PATCH 0/8] Sync AMD MP2 I2C driver with upstream version

Wed Apr 24 10:17:57 UTC 2019

On 24.04.19 11:52, Kai-Heng Feng wrote:
> at 17:41, Stefan Bader <stefan.bader at canonical.com> wrote:
> 
>> On 24.04.19 11:05, Kai-Heng Feng wrote:
>>> at 14:23, Khaled Elmously <khalid.elmously at canonical.com> wrote:
>>>
>>>> On 2019-04-17 22:55:42 , Kai-Heng Feng wrote:
>>>>> at 22:33, Stefan Bader <stefan.bader at canonical.com> wrote:
>>>>>
>>>>>> On 02.04.19 05:38, Kai-Heng Feng wrote:
>>>>>>> BugLink: https://bugs.launchpad.net/bugs/1787775
>>>>>>>
>>>>>>> [Impact]
>>>>>>> AMD MP2 I2C driver doesn't support more than one bus.
>>>>>>> This makes touchpad on Lenovo Yoga 530 doensn't work as expected.
>>>>>>>
>>>>>>> [Fix]
>>>>>>> Sync the MP2 driver with upstream version.
>>>>>>>
>>>>>>> [Test]
>>>>>>> The new upstream driver doesn't break on Latitude 5495.
>>>>>>> Users haven't given any feedback yet, but since Disco is freezing I
>>>>>>> am sending this out.
>>>>>>>
>>>>>>> [Regression Potential]
>>>>>>> Low. It's from upstream, and only a subset of Raven Ridge has this
>>>>>>> device.
>>>>>>>
>>>>>>> Elie Morisse (1):
>>>>>>>   i2c: Add drivers for the AMD PCIe MP2 I2C controller
>>>>>>>
>>>>>>> Kai-Heng Feng (4):
>>>>>>>   Revert "UBUNTU: SAUCE: i2c:amd Depends on ACPI"
>>>>>>>   Revert "UBUNTU: SAUCE: i2c:amd move out pointer in union
>>>>>>>     i2c_event_base"
>>>>>>>   Revert "UBUNTU: SAUCE: i2c:amd I2C Driver based on PCI Interface for
>>>>>>>     upcoming platform"
>>>>>>>   UBUNTU: [Config] Update config for AMD MP2 I2C driver
>>>>>>>
>>>>>>> Wolfram Sang (3):
>>>>>>>   i2c: add helpers to ease DMA handling
>>>>>>>   i2c: add a message flag for DMA safe buffers
>>>>>>>   i2c: add extra check to safe DMA buffer helper
>>>>>>>
>>>>>>>  Documentation/i2c/busses/i2c-amd-mp2          |  23 +
>>>>>>>  MAINTAINERS                                   |   8 +
>>>>>>>  .../config/amd64/config.common.amd64          |   2 +-
>>>>>>>  debian.master/config/i386/config.common.i386  |   2 +-
>>>>>>>  drivers/i2c/busses/Kconfig                    |  12 +-
>>>>>>>  drivers/i2c/busses/Makefile                   |   3 +-
>>>>>>>  drivers/i2c/busses/i2c-amd-mp2-pci.c          | 483 ++++++++++++++
>>>>>>>  drivers/i2c/busses/i2c-amd-mp2-plat.c         | 367 ++++++++++
>>>>>>>  drivers/i2c/busses/i2c-amd-mp2.h              | 219 ++++++
>>>>>>>  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 ----------
>>>>>>>  drivers/i2c/i2c-core-base.c                   |  51 ++
>>>>>>>  include/linux/i2c.h                           |   3 +
>>>>>>>  include/uapi/linux/i2c.h                      |   3 +
>>>>>>>  15 files changed, 1166 insertions(+), 1223 deletions(-)
>>>>>>>  create mode 100644 Documentation/i2c/busses/i2c-amd-mp2
>>>>>>>  create mode 100644 drivers/i2c/busses/i2c-amd-mp2-pci.c
>>>>>>>  create mode 100644 drivers/i2c/busses/i2c-amd-mp2-plat.c
>>>>>>>  create mode 100644 drivers/i2c/busses/i2c-amd-mp2.h
>>>>>>>  delete mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
>>>>>>>  delete mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
>>>>>>>  delete mode 100644 drivers/i2c/busses/i2c-amd-platdrv.c
>>>>>> I am a bit hesitant to do this especially in Bionic. Have you done any
>>>>>> wider
>>>>>> regression testing with other hardware which uses this driver? That one
>>>>>> Dell
>>>>>> might be better off but that would not necessarily guarantee that
>>>>>> nothing else
>>>>>> does regress.
>>>>>
>>>>> No we’ve only enabled one system with AMD MP2. So it’s the only system I can
>>>>> test.
>>>>>
>>>>> So there’s no guarantee. What I can confirm is that it used to only work on
>>>>> the Dell system, now it works on both the Dell one and Lenovo Yoga.
>>>>>
>>>>>> Also the config changes silently change things from being built-in for
>>>>>> 32bit and
>>>>>> 64bit kernels to only being available as a module for 64bit kernels.
>>>>>> That would
>>>>>> mean any user who was running 32bit will loose that driver support. And
>>>>>> you can
>>>>>> be sure that if there is any possibility of doing that, there will be
>>>>>> some user
>>>>>> who did.
>>>>>
>>>>> Ryzen based systems only support 64bits [1]:
>>>>> *OS Support
>>>>> Windows 10 - 64-Bit Edition
>>>>> RHEL x86 64-Bit
>>>>> Ubuntu x86 64-Bit
>>>>>
>>>>> [1] https://www.amd.com/en/products/apu/amd-ryzen-5-2500u
>>>>
>>>> I had thought the AMD64 specification explicitly supports i386 as "legacy
>>>> mode", as per page 2 of [2]
>>>
>>> CPU-wise yes. But the chipset and other components comes with Ryzen
>>>
>>> I just tried to boot a live 16.04.6 image on a Ryzen system, and it hangs at
>>> certain point.
>>>
>>>> Is there a reason the MP2 driver can no longer be built for 32-bit?
>>>
>>> Of course it can be built agains 32-bit, but Ryzen as a whole doesn’t really
>>> support 32-bit OS.
>>
>> This is a problem for a stable update. We have released with the driver built-in
>> for both amd64 and i386. So we cannot take away that support with any stable
>> update. Even if in theory nothing can use it.
> 
> Ok. it’s reasonable to keep it enabled in 3816. Do we need to change it back as
> builtin?

If possible I would keep both settings in sync. For the old driver both were
built-in. Since the new driver seems to allow it being a module, I would make it
a module in both amd64 and i386. It is just important that we do not change what
is available.

-Stefan

> 
> Kai-Heng
> 
>>
>> -Stefan
>>
>>> Kai-Heng
>>>
>>>> [2] https://www.amd.com/system/files/TechDocs/24592.pdf
>>>>
>>>>
>>>>
>>>>> Kai-Heng
>>>>>
>>>>>> -Stefan
>>>>>
>>>>>
>>>>>
>>>>> -- 
>>>>> kernel-team mailing list
>>>>> kernel-team at lists.ubuntu.com
>>>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190424/67813674/attachment-0001.sig>