[PATCH v1 0/4] pci: qcom: drop unrelated clock and add link_down reset for sa8775p

[PATCH v1 0/4] pci: qcom: drop unrelated clock and add link_down reset for sa8775p

[Ziyue Zhang]

This series drop gcc_aux_clock in pcie phy, the pcie aux clock should 
be gcc_phy_aux_clock. And sa8775p platform support link_down reset in
hardware, so add it for both pcie0 and pcie1 to provide a better user
experience.

Have follwing changes:
  - Update pcie phy bindings for sa8775p.
  - Document link_down reset.
  - Remove aux clock from pcie phy.
  - Add link_down reset for pcie.

Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
---

Ziyue Zhang (4):
  dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Update pcie phy bindings
    for sa8775p
  dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset
  arm64: dts: qcom: sa8775p: remove aux clock from pcie phy
  arm64: dts: qcom: sa8775p: add link_down reset for pcie

 .../bindings/pci/qcom,pcie-sa8775p.yaml       | 13 ++++--
 .../phy/qcom,sc8280xp-qmp-pcie-phy.yaml       |  4 +-
 arch/arm64/boot/dts/qcom/sa8775p.dtsi         | 42 ++++++++++++-------
 3 files changed, 37 insertions(+), 22 deletions(-)


base-commit: 3be1a7a31fbda82f3604b6c31e4f390110de1b46
-- 
2.34.1

[PATCH v1 1/4] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Update pcie phy bindings for sa8775p

[Ziyue Zhang]

The gcc_aux_clk is required by the PCIe controller but not by the PCIe
PHY. In PCIe PHY, the source of aux_clk used in low-power mode should
be gcc_phy_aux_clk. Hence, remove gcc_aux_clk and replace it with
gcc_phy_aux_clk.

Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
---
 .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
index 2c6c9296e4c0..17fd6547d3b4 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
@@ -176,6 +176,8 @@ allOf:
           contains:
             enum:
               - qcom,qcs615-qmp-gen3x1-pcie-phy
+              - qcom,sa8775p-qmp-gen4x2-pcie-phy
+              - qcom,sa8775p-qmp-gen4x4-pcie-phy
               - qcom,sc8280xp-qmp-gen3x1-pcie-phy
               - qcom,sc8280xp-qmp-gen3x2-pcie-phy
               - qcom,sc8280xp-qmp-gen3x4-pcie-phy
@@ -197,8 +199,6 @@ allOf:
           contains:
             enum:
               - qcom,qcs8300-qmp-gen4x2-pcie-phy
-              - qcom,sa8775p-qmp-gen4x2-pcie-phy
-              - qcom,sa8775p-qmp-gen4x4-pcie-phy
     then:
       properties:
         clocks:
-- 
2.34.1

[PATCH v1 2/4] dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset

[Ziyue Zhang]

Each PCIe controller on sa8775p supports 'link_down'reset on hardware,
document it.

Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
---
 .../devicetree/bindings/pci/qcom,pcie-sa8775p.yaml  | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
index e3fa232da2ca..805258cbcf2f 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
@@ -61,11 +61,14 @@ properties:
       - const: global
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   reset-names:
+    minItems: 1
     items:
-      - const: pci
+      - const: pci # PCIe core reset
+      - const: link_down # PCIe link down reset
 
 required:
   - interconnects
@@ -161,8 +164,10 @@ examples:
 
             power-domains = <&gcc PCIE_0_GDSC>;
 
-            resets = <&gcc GCC_PCIE_0_BCR>;
-            reset-names = "pci";
+            resets = <&gcc GCC_PCIE_0_BCR>,
+                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
+            reset-names = "pci",
+                          "link_down";
 
             perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
             wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>;
-- 
2.34.1

[PATCH v1 3/4] arm64: dts: qcom: sa8775p: remove aux clock from pcie phy

[Ziyue Zhang]

