| Message ID | 20230119140453.3942340-10-abel.vesa@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | sm8550: Add PCIe HC and PHY support | expand |
On 19/01/2023 15:04, Abel Vesa wrote: > Add the SM8550 platform to the binding. > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > > The v3 of this patchset is: > https://lore.kernel.org/all/20230119112453.3393911-1-abel.vesa@linaro.org/ > > Changes since v3: > * renamed noc_aggr to noc_aggr_4, as found in the driver > > Changes since v2: > * dropped the pipe from clock-names > * removed the pcie instance number from aggre clock-names comment > * renamed aggre clock-names to noc_aggr > * dropped the _pcie infix from cnoc_pcie_sf_axi > * renamed pcie_1_link_down_reset to simply link_down > * added enable-gpios back, since pcie1 node will use it > > Changes since v1: > * Switched to single compatible for both PCIes (qcom,pcie-sm8550) > * dropped enable-gpios property > * dropped interconnects related properties, the power-domains > * properties > and resets related properties the sm8550 specific allOf:if:then > * dropped pipe_mux, phy_pipe and ref clocks from the sm8550 specific > allOf:if:then clock-names array and decreased the minItems and > maxItems for clocks property accordingly > * added "minItems: 1" to interconnects, since sm8550 pcie uses just > * one, > same for interconnect-names > > > .../devicetree/bindings/pci/qcom,pcie.yaml | 44 +++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > index a5859bb3dc28..58f926666332 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > @@ -34,6 +34,7 @@ properties: > - qcom,pcie-sm8250 > - qcom,pcie-sm8450-pcie0 > - qcom,pcie-sm8450-pcie1 > + - qcom,pcie-sm8550 > - qcom,pcie-ipq6018 > > reg: > @@ -65,9 +66,11 @@ properties: > dma-coherent: true > > interconnects: > + minItems: 1 > maxItems: 2 > I don't see my concerns from v3 answered. This is a friendly reminder during the review process. It seems my previous comments were not fully addressed. Maybe my feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. Best regards, Krzysztof
On 23-01-22 15:10:59, Krzysztof Kozlowski wrote: > On 19/01/2023 15:04, Abel Vesa wrote: > > Add the SM8550 platform to the binding. > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > > --- > > > > The v3 of this patchset is: > > https://lore.kernel.org/all/20230119112453.3393911-1-abel.vesa@linaro.org/ > > > > Changes since v3: > > * renamed noc_aggr to noc_aggr_4, as found in the driver > > > > Changes since v2: > > * dropped the pipe from clock-names > > * removed the pcie instance number from aggre clock-names comment > > * renamed aggre clock-names to noc_aggr > > * dropped the _pcie infix from cnoc_pcie_sf_axi > > * renamed pcie_1_link_down_reset to simply link_down > > * added enable-gpios back, since pcie1 node will use it > > > > Changes since v1: > > * Switched to single compatible for both PCIes (qcom,pcie-sm8550) > > * dropped enable-gpios property > > * dropped interconnects related properties, the power-domains > > * properties > > and resets related properties the sm8550 specific allOf:if:then > > * dropped pipe_mux, phy_pipe and ref clocks from the sm8550 specific > > allOf:if:then clock-names array and decreased the minItems and > > maxItems for clocks property accordingly > > * added "minItems: 1" to interconnects, since sm8550 pcie uses just > > * one, > > same for interconnect-names > > > > > > .../devicetree/bindings/pci/qcom,pcie.yaml | 44 +++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > > index a5859bb3dc28..58f926666332 100644 > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > > @@ -34,6 +34,7 @@ properties: > > - qcom,pcie-sm8250 > > - qcom,pcie-sm8450-pcie0 > > - qcom,pcie-sm8450-pcie1 > > + - qcom,pcie-sm8550 > > - qcom,pcie-ipq6018 > > > > reg: > > @@ -65,9 +66,11 @@ properties: > > dma-coherent: true > > > > interconnects: > > + minItems: 1 > > maxItems: 2 > > > > I don't see my concerns from v3 answered. Check the dates for v4 and your reply to v3. v4 was sent a day before you sent your v3 comments. :) > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all > requested changes or keep discussing them. Will address your comments in next version. > > Thank you. > > Best regards, > Krzysztof >
On 23/01/2023 11:44, Abel Vesa wrote: > On 23-01-22 15:10:59, Krzysztof Kozlowski wrote: >> On 19/01/2023 15:04, Abel Vesa wrote: >>> Add the SM8550 platform to the binding. >>> >>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> >>> --- >>> >>> The v3 of this patchset is: >>> https://lore.kernel.org/all/20230119112453.3393911-1-abel.vesa@linaro.org/ >>> >>> Changes since v3: >>> * renamed noc_aggr to noc_aggr_4, as found in the driver >>> >>> Changes since v2: >>> * dropped the pipe from clock-names >>> * removed the pcie instance number from aggre clock-names comment >>> * renamed aggre clock-names to noc_aggr >>> * dropped the _pcie infix from cnoc_pcie_sf_axi >>> * renamed pcie_1_link_down_reset to simply link_down >>> * added enable-gpios back, since pcie1 node will use it >>> >>> Changes since v1: >>> * Switched to single compatible for both PCIes (qcom,pcie-sm8550) >>> * dropped enable-gpios property >>> * dropped interconnects related properties, the power-domains >>> * properties >>> and resets related properties the sm8550 specific allOf:if:then >>> * dropped pipe_mux, phy_pipe and ref clocks from the sm8550 specific >>> allOf:if:then clock-names array and decreased the minItems and >>> maxItems for clocks property accordingly >>> * added "minItems: 1" to interconnects, since sm8550 pcie uses just >>> * one, >>> same for interconnect-names >>> >>> >>> .../devicetree/bindings/pci/qcom,pcie.yaml | 44 +++++++++++++++++++ >>> 1 file changed, 44 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>> index a5859bb3dc28..58f926666332 100644 >>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml >>> @@ -34,6 +34,7 @@ properties: >>> - qcom,pcie-sm8250 >>> - qcom,pcie-sm8450-pcie0 >>> - qcom,pcie-sm8450-pcie1 >>> + - qcom,pcie-sm8550 >>> - qcom,pcie-ipq6018 >>> >>> reg: >>> @@ -65,9 +66,11 @@ properties: >>> dma-coherent: true >>> >>> interconnects: >>> + minItems: 1 >>> maxItems: 2 >>> >> >> I don't see my concerns from v3 answered. > > Check the dates for v4 and your reply to v3. > > v4 was sent a day before you sent your v3 comments. :) > >> >> This is a friendly reminder during the review process. >> >> It seems my previous comments were not fully addressed. Maybe my >> feedback got lost between the quotes, maybe you just forgot to apply it. >> Please go back to the previous discussion and either implement all >> requested changes or keep discussing them. > > Will address your comments in next version. Ah, then ok :) Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml index a5859bb3dc28..58f926666332 100644 --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml @@ -34,6 +34,7 @@ properties: - qcom,pcie-sm8250 - qcom,pcie-sm8450-pcie0 - qcom,pcie-sm8450-pcie1 + - qcom,pcie-sm8550 - qcom,pcie-ipq6018 reg: @@ -65,9 +66,11 @@ properties: dma-coherent: true interconnects: + minItems: 1 maxItems: 2 interconnect-names: + minItems: 1 items: - const: pcie-mem - const: cpu-pcie @@ -102,6 +105,10 @@ properties: power-domains: maxItems: 1 + enable-gpios: + description: GPIO controlled connection to ENABLE# signal + maxItems: 1 + perst-gpios: description: GPIO controlled connection to PERST# signal maxItems: 1 @@ -197,6 +204,7 @@ allOf: - qcom,pcie-sm8250 - qcom,pcie-sm8450-pcie0 - qcom,pcie-sm8450-pcie1 + - qcom,pcie-sm8550 then: properties: reg: @@ -611,6 +619,41 @@ allOf: items: - const: pci # PCIe core reset + - if: + properties: + compatible: + contains: + enum: + - qcom,pcie-sm8550 + then: + properties: + clocks: + minItems: 7 + maxItems: 8 + clock-names: + minItems: 7 + items: + - const: aux # Auxiliary clock + - const: cfg # Configuration clock + - const: bus_master # Master AXI clock + - const: bus_slave # Slave AXI clock + - const: slave_q2a # Slave Q2A clock + - const: ddrss_sf_tbu # PCIe SF TBU clock + - const: noc_aggr_4 # Aggre NoC PCIe AXI clock + - const: cnoc_sf_axi # Config NoC PCIe1 AXI clock + iommus: + maxItems: 1 + iommu-map: + maxItems: 2 + resets: + minItems: 1 + maxItems: 2 + reset-names: + minItems: 1 + items: + - const: pci # PCIe core reset + - const: link_down # PCIe link down reset + - if: properties: compatible: @@ -694,6 +737,7 @@ allOf: - qcom,pcie-sm8250 - qcom,pcie-sm8450-pcie0 - qcom,pcie-sm8450-pcie1 + - qcom,pcie-sm8550 then: oneOf: - properties:
Add the SM8550 platform to the binding. Signed-off-by: Abel Vesa <abel.vesa@linaro.org> --- The v3 of this patchset is: https://lore.kernel.org/all/20230119112453.3393911-1-abel.vesa@linaro.org/ Changes since v3: * renamed noc_aggr to noc_aggr_4, as found in the driver Changes since v2: * dropped the pipe from clock-names * removed the pcie instance number from aggre clock-names comment * renamed aggre clock-names to noc_aggr * dropped the _pcie infix from cnoc_pcie_sf_axi * renamed pcie_1_link_down_reset to simply link_down * added enable-gpios back, since pcie1 node will use it Changes since v1: * Switched to single compatible for both PCIes (qcom,pcie-sm8550) * dropped enable-gpios property * dropped interconnects related properties, the power-domains * properties and resets related properties the sm8550 specific allOf:if:then * dropped pipe_mux, phy_pipe and ref clocks from the sm8550 specific allOf:if:then clock-names array and decreased the minItems and maxItems for clocks property accordingly * added "minItems: 1" to interconnects, since sm8550 pcie uses just * one, same for interconnect-names .../devicetree/bindings/pci/qcom,pcie.yaml | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+)