2022-03-05 06:23:12

by Lizhi Hou

[permalink] [raw]
Subject: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

Create device tree binding document for PCIe endpoint bus.

Signed-off-by: Sonal Santan <[email protected]>
Signed-off-by: Max Zhen <[email protected]>
Signed-off-by: Lizhi Hou <[email protected]>
---
.../devicetree/bindings/bus/pci-ep-bus.yaml | 72 +++++++++++++++++++
1 file changed, 72 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml

diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
new file mode 100644
index 000000000000..0ca96298db6f
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PCIe Endpoint Bus binding
+
+description: |
+ PCIe device may use flattened device tree to describe apertures in its
+ PCIe BARs. The Bus PCIe endpoint node is created and attached under the
+ device tree root node for this kind of device. Then the flatten device
+ tree overlay for this device is attached under the endpoint node.
+
+ The aperture address which is under the endpoint node consists of BAR
+ index and offset. It uses the following encoding:
+
+ 0xIooooooo 0xoooooooo
+
+ Where:
+
+ I = BAR index
+ oooooo oooooooo = BAR offset
+
+ The endpoint is compatible with 'simple-bus' and contains 'ranges'
+ property for translating aperture address to CPU address.
+
+allOf:
+ - $ref: /schemas/simple-bus.yaml#
+
+maintainers:
+ - Lizhi Hou <[email protected]>
+
+properties:
+ compatible:
+ contains:
+ const: pci-ep-bus
+
+ "#address-cells":
+ const: 2
+
+ "#size-cells":
+ const: 2
+
+ ranges: true
+
+patternProperties:
+ "^.*@[0-9a-f]+$":
+ description: hardware apertures belong to this device.
+ type: object
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+ - ranges
+
+additionalProperties: false
+
+examples:
+ - |
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ pci-ep-bus@e0000000 {
+ compatible = "pci-ep-bus", "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges = <0x0 0x0 0x0 0xe0000000 0x0 0x2000000
+ 0x20000000 0x0 0x0 0xe4200000 0x0 0x40000>;
+ };
+ };
--
2.27.0


2022-03-06 22:20:52

by Tom Rix

[permalink] [raw]
Subject: Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

Lizhi,

Sorry for the delay, I am fighting with checking this with 'make
dt_binding_check'

There is a recent failure in linux-next around display/mediatek,*
between next-20220301 and next-20220302 that I am bisecting.

There are a couple of checkpatch --strict warnings for this set, the
obvious one is adding to the MAINTAINERS for new files.

Tom

On 3/4/22 9:23 PM, Lizhi Hou wrote:
> Create device tree binding document for PCIe endpoint bus.
>
> Signed-off-by: Sonal Santan <[email protected]>
> Signed-off-by: Max Zhen <[email protected]>
> Signed-off-by: Lizhi Hou <[email protected]>
> ---
> .../devicetree/bindings/bus/pci-ep-bus.yaml | 72 +++++++++++++++++++
> 1 file changed, 72 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>
> diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> new file mode 100644
> index 000000000000..0ca96298db6f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: PCIe Endpoint Bus binding
> +
> +description: |
> + PCIe device may use flattened device tree to describe apertures in its
> + PCIe BARs. The Bus PCIe endpoint node is created and attached under the
> + device tree root node for this kind of device. Then the flatten device
> + tree overlay for this device is attached under the endpoint node.
> +
> + The aperture address which is under the endpoint node consists of BAR
> + index and offset. It uses the following encoding:
> +
> + 0xIooooooo 0xoooooooo
> +
> + Where:
> +
> + I = BAR index
> + oooooo oooooooo = BAR offset
> +
> + The endpoint is compatible with 'simple-bus' and contains 'ranges'
> + property for translating aperture address to CPU address.
> +
> +allOf:
> + - $ref: /schemas/simple-bus.yaml#
> +
> +maintainers:
> + - Lizhi Hou <[email protected]>
> +
> +properties:
> + compatible:
> + contains:
> + const: pci-ep-bus
> +
> + "#address-cells":
> + const: 2
> +
> + "#size-cells":
> + const: 2
> +
> + ranges: true
> +
> +patternProperties:
> + "^.*@[0-9a-f]+$":
> + description: hardware apertures belong to this device.
> + type: object
> +
> +required:
> + - compatible
> + - "#address-cells"
> + - "#size-cells"
> + - ranges
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + pci-ep-bus@e0000000 {
> + compatible = "pci-ep-bus", "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0x0 0x0 0x0 0xe0000000 0x0 0x2000000
> + 0x20000000 0x0 0x0 0xe4200000 0x0 0x40000>;
> + };
> + };