gcc_aux_clk is used in PCIe RC and it is not required in pcie phy, in
pcie phy it should be gcc_phy_aux_clk, so remove gcc_aux_clk and
replace it with gcc_phy_aux_clk.

Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p.dtsi | 28 +++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 45f536633f64..d7248014368b 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -7224,16 +7224,18 @@ pcie0_phy: phy@1c04000 {
 		compatible = "qcom,sa8775p-qmp-gen4x2-pcie-phy";
 		reg = <0x0 0x1c04000 0x0 0x2000>;
 
-		clocks = <&gcc GCC_PCIE_0_AUX_CLK>,
+		clocks = <&gcc GCC_PCIE_0_PHY_AUX_CLK>,
 			 <&gcc GCC_PCIE_0_CFG_AHB_CLK>,
 			 <&gcc GCC_PCIE_CLKREF_EN>,
 			 <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>,
 			 <&gcc GCC_PCIE_0_PIPE_CLK>,
-			 <&gcc GCC_PCIE_0_PIPEDIV2_CLK>,
-			 <&gcc GCC_PCIE_0_PHY_AUX_CLK>;
-
-		clock-names = "aux", "cfg_ahb", "ref", "rchng", "pipe",
-			      "pipediv2", "phy_aux";
+			 <&gcc GCC_PCIE_0_PIPEDIV2_CLK>;
+		clock-names = "aux",
+			      "cfg_ahb",
+			      "ref",
+			      "rchng",
+			      "pipe",
+			      "pipediv2";
 
 		assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>;
 		assigned-clock-rates = <100000000>;
@@ -7382,16 +7384,18 @@ pcie1_phy: phy@1c14000 {
 		compatible = "qcom,sa8775p-qmp-gen4x4-pcie-phy";
 		reg = <0x0 0x1c14000 0x0 0x4000>;
 
-		clocks = <&gcc GCC_PCIE_1_AUX_CLK>,
+		clocks = <&gcc GCC_PCIE_1_PHY_AUX_CLK>,
 			 <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
 			 <&gcc GCC_PCIE_CLKREF_EN>,
 			 <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>,
 			 <&gcc GCC_PCIE_1_PIPE_CLK>,
-			 <&gcc GCC_PCIE_1_PIPEDIV2_CLK>,
-			 <&gcc GCC_PCIE_1_PHY_AUX_CLK>;
-
-		clock-names = "aux", "cfg_ahb", "ref", "rchng", "pipe",
-			      "pipediv2", "phy_aux";
+			 <&gcc GCC_PCIE_1_PIPEDIV2_CLK>;
+		clock-names = "aux",
+			      "cfg_ahb",
+			      "ref",
+			      "rchng",
+			      "pipe",
+			      "pipediv2";
 
 		assigned-clocks = <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>;
 		assigned-clock-rates = <100000000>;
-- 
2.34.1

[PATCH v1 4/4] arm64: dts: qcom: sa8775p: add link_down reset for pcie

[Ziyue Zhang]

SA8775p supports 'link_down' reset on hardware, so add it for both pcie0
and pcie1, which can provide a better user experience.

Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sa8775p.dtsi | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index d7248014368b..c8ce3d42c894 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -7152,8 +7152,11 @@ pcie0: pcie@1c00000 {
 		iommu-map = <0x0 &pcie_smmu 0x0000 0x1>,
 			    <0x100 &pcie_smmu 0x0001 0x1>;
 
-		resets = <&gcc GCC_PCIE_0_BCR>;
-		reset-names = "pci";
+		resets = <&gcc GCC_PCIE_0_BCR>,
+			 <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
+		reset-names = "pci",
+			      "link_down";
+
 		power-domains = <&gcc PCIE_0_GDSC>;
 
 		phys = <&pcie0_phy>;
@@ -7312,8 +7315,11 @@ pcie1: pcie@1c10000 {
 		iommu-map = <0x0 &pcie_smmu 0x0080 0x1>,
 			    <0x100 &pcie_smmu 0x0081 0x1>;
 
-		resets = <&gcc GCC_PCIE_1_BCR>;
-		reset-names = "pci";
+		resets = <&gcc GCC_PCIE_1_BCR>,
+			 <&gcc GCC_PCIE_1_LINK_DOWN_BCR>;
+		reset-names = "pci",
+			      "link_down";
+
 		power-domains = <&gcc PCIE_1_GDSC>;
 
 		phys = <&pcie1_phy>;
-- 
2.34.1

Re: [PATCH v1 2/4] dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset

[Dmitry Baryshkov]

On Thu, May 29, 2025 at 11:54:14AM +0800, Ziyue Zhang wrote:
> Each PCIe controller on sa8775p supports 'link_down'reset on hardware,
> document it.

I don't think it's possible to "support" reset in hardware. Either it
exists and is routed, or it is not.

> 
> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
> ---
>  .../devicetree/bindings/pci/qcom,pcie-sa8775p.yaml  | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> index e3fa232da2ca..805258cbcf2f 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> @@ -61,11 +61,14 @@ properties:
>        - const: global
>  
>    resets:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2

Shouldn't we just update this to maxItems:2 / minItems:2 and drop
minItems:1 from the next clause?

>  
>    reset-names:
> +    minItems: 1
>      items:
> -      - const: pci
> +      - const: pci # PCIe core reset
> +      - const: link_down # PCIe link down reset
>  
>  required:
>    - interconnects
> @@ -161,8 +164,10 @@ examples:
>  
>              power-domains = <&gcc PCIE_0_GDSC>;
>  
> -            resets = <&gcc GCC_PCIE_0_BCR>;
> -            reset-names = "pci";
> +            resets = <&gcc GCC_PCIE_0_BCR>,
> +                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
> +            reset-names = "pci",
> +                          "link_down";
>  
>              perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>              wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>;
> -- 
> 2.34.1
> 
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Dmitry

Re: [PATCH v1 2/4] dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset

[Ziyue Zhang]

On 6/3/2025 9:11 PM, Dmitry Baryshkov wrote:
> On Thu, May 29, 2025 at 11:54:14AM +0800, Ziyue Zhang wrote:
>> Each PCIe controller on sa8775p supports 'link_down'reset on hardware,
>> document it.
> I don't think it's possible to "support" reset in hardware. Either it
> exists and is routed, or it is not.

Hi Dmitry,

I will change the commit msg to
'Each PCIe controller on sa8775p includes 'link_down'reset on hardware,
document it.'
"Supports" implies that the PCIe controller has an active role in enabling
or managing the reset functionality—it suggests that the controller is designed
to accommodate or facilitate this feature.
  "Includes" simply states that the reset functionality is present in the
hardware—it exists, whether or not it's actively managed or configurable.
So I think change it to includes will be better.

BRs
Ziyue

>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
>> ---
>>   .../devicetree/bindings/pci/qcom,pcie-sa8775p.yaml  | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>> index e3fa232da2ca..805258cbcf2f 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>> @@ -61,11 +61,14 @@ properties:
>>         - const: global
>>   
>>     resets:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
> Shouldn't we just update this to maxItems:2 / minItems:2 and drop
> minItems:1 from the next clause?

Hi Dmitry,

link_down reset is optional. In many other platforms, like sm8550
and x1e80100, link_down reset is documented as a optional reset.
PCIe will works fine without link_down reset. So I think setting it
as optional is better.

BRs
Ziyue

>>   
>>     reset-names:
>> +    minItems: 1
>>       items:
>> -      - const: pci
>> +      - const: pci # PCIe core reset
>> +      - const: link_down # PCIe link down reset
>>   
>>   required:
>>     - interconnects
>> @@ -161,8 +164,10 @@ examples:
>>   
>>               power-domains = <&gcc PCIE_0_GDSC>;
>>   
>> -            resets = <&gcc GCC_PCIE_0_BCR>;
>> -            reset-names = "pci";
>> +            resets = <&gcc GCC_PCIE_0_BCR>,
>> +                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
>> +            reset-names = "pci",
>> +                          "link_down";
>>   
>>               perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>>               wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>;
>> -- 
>> 2.34.1
>>
>>
>> -- 
>> linux-phy mailing list
>> linux-phy@lists.infradead.org
>> https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v1 2/4] dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset

[Dmitry Baryshkov]

On Wed, Jun 04, 2025 at 03:58:33PM +0800, Ziyue Zhang wrote:
> 
> On 6/3/2025 9:11 PM, Dmitry Baryshkov wrote:
> > On Thu, May 29, 2025 at 11:54:14AM +0800, Ziyue Zhang wrote:
> > > Each PCIe controller on sa8775p supports 'link_down'reset on hardware,
> > > document it.
> > I don't think it's possible to "support" reset in hardware. Either it
> > exists and is routed, or it is not.
> 
> Hi Dmitry,
> 
> I will change the commit msg to
> 'Each PCIe controller on sa8775p includes 'link_down'reset on hardware,
> document it.'
> "Supports" implies that the PCIe controller has an active role in enabling
> or managing the reset functionality—it suggests that the controller is designed
> to accommodate or facilitate this feature.
>  "Includes" simply states that the reset functionality is present in the
> hardware—it exists, whether or not it's actively managed or configurable.
> So I think change it to includes will be better.
> 
> BRs
> Ziyue
> 
> > > Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
> > > ---
> > >   .../devicetree/bindings/pci/qcom,pcie-sa8775p.yaml  | 13 +++++++++----
> > >   1 file changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> > > index e3fa232da2ca..805258cbcf2f 100644
> > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> > > @@ -61,11 +61,14 @@ properties:
> > >         - const: global
> > >     resets:
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    maxItems: 2
> > Shouldn't we just update this to maxItems:2 / minItems:2 and drop
> > minItems:1 from the next clause?
> 
> Hi Dmitry,
> 
> link_down reset is optional. In many other platforms, like sm8550
> and x1e80100, link_down reset is documented as a optional reset.
> PCIe will works fine without link_down reset. So I think setting it
> as optional is better.

You are describing a hardware. How can a reset be optional in the
_hardware_? It's either routed or not.

> 
> BRs
> Ziyue
> 
> > >     reset-names:
> > > +    minItems: 1
> > >       items:
> > > -      - const: pci
> > > +      - const: pci # PCIe core reset
> > > +      - const: link_down # PCIe link down reset
> > >   required:
> > >     - interconnects
> > > @@ -161,8 +164,10 @@ examples:
> > >               power-domains = <&gcc PCIE_0_GDSC>;
> > > -            resets = <&gcc GCC_PCIE_0_BCR>;
> > > -            reset-names = "pci";
> > > +            resets = <&gcc GCC_PCIE_0_BCR>,
> > > +                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
> > > +            reset-names = "pci",
> > > +                          "link_down";
> > >               perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> > >               wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>;
> > > -- 
> > > 2.34.1
> > > 
> > > 
> > > -- 
> > > linux-phy mailing list
> > > linux-phy@lists.infradead.org
> > > https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Dmitry

Re: [PATCH v1 2/4] dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset

[Qiang Yu]

On 6/4/2025 5:15 PM, Dmitry Baryshkov wrote:
> On Wed, Jun 04, 2025 at 03:58:33PM +0800, Ziyue Zhang wrote:
>> On 6/3/2025 9:11 PM, Dmitry Baryshkov wrote:
>>> On Thu, May 29, 2025 at 11:54:14AM +0800, Ziyue Zhang wrote:
>>>> Each PCIe controller on sa8775p supports 'link_down'reset on hardware,
>>>> document it.
>>> I don't think it's possible to "support" reset in hardware. Either it
>>> exists and is routed, or it is not.
>> Hi Dmitry,
>>
>> I will change the commit msg to
>> 'Each PCIe controller on sa8775p includes 'link_down'reset on hardware,
>> document it.'
>> "Supports" implies that the PCIe controller has an active role in enabling
>> or managing the reset functionality—it suggests that the controller is designed
>> to accommodate or facilitate this feature.
>>  "Includes" simply states that the reset functionality is present in the
>> hardware—it exists, whether or not it's actively managed or configurable.
>> So I think change it to includes will be better.
>>
>> BRs
>> Ziyue
>>
>>>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
>>>> ---
>>>>   .../devicetree/bindings/pci/qcom,pcie-sa8775p.yaml  | 13 +++++++++----
>>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>> index e3fa232da2ca..805258cbcf2f 100644
>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>> @@ -61,11 +61,14 @@ properties:
>>>>         - const: global
>>>>     resets:
>>>> -    maxItems: 1
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>> Shouldn't we just update this to maxItems:2 / minItems:2 and drop
>>> minItems:1 from the next clause?
>> Hi Dmitry,
>>
>> link_down reset is optional. In many other platforms, like sm8550
>> and x1e80100, link_down reset is documented as a optional reset.
>> PCIe will works fine without link_down reset. So I think setting it
>> as optional is better.
> You are describing a hardware. How can a reset be optional in the
> _hardware_? It's either routed or not.
>
> I feel a bit confused. According to the theory above, everything seems to
> be non-optional when describing hardware, such as registers, clocks,
> resets, regulators, and interrupts—all of them either exist or do not.
>
> Seems like I misunderstand the concept of 'optional'? Is 'optional' only
> used for compatibility across different platforms?
>
> Additionally, we have documented the PCIe global interrupt as optional. I
> was taught that, in the PCIe driver, this interrupt is retrieved using the
> platform_get_irq_byname_optional API, so it can be documented as optional.
> However, this still seems to contradict the theory mentioned earlier.
>> BRs
>> Ziyue
>>
>>>>     reset-names:
>>>> +    minItems: 1
>>>>       items:
>>>> -      - const: pci
>>>> +      - const: pci # PCIe core reset
>>>> +      - const: link_down # PCIe link down reset
>>>>   required:
>>>>     - interconnects
>>>> @@ -161,8 +164,10 @@ examples:
>>>>               power-domains = <&gcc PCIE_0_GDSC>;
>>>> -            resets = <&gcc GCC_PCIE_0_BCR>;
>>>> -            reset-names = "pci";
>>>> +            resets = <&gcc GCC_PCIE_0_BCR>,
>>>> +                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
>>>> +            reset-names = "pci",
>>>> +                          "link_down";
>>>>               perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>>>>               wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>;
>>>> -- 
>>>> 2.34.1
>>>>
>>>>
>>>> -- 
>>>> linux-phy mailing list
>>>> linux-phy@lists.infradead.org
>>>> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Qiang Yu

Re: [PATCH v1 2/4] dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset

[Qiang Yu]

On 6/4/2025 5:15 PM, Dmitry Baryshkov wrote:
> On Wed, Jun 04, 2025 at 03:58:33PM +0800, Ziyue Zhang wrote:
>> On 6/3/2025 9:11 PM, Dmitry Baryshkov wrote:
>>> On Thu, May 29, 2025 at 11:54:14AM +0800, Ziyue Zhang wrote:
>>>> Each PCIe controller on sa8775p supports 'link_down'reset on hardware,
>>>> document it.
>>> I don't think it's possible to "support" reset in hardware. Either it
>>> exists and is routed, or it is not.
>> Hi Dmitry,
>>
>> I will change the commit msg to
>> 'Each PCIe controller on sa8775p includes 'link_down'reset on hardware,
>> document it.'
>> "Supports" implies that the PCIe controller has an active role in enabling
>> or managing the reset functionality—it suggests that the controller is designed
>> to accommodate or facilitate this feature.
>>  "Includes" simply states that the reset functionality is present in the
>> hardware—it exists, whether or not it's actively managed or configurable.
>> So I think change it to includes will be better.
>>
>> BRs
>> Ziyue
>>
>>>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
>>>> ---
>>>>   .../devicetree/bindings/pci/qcom,pcie-sa8775p.yaml  | 13 +++++++++----
>>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>> index e3fa232da2ca..805258cbcf2f 100644
>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>> @@ -61,11 +61,14 @@ properties:
>>>>         - const: global
>>>>     resets:
>>>> -    maxItems: 1
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>> Shouldn't we just update this to maxItems:2 / minItems:2 and drop
>>> minItems:1 from the next clause?
>> Hi Dmitry,
>>
>> link_down reset is optional. In many other platforms, like sm8550
>> and x1e80100, link_down reset is documented as a optional reset.
>> PCIe will works fine without link_down reset. So I think setting it
>> as optional is better.
> You are describing a hardware. How can a reset be optional in the
> _hardware_? It's either routed or not.

I feel a bit confused. According to the theory above, everything seems to
be non-optional when describing hardware, such as registers, clocks,
resets, regulators, and interrupts—all of them either exist or do not.

Seems like I misunderstand the concept of 'optional'? Is 'optional' only
used for compatibility across different platforms?

Additionally, we have documented the PCIe global interrupt as optional. I
was taught that, in the PCIe driver, this interrupt is retrieved using the
platform_get_irq_byname_optional API, so it can be documented as optional.
However, this still seems to contradict the theory mentioned earlier.

>> BRs
>> Ziyue
>>
>>>>     reset-names:
>>>> +    minItems: 1
>>>>       items:
>>>> -      - const: pci
>>>> +      - const: pci # PCIe core reset
>>>> +      - const: link_down # PCIe link down reset
>>>>   required:
>>>>     - interconnects
>>>> @@ -161,8 +164,10 @@ examples:
>>>>               power-domains = <&gcc PCIE_0_GDSC>;
>>>> -            resets = <&gcc GCC_PCIE_0_BCR>;
>>>> -            reset-names = "pci";
>>>> +            resets = <&gcc GCC_PCIE_0_BCR>,
>>>> +                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
>>>> +            reset-names = "pci",
>>>> +                          "link_down";
>>>>               perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>>>>               wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>;
>>>> -- 
>>>> 2.34.1
>>>>
>>>>
>>>> -- 
>>>> linux-phy mailing list
>>>> linux-phy@lists.infradead.org
>>>> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Qiang Yu

Re: [PATCH v1 2/4] dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset

[Krzysztof Kozlowski]

On 04/06/2025 12:05, Qiang Yu wrote:
>>>>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
>>>>> ---
>>>>>   .../devicetree/bindings/pci/qcom,pcie-sa8775p.yaml  | 13 +++++++++----
>>>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>>> index e3fa232da2ca..805258cbcf2f 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
>>>>> @@ -61,11 +61,14 @@ properties:
>>>>>         - const: global
>>>>>     resets:
>>>>> -    maxItems: 1
>>>>> +    minItems: 1
>>>>> +    maxItems: 2
>>>> Shouldn't we just update this to maxItems:2 / minItems:2 and drop
>>>> minItems:1 from the next clause?
>>> Hi Dmitry,
>>>
>>> link_down reset is optional. In many other platforms, like sm8550
>>> and x1e80100, link_down reset is documented as a optional reset.
>>> PCIe will works fine without link_down reset. So I think setting it
>>> as optional is better.
>> You are describing a hardware. How can a reset be optional in the
>> _hardware_? It's either routed or not.
> 
> I feel a bit confused. According to the theory above, everything seems to
> be non-optional when describing hardware, such as registers, clocks,
> resets, regulators, and interrupts—all of them either exist or do not.

Can you construct a DTS being fully complete and correct picture of
hardware without these? If not, they are not optional, because correct
hardware representation would need them.

> 
> Seems like I misunderstand the concept of 'optional'? Is 'optional' only
> used for compatibility across different platforms?
> 
> Additionally, we have documented the PCIe global interrupt as optional. I
> was taught that, in the PCIe driver, this interrupt is retrieved using the
> platform_get_irq_byname_optional API, so it can be documented as optional.
> However, this still seems to contradict the theory mentioned earlier.

ABI is just one side of the required properties.



Best regards,
Krzysztof

Re: [PATCH v1 2/4] dt-bindings: PCI: qcom,pcie-sa8775p: document link_down reset

[Dmitry Baryshkov]

On Wed, Jun 04, 2025 at 06:05:21PM +0800, Qiang Yu wrote:
> 
> On 6/4/2025 5:15 PM, Dmitry Baryshkov wrote:
> > On Wed, Jun 04, 2025 at 03:58:33PM +0800, Ziyue Zhang wrote:
> >> On 6/3/2025 9:11 PM, Dmitry Baryshkov wrote:
> >>> On Thu, May 29, 2025 at 11:54:14AM +0800, Ziyue Zhang wrote:
> >>>> Each PCIe controller on sa8775p supports 'link_down'reset on hardware,
> >>>> document it.
> >>> I don't think it's possible to "support" reset in hardware. Either it
> >>> exists and is routed, or it is not.
> >> Hi Dmitry,
> >>
> >> I will change the commit msg to
> >> 'Each PCIe controller on sa8775p includes 'link_down'reset on hardware,
> >> document it.'
> >> "Supports" implies that the PCIe controller has an active role in enabling
> >> or managing the reset functionality—it suggests that the controller is designed
> >> to accommodate or facilitate this feature.
> >>  "Includes" simply states that the reset functionality is present in the
> >> hardware—it exists, whether or not it's actively managed or configurable.
> >> So I think change it to includes will be better.
> >>
> >> BRs
> >> Ziyue
> >>
> >>>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
> >>>> ---
> >>>>   .../devicetree/bindings/pci/qcom,pcie-sa8775p.yaml  | 13 +++++++++----
> >>>>   1 file changed, 9 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> >>>> index e3fa232da2ca..805258cbcf2f 100644
> >>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> >>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8775p.yaml
> >>>> @@ -61,11 +61,14 @@ properties:
> >>>>         - const: global
> >>>>     resets:
> >>>> -    maxItems: 1
> >>>> +    minItems: 1
> >>>> +    maxItems: 2
> >>> Shouldn't we just update this to maxItems:2 / minItems:2 and drop
> >>> minItems:1 from the next clause?
> >> Hi Dmitry,
> >>
> >> link_down reset is optional. In many other platforms, like sm8550
> >> and x1e80100, link_down reset is documented as a optional reset.
> >> PCIe will works fine without link_down reset. So I think setting it
> >> as optional is better.
> > You are describing a hardware. How can a reset be optional in the
> > _hardware_? It's either routed or not.
> 
> I feel a bit confused. According to the theory above, everything seems to
> be non-optional when describing hardware, such as registers, clocks,
> resets, regulators, and interrupts—all of them either exist or do not.
> 
> Seems like I misunderstand the concept of 'optional'? Is 'optional' only
> used for compatibility across different platforms?
> 
> Additionally, we have documented the PCIe global interrupt as optional. I
> was taught that, in the PCIe driver, this interrupt is retrieved using the
> platform_get_irq_byname_optional API, so it can be documented as optional.
> However, this still seems to contradict the theory mentioned earlier.

I'd say, it should not be defined as optional.

> 
> >> BRs
> >> Ziyue
> >>
> >>>>     reset-names:
> >>>> +    minItems: 1
> >>>>       items:
> >>>> -      - const: pci
> >>>> +      - const: pci # PCIe core reset
> >>>> +      - const: link_down # PCIe link down reset
> >>>>   required:
> >>>>     - interconnects
> >>>> @@ -161,8 +164,10 @@ examples:
> >>>>               power-domains = <&gcc PCIE_0_GDSC>;
> >>>> -            resets = <&gcc GCC_PCIE_0_BCR>;
> >>>> -            reset-names = "pci";
> >>>> +            resets = <&gcc GCC_PCIE_0_BCR>,
> >>>> +                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
> >>>> +            reset-names = "pci",
> >>>> +                          "link_down";
> >>>>               perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> >>>>               wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>;
> >>>> -- 
> >>>> 2.34.1
> >>>>
> >>>>
> >>>> -- 
> >>>> linux-phy mailing list
> >>>> linux-phy@lists.infradead.org
> >>>> https://lists.infradead.org/mailman/listinfo/linux-phy
> 
> -- 
> With best wishes
> Qiang Yu
> 

-- 
With best wishes
Dmitry