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

On Thu, Mar 30, 2017 at 1:23 PM, dann frazier
<dann.frazier@canonical.com> wrote:
> 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).

We've also since given it a (successful) spin on s390x for good measure

>> 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! :)

Here's an updated PR that increases the buffer size to match the
length attribute which, as you noted off-list, follows a pattern used
elsewhere in the kernel. I've also provided this feedback in a review
of the current LPC patch series.

The following changes since commit 815d83d7188878caeaff28a15be14a8d676dc045:

  Linux 4.10.10 (2017-04-12 07:26:53 -0600)

are available in the git repository at:

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

for you to fetch changes up to 2ff710b9447e5e50533970c5740189fbf1fff58a:

  UBUNTU: SAUCE: LIBIO: Make the size of the acpi_resource buffer
match it's length property (2017-04-20 15:20:19 -0600)

----------------------------------------------------------------
dann frazier (4):
      UBUNTU: [Config] CONFIG_LIBIO=y on arm64 only
      UBUNTU: [Config] CONFIG_HISILICON_LPC=y
      UBUNTU: SAUCE: PCI: Restore codepath for !CONFIG_LIBIO
      UBUNTU: SAUCE: LIBIO: Make the size of the acpi_resource buffer
match it's length property

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

 .../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

On Thu, Apr 20, 2017 at 03:23:33PM -0600, dann frazier wrote:
> Here's an updated PR that increases the buffer size to match the
> length attribute which, as you noted off-list, follows a pattern used
> elsewhere in the kernel. I've also provided this feedback in a review
> of the current LPC patch series.
> 
> The following changes since commit 815d83d7188878caeaff28a15be14a8d676dc045:
> 
>   Linux 4.10.10 (2017-04-12 07:26:53 -0600)
> 
> are available in the git repository at:
> 
>   git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc-20170420
> 
> for you to fetch changes up to 2ff710b9447e5e50533970c5740189fbf1fff58a:
> 
>   UBUNTU: SAUCE: LIBIO: Make the size of the acpi_resource buffer
> match it's length property (2017-04-20 15:20:19 -0600)

Sorry for the delay in getting this review back to you.

One thing you might want to consider. I note that in the latest upstream
submission this has changed from libio to some other name. Adopting what
will (presumably) be the name used upstream might make it less
cumbersome when you need to backport future changes.

At this point though this looks safe enough to merge.

- Minimal impact to non-arm64 architectures
- New drivers will only be used on relevant platforms, so no impact on
  other arm64 platforms from those drivers
- Impact to other arm64 platforms is kept as minimal as possible
- Smoke testing against other architectures and platforms

Acked-by: Seth Forshee <seth.forshee@canonical.com>

On Mon, Apr 24, 2017 at 8:20 AM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Thu, Apr 20, 2017 at 03:23:33PM -0600, dann frazier wrote:
>> Here's an updated PR that increases the buffer size to match the
>> length attribute which, as you noted off-list, follows a pattern used
>> elsewhere in the kernel. I've also provided this feedback in a review
>> of the current LPC patch series.
>>
>> The following changes since commit 815d83d7188878caeaff28a15be14a8d676dc045:
>>
>>   Linux 4.10.10 (2017-04-12 07:26:53 -0600)
>>
>> are available in the git repository at:
>>
>>   git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc-20170420
>>
>> for you to fetch changes up to 2ff710b9447e5e50533970c5740189fbf1fff58a:
>>
>>   UBUNTU: SAUCE: LIBIO: Make the size of the acpi_resource buffer
>> match it's length property (2017-04-20 15:20:19 -0600)
>
> Sorry for the delay in getting this review back to you.
>
> One thing you might want to consider. I note that in the latest upstream
> submission this has changed from libio to some other name. Adopting what
> will (presumably) be the name used upstream might make it less
> cumbersome when you need to backport future changes.

The name has changed a few times already (extio->libio->logic pio),
and I'm not confident the last one will stick either. The current name
was not one of the ones upstream suggested, and it hasn't been ACK'd
yet. My preference would therefore be to merge as-is, unless you feel
strongly otherwise.

> At this point though this looks safe enough to merge.
>
> - Minimal impact to non-arm64 architectures
> - New drivers will only be used on relevant platforms, so no impact on
>   other arm64 platforms from those drivers
> - Impact to other arm64 platforms is kept as minimal as possible
> - Smoke testing against other architectures and platforms
>
> Acked-by: Seth Forshee <seth.forshee@canonical.com>

Thanks!

  -dann

On Mon, Apr 24, 2017 at 03:39:08PM -0600, dann frazier wrote:
> On Mon, Apr 24, 2017 at 8:20 AM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > On Thu, Apr 20, 2017 at 03:23:33PM -0600, dann frazier wrote:
> >> Here's an updated PR that increases the buffer size to match the
> >> length attribute which, as you noted off-list, follows a pattern used
> >> elsewhere in the kernel. I've also provided this feedback in a review
> >> of the current LPC patch series.
> >>
> >> The following changes since commit 815d83d7188878caeaff28a15be14a8d676dc045:
> >>
> >>   Linux 4.10.10 (2017-04-12 07:26:53 -0600)
> >>
> >> are available in the git repository at:
> >>
> >>   git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc-20170420
> >>
> >> for you to fetch changes up to 2ff710b9447e5e50533970c5740189fbf1fff58a:
> >>
> >>   UBUNTU: SAUCE: LIBIO: Make the size of the acpi_resource buffer
> >> match it's length property (2017-04-20 15:20:19 -0600)
> >
> > Sorry for the delay in getting this review back to you.
> >
> > One thing you might want to consider. I note that in the latest upstream
> > submission this has changed from libio to some other name. Adopting what
> > will (presumably) be the name used upstream might make it less
> > cumbersome when you need to backport future changes.
> 
> The name has changed a few times already (extio->libio->logic pio),
> and I'm not confident the last one will stick either. The current name
> was not one of the ones upstream suggested, and it hasn't been ACK'd
> yet. My preference would therefore be to merge as-is, unless you feel
> strongly otherwise.

No, it's fine. I only mentioned it because I thought it might make your
future backporting work easier.

Seth

Ack for this patch set.

 -Kamal

On Thu, Apr 20, 2017 at 03:23:33PM -0600, dann frazier wrote:
> [...]
> 
> Here's an updated PR that increases the buffer size to match the
> length attribute which, as you noted off-list, follows a pattern used
> elsewhere in the kernel. I've also provided this feedback in a review
> of the current LPC patch series.
> 
> The following changes since commit 815d83d7188878caeaff28a15be14a8d676dc045:
> 
>   Linux 4.10.10 (2017-04-12 07:26:53 -0600)
> 
> are available in the git repository at:
> 
>   git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc-20170420
> 
> for you to fetch changes up to 2ff710b9447e5e50533970c5740189fbf1fff58a:
> 
>   UBUNTU: SAUCE: LIBIO: Make the size of the acpi_resource buffer
> match it's length property (2017-04-20 15:20:19 -0600)
> 
> ----------------------------------------------------------------
> dann frazier (4):
>       UBUNTU: [Config] CONFIG_LIBIO=y on arm64 only
>       UBUNTU: [Config] CONFIG_HISILICON_LPC=y
>       UBUNTU: SAUCE: PCI: Restore codepath for !CONFIG_LIBIO
>       UBUNTU: SAUCE: LIBIO: Make the size of the acpi_resource buffer
> match it's length property
> 
> 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
> 
>  .../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
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team