2022-03-08 01:06:49

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

On Sun, Mar 6, 2022 at 9:37 AM Tom Rix <[email protected]> wrote:
>
> Lizhi,
>
> Sorry for the delay, I am fighting with checking this with 'make
> dt_binding_check'
>
> There is a recent failure in linux-next around display/mediatek,*
> between next-20220301 and next-20220302 that I am bisecting.

There's already patches for that posted.

Just use 'make -k'.

>
> There are a couple of checkpatch --strict warnings for this set, the
> obvious one is adding to the MAINTAINERS for new files.
>
> Tom
>
> On 3/4/22 9:23 PM, Lizhi Hou wrote:
> > Create device tree binding document for PCIe endpoint bus.
> >
> > Signed-off-by: Sonal Santan <[email protected]>
> > Signed-off-by: Max Zhen <[email protected]>
> > Signed-off-by: Lizhi Hou <[email protected]>
> > ---
> > .../devicetree/bindings/bus/pci-ep-bus.yaml | 72 +++++++++++++++++++
> > 1 file changed, 72 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> > new file mode 100644
> > index 000000000000..0ca96298db6f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> > @@ -0,0 +1,72 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: PCIe Endpoint Bus binding
> > +
> > +description: |
> > + PCIe device may use flattened device tree to describe apertures in its
> > + PCIe BARs. The Bus PCIe endpoint node is created and attached under the
> > + device tree root node for this kind of device. Then the flatten device
> > + tree overlay for this device is attached under the endpoint node.
> > +
> > + The aperture address which is under the endpoint node consists of BAR
> > + index and offset. It uses the following encoding:
> > +
> > + 0xIooooooo 0xoooooooo
> > +
> > + Where:
> > +
> > + I = BAR index
> > + oooooo oooooooo = BAR offset
> > +
> > + The endpoint is compatible with 'simple-bus' and contains 'ranges'
> > + property for translating aperture address to CPU address.


This binding is completely confusing because 'PCIe endpoint' is
generally used (in context of bindings and Linux) for describing the
endpoint's system (i.e. the internal structure of a PCIe device (e.g.
add-in card) from the view of its own processor (not the host
system)). This binding seems to be describing the host system's view
of a PCIe device. We already have that! That's just the PCI bus
binding[1] which has existed for ~25 years.

For a non-DT system, what you are going to need here is some way to
create DT nodes of the PCI bus hierarchy or at least from your device
back up to the host bridge. I would suggest you solve that problem
separately from implementing the FPGA driver by making it work first
on a DT based system.

Rob

[1] https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf

2022-04-22 23:17:56

by Lizhi Hou

[permalink] [raw]
Subject: Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

Hi Rob,

