Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Kishon Vijay Abraham I]

Hi,

On Tuesday 03 April 2018 06:43 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 03/04/2018 11:53, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 03 April 2018 04:22 PM, Kishon Vijay Abraham I wrote:
>>>
>>>
>>> On Tuesday 03 April 2018 04:03 PM, Gustavo Pimentel wrote:
 Hi Kishon,

 On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>> Changes the IP registers size to accommodate the ATU unroll space.
>>
>> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
>>
>> Replaces the pcie base address example by a real pcie base address in 
>> use.
>>
>> Signed-off-by: Gustavo Pimentel 
>> ---
>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 
>> ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
>> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> index 1da7ade..6300762 100644
>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> @@ -1,7 +1,8 @@
>>  * Synopsys DesignWare PCIe interface
>>  
>>  Required properties:
>> -- compatible: should contain "snps,dw-pcie" to identify the core.
>> +- compatible:
>> +"snps,dw-pcie" for RC mode;
>
> I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
> identify the pcie core?

 I guess so. What you suggest? I was just folling the same guideline present
 here: 
 https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2017_11_3_310=DwIC-g=DPL6_X_6JkXFx7AXWqB0tg=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs=G1MqB_DY6ZwWtvS60L9PZMHnMe6rClMnrakAyQT_hDc=BhQYkrcp6y3QkD23qn1I6lU882BDUfLiXjBVWQ91cmg=
>>>
>>> Okay, I think you should have
>>> "snps,dw-pcie-rc", "snps,dw-pcie" for RC mode;
>>>
>>> and in the later patch
>>> "snps,dw-pcie-ep", "snps,dw-pcie" for EP mode;
>>>
> 
> Ok, I'll change it.
> 

>>  - 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"
>> @@ -41,11 +42,11 @@ EP mode:
>>  
>>  Example configuration:
>>  
>> -pcie: pcie@d000 {
>> +pcie: pcie@dfc0 {
>>  compatible = "snps,dw-pcie";
>> -reg = <0xd000 0x1000>, /* Controller registers */
>> -  <0xd000 0x2000>; /* PCI config space */
>> -reg-names = "ctrlreg", "config";
>> +reg = <0xdfc0 0x302000>, /* IP registers */
>
> which version of synopsys IP is this. I think the ideal thing to do here 
> is to
> have a separate register space for iATU.

 I also agree with that. The unroll iATU was introduced on Synopsys IP 
 version
 4.80 and the kernel patch was release on 2016-08-10
 https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_657796_=DwIC-g=DPL6_X_6JkXFx7AXWqB0tg=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs=G1MqB_DY6ZwWtvS60L9PZMHnMe6rClMnrakAyQT_hDc=EgKKDbg4ywCxu4-lG_scYAgPMnxirsjS0uSS7SzBTeM=
 However a separate register space for iATU implies some extra code do 
 handle it
 (and of course some tests) that don't fit into this patch series, in my 
 point of
 view. Can this task enter in the backlog in order to be done in another 
 patch
 series?
>>>
>>> Yes sure. I think we should also make sure existing binding doesn't break.
> 
> I'll remove the any iATU unroll reference or change from this patch to avoid 
> any
> confusion.

No, I think it's fine to have iATU unroll reference (since kernel also seems to
support iATU unroll).

What I meant is, we should add more flexibility on defining iATU address space
(while making sure it doesn't break existing binding).

I'm okay with this patch once you fix the compatible comment but in general
this is what I'd like to have (in a future patch)

*)  Add a new compatible "snps,dw-pcie-4.80";
*) In the kernel "iatu_unroll_enabled" should be set based on the above 
compatible
*) Define the a new reg space for iATU in 4.80

And these have to be done in a way it doesn't break existing binding.

Thanks
Kishon

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Kishon Vijay Abraham I]

Hi,

On Tuesday 03 April 2018 06:43 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 03/04/2018 11:53, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 03 April 2018 04:22 PM, Kishon Vijay Abraham I wrote:
>>>
>>>
>>> On Tuesday 03 April 2018 04:03 PM, Gustavo Pimentel wrote:
 Hi Kishon,

 On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>> Changes the IP registers size to accommodate the ATU unroll space.
