mbox

[PULL,Zesty] Low-pin-count (LPC) controller support for arm64

Message ID CALdTtnt1W=grqAEvNwwD-tq6v04EQSSmH=dbXkByRpVh7cx4Eg@mail.gmail.com
State New
Headers show

Pull-request

git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc

Message

dann frazier March 30, 2017, 12:58 a.m. UTC
This is a port of the LIBIO infrastructure being prepared upstream to
handle the low-pin-count (LPC) controller support needed for HiSilicon
server SoCs, and which is also evolving into a generic PIO mapping
system for the kernel. While the direction has upstream approval and
some patches are acked, it is still evolving. I've therefore only
enabled CONFIG_LIBIO on arm64, and also added a patch to revert to the
existing PCI PIO mapping code when CONFIG_LIBIO is disabled.

Tested on a HiSilicon D05 board. Regression tested on ppc64el by Manoj
Iyer (for the OF change). Also regression tested on a Cavium ThunderX
system. Built tested on all Ubuntu archs in ppa:dannf/test.

Addresses: https://bugs.launchpad.net/bugs/1677319

The following changes since commit 4bd7900ebaac4fd554f8b062954175a1de04686f:

  UBUNTU: SAUCE: (no-up) net/mlx5: Avoid dereferencing uninitialized
pointer (2017-03-29 06:19:51 -0600)

are available in the git repository at:

  git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc

for you to fetch changes up to 4354f8bbb202ae8374664b907d4a432e48cc057f:

  UBUNTU: SAUCE: PCI: Restore codepath for !CONFIG_LIBIO (2017-03-29
15:33:44 -0600)

----------------------------------------------------------------
dann frazier (3):
      UBUNTU: [Config] CONFIG_LIBIO=y on arm64 only
      UBUNTU: [Config] CONFIG_HISILICON_LPC=y
      UBUNTU: SAUCE: PCI: Restore codepath for !CONFIG_LIBIO

zhichang.yuan (6):
      UBUNTU: SAUCE: LIBIO: Introduce a generic PIO mapping method
      UBUNTU: SAUCE: OF: Add missing I/O range exception for indirect-IO devices
      UBUNTU: SAUCE: LPC: Support the device-tree LPC host on Hip06/Hip07
      UBUNTU: SAUCE: LIBIO: Support the dynamically logical PIO
registration of ACPI host I/O
      UBUNTU: SAUCE: LPC: Add the ACPI LPC support
      UBUNTU: SAUCE: PCI: Apply the new generic I/O management on PCI IO hosts

 Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
|  33 +++
 MAINTAINERS
      |   8 +
 arch/arm64/boot/dts/hisilicon/hip06-d03.dts
      |   4 +
 arch/arm64/boot/dts/hisilicon/hip06.dtsi
      |  14 +
 arch/arm64/boot/dts/hisilicon/hip07-d05.dts
      |   4 +
 arch/arm64/boot/dts/hisilicon/hip07.dtsi
      |  14 +
 debian.master/config/amd64/config.common.amd64
      |   1 +
 debian.master/config/arm64/config.common.arm64
      |   1 +
 debian.master/config/armhf/config.common.armhf
      |   1 +
 debian.master/config/config.common.ubuntu
      |   1 +
 debian.master/config/i386/config.common.i386
      |   1 +
 debian.master/config/ppc64el/config.common.ppc64el
      |   1 +
 debian.master/config/s390x/config.common.s390x
      |   1 +
 drivers/acpi/pci_root.c
      |   8 +-
 drivers/bus/Kconfig
      |   8 +
 drivers/bus/Makefile
      |   1 +
 drivers/bus/hisi_lpc.c
      | 640 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/address.c
      |  95 +++++--
 drivers/pci/pci.c
      |  47 +++-
 include/asm-generic/io.h
      |  50 ++++
 include/linux/libio.h
      | 120 +++++++++
 include/linux/pci.h
      |   3 +-
 lib/Kconfig
      |  14 +
 lib/Makefile
      |   2 +
 lib/libio.c
      | 553 +++++++++++++++++++++++++++++++++++++++
 25 files changed, 1599 insertions(+), 26 deletions(-)
 create mode 100644
Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
 create mode 100644 drivers/bus/hisi_lpc.c
 create mode 100644 include/linux/libio.h
 create mode 100644 lib/libio.c

Comments

Seth Forshee March 30, 2017, 4:04 p.m. UTC | #1
On Wed, Mar 29, 2017 at 06:58:58PM -0600, dann frazier wrote:
> This is a port of the LIBIO infrastructure being prepared upstream to
> handle the low-pin-count (LPC) controller support needed for HiSilicon
> server SoCs, and which is also evolving into a generic PIO mapping
> system for the kernel. While the direction has upstream approval and
> some patches are acked, it is still evolving. I've therefore only
> enabled CONFIG_LIBIO on arm64, and also added a patch to revert to the
> existing PCI PIO mapping code when CONFIG_LIBIO is disabled.
> 
> Tested on a HiSilicon D05 board. Regression tested on ppc64el by Manoj
> Iyer (for the OF change). Also regression tested on a Cavium ThunderX
> system. Built tested on all Ubuntu archs in ppa:dannf/test.
> 
> Addresses: https://bugs.launchpad.net/bugs/1677319
> 
> The following changes since commit 4bd7900ebaac4fd554f8b062954175a1de04686f:
> 
>   UBUNTU: SAUCE: (no-up) net/mlx5: Avoid dereferencing uninitialized
> pointer (2017-03-29 06:19:51 -0600)
> 
> are available in the git repository at:
> 
>   git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc
> 
> for you to fetch changes up to 4354f8bbb202ae8374664b907d4a432e48cc057f:
> 
>   UBUNTU: SAUCE: PCI: Restore codepath for !CONFIG_LIBIO (2017-03-29
> 15:33:44 -0600)

