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@xxxxxxxxxxx> >>>> --- >>>> .../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@xxxxxxxxxxxxxxxxxxx >>>> https://lists.infradead.org/mailman/listinfo/linux-phy -- With best wishes Qiang Yu