>>
>> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
>>
>> Replaces the pcie base address example by a real pcie base address in 
>> use.
>>
>> Signed-off-by: Gustavo Pimentel 
>> ---
>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 
>> ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
>> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> index 1da7ade..6300762 100644
>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> @@ -1,7 +1,8 @@
>>  * Synopsys DesignWare PCIe interface
>>  
>>  Required properties:
>> -- compatible: should contain "snps,dw-pcie" to identify the core.
>> +- compatible:
>> +"snps,dw-pcie" for RC mode;
>
> I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
> identify the pcie core?

 I guess so. What you suggest? I was just folling the same guideline present
 here: 
 https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2017_11_3_310=DwIC-g=DPL6_X_6JkXFx7AXWqB0tg=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs=G1MqB_DY6ZwWtvS60L9PZMHnMe6rClMnrakAyQT_hDc=BhQYkrcp6y3QkD23qn1I6lU882BDUfLiXjBVWQ91cmg=
>>>
>>> Okay, I think you should have
>>> "snps,dw-pcie-rc", "snps,dw-pcie" for RC mode;
>>>
>>> and in the later patch
>>> "snps,dw-pcie-ep", "snps,dw-pcie" for EP mode;
>>>
> 
> Ok, I'll change it.
> 

>>  - 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"
>> @@ -41,11 +42,11 @@ EP mode:
>>  
>>  Example configuration:
>>  
>> -pcie: pcie@d000 {
>> +pcie: pcie@dfc0 {
>>  compatible = "snps,dw-pcie";
>> -reg = <0xd000 0x1000>, /* Controller registers */
>> -  <0xd000 0x2000>; /* PCI config space */
>> -reg-names = "ctrlreg", "config";
>> +reg = <0xdfc0 0x302000>, /* IP registers */
>
> which version of synopsys IP is this. I think the ideal thing to do here 
> is to
> have a separate register space for iATU.

 I also agree with that. The unroll iATU was introduced on Synopsys IP 
 version
 4.80 and the kernel patch was release on 2016-08-10
 https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_657796_=DwIC-g=DPL6_X_6JkXFx7AXWqB0tg=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs=G1MqB_DY6ZwWtvS60L9PZMHnMe6rClMnrakAyQT_hDc=EgKKDbg4ywCxu4-lG_scYAgPMnxirsjS0uSS7SzBTeM=
 However a separate register space for iATU implies some extra code do 
 handle it
 (and of course some tests) that don't fit into this patch series, in my 
 point of
 view. Can this task enter in the backlog in order to be done in another 
 patch
 series?
>>>
>>> Yes sure. I think we should also make sure existing binding doesn't break.
> 
> I'll remove the any iATU unroll reference or change from this patch to avoid 
> any
> confusion.

No, I think it's fine to have iATU unroll reference (since kernel also seems to
support iATU unroll).

What I meant is, we should add more flexibility on defining iATU address space
(while making sure it doesn't break existing binding).

I'm okay with this patch once you fix the compatible comment but in general
this is what I'd like to have (in a future patch)

*)  Add a new compatible "snps,dw-pcie-4.80";
*) In the kernel "iatu_unroll_enabled" should be set based on the above 
compatible
*) Define the a new reg space for iATU in 4.80

And these have to be done in a way it doesn't break existing binding.

Thanks
Kishon

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Gustavo Pimentel]

Hi Kishon,

On 03/04/2018 11:53, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 03 April 2018 04:22 PM, Kishon Vijay Abraham I wrote:
>>
>>
>> On Tuesday 03 April 2018 04:03 PM, Gustavo Pimentel wrote:
>>> Hi Kishon,
>>>
>>> On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
 Hi,

 On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
> Changes the IP registers size to accommodate the ATU unroll space.
>
> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
>
> Replaces the pcie base address example by a real pcie base address in use.
>
> Signed-off-by: Gustavo Pimentel 
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 
> ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 1da7ade..6300762 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -1,7 +1,8 @@
>  * Synopsys DesignWare PCIe interface
>  
>  Required properties:
> -- compatible: should contain "snps,dw-pcie" to identify the core.
> +- compatible:
> + "snps,dw-pcie" for RC mode;

 I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
 identify the pcie core?
