[RFC,02/21] dt-bindings: PCI: Endpoint: Add DT bindings for PCI EPF Device

Message ID 20190926112933.8922-3-kishon@ti.com
State New
Headers show
Series
  • Implement NTB Controller using multiple PCI
Related show

Commit Message

Kishon Vijay Abraham I Sept. 26, 2019, 11:29 a.m.
Add device tree bindings for PCI endpoint function device. The
nodes for PCI endpoint function device should be attached to
PCI endpoint function bus.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

---
 .../bindings/pci/endpoint/pci-epf.txt         | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt

-- 
2.17.1

Comments

Rob Herring Oct. 15, 2019, 6:42 p.m. | #1
On Thu, Sep 26, 2019 at 04:59:14PM +0530, Kishon Vijay Abraham I wrote:
> Add device tree bindings for PCI endpoint function device. The

> nodes for PCI endpoint function device should be attached to

> PCI endpoint function bus.

> 

> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

> ---

>  .../bindings/pci/endpoint/pci-epf.txt         | 28 +++++++++++++++++++

>  1 file changed, 28 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt


This and the previous patch for the bus should be combined and please 
convert to a schema.

> 

> diff --git a/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt b/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt

> new file mode 100644

> index 000000000000..f006395fd526

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt

> @@ -0,0 +1,28 @@

> +PCI Endpoint Function Device

> +

> +This describes the generic bindings to be used when a device has to be

> +exposed to the remote host over PCIe. The device could be an actual

> +peripheral in the platform or a virtual device created by the software.

> +

> +epcs : phandle to the endpoint controller device

> +epc-names : the names of the endpoint controller device corresponding

> +	    to the EPCs present in the *epcs* phandle


Other than the NTB case, I'd expect the parent device to be the 
controller. Let's make NTB the exception...


> +vendor-id: used to identify device manufacturer

> +device-id: used to identify a particular device

> +baseclass-code: used to classify the type of function the device performs

> +subclass-code: used to identify more specifically the function of the device


Are these codes standard?

Powerpc has "class-code" already...

> +subsys-vendor-id: used to identify vendor of the add-in card or subsystem


Powerpc has "subsystem-vendor-id" already...

> +subsys-id: used to specify an id that is specific to a vendor

> +

> +Example:

> +Following is an example of NTB device exposed to the remote host.

> +

> +ntb {


This is going to need some sort of addressing (which implies 'reg')? If 
not, I don't understand why you have 2 levels.

> +	compatible = "pci-epf-ntb";

> +	epcs = <&pcie0_ep>, <&pcie1_ep>;

> +	epc-names = "primary", "secondary";

> +	vendor-id = /bits/ 16 <0x104c>;

> +	device-id = /bits/ 16 <0xb00d>;


These have a long history in OF and should be 32-bits (yes, we've let 
some cases of 16-bit creep in).

> +	num-mws = <4>;


Doesn't this apply to more than NTB?

Can't you just get the length of 'mws-size'?

> +	mws-size = <0x100000>, <0x100000>, <0x100000>, <0x100000>;


Need to support 64-bit sizes?

> +};

> -- 

> 2.17.1

>
Kishon Vijay Abraham I Oct. 16, 2019, 4:45 a.m. | #2
On 16/10/19 12:12 AM, Rob Herring wrote:
> On Thu, Sep 26, 2019 at 04:59:14PM +0530, Kishon Vijay Abraham I wrote:

>> Add device tree bindings for PCI endpoint function device. The

>> nodes for PCI endpoint function device should be attached to

>> PCI endpoint function bus.

>>

>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

>> ---

>>  .../bindings/pci/endpoint/pci-epf.txt         | 28 +++++++++++++++++++

>>  1 file changed, 28 insertions(+)

>>  create mode 100644 Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt

> 

> This and the previous patch for the bus should be combined and please 

> convert to a schema.


Sure Rob. Thanks for the review.

-Kishon
> 

>>

>> diff --git a/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt b/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt

>> new file mode 100644

>> index 000000000000..f006395fd526

>> --- /dev/null

>> +++ b/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt

>> @@ -0,0 +1,28 @@

>> +PCI Endpoint Function Device

>> +

>> +This describes the generic bindings to be used when a device has to be

>> +exposed to the remote host over PCIe. The device could be an actual

>> +peripheral in the platform or a virtual device created by the software.

>> +

>> +epcs : phandle to the endpoint controller device

>> +epc-names : the names of the endpoint controller device corresponding

>> +	    to the EPCs present in the *epcs* phandle

> 

> Other than the NTB case, I'd expect the parent device to be the 

> controller. Let's make NTB the exception...

> 

> 

>> +vendor-id: used to identify device manufacturer

>> +device-id: used to identify a particular device

>> +baseclass-code: used to classify the type of function the device performs

>> +subclass-code: used to identify more specifically the function of the device

> 

> Are these codes standard?

> 

> Powerpc has "class-code" already...

> 

>> +subsys-vendor-id: used to identify vendor of the add-in card or subsystem

> 

> Powerpc has "subsystem-vendor-id" already...

> 

>> +subsys-id: used to specify an id that is specific to a vendor

>> +

>> +Example:

>> +Following is an example of NTB device exposed to the remote host.

>> +

>> +ntb {

> 

> This is going to need some sort of addressing (which implies 'reg')? If 

> not, I don't understand why you have 2 levels.

> 

>> +	compatible = "pci-epf-ntb";

>> +	epcs = <&pcie0_ep>, <&pcie1_ep>;

>> +	epc-names = "primary", "secondary";

>> +	vendor-id = /bits/ 16 <0x104c>;

>> +	device-id = /bits/ 16 <0xb00d>;

> 

> These have a long history in OF and should be 32-bits (yes, we've let 

> some cases of 16-bit creep in).

> 

>> +	num-mws = <4>;

> 

> Doesn't this apply to more than NTB?

> 

> Can't you just get the length of 'mws-size'?

> 

>> +	mws-size = <0x100000>, <0x100000>, <0x100000>, <0x100000>;

> 

> Need to support 64-bit sizes?

> 

>> +};

>> -- 

>> 2.17.1

>>

Patch

diff --git a/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt b/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt
new file mode 100644
index 000000000000..f006395fd526
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/endpoint/pci-epf.txt
@@ -0,0 +1,28 @@ 
+PCI Endpoint Function Device
+
+This describes the generic bindings to be used when a device has to be
+exposed to the remote host over PCIe. The device could be an actual
+peripheral in the platform or a virtual device created by the software.
+
+epcs : phandle to the endpoint controller device
+epc-names : the names of the endpoint controller device corresponding
+	    to the EPCs present in the *epcs* phandle
+vendor-id: used to identify device manufacturer
+device-id: used to identify a particular device
+baseclass-code: used to classify the type of function the device performs
+subclass-code: used to identify more specifically the function of the device
+subsys-vendor-id: used to identify vendor of the add-in card or subsystem
+subsys-id: used to specify an id that is specific to a vendor
+
+Example:
+Following is an example of NTB device exposed to the remote host.
+
+ntb {
+	compatible = "pci-epf-ntb";
+	epcs = <&pcie0_ep>, <&pcie1_ep>;
+	epc-names = "primary", "secondary";
+	vendor-id = /bits/ 16 <0x104c>;
+	device-id = /bits/ 16 <0xb00d>;
+	num-mws = <4>;
+	mws-size = <0x100000>, <0x100000>, <0x100000>, <0x100000>;
+};