diff mbox series

[3/8] bindings: PCI: designware: Add support for the EP in designware driver

Message ID dd582bfc2780679b7a092d23547c316234a84c34.1522235224.git.gustavo.pimentel@synopsys.com
State Superseded
Delegated to: Lorenzo Pieralisi
Headers show
Series Designware EP support and code clenan up | expand

Commit Message

Gustavo Pimentel March 28, 2018, 11:38 a.m. UTC
Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Kishon Vijay Abraham I April 2, 2018, 5:35 a.m. UTC | #1
On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

Please add a commit message.
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 6300762..4bb2e08 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -3,6 +3,7 @@
>  Required properties:
>  - compatible:
>  	"snps,dw-pcie" for RC mode;
> +	"snps,dw-pcie-ep" for EP mode;
>  - reg: Should contain the configuration address space.
>  - reg-names: Must be "config" for the PCIe configuration space.
>      (The old way of getting the configuration address space from "ranges"
> @@ -56,3 +57,15 @@ Example configuration:
>  		#interrupt-cells = <1>;
>  		num-lanes = <1>;
>  	};
> +or
> +	pcie_ep: pcie_ep@dfc00000 {
> +		compatible = "snps,dw-pcie-ep";
> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */

Doesn't this have iATU unroll space?

Thanks
Kishon
Gustavo Pimentel April 3, 2018, 10:43 a.m. UTC | #2
Hi Kishon,

On 02/04/2018 06:35, Kishon Vijay Abraham I wrote:
> 
> 
> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> 
> Please add a commit message.

Ok. I'll add. Thanks for noticing it.

>> ---
>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> index 6300762..4bb2e08 100644
>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> @@ -3,6 +3,7 @@
>>  Required properties:
>>  - compatible:
>>  	"snps,dw-pcie" for RC mode;
>> +	"snps,dw-pcie-ep" for EP mode;
>>  - reg: Should contain the configuration address space.
>>  - reg-names: Must be "config" for the PCIe configuration space.
>>      (The old way of getting the configuration address space from "ranges"
>> @@ -56,3 +57,15 @@ Example configuration:
>>  		#interrupt-cells = <1>;
>>  		num-lanes = <1>;
>>  	};
>> +or
>> +	pcie_ep: pcie_ep@dfc00000 {
>> +		compatible = "snps,dw-pcie-ep";
>> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
>> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */
> 
> Doesn't this have iATU unroll space?

I don't think EP has it, but I'm no expert on this matter. Can you provide me
some example of having iATU unroll space mapping would be useful in EP scope?

> 
> Thanks
> Kishon
> 

Regards,
Gustavo
Kishon Vijay Abraham I April 3, 2018, 10:55 a.m. UTC | #3
Hi,

On Tuesday 03 April 2018 04:13 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 02/04/2018 06:35, Kishon Vijay Abraham I wrote:
>>
>>
>> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>
>> Please add a commit message.
> 
> Ok. I'll add. Thanks for noticing it.
> 
>>> ---
>>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> index 6300762..4bb2e08 100644
>>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> @@ -3,6 +3,7 @@
>>>  Required properties:
>>>  - compatible:
>>>  	"snps,dw-pcie" for RC mode;
>>> +	"snps,dw-pcie-ep" for EP mode;
>>>  - reg: Should contain the configuration address space.
>>>  - reg-names: Must be "config" for the PCIe configuration space.
>>>      (The old way of getting the configuration address space from "ranges"
>>> @@ -56,3 +57,15 @@ Example configuration:
>>>  		#interrupt-cells = <1>;
>>>  		num-lanes = <1>;
>>>  	};
>>> +or
>>> +	pcie_ep: pcie_ep@dfc00000 {
>>> +		compatible = "snps,dw-pcie-ep";
>>> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
>>> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */
>>
>> Doesn't this have iATU unroll space?
> 
> I don't think EP has it, but I'm no expert on this matter. Can you provide me
> some example of having iATU unroll space mapping would be useful in EP scope?

I'm not sure. I thought if the dwc3 core version is 4.80, then it'll have a
separate ATU space irrespective of RC mode or EP mode.

Thanks
Kishon
Gustavo Pimentel April 3, 2018, 1:20 p.m. UTC | #4
Hi Kishon,

On 03/04/2018 11:55, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 03 April 2018 04:13 PM, Gustavo Pimentel wrote:
>> Hi Kishon,
>>
>> On 02/04/2018 06:35, Kishon Vijay Abraham I wrote:
>>>
>>>
>>> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>
>>> Please add a commit message.
>>
>> Ok. I'll add. Thanks for noticing it.
>>
>>>> ---
>>>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>>>>  1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>> index 6300762..4bb2e08 100644
>>>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>> @@ -3,6 +3,7 @@
>>>>  Required properties:
>>>>  - compatible:
>>>>  	"snps,dw-pcie" for RC mode;
>>>> +	"snps,dw-pcie-ep" for EP mode;
>>>>  - reg: Should contain the configuration address space.
>>>>  - reg-names: Must be "config" for the PCIe configuration space.
>>>>      (The old way of getting the configuration address space from "ranges"
>>>> @@ -56,3 +57,15 @@ Example configuration:
>>>>  		#interrupt-cells = <1>;
>>>>  		num-lanes = <1>;
>>>>  	};
>>>> +or
>>>> +	pcie_ep: pcie_ep@dfc00000 {
>>>> +		compatible = "snps,dw-pcie-ep";
>>>> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
>>>> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */
>>>
>>> Doesn't this have iATU unroll space?
>>
>> I don't think EP has it, but I'm no expert on this matter. Can you provide me
>> some example of having iATU unroll space mapping would be useful in EP scope?
> 
> I'm not sure. I thought if the dwc3 core version is 4.80, then it'll have a
> separate ATU space irrespective of RC mode or EP mode.

As replied on patch 1, let's leave out  any reference of iATU unroll to avoid
any confusion. Agree?

> 
> Thanks
> Kishon
> 

Regards,
Gustavo
Lorenzo Pieralisi April 4, 2018, 11:50 a.m. UTC | #5
On Wed, Mar 28, 2018 at 12:38:33PM +0100, Gustavo Pimentel wrote:

Please always write a commit log even if it is trivial.

Thanks,
Lorenzo

> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 6300762..4bb2e08 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -3,6 +3,7 @@
>  Required properties:
>  - compatible:
>  	"snps,dw-pcie" for RC mode;
> +	"snps,dw-pcie-ep" for EP mode;
>  - reg: Should contain the configuration address space.
>  - reg-names: Must be "config" for the PCIe configuration space.
>      (The old way of getting the configuration address space from "ranges"
> @@ -56,3 +57,15 @@ Example configuration:
>  		#interrupt-cells = <1>;
>  		num-lanes = <1>;
>  	};
> +or
> +	pcie_ep: pcie_ep@dfc00000 {
> +		compatible = "snps,dw-pcie-ep";
> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */
> +		      <0xd0000000 0x2000000>; /* Configuration space */
> +		reg-names = "dbi", "dbi2", "addr_space";
> +		device_type = "pci";
> +		num-ib-windows = <6>;
> +		num-ob-windows = <2>;
> +		num-lanes = <1>;
> +	};
> -- 
> 2.7.4
> 
>
Gustavo Pimentel April 4, 2018, 11:56 a.m. UTC | #6
Hi Lorenzo,

On 04/04/2018 12:50, Lorenzo Pieralisi wrote:
> On Wed, Mar 28, 2018 at 12:38:33PM +0100, Gustavo Pimentel wrote:
> 
> Please always write a commit log even if it is trivial.

Ok, Kishon has also refered that. On next patch version it'll contain a log
description.

> 
> Thanks,
> Lorenzo
> 
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> index 6300762..4bb2e08 100644
>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> @@ -3,6 +3,7 @@
>>  Required properties:
>>  - compatible:
>>  	"snps,dw-pcie" for RC mode;
>> +	"snps,dw-pcie-ep" for EP mode;
>>  - reg: Should contain the configuration address space.
>>  - reg-names: Must be "config" for the PCIe configuration space.
>>      (The old way of getting the configuration address space from "ranges"
>> @@ -56,3 +57,15 @@ Example configuration:
>>  		#interrupt-cells = <1>;
>>  		num-lanes = <1>;
>>  	};
>> +or
>> +	pcie_ep: pcie_ep@dfc00000 {
>> +		compatible = "snps,dw-pcie-ep";
>> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
>> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */
>> +		      <0xd0000000 0x2000000>; /* Configuration space */
>> +		reg-names = "dbi", "dbi2", "addr_space";
>> +		device_type = "pci";
>> +		num-ib-windows = <6>;
>> +		num-ob-windows = <2>;
>> +		num-lanes = <1>;
>> +	};
>> -- 
>> 2.7.4
>>
>>

Regards,
Gustavo
Kishon Vijay Abraham I April 6, 2018, 7:04 a.m. UTC | #7
Hi,

On Tuesday 03 April 2018 06:50 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 03/04/2018 11:55, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 03 April 2018 04:13 PM, Gustavo Pimentel wrote:
>>> Hi Kishon,
>>>
>>> On 02/04/2018 06:35, Kishon Vijay Abraham I wrote:
>>>>
>>>>
>>>> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>>>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>>
>>>> Please add a commit message.
>>>
>>> Ok. I'll add. Thanks for noticing it.
>>>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>>>>>  1 file changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>>> index 6300762..4bb2e08 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>>> @@ -3,6 +3,7 @@
>>>>>  Required properties:
>>>>>  - compatible:
>>>>>  	"snps,dw-pcie" for RC mode;
>>>>> +	"snps,dw-pcie-ep" for EP mode;
>>>>>  - reg: Should contain the configuration address space.
>>>>>  - reg-names: Must be "config" for the PCIe configuration space.
>>>>>      (The old way of getting the configuration address space from "ranges"
>>>>> @@ -56,3 +57,15 @@ Example configuration:
>>>>>  		#interrupt-cells = <1>;
>>>>>  		num-lanes = <1>;
>>>>>  	};
>>>>> +or
>>>>> +	pcie_ep: pcie_ep@dfc00000 {
>>>>> +		compatible = "snps,dw-pcie-ep";
>>>>> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
>>>>> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */
>>>>
>>>> Doesn't this have iATU unroll space?
>>>
>>> I don't think EP has it, but I'm no expert on this matter. Can you provide me
>>> some example of having iATU unroll space mapping would be useful in EP scope?
>>
>> I'm not sure. I thought if the dwc3 core version is 4.80, then it'll have a
>> separate ATU space irrespective of RC mode or EP mode.
> 
> As replied on patch 1, let's leave out  any reference of iATU unroll to avoid
> any confusion. Agree?

Mentioning iATU is fine as long as you change the size field in the "reg" property.

Thanks
Kishon
Rob Herring April 9, 2018, 7:12 p.m. UTC | #8
On Wed, Mar 28, 2018 at 12:38:33PM +0100, Gustavo Pimentel wrote:
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 6300762..4bb2e08 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -3,6 +3,7 @@
>  Required properties:
>  - compatible:
>  	"snps,dw-pcie" for RC mode;
> +	"snps,dw-pcie-ep" for EP mode;
>  - reg: Should contain the configuration address space.
>  - reg-names: Must be "config" for the PCIe configuration space.
>      (The old way of getting the configuration address space from "ranges"
> @@ -56,3 +57,15 @@ Example configuration:
>  		#interrupt-cells = <1>;
>  		num-lanes = <1>;
>  	};
> +or
> +	pcie_ep: pcie_ep@dfc00000 {

pcie-ep@...

Or what others have used. We should define a standard name in the DT 
spec for this.

> +		compatible = "snps,dw-pcie-ep";
> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */
> +		      <0xd0000000 0x2000000>; /* Configuration space */
> +		reg-names = "dbi", "dbi2", "addr_space";
> +		device_type = "pci";
> +		num-ib-windows = <6>;
> +		num-ob-windows = <2>;
> +		num-lanes = <1>;
> +	};
> -- 
> 2.7.4
> 
>
Gustavo Pimentel April 10, 2018, 11:11 a.m. UTC | #9
Hi Rob,

On 09/04/2018 20:12, Rob Herring wrote:
> On Wed, Mar 28, 2018 at 12:38:33PM +0100, Gustavo Pimentel wrote:
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> index 6300762..4bb2e08 100644
>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> @@ -3,6 +3,7 @@
>>  Required properties:
>>  - compatible:
>>  	"snps,dw-pcie" for RC mode;
>> +	"snps,dw-pcie-ep" for EP mode;
>>  - reg: Should contain the configuration address space.
>>  - reg-names: Must be "config" for the PCIe configuration space.
>>      (The old way of getting the configuration address space from "ranges"
>> @@ -56,3 +57,15 @@ Example configuration:
>>  		#interrupt-cells = <1>;
>>  		num-lanes = <1>;
>>  	};
>> +or
>> +	pcie_ep: pcie_ep@dfc00000 {
> 
> pcie-ep@...
> 
> Or what others have used. We should define a standard name in the DT 
> spec for this.
> 

By looking to [1] and [2], I think I should maintain the pcie name instead of
pcie-ep.

[1] ->
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pci/ti-pci.txt
[2] ->
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt

Thanks for noticing it.

>> +		compatible = "snps,dw-pcie-ep";
>> +		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
>> +		      <0xdfc01000 0x0001000>, /* IP registers 2 */
>> +		      <0xd0000000 0x2000000>; /* Configuration space */
>> +		reg-names = "dbi", "dbi2", "addr_space";
>> +		device_type = "pci";
>> +		num-ib-windows = <6>;
>> +		num-ob-windows = <2>;
>> +		num-lanes = <1>;
>> +	};
>> -- 
>> 2.7.4
>>
>>

Regards,
Gustavo
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index 6300762..4bb2e08 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -3,6 +3,7 @@ 
 Required properties:
 - compatible:
 	"snps,dw-pcie" for RC mode;
+	"snps,dw-pcie-ep" for EP mode;
 - reg: Should contain the configuration address space.
 - reg-names: Must be "config" for the PCIe configuration space.
     (The old way of getting the configuration address space from "ranges"
@@ -56,3 +57,15 @@  Example configuration:
 		#interrupt-cells = <1>;
 		num-lanes = <1>;
 	};
+or
+	pcie_ep: pcie_ep@dfc00000 {
+		compatible = "snps,dw-pcie-ep";
+		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
+		      <0xdfc01000 0x0001000>, /* IP registers 2 */
+		      <0xd0000000 0x2000000>; /* Configuration space */
+		reg-names = "dbi", "dbi2", "addr_space";
+		device_type = "pci";
+		num-ib-windows = <6>;
+		num-ob-windows = <2>;
+		num-lanes = <1>;
+	};