On Thu, Dec 05, 2024 at 10:38:05AM +0100, Krzysztof Kozlowski wrote: > On Wed, Dec 04, 2024 at 05:03:24PM +0530, Varadarajan Narayanan wrote: > > From: Nitheesh Sekar <quic_nsekar@xxxxxxxxxxx> > > > > Document the Qualcomm UNIPHY PCIe 28LP present in IPQ5332. > > > > Signed-off-by: Nitheesh Sekar <quic_nsekar@xxxxxxxxxxx> > > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> > > --- > > v2: Rename the file to match the compatible > > Either I look at wrong v1 from your cover letter or there was no such > file in v1, so how it can be a rename? > > What happened here? This driver was pulled in from [1] "Enable IPQ5018 PCI support (Nitheesh Sekar)" In that review, there was this feedback [4] ------------------------------- > +++ > b/Documentation/devicetree/bindings/phy/qcom,uniphy-pcie-28lp.yaml Filename should match compatibles and they do not use 28lp. ------------------------------- > > Drop 'driver' from title > > Dropped 'clock-names' > > Fixed 'reset-names' > > -- > > .../bindings/phy/qcom,uniphy-pcie.yaml | 82 +++++++++++++++++++ > > 1 file changed, 82 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/phy/qcom,uniphy-pcie.yaml > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy-pcie.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy-pcie.yaml > > new file mode 100644 > > index 000000000000..e0ad98a9f324 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy-pcie.yaml > > This does not match compatible, so I don't see how it even matches your > changelog. Since this phy has both single and dual line capabilities I used the phy's name alone for the file name. Will rename this as qcom,ipq5332-uniphy-pcie-phy.yaml If this is not suitable, can you please suggest one that would be apt for this phy. > > @@ -0,0 +1,82 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/phy/qcom,uniphy-pcie.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm UNIPHY PCIe 28LP PHY > > + > > +maintainers: > > + - Nitheesh Sekar <quic_nsekar@xxxxxxxxxxx> > > + - Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> > > + > > +description: > > + PCIe and USB combo PHY found in Qualcomm IPQ5332 SoC > > + > > +properties: > > + compatible: > > + enum: > > + - qcom,ipq5332-uniphy-pcie-gen3x1 > > Odd naming. Did anyone suggest this? I would expect something matches > like everything else recent (see X1 for example). It was not suggested by anyone. Since [4] didn't comment on this continued to use it. Will change it as follows (similar to qcom,x1e80100-qmp-gen4x2-pcie-phy) qcom,ipq5332-uniphy-gen3x1-pcie-phy qcom,ipq5332-uniphy-gen3x2-pcie-phy > > > + - qcom,ipq5332-uniphy-pcie-gen3x2 > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + minItems: 2 > > What happened here? This cannot be minItems and it never was. Will fix this. > > + > > + resets: > > + minItems: 2 > > + maxItems: 3 > > Why this varies? > > This patch is odd. Confusing changelog, v1 entirely different and not > matching what is here, unusual and incorrect code in the binding itself. > > Provide changelog explaining WHY you did such odd changes. This series combines [1] and [2]. [1] introduces IPQ5018 PCIe support and [2] depends on [1] to introduce IPQ5332 PCIe support. Since the community was interested in [2] (please see [3]), tried to revive IPQ5332's PCIe support with this patch. Apologies for not expressing this in the cover letter. > Open *LATEST* existing Qcom bindings and look how they do it. Do not > implement things differently. Sure. Thanks Varada 1. Enable IPQ5018 PCI support (Nitheesh Sekar) - https://lore.kernel.org/all/20231003120846.28626-1-quic_nsekar@xxxxxxxxxxx/ 2. Add PCIe support for Qualcomm IPQ5332 (Praveenkumar I) - https://lore.kernel.org/linux-arm-msm/20231214062847.2215542-1-quic_ipkumar@xxxxxxxxxxx/ 3. Community interest - https://lore.kernel.org/linux-arm-msm/20240310132915.GE3390@thinkpad/ 4. dt-bindings feedback - https://lore.kernel.org/all/4bc021c1-0198-41a4-aa73-bf0cf0c0420a@xxxxxxxxxx/
