diff mbox

[v9,5/6] Documentation: DT: Add HiSilicon PCIe host binding

Message ID 1442321361-174300-6-git-send-email-wangzhou1@hisilicon.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhou Wang Sept. 15, 2015, 12:49 p.m. UTC 
This patch adds related DTS binding document for HiSilicon PCIe host driver.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt

Comments

Rob Herring (Arm) Sept. 15, 2015, 7:43 p.m. UTC | #1 
On 09/15/2015 07:49 AM, Zhou Wang wrote:
> This patch adds related DTS binding document for HiSilicon PCIe host driver.
> 
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> new file mode 100644
> index 0000000..2afc9d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -0,0 +1,46 @@
> +HiSilicon PCIe host bridge DT description
> +
> +HiSilicon PCIe host controller is based on Designware PCI core.
> +It shares common functions with PCIe Designware core driver and inherits
> +common properties defined in
> +Documentation/devicetree/bindings/pci/designware-pci.txt.
> +
> +Additional properties are described here:
> +
> +Required properties:
> +- compatible: Should contain "hisilicon,hip05-pcie".
> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
> +- reg-names: Must include the following entries:
> +  "rc_dbi": controller configuration registers;
> +  "subctrl": whole PCIe hosts configuration registers;
> +  "config": PCIe configuration space registers.
> +- msi-parent: Should be its_pcie which is an ITS receiving MSI interrupts.
> +- port-id: Should be 0, 1, 2 or 3.

What is port-id for? Use of instance indexes need to have good reason.

Rob
Zhou Wang Sept. 16, 2015, 1:14 a.m. UTC | #2 
On 2015/9/16 3:43, Rob Herring wrote:
> On 09/15/2015 07:49 AM, Zhou Wang wrote:
>> This patch adds related DTS binding document for HiSilicon PCIe host driver.
>>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>> ---
>>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>> new file mode 100644
>> index 0000000..2afc9d1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>> @@ -0,0 +1,46 @@
>> +HiSilicon PCIe host bridge DT description
>> +
>> +HiSilicon PCIe host controller is based on Designware PCI core.
>> +It shares common functions with PCIe Designware core driver and inherits
>> +common properties defined in
>> +Documentation/devicetree/bindings/pci/designware-pci.txt.
>> +
>> +Additional properties are described here:
>> +
>> +Required properties:
>> +- compatible: Should contain "hisilicon,hip05-pcie".
>> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
>> +- reg-names: Must include the following entries:
>> +  "rc_dbi": controller configuration registers;
>> +  "subctrl": whole PCIe hosts configuration registers;
>> +  "config": PCIe configuration space registers.
>> +- msi-parent: Should be its_pcie which is an ITS receiving MSI interrupts.
>> +- port-id: Should be 0, 1, 2 or 3.
> 
> What is port-id for? Use of instance indexes need to have good reason.
> 
> Rob

There are four PCIe controllers in HiSilicon Hip05 SoC, port-id just indicates
which one we use. And we will use port-id to locate related registers in driver.

Thanks,
Zhou

>
> 
> .
>
Rob Herring (Arm) Sept. 16, 2015, 2:17 a.m. UTC | #3 
On 09/15/2015 08:14 PM, Zhou Wang wrote:
> On 2015/9/16 3:43, Rob Herring wrote:
>> On 09/15/2015 07:49 AM, Zhou Wang wrote:
>>> This patch adds related DTS binding document for HiSilicon PCIe host driver.
>>>
>>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>>> ---
>>>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>>>  1 file changed, 46 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>> new file mode 100644
>>> index 0000000..2afc9d1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>> @@ -0,0 +1,46 @@
>>> +HiSilicon PCIe host bridge DT description
>>> +
>>> +HiSilicon PCIe host controller is based on Designware PCI core.
>>> +It shares common functions with PCIe Designware core driver and inherits
>>> +common properties defined in
>>> +Documentation/devicetree/bindings/pci/designware-pci.txt.
>>> +
>>> +Additional properties are described here:
>>> +
>>> +Required properties:
>>> +- compatible: Should contain "hisilicon,hip05-pcie".
>>> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
>>> +- reg-names: Must include the following entries:
>>> +  "rc_dbi": controller configuration registers;
>>> +  "subctrl": whole PCIe hosts configuration registers;
>>> +  "config": PCIe configuration space registers.
>>> +- msi-parent: Should be its_pcie which is an ITS receiving MSI interrupts.
>>> +- port-id: Should be 0, 1, 2 or 3.
>>
>> What is port-id for? Use of instance indexes need to have good reason.
>>
>> Rob
> 
> There are four PCIe controllers in HiSilicon Hip05 SoC, port-id just indicates
> which one we use. And we will use port-id to locate related registers in driver.