On 3/7/22 06:07, Rob Herring wrote:
> On Sun, Mar 6, 2022 at 9:37 AM Tom Rix <[email protected]> wrote:
>> Lizhi,
>>
>> Sorry for the delay, I am fighting with checking this with 'make
>> dt_binding_check'
>>
>> There is a recent failure in linux-next around display/mediatek,*
>> between next-20220301 and next-20220302 that I am bisecting.
> There's already patches for that posted.
>
> Just use 'make -k'.
>
>> There are a couple of checkpatch --strict warnings for this set, the
>> obvious one is adding to the MAINTAINERS for new files.
>>
>> Tom
>>
>> On 3/4/22 9:23 PM, Lizhi Hou wrote:
>>> Create device tree binding document for PCIe endpoint bus.
>>>
>>> Signed-off-by: Sonal Santan <[email protected]>
>>> Signed-off-by: Max Zhen <[email protected]>
>>> Signed-off-by: Lizhi Hou <[email protected]>
>>> ---
>>> .../devicetree/bindings/bus/pci-ep-bus.yaml | 72 +++++++++++++++++++
>>> 1 file changed, 72 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>> new file mode 100644
>>> index 000000000000..0ca96298db6f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>> @@ -0,0 +1,72 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: PCIe Endpoint Bus binding
>>> +
>>> +description: |
>>> + PCIe device may use flattened device tree to describe apertures in its
>>> + PCIe BARs. The Bus PCIe endpoint node is created and attached under the
>>> + device tree root node for this kind of device. Then the flatten device
>>> + tree overlay for this device is attached under the endpoint node.
>>> +
>>> + The aperture address which is under the endpoint node consists of BAR
>>> + index and offset. It uses the following encoding:
>>> +
>>> + 0xIooooooo 0xoooooooo
>>> +
>>> + Where:
>>> +
>>> + I = BAR index
>>> + oooooo oooooooo = BAR offset
>>> +
>>> + The endpoint is compatible with 'simple-bus' and contains 'ranges'
>>> + property for translating aperture address to CPU address.
>
> This binding is completely confusing because 'PCIe endpoint' is
> generally used (in context of bindings and Linux) for describing the
> endpoint's system (i.e. the internal structure of a PCIe device (e.g.
> add-in card) from the view of its own processor (not the host
> system)). This binding seems to be describing the host system's view
> of a PCIe device. We already have that! That's just the PCI bus
> binding[1] which has existed for ~25 years.
>
> For a non-DT system, what you are going to need here is some way to
> create DT nodes of the PCI bus hierarchy or at least from your device
> back up to the host bridge. I would suggest you solve that problem
> separately from implementing the FPGA driver by making it work first
> on a DT based system.
>
> Rob
>
> [1] https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
>
>
I investigated the implementation detail for adding device tree node for
PCIe devices. Based on my findings this is quite involved and so would
like to bounce off my approach with you before I start making changes.

We will start with DT-Base system which already has device node for PCIe
controller in base device tree. And we will focus on:

- Adding functions to generate device tree node for all PCIe devices.
- Linking dynamically generated DT nodes to the PCIe to the PCIe devices.

So the first question to resolve is when the PCIe DT node will be created
and destroyed. Here are the different options:

- Option #1: Add functions in pci_bus_add_device()/pci_stop_dev()
  - The same place for creating/destroying device sysfs node. This should
    be able to handle different cases: SR-IOV vf, hotplug, virtual device
    etc.
  - Leverage existing PCI enumeration and get the device information
    directly from pci_dev structure.

-  Option #2: Enumerate PCIe devices and create DT node without relying
   on PCI subsystem enumeration.
  - E.g. Enumerating and creating PCIe dt node in an init callback
    pure_initcall()
  - Linking dt node to PCIe device is already supported in
    pci_setup_device()
  - Eventually need to handle hotplug case separately.

Option #1 looks an easier and cleaner way to implement.

The second question is for linking the dynamic generated dt node to PCIe
device through pci_dev->dev.of_node. The biggest concern is that current
kernel and driver code may check of_node pointer and run complete different
code path if of_node is not NULL. Here are some examples.

 - of_irq_parse_pci():
https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/of.c#L492
 - pci_msi_domain_get_msi_rid():
https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/msi/irqdomain.c#L233
 - pci_dma_configure():
https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/pci-driver.c#L1610

