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