On 6/4/2025 5:15 PM, Dmitry Baryshkov wrote:

On Wed, Jun 04, 2025 at 03:58:33PM +0800, Ziyue Zhang wrote:

quoted

On 6/3/2025 9:11 PM, Dmitry Baryshkov wrote:

quoted

On Thu, May 29, 2025 at 11:54:14AM +0800, Ziyue Zhang wrote:

quoted

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

quoted

quoted

Signed-off-by: Ziyue Zhang <redacted>
---
  .../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.

quoted

BRs
Ziyue

quoted

quoted

    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