>>>
>>> I guess so. What you suggest? I was just folling the same guideline present
>>> here: 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2017_11_3_310=DwIC-g=DPL6_X_6JkXFx7AXWqB0tg=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs=G1MqB_DY6ZwWtvS60L9PZMHnMe6rClMnrakAyQT_hDc=BhQYkrcp6y3QkD23qn1I6lU882BDUfLiXjBVWQ91cmg=
>>
>> Okay, I think you should have
>> "snps,dw-pcie-rc", "snps,dw-pcie" for RC mode;
>>
>> and in the later patch
>> "snps,dw-pcie-ep", "snps,dw-pcie" for EP mode;
>>

Ok, I'll change it.

>>>
>  - 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"
> @@ -41,11 +42,11 @@ EP mode:
>  
>  Example configuration:
>  
> - pcie: pcie@d000 {
> + pcie: pcie@dfc0 {
>   compatible = "snps,dw-pcie";
> - reg = <0xd000 0x1000>, /* Controller registers */
> -   <0xd000 0x2000>; /* PCI config space */
> - reg-names = "ctrlreg", "config";
> + reg = <0xdfc0 0x302000>, /* IP registers */

 which version of synopsys IP is this. I think the ideal thing to do here 
 is to
 have a separate register space for iATU.
>>>
>>> I also agree with that. The unroll iATU was introduced on Synopsys IP 
>>> version
>>> 4.80 and the kernel patch was release on 2016-08-10
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_657796_=DwIC-g=DPL6_X_6JkXFx7AXWqB0tg=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs=G1MqB_DY6ZwWtvS60L9PZMHnMe6rClMnrakAyQT_hDc=EgKKDbg4ywCxu4-lG_scYAgPMnxirsjS0uSS7SzBTeM=
>>> However a separate register space for iATU implies some extra code do 
>>> handle it
>>> (and of course some tests) that don't fit into this patch series, in my 
>>> point of
>>> view. Can this task enter in the backlog in order to be done in another 
>>> patch
>>> series?
>>
>> Yes sure. I think we should also make sure existing binding doesn't break.

I'll remove the any iATU unroll reference or change from this patch to avoid any
confusion.

> 
> And IMO we should have a new compatible "snps,dw-pcie-4.80" in order to enable
> iATU unroll.
> 
> Thanks
> Kishon
> 

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Gustavo Pimentel]

Hi Kishon,

On 03/04/2018 11:53, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 03 April 2018 04:22 PM, Kishon Vijay Abraham I wrote:
>>
>>
>> On Tuesday 03 April 2018 04:03 PM, Gustavo Pimentel wrote:
>>> Hi Kishon,
>>>
>>> On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
 Hi,

 On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
> Changes the IP registers size to accommodate the ATU unroll space.
>
> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
>
> Replaces the pcie base address example by a real pcie base address in use.
>
> Signed-off-by: Gustavo Pimentel 
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 
> ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 1da7ade..6300762 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -1,7 +1,8 @@
>  * Synopsys DesignWare PCIe interface
>  
>  Required properties:
> -- compatible: should contain "snps,dw-pcie" to identify the core.
> +- compatible:
> + "snps,dw-pcie" for RC mode;

 I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
 identify the pcie core?
>>>
>>> I guess so. What you suggest? I was just folling the same guideline present
>>> here: 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2017_11_3_310=DwIC-g=DPL6_X_6JkXFx7AXWqB0tg=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs=G1MqB_DY6ZwWtvS60L9PZMHnMe6rClMnrakAyQT_hDc=BhQYkrcp6y3QkD23qn1I6lU882BDUfLiXjBVWQ91cmg=
>>
>> Okay, I think you should have
>> "snps,dw-pcie-rc", "snps,dw-pcie" for RC mode;
>>
>> and in the later patch
>> "snps,dw-pcie-ep", "snps,dw-pcie" for EP mode;
>>

Ok, I'll change it.

>>>
>  - 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"
> @@ -41,11 +42,11 @@ EP mode:
>  
>  Example configuration:
>  
> - pcie: pcie@d000 {
> + pcie: pcie@dfc0 {
>   compatible = "snps,dw-pcie";
> - reg = <0xd000 0x1000>, /* Controller registers */
> -   <0xd000 0x2000>; /* PCI config space */
> - reg-names = "ctrlreg", "config";
> + reg = <0xdfc0 0x302000>, /* IP registers */

 which version of synopsys IP is this. I think the ideal thing to do here 
 is to
 have a separate register space for iATU.
>>>
>>> I also agree with that. The unroll iATU was introduced on Synopsys IP 
>>> version
>>> 4.80 and the kernel patch was release on 2016-08-10
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_657796_=DwIC-g=DPL6_X_6JkXFx7AXWqB0tg=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs=G1MqB_DY6ZwWtvS60L9PZMHnMe6rClMnrakAyQT_hDc=EgKKDbg4ywCxu4-lG_scYAgPMnxirsjS0uSS7SzBTeM=
>>> However a separate register space for iATU implies some extra code do 
>>> handle it
>>> (and of course some tests) that don't fit into this patch series, in my 
>>> point of
>>> view. Can this task enter in the backlog in order to be done in another 
>>> patch
>>> series?
>>
>> Yes sure. I think we should also make sure existing binding doesn't break.

I'll remove the any iATU unroll reference or change from this patch to avoid any
confusion.

> 
> And IMO we should have a new compatible "snps,dw-pcie-4.80" in order to enable
> iATU unroll.
> 
> Thanks
> Kishon
> 

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Kishon Vijay Abraham I]

On Tuesday 03 April 2018 04:03 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>>> Changes the IP registers size to accommodate the ATU unroll space.
>>>
>>> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
>>>
>>> Replaces the pcie base address example by a real pcie base address in use.
>>>
>>> Signed-off-by: Gustavo Pimentel 
>>> ---
>>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 ++--
>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
>>> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> index 1da7ade..6300762 100644
>>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> @@ -1,7 +1,8 @@
>>>  * Synopsys DesignWare PCIe interface
>>>  
>>>  Required properties:
>>> -- compatible: should contain "snps,dw-pcie" to identify the core.
>>> +- compatible:
>>> +   "snps,dw-pcie" for RC mode;
>>
>> I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
>> identify the pcie core?
> 
> I guess so. What you suggest? I was just folling the same guideline present
> here: https://lkml.org/lkml/2017/11/3/310

Okay, I think you should have
"snps,dw-pcie-rc", "snps,dw-pcie" for RC mode;

and in the later patch
"snps,dw-pcie-ep", "snps,dw-pcie" 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"
>>> @@ -41,11 +42,11 @@ EP mode:
>>>  
>>>  Example configuration:
>>>  
>>> -   pcie: pcie@d000 {
>>> +   pcie: pcie@dfc0 {
>>> compatible = "snps,dw-pcie";
>>> -   reg = <0xd000 0x1000>, /* Controller registers */
>>> - <0xd000 0x2000>; /* PCI config space */
>>> -   reg-names = "ctrlreg", "config";
>>> +   reg = <0xdfc0 0x302000>, /* IP registers */
>>
>> which version of synopsys IP is this. I think the ideal thing to do here is 
>> to
>> have a separate register space for iATU.
> 
> I also agree with that. The unroll iATU was introduced on Synopsys IP version
> 4.80 and the kernel patch was release on 2016-08-10
> https://patchwork.ozlabs.org/patch/657796/
> However a separate register space for iATU implies some extra code do handle 
> it
> (and of course some tests) that don't fit into this patch series, in my point 
> of
> view. Can this task enter in the backlog in order to be done in another patch
> series?

Yes sure. I think we should also make sure existing binding doesn't break.

Thanks
Kishon

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Kishon Vijay Abraham I]

On Tuesday 03 April 2018 04:03 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>>> Changes the IP registers size to accommodate the ATU unroll space.
>>>
>>> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
>>>
>>> Replaces the pcie base address example by a real pcie base address in use.
>>>
>>> Signed-off-by: Gustavo Pimentel 
>>> ---
>>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 ++--
>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
>>> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> index 1da7ade..6300762 100644
>>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>> @@ -1,7 +1,8 @@
>>>  * Synopsys DesignWare PCIe interface
>>>  
>>>  Required properties:
>>> -- compatible: should contain "snps,dw-pcie" to identify the core.
>>> +- compatible:
>>> +   "snps,dw-pcie" for RC mode;
>>
>> I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
>> identify the pcie core?
> 
> I guess so. What you suggest? I was just folling the same guideline present
> here: https://lkml.org/lkml/2017/11/3/310

Okay, I think you should have
"snps,dw-pcie-rc", "snps,dw-pcie" for RC mode;

and in the later patch
"snps,dw-pcie-ep", "snps,dw-pcie" 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"
>>> @@ -41,11 +42,11 @@ EP mode:
>>>  
>>>  Example configuration:
>>>  
>>> -   pcie: pcie@d000 {
>>> +   pcie: pcie@dfc0 {
>>> compatible = "snps,dw-pcie";
>>> -   reg = <0xd000 0x1000>, /* Controller registers */
>>> - <0xd000 0x2000>; /* PCI config space */
>>> -   reg-names = "ctrlreg", "config";
>>> +   reg = <0xdfc0 0x302000>, /* IP registers */
>>
>> which version of synopsys IP is this. I think the ideal thing to do here is 
>> to
>> have a separate register space for iATU.
> 
> I also agree with that. The unroll iATU was introduced on Synopsys IP version
> 4.80 and the kernel patch was release on 2016-08-10
> https://patchwork.ozlabs.org/patch/657796/
> However a separate register space for iATU implies some extra code do handle 
> it
> (and of course some tests) that don't fit into this patch series, in my point 
> of
> view. Can this task enter in the backlog in order to be done in another patch
> series?

Yes sure. I think we should also make sure existing binding doesn't break.

Thanks
Kishon

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Kishon Vijay Abraham I]

Hi,

On Tuesday 03 April 2018 04:22 PM, Kishon Vijay Abraham I wrote:
> 
> 
> On Tuesday 03 April 2018 04:03 PM, Gustavo Pimentel wrote:
>> Hi Kishon,
>>
>> On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
 Changes the IP registers size to accommodate the ATU unroll space.

 Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.

 Replaces the pcie base address example by a real pcie base address in use.

 Signed-off-by: Gustavo Pimentel 
 ---
  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 
 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

 diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
 b/Documentation/devicetree/bindings/pci/designware-pcie.txt
 index 1da7ade..6300762 100644
 --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
 +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
 @@ -1,7 +1,8 @@
  * Synopsys DesignWare PCIe interface
  
  Required properties:
 -- compatible: should contain "snps,dw-pcie" to identify the core.
 +- compatible:
 +  "snps,dw-pcie" for RC mode;
>>>
>>> I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
>>> identify the pcie core?
>>
>> I guess so. What you suggest? I was just folling the same guideline present
>> here: https://lkml.org/lkml/2017/11/3/310
> 
> Okay, I think you should have
> "snps,dw-pcie-rc", "snps,dw-pcie" for RC mode;
> 
> and in the later patch
> "snps,dw-pcie-ep", "snps,dw-pcie" 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"
 @@ -41,11 +42,11 @@ EP mode:
  
  Example configuration:
  
 -  pcie: pcie@d000 {
 +  pcie: pcie@dfc0 {
compatible = "snps,dw-pcie";
 -  reg = <0xd000 0x1000>, /* Controller registers */
 -<0xd000 0x2000>; /* PCI config space */
 -  reg-names = "ctrlreg", "config";
 +  reg = <0xdfc0 0x302000>, /* IP registers */
>>>
>>> which version of synopsys IP is this. I think the ideal thing to do here is 
>>> to
>>> have a separate register space for iATU.
>>
>> I also agree with that. The unroll iATU was introduced on Synopsys IP version
>> 4.80 and the kernel patch was release on 2016-08-10
>> https://patchwork.ozlabs.org/patch/657796/
>> However a separate register space for iATU implies some extra code do handle 
>> it
>> (and of course some tests) that don't fit into this patch series, in my 
>> point of
>> view. Can this task enter in the backlog in order to be done in another patch
>> series?
> 
> Yes sure. I think we should also make sure existing binding doesn't break.

And IMO we should have a new compatible "snps,dw-pcie-4.80" in order to enable
iATU unroll.

Thanks
Kishon

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Kishon Vijay Abraham I]

Hi,

On Tuesday 03 April 2018 04:22 PM, Kishon Vijay Abraham I wrote:
> 
> 
> On Tuesday 03 April 2018 04:03 PM, Gustavo Pimentel wrote:
>> Hi Kishon,
>>
>> On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
 Changes the IP registers size to accommodate the ATU unroll space.

 Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.

 Replaces the pcie base address example by a real pcie base address in use.

 Signed-off-by: Gustavo Pimentel 
 ---
  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 
 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

 diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
 b/Documentation/devicetree/bindings/pci/designware-pcie.txt
 index 1da7ade..6300762 100644
 --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
 +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
 @@ -1,7 +1,8 @@
  * Synopsys DesignWare PCIe interface
  
  Required properties:
 -- compatible: should contain "snps,dw-pcie" to identify the core.
 +- compatible:
 +  "snps,dw-pcie" for RC mode;
>>>
>>> I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
>>> identify the pcie core?
>>
>> I guess so. What you suggest? I was just folling the same guideline present
>> here: https://lkml.org/lkml/2017/11/3/310
> 
> Okay, I think you should have
> "snps,dw-pcie-rc", "snps,dw-pcie" for RC mode;
> 
> and in the later patch
> "snps,dw-pcie-ep", "snps,dw-pcie" 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"
 @@ -41,11 +42,11 @@ EP mode:
  
  Example configuration:
  
 -  pcie: pcie@d000 {
 +  pcie: pcie@dfc0 {
compatible = "snps,dw-pcie";
 -  reg = <0xd000 0x1000>, /* Controller registers */
 -<0xd000 0x2000>; /* PCI config space */
 -  reg-names = "ctrlreg", "config";
 +  reg = <0xdfc0 0x302000>, /* IP registers */
>>>
>>> which version of synopsys IP is this. I think the ideal thing to do here is 
>>> to
>>> have a separate register space for iATU.
>>
>> I also agree with that. The unroll iATU was introduced on Synopsys IP version
>> 4.80 and the kernel patch was release on 2016-08-10
>> https://patchwork.ozlabs.org/patch/657796/
>> However a separate register space for iATU implies some extra code do handle 
>> it
>> (and of course some tests) that don't fit into this patch series, in my 
>> point of
>> view. Can this task enter in the backlog in order to be done in another patch
>> series?
> 
> Yes sure. I think we should also make sure existing binding doesn't break.

And IMO we should have a new compatible "snps,dw-pcie-4.80" in order to enable
iATU unroll.

Thanks
Kishon

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Gustavo Pimentel]

Hi Kishon,

On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>> Changes the IP registers size to accommodate the ATU unroll space.
>>
>> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
>>
>> Replaces the pcie base address example by a real pcie base address in use.
>>
>> Signed-off-by: Gustavo Pimentel 
>> ---
>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
>> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> index 1da7ade..6300762 100644
>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> @@ -1,7 +1,8 @@
>>  * Synopsys DesignWare PCIe interface
>>  
>>  Required properties:
>> -- compatible: should contain "snps,dw-pcie" to identify the core.
>> +- compatible:
>> +"snps,dw-pcie" for RC mode;
> 
> I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
> identify the pcie core?

I guess so. What you suggest? I was just folling the same guideline present
here: https://lkml.org/lkml/2017/11/3/310

>>  - 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"
>> @@ -41,11 +42,11 @@ EP mode:
>>  
>>  Example configuration:
>>  
>> -pcie: pcie@d000 {
>> +pcie: pcie@dfc0 {
>>  compatible = "snps,dw-pcie";
>> -reg = <0xd000 0x1000>, /* Controller registers */
>> -  <0xd000 0x2000>; /* PCI config space */
>> -reg-names = "ctrlreg", "config";
>> +reg = <0xdfc0 0x302000>, /* IP registers */
> 
> which version of synopsys IP is this. I think the ideal thing to do here is to
> have a separate register space for iATU.

I also agree with that. The unroll iATU was introduced on Synopsys IP version
4.80 and the kernel patch was release on 2016-08-10
https://patchwork.ozlabs.org/patch/657796/
However a separate register space for iATU implies some extra code do handle it
(and of course some tests) that don't fit into this patch series, in my point of
view. Can this task enter in the backlog in order to be done in another patch
series?

> 
> Thanks
> Kishon
> 

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Gustavo Pimentel]

Hi Kishon,

On 02/04/2018 06:23, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
>> Changes the IP registers size to accommodate the ATU unroll space.
>>
>> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
>>
>> Replaces the pcie base address example by a real pcie base address in use.
>>
>> Signed-off-by: Gustavo Pimentel 
>> ---
>>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
>> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> index 1da7ade..6300762 100644
>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>> @@ -1,7 +1,8 @@
>>  * Synopsys DesignWare PCIe interface
>>  
>>  Required properties:
>> -- compatible: should contain "snps,dw-pcie" to identify the core.
>> +- compatible:
>> +"snps,dw-pcie" for RC mode;
> 
> I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
> identify the pcie core?

I guess so. What you suggest? I was just folling the same guideline present
here: https://lkml.org/lkml/2017/11/3/310

>>  - 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"
>> @@ -41,11 +42,11 @@ EP mode:
>>  
>>  Example configuration:
>>  
>> -pcie: pcie@d000 {
>> +pcie: pcie@dfc0 {
>>  compatible = "snps,dw-pcie";
>> -reg = <0xd000 0x1000>, /* Controller registers */
>> -  <0xd000 0x2000>; /* PCI config space */
>> -reg-names = "ctrlreg", "config";
>> +reg = <0xdfc0 0x302000>, /* IP registers */
> 
> which version of synopsys IP is this. I think the ideal thing to do here is to
> have a separate register space for iATU.

I also agree with that. The unroll iATU was introduced on Synopsys IP version
4.80 and the kernel patch was release on 2016-08-10
https://patchwork.ozlabs.org/patch/657796/
However a separate register space for iATU implies some extra code do handle it
(and of course some tests) that don't fit into this patch series, in my point of
view. Can this task enter in the backlog in order to be done in another patch
series?

> 
> Thanks
> Kishon
> 

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Kishon Vijay Abraham I]

Hi,

On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
> Changes the IP registers size to accommodate the ATU unroll space.
> 
> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
> 
> Replaces the pcie base address example by a real pcie base address in use.
> 
> Signed-off-by: Gustavo Pimentel 
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 1da7ade..6300762 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -1,7 +1,8 @@
>  * Synopsys DesignWare PCIe interface
>  
>  Required properties:
> -- compatible: should contain "snps,dw-pcie" to identify the core.
> +- compatible:
> + "snps,dw-pcie" for RC mode;

I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
identify the pcie core?
>  - 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"
> @@ -41,11 +42,11 @@ EP mode:
>  
>  Example configuration:
>  
> - pcie: pcie@d000 {
> + pcie: pcie@dfc0 {
>   compatible = "snps,dw-pcie";
> - reg = <0xd000 0x1000>, /* Controller registers */
> -   <0xd000 0x2000>; /* PCI config space */
> - reg-names = "ctrlreg", "config";
> + reg = <0xdfc0 0x302000>, /* IP registers */

which version of synopsys IP is this. I think the ideal thing to do here is to
have a separate register space for iATU.

Thanks
Kishon

Re: [PATCH 1/8] bindings: PCI: designware: Example update

[Kishon Vijay Abraham I]

Hi,

On Wednesday 28 March 2018 05:08 PM, Gustavo Pimentel wrote:
> Changes the IP registers size to accommodate the ATU unroll space.
> 
> Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.
> 
> Replaces the pcie base address example by a real pcie base address in use.
> 
> Signed-off-by: Gustavo Pimentel 
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 1da7ade..6300762 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -1,7 +1,8 @@
>  * Synopsys DesignWare PCIe interface
>  
>  Required properties:
> -- compatible: should contain "snps,dw-pcie" to identify the core.
> +- compatible:
> + "snps,dw-pcie" for RC mode;

I think irrespective of RC mode or EP mode, "snps,dw-pcie" can be used to
identify the pcie core?
>  - 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"
> @@ -41,11 +42,11 @@ EP mode:
>  
>  Example configuration:
>  
> - pcie: pcie@d000 {
> + pcie: pcie@dfc0 {
>   compatible = "snps,dw-pcie";
> - reg = <0xd000 0x1000>, /* Controller registers */
> -   <0xd000 0x2000>; /* PCI config space */
> - reg-names = "ctrlreg", "config";
> + reg = <0xdfc0 0x302000>, /* IP registers */

which version of synopsys IP is this. I think the ideal thing to do here is to
have a separate register space for iATU.

Thanks
Kishon

[PATCH 1/8] bindings: PCI: designware: Example update

[Gustavo Pimentel]

Changes the IP registers size to accommodate the ATU unroll space.

Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.

Replaces the pcie base address example by a real pcie base address in use.

Signed-off-by: Gustavo Pimentel 
---
 Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index 1da7ade..6300762 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -1,7 +1,8 @@
 * Synopsys DesignWare PCIe interface
 
 Required properties:
-- compatible: should contain "snps,dw-pcie" to identify the core.
+- compatible:
+   "snps,dw-pcie" for RC 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"
@@ -41,11 +42,11 @@ EP mode:
 
 Example configuration:
 
-   pcie: pcie@d000 {
+   pcie: pcie@dfc0 {
compatible = "snps,dw-pcie";
-   reg = <0xd000 0x1000>, /* Controller registers */
- <0xd000 0x2000>; /* PCI config space */
-   reg-names = "ctrlreg", "config";
+   reg = <0xdfc0 0x302000>, /* IP registers */
+ <0xd000 0x002000>; /* Configuration space */
+   reg-names = "dbi", "config";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -54,5 +55,4 @@ Example configuration:
interrupts = <25>, <24>;
#interrupt-cells = <1>;
num-lanes = <1>;
-   num-viewport = <3>;
};
-- 
2.7.4

[PATCH 1/8] bindings: PCI: designware: Example update

[Gustavo Pimentel]

Changes the IP registers size to accommodate the ATU unroll space.

Replaces "ctrlreg" reg-name by "dbi" to be coherent with similar drivers.

Replaces the pcie base address example by a real pcie base address in use.

Signed-off-by: Gustavo Pimentel 
---
 Documentation/devicetree/bindings/pci/designware-pcie.txt | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt 
b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index 1da7ade..6300762 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -1,7 +1,8 @@
 * Synopsys DesignWare PCIe interface
 
 Required properties:
-- compatible: should contain "snps,dw-pcie" to identify the core.
+- compatible:
+   "snps,dw-pcie" for RC 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"
@@ -41,11 +42,11 @@ EP mode:
 
 Example configuration:
 
-   pcie: pcie@d000 {
+   pcie: pcie@dfc0 {
compatible = "snps,dw-pcie";
-   reg = <0xd000 0x1000>, /* Controller registers */
- <0xd000 0x2000>; /* PCI config space */
-   reg-names = "ctrlreg", "config";
+   reg = <0xdfc0 0x302000>, /* IP registers */
+ <0xd000 0x002000>; /* Configuration space */
+   reg-names = "dbi", "config";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -54,5 +55,4 @@ Example configuration:
interrupts = <25>, <24>;
#interrupt-cells = <1>;
num-lanes = <1>;
-   num-viewport = <3>;
};
-- 
2.7.4