It needs to identify all the places which use of_node and make sure using
dynamic generated of_node is equivalent with the code without using
of_node. Considering there are so many hardware and virtualization Linux
support, the risk might high for changing the code which has been existing
for long time. And how to validate the change could be another challenge.
Introducing compiling option (e.g. CONFIG_PCI_DT_NODE) may lower the risk.

There are other approaches for linking dt node to PCIe device.
- Option #a: Add a special flag or property to dynamic generated PCIe DT
  node. Then convert the code.

      if (pdev->dev.of_node)
          funcA();
      else
          funcB();

   to
      struct device_node *pci_get_of_node(pdev)
      {
          if (pdev->dev.of_node is dynamic generated)
              return NULL;
          return pdev->dev.of_node;
      }

      if (pci_get_of_node(pdev))
          funcA ();
      else
          funcB ();

- Option #b: Introduce a new data member "dyn_of_node" in struct device
  to link the dynamic generated PCIe dt node.

      struct device {
          ...
          of_node: Associated device tree node.
          fwnode: Associated device node supplied by platform firmware.
          dyn_of_node: Associated dynamic generated device tree node.
          ...

Could we implement Option #b for now because it is lower risk?

The last question is about the properties for each dynamic generated
PCIe dt node. To keep the generated device node lightweight, what will be
the desired (minimum) set of properties to generate? Besides the properties
defined in IEEE Std 1275, it looks more properties are needed. E.g.
    interrupt-map, interrupt-map-mask, ...

Thanks,
Lizhi

2022-05-14 03:51:02

by Lizhi Hou

[permalink] [raw]
Subject: Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

Hi Rob,


Do you have any comment on this? We are looking for guidance on the
approaches suggested.


Thanks,

Lizhi

On 4/22/22 14:57, Lizhi Hou wrote:
> Hi Rob,
>
> On 3/7/22 06:07, Rob Herring wrote:
>> On Sun, Mar 6, 2022 at 9:37 AM Tom Rix <[email protected]> wrote:
>>> Lizhi,
>>>
>>> Sorry for the delay, I am fighting with checking this with 'make
>>> dt_binding_check'
>>>
>>> There is a recent failure in linux-next around display/mediatek,*
>>> between next-20220301 and next-20220302 that I am bisecting.
>> There's already patches for that posted.
>>
>> Just use 'make -k'.
>>
>>> There are a couple of checkpatch --strict warnings for this set, the
>>> obvious one is adding to the MAINTAINERS for new files.
>>>
>>> Tom
>>>
>>> On 3/4/22 9:23 PM, Lizhi Hou wrote:
>>>> Create device tree binding document for PCIe endpoint bus.
>>>>
>>>> Signed-off-by: Sonal Santan <[email protected]>
>>>> Signed-off-by: Max Zhen <[email protected]>
>>>> Signed-off-by: Lizhi Hou <[email protected]>
>>>> ---
>>>>    .../devicetree/bindings/bus/pci-ep-bus.yaml   | 72
>>>> +++++++++++++++++++
>>>>    1 file changed, 72 insertions(+)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>> b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>> new file mode 100644
>>>> index 000000000000..0ca96298db6f
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>> @@ -0,0 +1,72 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: PCIe Endpoint Bus binding
>>>> +
>>>> +description: |
>>>> +  PCIe device may use flattened device tree to describe apertures
>>>> in its
>>>> +  PCIe BARs. The Bus PCIe endpoint node is created and attached
>>>> under the
>>>> +  device tree root node for this kind of device. Then the flatten
>>>> device
>>>> +  tree overlay for this device is attached under the endpoint node.
>>>> +
>>>> +  The aperture address which is under the endpoint node consists
>>>> of BAR
>>>> +  index and offset. It uses the following encoding:
>>>> +
>>>> +    0xIooooooo 0xoooooooo
>>>> +
>>>> +  Where:
>>>> +
>>>> +    I = BAR index
>>>> +    oooooo oooooooo = BAR offset
>>>> +
>>>> +  The endpoint is compatible with 'simple-bus' and contains 'ranges'
>>>> +  property for translating aperture address to CPU address.
>>
>> This binding is completely confusing because 'PCIe endpoint' is
>> generally used (in context of bindings and Linux) for describing the
>> endpoint's system (i.e. the internal structure of a PCIe device (e.g.
>> add-in card) from the view of its own processor (not the host
>> system)). This binding seems to be describing the host system's view
>> of a PCIe device. We already have that! That's just the PCI bus
>> binding[1] which has existed for ~25 years.
>>
>> For a non-DT system, what you are going to need here is some way to
>> create DT nodes of the PCI bus hierarchy or at least from your device
>> back up to the host bridge. I would suggest you solve that problem
>> separately from implementing the FPGA driver by making it work first
>> on a DT based system.
>>
>> Rob
>>
>> [1] https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
>>
>>
> I investigated the implementation detail for adding device tree node for
> PCIe devices. Based on my findings this is quite involved and so would
> like to bounce off my approach with you before I start making changes.
>
> We will start with DT-Base system which already has device node for PCIe
> controller in base device tree. And we will focus on:
>
> - Adding functions to generate device tree node for all PCIe devices.
> - Linking dynamically generated DT nodes to the PCIe to the PCIe devices.
>
> So the first question to resolve is when the PCIe DT node will be created
> and destroyed. Here are the different options:
>
> - Option #1: Add functions in pci_bus_add_device()/pci_stop_dev()
>   - The same place for creating/destroying device sysfs node. This should
>     be able to handle different cases: SR-IOV vf, hotplug, virtual device
>     etc.
>   - Leverage existing PCI enumeration and get the device information
>     directly from pci_dev structure.
>
> -  Option #2: Enumerate PCIe devices and create DT node without relying
>    on PCI subsystem enumeration.
>   - E.g. Enumerating and creating PCIe dt node in an init callback
>     pure_initcall()
>   - Linking dt node to PCIe device is already supported in
>     pci_setup_device()
>   - Eventually need to handle hotplug case separately.
>
> Option #1 looks an easier and cleaner way to implement.
>
> The second question is for linking the dynamic generated dt node to PCIe
> device through pci_dev->dev.of_node. The biggest concern is that current
> kernel and driver code may check of_node pointer and run complete
> different
> code path if of_node is not NULL. Here are some examples.
>
>  - of_irq_parse_pci():
> https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/of.c#L492
>  - pci_msi_domain_get_msi_rid():
> https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/msi/irqdomain.c#L233
>  - pci_dma_configure():
> https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/pci-driver.c#L1610
>
> It needs to identify all the places which use of_node and make sure using
> dynamic generated of_node is equivalent with the code without using
> of_node. Considering there are so many hardware and virtualization Linux
> support, the risk might high for changing the code which has been
> existing
> for long time. And how to validate the change could be another challenge.
> Introducing compiling option (e.g. CONFIG_PCI_DT_NODE) may lower the
> risk.
>
> There are other approaches for linking dt node to PCIe device.
> - Option #a: Add a special flag or property to dynamic generated PCIe DT
>   node. Then convert the code.
>
>       if (pdev->dev.of_node)
>           funcA();
>       else
>           funcB();
>
>    to
>       struct device_node *pci_get_of_node(pdev)
>       {
>           if (pdev->dev.of_node is dynamic generated)
>               return NULL;
>           return pdev->dev.of_node;
>       }
>
>       if (pci_get_of_node(pdev))
>           funcA ();
>       else
>           funcB ();
>
> - Option #b: Introduce a new data member "dyn_of_node" in struct device
>   to link the dynamic generated PCIe dt node.
>
>       struct device {
>           ...
>           of_node: Associated device tree node.
>           fwnode: Associated device node supplied by platform firmware.
>           dyn_of_node: Associated dynamic generated device tree node.
>           ...
>
> Could we implement Option #b for now because it is lower risk?
>
> The last question is about the properties for each dynamic generated
> PCIe dt node. To keep the generated device node lightweight, what will be
> the desired (minimum) set of properties to generate? Besides the
> properties
> defined in IEEE Std 1275, it looks more properties are needed. E.g.
>     interrupt-map, interrupt-map-mask, ...
>
> Thanks,
> Lizhi
>

2022-06-21 15:12:57

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

Hi,

+ Kishon

On Fri, Mar 04, 2022 at 09:23:02PM -0800, Lizhi Hou wrote:
> Create device tree binding document for PCIe endpoint bus.
>

I'm currently working on a PCI endpoint function driver for MHI bus [1] and
hence interested in this topic.

Comments below.

[1] https://lore.kernel.org/lkml/[email protected]/

> Signed-off-by: Sonal Santan <[email protected]>
> Signed-off-by: Max Zhen <[email protected]>
> Signed-off-by: Lizhi Hou <[email protected]>
> ---
> .../devicetree/bindings/bus/pci-ep-bus.yaml | 72 +++++++++++++++++++
> 1 file changed, 72 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>
> diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> new file mode 100644
> index 000000000000..0ca96298db6f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: PCIe Endpoint Bus binding
> +
> +description: |
> + PCIe device may use flattened device tree to describe apertures in its
> + PCIe BARs. The Bus PCIe endpoint node is created and attached under the
> + device tree root node for this kind of device. Then the flatten device
> + tree overlay for this device is attached under the endpoint node.
> +
> + The aperture address which is under the endpoint node consists of BAR
> + index and offset. It uses the following encoding:
> +

On top of Rob's reply:

Currently, the BAR memory for the PCI endpoint device is either allocated
dynamically using pci_epf_alloc_space() or we need to pass the address in
"phys_addr" field of "struct pci_epf_bar".

In most of the PCI endpoint devices, we need to use a fixed memory region as
the BAR. Since there is no devicetree integration for PCI endpoint subsystem,
I've been using the 2nd approach of obtaining the BAR address from PCI endpoint
controller devicetree node and passing it to "phys_addr" of
"struct pci_epf_bar".

Ideally, the BAR information should come from the devicetree. But we cannot use
just "pci-ep-bus" node. I've been thinking about the below structure:

pcie_ep: pcie-ep@40000000 {
compatible = "pcie-ep";
....

pci_epf_0: pci-epf@100000 {
reg = <0x100000 0x1000>; # BAR0
};

pci_epf_1: pci-epf@200000 {
reg = <0x200000 0x1000>; # BAR1
};
};

Where, "pci-epf@" represents each of the PCI endpoint functions implemented by
this device (note that there can be more than one function per endpoint device)
and "reg" has the BAR address/size for that function.

Rob, what do you think?

Thanks,
Mani

> + 0xIooooooo 0xoooooooo
> +
> + Where:
> +
> + I = BAR index
> + oooooo oooooooo = BAR offset
> +
> + The endpoint is compatible with 'simple-bus' and contains 'ranges'
> + property for translating aperture address to CPU address.
> +
> +allOf:
> + - $ref: /schemas/simple-bus.yaml#
> +
> +maintainers:
> + - Lizhi Hou <[email protected]>
> +
> +properties:
> + compatible:
> + contains:
> + const: pci-ep-bus
> +
> + "#address-cells":
> + const: 2
> +
> + "#size-cells":
> + const: 2
> +
> + ranges: true
> +
> +patternProperties:
> + "^.*@[0-9a-f]+$":
> + description: hardware apertures belong to this device.
> + type: object
> +
> +required:
> + - compatible
> + - "#address-cells"
> + - "#size-cells"
> + - ranges
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + pci-ep-bus@e0000000 {
> + compatible = "pci-ep-bus", "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0x0 0x0 0x0 0xe0000000 0x0 0x2000000
> + 0x20000000 0x0 0x0 0xe4200000 0x0 0x40000>;
> + };
> + };
> --
> 2.27.0
>

--
மணிவண்ணன் சதாசிவம்