Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-14 Thread Vivek Gautam 
Hi,


On Mon, Nov 14, 2016 at 4:02 PM, Srinivas Kandagatla
 wrote:
>
>
> On 09/11/16 10:37, Vivek Gautam wrote:
>>
>> Hi,
>>
>> On Fri, Nov 4, 2016 at 6:29 PM, Srinivas Kandagatla
>>  wrote:
>>>
>>> This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
>>> Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
>>> legacy interrupts and it conforms to PCI Express Base 2.1 specification.
>>>
>>> This patch adds post_init callback to qcom_pcie_ops, as this is pcie
>>> pipe clocks are only setup after the phy is powered on.
>>> It also adds ltssm_enable callback as it is very much different to other
>>> supported SOCs in the driver.
>>>
>>> Signed-off-by: Srinivas Kandagatla 
>>> ---
>>
>>
>> Few minor nits.
>>
>>>  .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
>>>  drivers/pci/host/pcie-qcom.c   | 177
>>> -
>>>  2 files changed, 239 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> index 4059a6f..4a0538d 100644
>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> @@ -7,6 +7,7 @@
>>> - "qcom,pcie-ipq8064" for ipq8064
>>> - "qcom,pcie-apq8064" for apq8064
>>> - "qcom,pcie-apq8084" for apq8084
>>> +   - "qcom,pcie-msm8996" for msm8996 or apq8096
>>
>>
>> Since this works for both apq8096 and msm8996, compatible -
>> "qcom,pcie-apq8096" for uniformity ?
>
>
> AFAIK, compatible is selected based on SOC on which this IP is integrated
> first, So msm8996 seems to be correct, in that way.
>
> Also if we look at clk controller compatible strings, you would see them as
> *msm8996* rather than *8096*.

ok, cool. I didn't notice that. This looks good then.


Thanks
Vivek

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-14 Thread Vivek Gautam 
Hi,


On Mon, Nov 14, 2016 at 4:02 PM, Srinivas Kandagatla
 wrote:
>
>
> On 09/11/16 10:37, Vivek Gautam wrote:
>>
>> Hi,
>>
>> On Fri, Nov 4, 2016 at 6:29 PM, Srinivas Kandagatla
>>  wrote:
>>>
>>> This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
>>> Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
>>> legacy interrupts and it conforms to PCI Express Base 2.1 specification.
>>>
>>> This patch adds post_init callback to qcom_pcie_ops, as this is pcie
>>> pipe clocks are only setup after the phy is powered on.
>>> It also adds ltssm_enable callback as it is very much different to other
>>> supported SOCs in the driver.
>>>
>>> Signed-off-by: Srinivas Kandagatla 
>>> ---
>>
>>
>> Few minor nits.
>>
>>>  .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
>>>  drivers/pci/host/pcie-qcom.c   | 177
>>> -
>>>  2 files changed, 239 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> index 4059a6f..4a0538d 100644
>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> @@ -7,6 +7,7 @@
>>> - "qcom,pcie-ipq8064" for ipq8064
>>> - "qcom,pcie-apq8064" for apq8064
>>> - "qcom,pcie-apq8084" for apq8084
>>> +   - "qcom,pcie-msm8996" for msm8996 or apq8096
>>
>>
>> Since this works for both apq8096 and msm8996, compatible -
>> "qcom,pcie-apq8096" for uniformity ?
>
>
> AFAIK, compatible is selected based on SOC on which this IP is integrated
> first, So msm8996 seems to be correct, in that way.
>
> Also if we look at clk controller compatible strings, you would see them as
> *msm8996* rather than *8096*.

ok, cool. I didn't notice that. This looks good then.


Thanks
Vivek

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-14 Thread Srinivas Kandagatla 



On 09/11/16 10:37, Vivek Gautam wrote:

Hi,

On Fri, Nov 4, 2016 at 6:29 PM, Srinivas Kandagatla
 wrote:

This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
legacy interrupts and it conforms to PCI Express Base 2.1 specification.

This patch adds post_init callback to qcom_pcie_ops, as this is pcie
pipe clocks are only setup after the phy is powered on.
It also adds ltssm_enable callback as it is very much different to other
supported SOCs in the driver.

Signed-off-by: Srinivas Kandagatla 
---