Just having multiple instances is not a reason. So looking at the
driver, port-id is used to calculate register addresses for these registers:

#define PCIE_SUBCTRL_MODE_REG                           0x2800
#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818

Is the base address of subctrl the same on all 4 ports? If so, you
should not have overlapping resources in the DT. Either split these 2
registers into 2 reg regions (for each register) or use syscon to
provide access to the region.

Rob
Zhou Wang Sept. 16, 2015, 3:24 a.m. UTC | #4 
On 2015/9/16 10:17, Rob Herring wrote:
> On 09/15/2015 08:14 PM, Zhou Wang wrote:
>> On 2015/9/16 3:43, Rob Herring wrote:
>>> On 09/15/2015 07:49 AM, Zhou Wang wrote:
>>>> This patch adds related DTS binding document for HiSilicon PCIe host driver.
>>>>
>>>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>>>> ---
>>>>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>>>>  1 file changed, 46 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>>> new file mode 100644
>>>> index 0000000..2afc9d1
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>>> @@ -0,0 +1,46 @@
>>>> +HiSilicon PCIe host bridge DT description
>>>> +
>>>> +HiSilicon PCIe host controller is based on Designware PCI core.
>>>> +It shares common functions with PCIe Designware core driver and inherits
>>>> +common properties defined in
>>>> +Documentation/devicetree/bindings/pci/designware-pci.txt.
>>>> +
>>>> +Additional properties are described here:
>>>> +
>>>> +Required properties:
>>>> +- compatible: Should contain "hisilicon,hip05-pcie".
>>>> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
>>>> +- reg-names: Must include the following entries:
>>>> +  "rc_dbi": controller configuration registers;
>>>> +  "subctrl": whole PCIe hosts configuration registers;
>>>> +  "config": PCIe configuration space registers.
>>>> +- msi-parent: Should be its_pcie which is an ITS receiving MSI interrupts.
>>>> +- port-id: Should be 0, 1, 2 or 3.
>>>
>>> What is port-id for? Use of instance indexes need to have good reason.
>>>
>>> Rob
>>
>> There are four PCIe controllers in HiSilicon Hip05 SoC, port-id just indicates
>> which one we use. And we will use port-id to locate related registers in driver.
> 
> Just having multiple instances is not a reason. So looking at the
> driver, port-id is used to calculate register addresses for these registers:
> 
> #define PCIE_SUBCTRL_MODE_REG                           0x2800
> #define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
> 
> Is the base address of subctrl the same on all 4 ports? If so, you

Yes, same subctrl address for 4 ports. Will use syscon to access subctrl
registers in next version.

Many thanks for pointing this,
Zhou

> should not have overlapping resources in the DT. Either split these 2
> registers into 2 reg regions (for each register) or use syscon to
> provide access to the region.
> 
> Rob
> 
> 
> .
>
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
new file mode 100644
index 0000000..2afc9d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -0,0 +1,46 @@ 
+HiSilicon PCIe host bridge DT description
+
+HiSilicon PCIe host controller is based on Designware PCI core.
+It shares common functions with PCIe Designware core driver and inherits
+common properties defined in
+Documentation/devicetree/bindings/pci/designware-pci.txt.
+
+Additional properties are described here:
+
+Required properties:
+- compatible: Should contain "hisilicon,hip05-pcie".
+- reg: Should contain rc_dbi, subctrl, config registers location and length.
+- reg-names: Must include the following entries:
+  "rc_dbi": controller configuration registers;
+  "subctrl": whole PCIe hosts configuration registers;
+  "config": PCIe configuration space registers.
+- msi-parent: Should be its_pcie which is an ITS receiving MSI interrupts.
+- port-id: Should be 0, 1, 2 or 3.
+
+Optional properties:
+- status: Either "ok" or "disabled".
+- dma-coherent: Present if DMA operations are coherent.
+
+Example:
+	pcie@0xb0080000 {
+		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
+		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
+		      <0x220 0x00000000 0 0x2000>;
+		reg-names = "rc_dbi", "subctrl", "config";
+		bus-range = <0  15>;
+		msi-parent = <&its_pcie>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		dma-coherent;
+		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
+		num-lanes = <8>;
+		port-id = <1>;
+		#interrupts-cells = <1>;
+		interrupts-map-mask = <0xf800 0 0 7>;
+		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
+				  0x0 0 0 2 &mbigen_pcie 2 11
+				  0x0 0 0 3 &mbigen_pcie 3 12
+				  0x0 0 0 4 &mbigen_pcie 4 13>;
+		status = "ok";
+	};