Some of this isn't exactly pretty. But it does look to keep nearly all
the changes in behavior isolated to arm64. Additionally, a good deal of
it is isolated to device-specific code. I think the OF changes look
safe, and testing seems to bear that out.

Given that we're very late in the zesty development cycle, the major
concern I have is the extent of regression testing on other arm64
platforms, with regard to the libio/pci changes in particular. I think
it would be better to try and get some broader testing, then SRU these a
bit later.

One curious thing I encountered while looking through the patches that I
hadn't noticed previously:

+static acpi_status
+acpi_build_libiores_template(struct acpi_device *adev,
+                       struct acpi_buffer *buffer)
+{
+       acpi_handle handle = adev->handle;
+       struct acpi_resource *resource;
+       acpi_status status;
+       int res_cnt = 0;
+
+       status = acpi_walk_resources(handle, METHOD_NAME__CRS,
+                                    acpi_count_libiores, &res_cnt);
+       if (ACPI_FAILURE(status) || !res_cnt) {
+               dev_err(&adev->dev, "can't evaluate _CRS: %d\n", status);
+               return -EINVAL;
+       }
+
+       buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1;
+       buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL);

I don't understand allocating the buffer one byte less than
buffer->length. Can you confirm that this isn't a bug?

Thanks,
Seth
dann frazier March 30, 2017, 7:23 p.m. UTC | #2
On Thu, Mar 30, 2017 at 10:04 AM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Wed, Mar 29, 2017 at 06:58:58PM -0600, dann frazier wrote:
>> This is a port of the LIBIO infrastructure being prepared upstream to
>> handle the low-pin-count (LPC) controller support needed for HiSilicon
>> server SoCs, and which is also evolving into a generic PIO mapping
>> system for the kernel. While the direction has upstream approval and
>> some patches are acked, it is still evolving. I've therefore only
>> enabled CONFIG_LIBIO on arm64, and also added a patch to revert to the
>> existing PCI PIO mapping code when CONFIG_LIBIO is disabled.
>>
>> Tested on a HiSilicon D05 board. Regression tested on ppc64el by Manoj
>> Iyer (for the OF change). Also regression tested on a Cavium ThunderX
>> system. Built tested on all Ubuntu archs in ppa:dannf/test.
>>
>> Addresses: https://bugs.launchpad.net/bugs/1677319
>>
>> The following changes since commit 4bd7900ebaac4fd554f8b062954175a1de04686f:
>>
>>   UBUNTU: SAUCE: (no-up) net/mlx5: Avoid dereferencing uninitialized
>> pointer (2017-03-29 06:19:51 -0600)
>>
>> are available in the git repository at:
>>
>>   git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc
>>
>> for you to fetch changes up to 4354f8bbb202ae8374664b907d4a432e48cc057f:
>>
>>   UBUNTU: SAUCE: PCI: Restore codepath for !CONFIG_LIBIO (2017-03-29
>> 15:33:44 -0600)
>
> Some of this isn't exactly pretty.

I don't disagree.

> But it does look to keep nearly all
> the changes in behavior isolated to arm64. Additionally, a good deal of
> it is isolated to device-specific code. I think the OF changes look
> safe, and testing seems to bear that out.

Yeah - tested on Power, and 2 arm64 systems in device-tree mode that
have PCIe (Cavium ThunderX CRB, HP X-Gene m400).

> Given that we're very late in the zesty development cycle, the major
> concern I have is the extent of regression testing on other arm64
> platforms, with regard to the libio/pci changes in particular. I think
> it would be better to try and get some broader testing, then SRU these a
> bit later.

Also tested on a QDF2400, which is an arm64 server that boots in ACPI mode.

> One curious thing I encountered while looking through the patches that I
> hadn't noticed previously:
>
> +static acpi_status
> +acpi_build_libiores_template(struct acpi_device *adev,
> +                       struct acpi_buffer *buffer)
> +{
> +       acpi_handle handle = adev->handle;
> +       struct acpi_resource *resource;
> +       acpi_status status;
> +       int res_cnt = 0;
> +
> +       status = acpi_walk_resources(handle, METHOD_NAME__CRS,
> +                                    acpi_count_libiores, &res_cnt);
> +       if (ACPI_FAILURE(status) || !res_cnt) {
> +               dev_err(&adev->dev, "can't evaluate _CRS: %d\n", status);
> +               return -EINVAL;
> +       }
> +
> +       buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1;
> +       buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL);
>
> I don't understand allocating the buffer one byte less than
> buffer->length. Can you confirm that this isn't a bug?

Will do. Thanks for the (what is this now, 5th?) review! :)

  -dann
Stefan Bader May 4, 2017, 9:19 a.m. UTC | #3
Applied to master-next on Zesty.