Few minor nits.


 .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
 drivers/pci/host/pcie-qcom.c   | 177 -
 2 files changed, 239 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt 
b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 4059a6f..4a0538d 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -7,6 +7,7 @@
- "qcom,pcie-ipq8064" for ipq8064
- "qcom,pcie-apq8064" for apq8064
- "qcom,pcie-apq8084" for apq8084
+   - "qcom,pcie-msm8996" for msm8996 or apq8096


Since this works for both apq8096 and msm8996, compatible -
"qcom,pcie-apq8096" for uniformity ?


AFAIK, compatible is selected based on SOC on which this IP is 
integrated first, So msm8996 seems to be correct, in that way.


Also if we look at clk controller compatible strings, you would see them 
as *msm8996* rather than *8096*.

+
+static int qcom_pcie_init_v2(struct qcom_pcie *pcie)
+{
+   struct qcom_pcie_resources_v2 *res = >res.v2;
+   struct device *dev = pcie->pp.dev;
+   u32 val;
+   int ret = 0;


you don't need to initialize ret here.

Yep, I will fix it.



+
+   ret = clk_prepare_enable(res->aux_clk);
+   if (ret) {
+   dev_err(dev, "cannot prepare/enable aux clock\n");
+   return ret;
+   }


[snip]


@@ -429,6 +571,17 @@ static int qcom_pcie_link_up(struct pcie_port *pp)
return !!(val & PCI_EXP_LNKSTA_DLLLA);
 }

