Re: [RESEND PATCH RFC V4 0/3] Generate device tree node for pci devices

[Lizhi Hou <lizhi.hou@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 8439 bytes --]


On 12/1/22 13:20, Rob Herring wrote:
> On Mon, Nov 21, 2022 at 08:43:01AM -0800, Lizhi Hou wrote:
>> This patch series introduces OF overlay support for PCI devices which
>> primarily addresses two use cases. First, it provides a data driven method
>> to describe hardware peripherals that are present in a PCI endpoint and
>> hence can be accessed by the PCI host. Second, it allows reuse of a OF
>> compatible driver -- often used in SoC platforms -- in a PCI host based
>> system.
>>
>> There are 2 series devices rely on this patch:
>>
>>    1) Xilinx Alveo Accelerator cards (FPGA based device)
>>    2) Microchip LAN9662 Ethernet Controller
>>
>>       Please see: https://lore.kernel.org/lkml/20220427094502.456111-1-clement.leger@bootlin.com/
>>
>> Normally, the PCI core discovers PCI devices and their BARs using the
>> PCI enumeration process. However, the process does not provide a way to
>> discover the hardware peripherals that are present in a PCI device, and
>> which can be accessed through the PCI BARs. Also, the enumeration process
>> does not provide a way to associate MSI-X vectors of a PCI device with the
>> hardware peripherals that are present in the device. PCI device drivers
>> often use header files to describe the hardware peripherals and their
>> resources as there is no standard data driven way to do so. This patch
>> series proposes to use flattened device tree blob to describe the
>> peripherals in a data driven way. Based on previous discussion, using
>> device tree overlay is the best way to unflatten the blob and populate
>> platform devices. To use device tree overlay, there are three obvious
>> problems that need to be resolved.
>>
>> First, we need to create a base tree for non-DT system such as x86_64. A
>> patch series has been submitted for this:
>> https://lore.kernel.org/lkml/20220624034327.2542112-1-frowand.list@gmail.com/
>> https://lore.kernel.org/lkml/20220216050056.311496-1-lizhi.hou@xilinx.com/
>>
>> Second, a device tree node corresponding to the PCI endpoint is required
>> for overlaying the flattened device tree blob for that PCI endpoint.
>> Because PCI is a self-discoverable bus, a device tree node is usually not
>> created for PCI devices. This series adds support to generate a device
>> tree node for a PCI device which advertises itself using PCI quirks
>> infrastructure.
>>
>> Third, we need to generate device tree nodes for PCI bridges since a child
>> PCI endpoint may choose to have a device tree node created.
>>
>> This patch series is made up of three patches.
>>
>> The first patch is adding OF interface to create or destroy OF node
>> dynamically.
>>
>> The second patch introduces a kernel option, CONFIG_DYNAMIC_PCI_OF_NODEX.
>> When the option is turned on, the kernel will generate device tree nodes
>> for all PCI bridges unconditionally. The patch also shows how to use the
>> PCI quirks infrastructure, DECLARE_PCI_FIXUP_FINAL to generate a device
>> tree node for a device. Specifically, the patch generates a device tree
>> node for Xilinx Alveo U50 PCIe accelerator device. The generated device
>> tree nodes do not have any property.
>>
>> The third patch adds basic properties ('reg', 'compatible' and
>> 'device_type') to the dynamically generated device tree nodes. More
>> properties can be added in the future.
>>
>> Here is the example of device tree nodes generated within the ARM64 QEMU.
> Can you share the setup for QEMU.
I attached the VM xml config file. Should I include the configure in 
cover letter next time?
>
>
>> # lspci -t
>> -[0000:00]-+-00.0
>>             +-01.0-[01]--
>>             +-01.1-[02]----00.0
>>             +-01.2-[03]----00.0
>>             +-01.3-[04]----00.0
>>             +-01.4-[05]----00.0
>>             +-01.5-[06]--
>>             +-01.6-[07]--
>>             +-01.7-[08]--
>>             +-02.0-[09-0b]----00.0-[0a-0b]----00.0-[0b]--+-00.0
>>             |                                            \-00.1
>>             +-02.1-[0c]--
>>             \-03.0-[0d-0e]----00.0-[0e]----01.0
>>
>> # tree /sys/firmware/devicetree/base/pcie\@10000000
>> /sys/firmware/devicetree/base/pcie@10000000
>> |-- #address-cells
>> |-- #interrupt-cells
>> |-- #size-cells
>> |-- bus-range
>> |-- compatible
>> |-- device_type
>> |-- dma-coherent
>> |-- interrupt-map
>> |-- interrupt-map-mask
>> |-- linux,pci-domain
>> |-- msi-parent
>> |-- name
>> |-- pci@1,0
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@1,1
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@1,2
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@1,3
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@1,4
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@1,5
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@1,6
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@1,7
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@2,0
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- pci@0,0
>> |   |   |-- #address_cells
>> |   |   |-- #size_cells
>> |   |   |-- compatible
>> |   |   |-- device_type
>> |   |   |-- pci@0,0
>> |   |   |   |-- #address_cells
>> |   |   |   |-- #size_cells
>> |   |   |   |-- compatible
>> |   |   |   |-- dev@0,0
> I don't see anywhere in the patch series defining 'dev'.