+static void qcom_pcie_deinit_v2(struct qcom_pcie *pcie)
+{
+   struct qcom_pcie_resources_v2 *res = >res.v2;
+
+   clk_disable_unprepare(res->slave_clk);
+   clk_disable_unprepare(res->master_clk);
+   clk_disable_unprepare(res->cfg_clk);
+   clk_disable_unprepare(res->aux_clk);
+   clk_disable_unprepare(res->pipe_clk);


i am sure, this is not affecting the functionality, but the pipe clock
is enabled after all the clocks.
so it makes sense to disable it in the first place. you can just move
this above slave_clk.


Sure.. will fix it.


[snip]

Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-14 Thread Srinivas Kandagatla 



On 09/11/16 10:37, Vivek Gautam wrote:

Hi,

On Fri, Nov 4, 2016 at 6:29 PM, Srinivas Kandagatla
 wrote:

This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
legacy interrupts and it conforms to PCI Express Base 2.1 specification.

This patch adds post_init callback to qcom_pcie_ops, as this is pcie
pipe clocks are only setup after the phy is powered on.
It also adds ltssm_enable callback as it is very much different to other
supported SOCs in the driver.

Signed-off-by: Srinivas Kandagatla 
---


Few minor nits.


 .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
 drivers/pci/host/pcie-qcom.c   | 177 -
 2 files changed, 239 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt 
b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 4059a6f..4a0538d 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -7,6 +7,7 @@
- "qcom,pcie-ipq8064" for ipq8064
- "qcom,pcie-apq8064" for apq8064
- "qcom,pcie-apq8084" for apq8084
+   - "qcom,pcie-msm8996" for msm8996 or apq8096


Since this works for both apq8096 and msm8996, compatible -
"qcom,pcie-apq8096" for uniformity ?


AFAIK, compatible is selected based on SOC on which this IP is 
integrated first, So msm8996 seems to be correct, in that way.


Also if we look at clk controller compatible strings, you would see them 
as *msm8996* rather than *8096*.

+
+static int qcom_pcie_init_v2(struct qcom_pcie *pcie)
+{
+   struct qcom_pcie_resources_v2 *res = >res.v2;
+   struct device *dev = pcie->pp.dev;
+   u32 val;
+   int ret = 0;


you don't need to initialize ret here.

Yep, I will fix it.



+
+   ret = clk_prepare_enable(res->aux_clk);
+   if (ret) {
+   dev_err(dev, "cannot prepare/enable aux clock\n");
+   return ret;
+   }


[snip]


@@ -429,6 +571,17 @@ static int qcom_pcie_link_up(struct pcie_port *pp)
return !!(val & PCI_EXP_LNKSTA_DLLLA);
 }

+static void qcom_pcie_deinit_v2(struct qcom_pcie *pcie)
+{
+   struct qcom_pcie_resources_v2 *res = >res.v2;
+
+   clk_disable_unprepare(res->slave_clk);
+   clk_disable_unprepare(res->master_clk);
+   clk_disable_unprepare(res->cfg_clk);
+   clk_disable_unprepare(res->aux_clk);
+   clk_disable_unprepare(res->pipe_clk);


i am sure, this is not affecting the functionality, but the pipe clock
is enabled after all the clocks.
so it makes sense to disable it in the first place. you can just move
this above slave_clk.


Sure.. will fix it.


[snip]

Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-10 Thread Rob Herring 
On Fri, Nov 04, 2016 at 12:59:46PM +, Srinivas Kandagatla wrote:
> This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
> Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
> legacy interrupts and it conforms to PCI Express Base 2.1 specification.
> 
> This patch adds post_init callback to qcom_pcie_ops, as this is pcie
> pipe clocks are only setup after the phy is powered on.
> It also adds ltssm_enable callback as it is very much different to other
> supported SOCs in the driver.
> 
> Signed-off-by: Srinivas Kandagatla 
> ---
>  .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
>  drivers/pci/host/pcie-qcom.c   | 177 
> -
>  2 files changed, 239 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt 
> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 4059a6f..4a0538d 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -7,6 +7,7 @@
>   - "qcom,pcie-ipq8064" for ipq8064
>   - "qcom,pcie-apq8064" for apq8064
>   - "qcom,pcie-apq8084" for apq8084
> + - "qcom,pcie-msm8996" for msm8996 or apq8096
>  
>  - reg:
>   Usage: required
> @@ -92,6 +93,16 @@
>   - "aux" Auxiliary (AUX) clock
>   - "bus_master"  Master AXI clock
>   - "bus_slave"   Slave AXI clock
> +
> +- clock-names:
> + Usage: required for msm8996/apq8096
> + Value type: 
> + Definition: Should contain the following entries
> + - "aux" Auxiliary (AUX) clock.
> + - "bus_master"  Master AXI clock.
> + - "bus_slave"   Slave AXI clock.
> + - "pipe"Pipe Clock driving internal logic.
> + - "cfg" Configuration clk.

The order here and the order in the example don't match. The order 
should be defined.

>  - resets:
>   Usage: required
>   Value type: 
> @@ -115,7 +126,7 @@
>   - "core" Core reset
>  
>  - power-domains:
> - Usage: required for apq8084
> + Usage: required for apq8084 and msm8996/apq8096
>   Value type: 
>   Definition: A phandle and power domain specifier pair to the
>   power domain which is responsible for collapsing
> @@ -231,3 +242,58 @@
>   pinctrl-0 = <_pins_default>;
>   pinctrl-names = "default";
>   };
> +
> +* Example for apq8096:
> +
> + pcie@00608000{

Drop leading 0s.

> + compatible = "qcom,pcie-msm8996", "snps,dw-pcie";
> + power-domains = < PCIE1_GDSC>;
> + bus-range = <0x00 0xff>;
> + num-lanes = <1>;
> +
> + status  = "disabled";

No point to have status in the example.

> +
> + reg = <0x00608000 0x2000>,
> +   <0x0d00 0xf1d>,
> +   <0x0d000f20 0xa8>,
> +   <0x0d10 0x10>;
> +
> + reg-names = "parf", "dbi", "elbi", "config";
> +
> + phys = <_phy 1>;
> + phy-names = "pciephy";
> +
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x0100 0x0 0x0d20 0x0d20 0x0 0x10>,
> + <0x0200 0x0 0x0d30 0x0d30 0x0 0xd0>;
> +
> + interrupts = ;
> + interrupt-names = "msi";
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1  0 272 IRQ_TYPE_LEVEL_HIGH>, /* 
> int_a */
> + <0 0 0 2  0 273 IRQ_TYPE_LEVEL_HIGH>, /* 
> int_b */
> + <0 0 0 3  0 274 IRQ_TYPE_LEVEL_HIGH>, /* 
> int_c */
> + <0 0 0 4  0 275 IRQ_TYPE_LEVEL_HIGH>; /* 
> int_d */
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <_clkreq_default _perst_default 
> _wake_default>;
> + pinctrl-1 = <_clkreq_sleep _perst_default 
> _wake_sleep>;
> +
> + vdda-1p8-supply = <_l12>;
> + vdda-supply = <_l28>;
> + linux,pci-domain = <1>;
> +
> + clocks = < GCC_PCIE_1_PIPE_CLK>,
> + < GCC_PCIE_1_AUX_CLK>,
> + < GCC_PCIE_1_CFG_AHB_CLK>,
> + < GCC_PCIE_1_MSTR_AXI_CLK>,
> + < GCC_PCIE_1_SLV_AXI_CLK>;
> +
> + clock-names =  "pipe",
> + "aux",
> + "cfg",
> + "bus_master",
> + "bus_slave";
> + };

Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-10 Thread Rob Herring 
On Fri, Nov 04, 2016 at 12:59:46PM +, Srinivas Kandagatla wrote:
> This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
> Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
> legacy interrupts and it conforms to PCI Express Base 2.1 specification.
> 
> This patch adds post_init callback to qcom_pcie_ops, as this is pcie
> pipe clocks are only setup after the phy is powered on.
> It also adds ltssm_enable callback as it is very much different to other
> supported SOCs in the driver.
> 
> Signed-off-by: Srinivas Kandagatla 
> ---
>  .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
>  drivers/pci/host/pcie-qcom.c   | 177 
> -
>  2 files changed, 239 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt 
> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 4059a6f..4a0538d 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -7,6 +7,7 @@
>   - "qcom,pcie-ipq8064" for ipq8064
>   - "qcom,pcie-apq8064" for apq8064
>   - "qcom,pcie-apq8084" for apq8084
> + - "qcom,pcie-msm8996" for msm8996 or apq8096
>  
>  - reg:
>   Usage: required
> @@ -92,6 +93,16 @@
>   - "aux" Auxiliary (AUX) clock
>   - "bus_master"  Master AXI clock
>   - "bus_slave"   Slave AXI clock
> +
> +- clock-names:
> + Usage: required for msm8996/apq8096
> + Value type: 
> + Definition: Should contain the following entries
> + - "aux" Auxiliary (AUX) clock.
> + - "bus_master"  Master AXI clock.
> + - "bus_slave"   Slave AXI clock.
> + - "pipe"Pipe Clock driving internal logic.
> + - "cfg" Configuration clk.

The order here and the order in the example don't match. The order 
should be defined.

>  - resets:
>   Usage: required
>   Value type: 
> @@ -115,7 +126,7 @@
>   - "core" Core reset
>  
>  - power-domains:
> - Usage: required for apq8084
> + Usage: required for apq8084 and msm8996/apq8096
>   Value type: 
>   Definition: A phandle and power domain specifier pair to the
>   power domain which is responsible for collapsing
> @@ -231,3 +242,58 @@
>   pinctrl-0 = <_pins_default>;
>   pinctrl-names = "default";
>   };
> +
> +* Example for apq8096:
> +
> + pcie@00608000{

Drop leading 0s.

> + compatible = "qcom,pcie-msm8996", "snps,dw-pcie";
> + power-domains = < PCIE1_GDSC>;
> + bus-range = <0x00 0xff>;
> + num-lanes = <1>;
> +
> + status  = "disabled";

No point to have status in the example.

> +
> + reg = <0x00608000 0x2000>,
> +   <0x0d00 0xf1d>,
> +   <0x0d000f20 0xa8>,
> +   <0x0d10 0x10>;
> +
> + reg-names = "parf", "dbi", "elbi", "config";
> +
> + phys = <_phy 1>;
> + phy-names = "pciephy";
> +
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x0100 0x0 0x0d20 0x0d20 0x0 0x10>,
> + <0x0200 0x0 0x0d30 0x0d30 0x0 0xd0>;
> +
> + interrupts = ;
> + interrupt-names = "msi";
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1  0 272 IRQ_TYPE_LEVEL_HIGH>, /* 
> int_a */
> + <0 0 0 2  0 273 IRQ_TYPE_LEVEL_HIGH>, /* 
> int_b */
> + <0 0 0 3  0 274 IRQ_TYPE_LEVEL_HIGH>, /* 
> int_c */
> + <0 0 0 4  0 275 IRQ_TYPE_LEVEL_HIGH>; /* 
> int_d */
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <_clkreq_default _perst_default 
> _wake_default>;
> + pinctrl-1 = <_clkreq_sleep _perst_default 
> _wake_sleep>;
> +
> + vdda-1p8-supply = <_l12>;
> + vdda-supply = <_l28>;
> + linux,pci-domain = <1>;
> +
> + clocks = < GCC_PCIE_1_PIPE_CLK>,
> + < GCC_PCIE_1_AUX_CLK>,
> + < GCC_PCIE_1_CFG_AHB_CLK>,
> + < GCC_PCIE_1_MSTR_AXI_CLK>,
> + < GCC_PCIE_1_SLV_AXI_CLK>;
> +
> + clock-names =  "pipe",
> + "aux",
> + "cfg",
> + "bus_master",
> + "bus_slave";
> + };

Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-09 Thread Vivek Gautam 
Hi,

On Fri, Nov 4, 2016 at 6:29 PM, Srinivas Kandagatla
 wrote:
> This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
> Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
> legacy interrupts and it conforms to PCI Express Base 2.1 specification.
>
> This patch adds post_init callback to qcom_pcie_ops, as this is pcie
> pipe clocks are only setup after the phy is powered on.
> It also adds ltssm_enable callback as it is very much different to other
> supported SOCs in the driver.
>
> Signed-off-by: Srinivas Kandagatla 
> ---

Few minor nits.

>  .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
>  drivers/pci/host/pcie-qcom.c   | 177 
> -
>  2 files changed, 239 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt 
> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 4059a6f..4a0538d 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -7,6 +7,7 @@
> - "qcom,pcie-ipq8064" for ipq8064
> - "qcom,pcie-apq8064" for apq8064
> - "qcom,pcie-apq8084" for apq8084
> +   - "qcom,pcie-msm8996" for msm8996 or apq8096

Since this works for both apq8096 and msm8996, compatible -
"qcom,pcie-apq8096" for uniformity ?
[snip]

> @@ -231,3 +242,58 @@
> pinctrl-0 = <_pins_default>;
> pinctrl-names = "default";
> };
> +
> +* Example for apq8096:
> +
> +   pcie@00608000{
> +   compatible = "qcom,pcie-msm8996", "snps,dw-pcie";

this will change accordingly.

> +   power-domains = < PCIE1_GDSC>;
> +   bus-range = <0x00 0xff>;
> +   num-lanes = <1>;
> +

[snip]

> +
> +static int qcom_pcie_init_v2(struct qcom_pcie *pcie)
> +{
> +   struct qcom_pcie_resources_v2 *res = >res.v2;
> +   struct device *dev = pcie->pp.dev;
> +   u32 val;
> +   int ret = 0;

you don't need to initialize ret here.

> +
> +   ret = clk_prepare_enable(res->aux_clk);
> +   if (ret) {
> +   dev_err(dev, "cannot prepare/enable aux clock\n");
> +   return ret;
> +   }

[snip]

> @@ -429,6 +571,17 @@ static int qcom_pcie_link_up(struct pcie_port *pp)
> return !!(val & PCI_EXP_LNKSTA_DLLLA);
>  }
>
> +static void qcom_pcie_deinit_v2(struct qcom_pcie *pcie)
> +{
> +   struct qcom_pcie_resources_v2 *res = >res.v2;
> +
> +   clk_disable_unprepare(res->slave_clk);
> +   clk_disable_unprepare(res->master_clk);
> +   clk_disable_unprepare(res->cfg_clk);
> +   clk_disable_unprepare(res->aux_clk);
> +   clk_disable_unprepare(res->pipe_clk);

i am sure, this is not affecting the functionality, but the pipe clock
is enabled after all the clocks.
so it makes sense to disable it in the first place. you can just move
this above slave_clk.

[snip]

> @@ -572,6 +738,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-ipq8064", .data = _v0 },
> { .compatible = "qcom,pcie-apq8064", .data = _v0 },
> { .compatible = "qcom,pcie-apq8084", .data = _v1 },
> +   { .compatible = "qcom,pcie-msm8996", .data = _v2 },

this will change according to earlier comment in bindings.


Thanks
Vivek


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-09 Thread Vivek Gautam 
Hi,

On Fri, Nov 4, 2016 at 6:29 PM, Srinivas Kandagatla
 wrote:
> This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
> Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
> legacy interrupts and it conforms to PCI Express Base 2.1 specification.
>
> This patch adds post_init callback to qcom_pcie_ops, as this is pcie
> pipe clocks are only setup after the phy is powered on.
> It also adds ltssm_enable callback as it is very much different to other
> supported SOCs in the driver.
>
> Signed-off-by: Srinivas Kandagatla 
> ---

Few minor nits.

>  .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
>  drivers/pci/host/pcie-qcom.c   | 177 
> -
>  2 files changed, 239 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt 
> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 4059a6f..4a0538d 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -7,6 +7,7 @@
> - "qcom,pcie-ipq8064" for ipq8064
> - "qcom,pcie-apq8064" for apq8064
> - "qcom,pcie-apq8084" for apq8084
> +   - "qcom,pcie-msm8996" for msm8996 or apq8096

Since this works for both apq8096 and msm8996, compatible -
"qcom,pcie-apq8096" for uniformity ?
[snip]

> @@ -231,3 +242,58 @@
> pinctrl-0 = <_pins_default>;
> pinctrl-names = "default";
> };
> +
> +* Example for apq8096:
> +
> +   pcie@00608000{
> +   compatible = "qcom,pcie-msm8996", "snps,dw-pcie";

this will change accordingly.

> +   power-domains = < PCIE1_GDSC>;
> +   bus-range = <0x00 0xff>;
> +   num-lanes = <1>;
> +

[snip]

> +
> +static int qcom_pcie_init_v2(struct qcom_pcie *pcie)
> +{
> +   struct qcom_pcie_resources_v2 *res = >res.v2;
> +   struct device *dev = pcie->pp.dev;
> +   u32 val;
> +   int ret = 0;

you don't need to initialize ret here.

> +
> +   ret = clk_prepare_enable(res->aux_clk);
> +   if (ret) {
> +   dev_err(dev, "cannot prepare/enable aux clock\n");
> +   return ret;
> +   }

[snip]

> @@ -429,6 +571,17 @@ static int qcom_pcie_link_up(struct pcie_port *pp)
> return !!(val & PCI_EXP_LNKSTA_DLLLA);
>  }
>
> +static void qcom_pcie_deinit_v2(struct qcom_pcie *pcie)
> +{
> +   struct qcom_pcie_resources_v2 *res = >res.v2;
> +
> +   clk_disable_unprepare(res->slave_clk);
> +   clk_disable_unprepare(res->master_clk);
> +   clk_disable_unprepare(res->cfg_clk);
> +   clk_disable_unprepare(res->aux_clk);
> +   clk_disable_unprepare(res->pipe_clk);

i am sure, this is not affecting the functionality, but the pipe clock
is enabled after all the clocks.
so it makes sense to disable it in the first place. you can just move
this above slave_clk.

[snip]

> @@ -572,6 +738,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-ipq8064", .data = _v0 },
> { .compatible = "qcom,pcie-apq8064", .data = _v0 },
> { .compatible = "qcom,pcie-apq8084", .data = _v1 },
> +   { .compatible = "qcom,pcie-msm8996", .data = _v2 },

this will change according to earlier comment in bindings.


Thanks
Vivek


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-04 Thread Srinivas Kandagatla 
This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
legacy interrupts and it conforms to PCI Express Base 2.1 specification.

This patch adds post_init callback to qcom_pcie_ops, as this is pcie
pipe clocks are only setup after the phy is powered on.
It also adds ltssm_enable callback as it is very much different to other
supported SOCs in the driver.

Signed-off-by: Srinivas Kandagatla 
---
 .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
 drivers/pci/host/pcie-qcom.c   | 177 -
 2 files changed, 239 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt 
b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 4059a6f..4a0538d 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -7,6 +7,7 @@
- "qcom,pcie-ipq8064" for ipq8064
- "qcom,pcie-apq8064" for apq8064
- "qcom,pcie-apq8084" for apq8084
+   - "qcom,pcie-msm8996" for msm8996 or apq8096
 
 - reg:
Usage: required
@@ -92,6 +93,16 @@
- "aux" Auxiliary (AUX) clock
- "bus_master"  Master AXI clock
- "bus_slave"   Slave AXI clock
+
+- clock-names:
+   Usage: required for msm8996/apq8096
+   Value type: 
+   Definition: Should contain the following entries
+   - "aux" Auxiliary (AUX) clock.
+   - "bus_master"  Master AXI clock.
+   - "bus_slave"   Slave AXI clock.
+   - "pipe"Pipe Clock driving internal logic.
+   - "cfg" Configuration clk.
 - resets:
Usage: required
Value type: 
@@ -115,7 +126,7 @@
- "core" Core reset
 
 - power-domains:
-   Usage: required for apq8084
+   Usage: required for apq8084 and msm8996/apq8096
Value type: 
Definition: A phandle and power domain specifier pair to the
power domain which is responsible for collapsing
@@ -231,3 +242,58 @@
pinctrl-0 = <_pins_default>;
pinctrl-names = "default";
};
+
+* Example for apq8096:
+
+   pcie@00608000{
+   compatible = "qcom,pcie-msm8996", "snps,dw-pcie";
+   power-domains = < PCIE1_GDSC>;
+   bus-range = <0x00 0xff>;
+   num-lanes = <1>;
+
+   status  = "disabled";
+
+   reg = <0x00608000 0x2000>,
+ <0x0d00 0xf1d>,
+ <0x0d000f20 0xa8>,
+ <0x0d10 0x10>;
+
+   reg-names = "parf", "dbi", "elbi", "config";
+
+   phys = <_phy 1>;
+   phy-names = "pciephy";
+
+   #address-cells = <3>;
+   #size-cells = <2>;
+   ranges = <0x0100 0x0 0x0d20 0x0d20 0x0 0x10>,
+   <0x0200 0x0 0x0d30 0x0d30 0x0 0xd0>;
+
+   interrupts = ;
+   interrupt-names = "msi";
+   #interrupt-cells = <1>;
+   interrupt-map-mask = <0 0 0 0x7>;
+   interrupt-map = <0 0 0 1  0 272 IRQ_TYPE_LEVEL_HIGH>, /* 
int_a */
+   <0 0 0 2  0 273 IRQ_TYPE_LEVEL_HIGH>, /* 
int_b */
+   <0 0 0 3  0 274 IRQ_TYPE_LEVEL_HIGH>, /* 
int_c */
+   <0 0 0 4  0 275 IRQ_TYPE_LEVEL_HIGH>; /* 
int_d */
+
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <_clkreq_default _perst_default 
_wake_default>;
+   pinctrl-1 = <_clkreq_sleep _perst_default 
_wake_sleep>;
+
+   vdda-1p8-supply = <_l12>;
+   vdda-supply = <_l28>;
+   linux,pci-domain = <1>;
+
+   clocks = < GCC_PCIE_1_PIPE_CLK>,
+   < GCC_PCIE_1_AUX_CLK>,
+   < GCC_PCIE_1_CFG_AHB_CLK>,
+   < GCC_PCIE_1_MSTR_AXI_CLK>,
+   < GCC_PCIE_1_SLV_AXI_CLK>;
+
+   clock-names =  "pipe",
+   "aux",
+   "cfg",
+   "bus_master",
+   "bus_slave";
+   };
diff --git a/drivers/pci/host/pcie-qcom.c b/drivers/pci/host/pcie-qcom.c
index 3593640..f37c690 100644
--- a/drivers/pci/host/pcie-qcom.c
+++ b/drivers/pci/host/pcie-qcom.c
@@ -36,11 +36,19 @@
 
 #include "pcie-designware.h"
 
+#define PCIE20_PARF_DBI_BASE_ADDR  0x168
+
+#define PCIE20_PARF_SYS_CTRL   0x00
 #define PCIE20_PARF_PHY_CTRL   0x40
 #define PCIE20_PARF_PHY_REFCLK 0x4C
 #define

[PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller

2016-11-04 Thread Srinivas Kandagatla 
This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
legacy interrupts and it conforms to PCI Express Base 2.1 specification.

This patch adds post_init callback to qcom_pcie_ops, as this is pcie
pipe clocks are only setup after the phy is powered on.
It also adds ltssm_enable callback as it is very much different to other
supported SOCs in the driver.

Signed-off-by: Srinivas Kandagatla 
---
 .../devicetree/bindings/pci/qcom,pcie.txt  |  68 +++-
 drivers/pci/host/pcie-qcom.c   | 177 -
 2 files changed, 239 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt 
b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 4059a6f..4a0538d 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -7,6 +7,7 @@
- "qcom,pcie-ipq8064" for ipq8064
- "qcom,pcie-apq8064" for apq8064
- "qcom,pcie-apq8084" for apq8084
+   - "qcom,pcie-msm8996" for msm8996 or apq8096
 
 - reg:
Usage: required
@@ -92,6 +93,16 @@
- "aux" Auxiliary (AUX) clock
- "bus_master"  Master AXI clock
- "bus_slave"   Slave AXI clock
+
+- clock-names:
+   Usage: required for msm8996/apq8096
+   Value type: 
+   Definition: Should contain the following entries
+   - "aux" Auxiliary (AUX) clock.
+   - "bus_master"  Master AXI clock.
+   - "bus_slave"   Slave AXI clock.
+   - "pipe"Pipe Clock driving internal logic.
+   - "cfg" Configuration clk.
 - resets:
Usage: required
Value type: 
@@ -115,7 +126,7 @@
- "core" Core reset
 
 - power-domains:
-   Usage: required for apq8084
+   Usage: required for apq8084 and msm8996/apq8096
Value type: 
Definition: A phandle and power domain specifier pair to the
power domain which is responsible for collapsing
@@ -231,3 +242,58 @@
pinctrl-0 = <_pins_default>;
pinctrl-names = "default";
};
+
+* Example for apq8096:
+
+   pcie@00608000{
+   compatible = "qcom,pcie-msm8996", "snps,dw-pcie";
+   power-domains = < PCIE1_GDSC>;
+   bus-range = <0x00 0xff>;
+   num-lanes = <1>;
+
+   status  = "disabled";
+
+   reg = <0x00608000 0x2000>,
+ <0x0d00 0xf1d>,
+ <0x0d000f20 0xa8>,
+ <0x0d10 0x10>;
+
+   reg-names = "parf", "dbi", "elbi", "config";
+
+   phys = <_phy 1>;
+   phy-names = "pciephy";
+
+   #address-cells = <3>;
+   #size-cells = <2>;
+   ranges = <0x0100 0x0 0x0d20 0x0d20 0x0 0x10>,
+   <0x0200 0x0 0x0d30 0x0d30 0x0 0xd0>;
+
+   interrupts = ;
+   interrupt-names = "msi";
+   #interrupt-cells = <1>;
+   interrupt-map-mask = <0 0 0 0x7>;
+   interrupt-map = <0 0 0 1  0 272 IRQ_TYPE_LEVEL_HIGH>, /* 
int_a */
+   <0 0 0 2  0 273 IRQ_TYPE_LEVEL_HIGH>, /* 
int_b */
+   <0 0 0 3  0 274 IRQ_TYPE_LEVEL_HIGH>, /* 
int_c */
+   <0 0 0 4  0 275 IRQ_TYPE_LEVEL_HIGH>; /* 
int_d */
+
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <_clkreq_default _perst_default 
_wake_default>;
+   pinctrl-1 = <_clkreq_sleep _perst_default 
_wake_sleep>;
+
+   vdda-1p8-supply = <_l12>;
+   vdda-supply = <_l28>;
+   linux,pci-domain = <1>;
+
+   clocks = < GCC_PCIE_1_PIPE_CLK>,
+   < GCC_PCIE_1_AUX_CLK>,
+   < GCC_PCIE_1_CFG_AHB_CLK>,
+   < GCC_PCIE_1_MSTR_AXI_CLK>,
+   < GCC_PCIE_1_SLV_AXI_CLK>;
+
+   clock-names =  "pipe",
+   "aux",
+   "cfg",
+   "bus_master",
+   "bus_slave";
+   };
diff --git a/drivers/pci/host/pcie-qcom.c b/drivers/pci/host/pcie-qcom.c
index 3593640..f37c690 100644
--- a/drivers/pci/host/pcie-qcom.c
+++ b/drivers/pci/host/pcie-qcom.c
@@ -36,11 +36,19 @@
 
 #include "pcie-designware.h"
 
+#define PCIE20_PARF_DBI_BASE_ADDR  0x168
+
+#define PCIE20_PARF_SYS_CTRL   0x00
 #define PCIE20_PARF_PHY_CTRL   0x40
 #define PCIE20_PARF_PHY_REFCLK 0x4C
 #define PCIE20_PARF_DBI_BASE_ADDR  0x168
 #define