It is defined here:

 > +void of_pci_make_dev_node(struct pci_dev *pdev)
 > +{
 > +    struct device_node *parent, *dt_node = NULL;
 > +    const char *pci_type = "dev";


Thanks,

Lizhi

>
>> |   |   |   |   |-- compatible
>> |   |   |   |   `-- reg
>> |   |   |   |-- dev@0,1
>> |   |   |   |   |-- compatible
>> |   |   |   |   `-- reg
>> |   |   |   |-- device_type
>> |   |   |   |-- ranges
>> |   |   |   `-- reg
>> |   |   |-- ranges
>> |   |   `-- reg
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@2,1
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- ranges
>> |   `-- reg
>> |-- pci@3,0
>> |   |-- #address_cells
>> |   |-- #size_cells
>> |   |-- compatible
>> |   |-- device_type
>> |   |-- pci@0,0
>> |   |   |-- #address_cells
>> |   |   |-- #size_cells
>> |   |   |-- compatible
>> |   |   |-- device_type
>> |   |   |-- ranges
>> |   |   `-- reg
>> |   |-- ranges
>> |   `-- reg
>> |-- ranges
>> `-- reg
>>
>> Changes since RFC v3:
>> - Split the Xilinx Alveo U50 PCI quirk to a separate patch
>> - Minor changes in commit description and code comment
>>
>> Changes since RFC v2:
>> - Merged patch 3 with patch 2
>> - Added OF interfaces of_changeset_add_prop_* and use them to create
>>    properties.
>> - Added '#address-cells', '#size-cells' and 'ranges' properties.
>>
>> Changes since RFC v1:
>> - Added one patch to create basic properties.
>> - To move DT related code out of PCI subsystem, replaced of_node_alloc()
>>    with of_create_node()/of_destroy_node()
>>
>> Lizhi Hou (3):
>>    of: dynamic: Add interfaces for creating device node dynamically
>>    PCI: Create device tree node for selected devices
>>    PCI: Add PCI quirks to generate device tree node for Xilinx Alveo U50
>>
>>   drivers/of/dynamic.c        | 187 ++++++++++++++++++++++++++
>>   drivers/pci/Kconfig         |  12 ++
>>   drivers/pci/Makefile        |   1 +
>>   drivers/pci/bus.c           |   2 +
>>   drivers/pci/msi/irqdomain.c |   6 +-
>>   drivers/pci/of.c            |  71 ++++++++++
>>   drivers/pci/of_property.c   | 256 ++++++++++++++++++++++++++++++++++++
>>   drivers/pci/pci-driver.c    |   3 +-
>>   drivers/pci/pci.h           |  19 +++
>>   drivers/pci/quirks.c        |  11 ++
>>   drivers/pci/remove.c        |   1 +
>>   include/linux/of.h          |  24 ++++
>>   12 files changed, 590 insertions(+), 3 deletions(-)
>>   create mode 100644 drivers/pci/of_property.c
>>
>> -- 
>> 2.17.1
>>
>>

[-- Attachment #2: arm64-vm.xml --]
[-- Type: text/xml, Size: 6195 bytes --]

<domain type='qemu'>
  <name>arm64-vm</name>
  <uuid>53d819ef-d628-4337-9a21-14468e6c4436</uuid>
  <memory unit='KiB'>2097152</memory>
  <currentMemory unit='KiB'>2097152</currentMemory>
  <vcpu placement='static'>2</vcpu>
  <os>
    <type arch='aarch64' machine='virt-2.11'>hvm</type>
    <kernel>/root/pcie-dt-images/vmlinuz-6.0.0-rc5+</kernel>
    <initrd>/root/pcie-dt-images/initrd.img-6.0.0-rc5+</initrd>
    <cmdline>console=ttyAMA0,115200 root=/dev/vda net.ifnames=0 biosdevname=0</cmdline>
    <boot dev='hd'/>
    <bootmenu enable='no'/>
  </os>
  <features>
    <gic version='2'/>
  </features>
  <cpu mode='custom' match='exact' check='none'>
    <model fallback='allow'>cortex-a57</model>
  </cpu>
  <clock offset='utc'/>
  <on_poweroff>destroy</on_poweroff>
  <on_reboot>restart</on_reboot>
  <on_crash>destroy</on_crash>
  <devices>
    <emulator>/usr/local/bin/qemu-system-aarch64</emulator>
    <disk type='file' device='disk'>
      <driver name='qemu' type='raw'/>
      <source file='/root/arm64-vm.img'/>
      <target dev='vda' bus='virtio'/>
      <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/>
    </disk>
    <controller type='usb' index='0' model='qemu-xhci' ports='8'>
      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
    </controller>
    <controller type='pci' index='0' model='pcie-root'/>
    <controller type='pci' index='1' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='1' port='0x8'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/>
    </controller>
    <controller type='pci' index='2' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='2' port='0x9'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
    </controller>
    <controller type='pci' index='3' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='3' port='0xa'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
    </controller>
    <controller type='pci' index='4' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='4' port='0xb'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x3'/>
    </controller>
    <controller type='pci' index='5' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='5' port='0xc'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x4'/>
    </controller>
    <controller type='pci' index='6' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='6' port='0xd'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x5'/>
    </controller>
    <controller type='pci' index='7' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='7' port='0xe'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x6'/>
    </controller>
    <controller type='pci' index='8' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='8' port='0xf'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x7'/>
    </controller>
    <controller type='pci' index='9' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='9' port='0x10'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0' multifunction='on'/>
    </controller>
    <controller type='pci' index='10' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='10' port='0x11'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/>
    </controller>
    <controller type='pci' index='11' model='pcie-switch-upstream-port'>
      <model name='x3130-upstream'/>
      <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/>
    </controller>
    <controller type='pci' index='12' model='pcie-switch-downstream-port'>
      <model name='xio3130-downstream'/>
      <target chassis='11' port='0x0'/>
      <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0' multifunction='on'/>
    </controller>
    <controller type='pci' index='13' model='dmi-to-pci-bridge'>
      <model name='i82801b11-bridge'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </controller>
    <controller type='pci' index='14' model='pci-bridge'>
      <model name='pci-bridge'/>
      <target chassisNr='14'/>
      <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/>
    </controller>
    <controller type='virtio-serial' index='0'>
      <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/>
    </controller>
    <interface type='network'>
      <mac address='52:54:00:65:8e:e4'/>
      <source network='default'/>
      <model type='rtl8139'/>
      <address type='pci' domain='0x0000' bus='0x0e' slot='0x01' function='0x0'/>
    </interface>
    <serial type='pty'>
      <target type='system-serial' port='0'>
        <model name='pl011'/>
      </target>
    </serial>
    <console type='pty'>
      <target type='serial' port='0'/>
    </console>
    <channel type='unix'>
      <target type='virtio' name='org.qemu.guest_agent.0'/>
      <address type='virtio-serial' controller='0' bus='0' port='1'/>
    </channel>
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <source>
        <address domain='0x0000' bus='0x65' slot='0x00' function='0x0'/>
      </source>
      <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0' multifunction='on'/>
    </hostdev>
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <source>
        <address domain='0x0000' bus='0x65' slot='0x00' function='0x1'/>
      </source>
      <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x1'/>
    </hostdev>
    <rng model='virtio'>
      <backend model='random'>/dev/urandom</backend>
      <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
    </rng>
  </devices>
</domain>