Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY
On 17-06-2021 20:37, Jonathan Marek wrote: On 6/16/21 1:50 AM, rajee...@codeaurora.org wrote: On 03-06-2021 01:32, rajee...@codeaurora.org wrote: On 02-06-2021 02:28, Rob Herring wrote: On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote: + +properties: + compatible: + oneOf: + - const: qcom,dsi-phy-7nm When would one use this? This is for SM8250. + - const: qcom,dsi-phy-7nm-7280 + - const: qcom,dsi-phy-7nm-8150 These don't look like full SoC names (sm8150?) and it's ,-. Thanks, Rob, for the review. I just took the `compatible` property currently used in the DSI PHY driver (drivers/gpu/drm/msm/dsi/phy/dsi_phy.c), and added a new entry for sc7280. A similar pattern of `compatible` names are used in other variants of the DSI PHY driver e.g. qcom,qcom,dsi-phy-10nm-8998, qcom,dsi-phy-14nm-660 etc. The existing compatible names "qcom,dsi-phy-7nm-8150" (SoC at the end) make some sense, if we look at the organization of the dsi phy driver code. I am new to this and don't know the reason behind the current code organization and this naming. Yes, I agree with you, we should use full SoC names. Adding the SoC name at the end does not feel very convincing, so I will change this to the suggested format e.g. "qcom,sm8250-dsi-phy-7nm", and will rename the occurrences in the driver and device tree accordingly. Do I need to make changes for 10nm, 14nm, 20nm, and 28nm DSI PHY too? Bindings doc for these PHYs recently got merged to msm-next [1] [1] https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e Hi Rob, I missed adding "robh...@kernel.org" earlier in this thread. Please check my response to your review comments. Regarding your suggestion to use ,- format for compatible property, should I also upload a new patch to make changes in 10nm, 14nm, 20nm, and 28nm DSI PHY DT bindings? Thanks, Rajeev Hi, I missed this and ended up sending a similar patch a week later (as part of my cphy series, because I needed it to add a "phy-type" property). "qcom,dsi-phy-7nm" and "qcom,dsi-phy-7nm-8150" aren't new compatibles, they were previously documented in the .txt bindings, which are getting removed, but the new .yaml bindings didn't include them. Documenting them is just a fixup to that patch [1] which is already R-B'd by RobH (and has similar compatibles such as "qcom,dsi-phy-10nm" and "qcom,dsi-phy-10nm-8998 "). You can use a different/better naming scheme for sc7280, but changing the others has nothing to do with adding support for sc7280. [1] https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e Hi Jonathan, I will discard this patch and will add the bindings for the sc7280 on top of your patch [1]. [1] https://lore.kernel.org/linux-arm-msm/20210617144349.28448-2-jonat...@marek.ca/ Thanks, Rajeev
Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY
On 6/16/21 1:50 AM, rajee...@codeaurora.org wrote: On 03-06-2021 01:32, rajee...@codeaurora.org wrote: On 02-06-2021 02:28, Rob Herring wrote: On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote: + +properties: + compatible: + oneOf: + - const: qcom,dsi-phy-7nm When would one use this? This is for SM8250. + - const: qcom,dsi-phy-7nm-7280 + - const: qcom,dsi-phy-7nm-8150 These don't look like full SoC names (sm8150?) and it's ,-. Thanks, Rob, for the review. I just took the `compatible` property currently used in the DSI PHY driver (drivers/gpu/drm/msm/dsi/phy/dsi_phy.c), and added a new entry for sc7280. A similar pattern of `compatible` names are used in other variants of the DSI PHY driver e.g. qcom,qcom,dsi-phy-10nm-8998, qcom,dsi-phy-14nm-660 etc. The existing compatible names "qcom,dsi-phy-7nm-8150" (SoC at the end) make some sense, if we look at the organization of the dsi phy driver code. I am new to this and don't know the reason behind the current code organization and this naming. Yes, I agree with you, we should use full SoC names. Adding the SoC name at the end does not feel very convincing, so I will change this to the suggested format e.g. "qcom,sm8250-dsi-phy-7nm", and will rename the occurrences in the driver and device tree accordingly. Do I need to make changes for 10nm, 14nm, 20nm, and 28nm DSI PHY too? Bindings doc for these PHYs recently got merged to msm-next [1] [1] https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e Hi Rob, I missed adding "robh...@kernel.org" earlier in this thread. Please check my response to your review comments. Regarding your suggestion to use ,- format for compatible property, should I also upload a new patch to make changes in 10nm, 14nm, 20nm, and 28nm DSI PHY DT bindings? Thanks, Rajeev Hi, I missed this and ended up sending a similar patch a week later (as part of my cphy series, because I needed it to add a "phy-type" property). "qcom,dsi-phy-7nm" and "qcom,dsi-phy-7nm-8150" aren't new compatibles, they were previously documented in the .txt bindings, which are getting removed, but the new .yaml bindings didn't include them. Documenting them is just a fixup to that patch [1] which is already R-B'd by RobH (and has similar compatibles such as "qcom,dsi-phy-10nm" and "qcom,dsi-phy-10nm-8998 "). You can use a different/better naming scheme for sc7280, but changing the others has nothing to do with adding support for sc7280. [1] https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e
Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY
On 03-06-2021 01:32, rajee...@codeaurora.org wrote: On 02-06-2021 02:28, Rob Herring wrote: On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote: + +properties: + compatible: +oneOf: + - const: qcom,dsi-phy-7nm When would one use this? This is for SM8250. + - const: qcom,dsi-phy-7nm-7280 + - const: qcom,dsi-phy-7nm-8150 These don't look like full SoC names (sm8150?) and it's ,-. Thanks, Rob, for the review. I just took the `compatible` property currently used in the DSI PHY driver (drivers/gpu/drm/msm/dsi/phy/dsi_phy.c), and added a new entry for sc7280. A similar pattern of `compatible` names are used in other variants of the DSI PHY driver e.g. qcom,qcom,dsi-phy-10nm-8998, qcom,dsi-phy-14nm-660 etc. The existing compatible names "qcom,dsi-phy-7nm-8150" (SoC at the end) make some sense, if we look at the organization of the dsi phy driver code. I am new to this and don't know the reason behind the current code organization and this naming. Yes, I agree with you, we should use full SoC names. Adding the SoC name at the end does not feel very convincing, so I will change this to the suggested format e.g. "qcom,sm8250-dsi-phy-7nm", and will rename the occurrences in the driver and device tree accordingly. Do I need to make changes for 10nm, 14nm, 20nm, and 28nm DSI PHY too? Bindings doc for these PHYs recently got merged to msm-next [1] [1] https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e Hi Rob, I missed adding "robh...@kernel.org" earlier in this thread. Please check my response to your review comments. Regarding your suggestion to use ,- format for compatible property, should I also upload a new patch to make changes in 10nm, 14nm, 20nm, and 28nm DSI PHY DT bindings? Thanks, Rajeev
Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY
On 02-06-2021 02:28, Rob Herring wrote: On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote: + +properties: + compatible: +oneOf: + - const: qcom,dsi-phy-7nm When would one use this? This is for SM8250. + - const: qcom,dsi-phy-7nm-7280 + - const: qcom,dsi-phy-7nm-8150 These don't look like full SoC names (sm8150?) and it's ,-. Thanks, Rob, for the review. I just took the `compatible` property currently used in the DSI PHY driver (drivers/gpu/drm/msm/dsi/phy/dsi_phy.c), and added a new entry for sc7280. A similar pattern of `compatible` names are used in other variants of the DSI PHY driver e.g. qcom,qcom,dsi-phy-10nm-8998, qcom,dsi-phy-14nm-660 etc. The existing compatible names "qcom,dsi-phy-7nm-8150" (SoC at the end) make some sense, if we look at the organization of the dsi phy driver code. I am new to this and don't know the reason behind the current code organization and this naming. Yes, I agree with you, we should use full SoC names. Adding the SoC name at the end does not feel very convincing, so I will change this to the suggested format e.g. "qcom,sm8250-dsi-phy-7nm", and will rename the occurrences in the driver and device tree accordingly. Do I need to make changes for 10nm, 14nm, 20nm, and 28nm DSI PHY too? Bindings doc for these PHYs recently got merged to msm-next [1] [1] https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e Thanks, Rajeev
Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY
On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote: > Add YAML schema for the device tree bindings for MSM 7nm DSI PHY driver. > > Cc: Jonathan Marek > Signed-off-by: Rajeev Nandan > --- > .../bindings/display/msm/dsi-phy-7nm.yaml | 68 > ++ > 1 file changed, 68 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml > b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml > new file mode 100644 > index ..f17cfde > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml > @@ -0,0 +1,68 @@ > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/msm/dsi-phy-7nm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Display DSI 7nm PHY > + > +maintainers: > + - Rajeev Nandan > + > +allOf: > + - $ref: dsi-phy-common.yaml# > + > +properties: > + compatible: > +oneOf: > + - const: qcom,dsi-phy-7nm When would one use this? > + - const: qcom,dsi-phy-7nm-7280 > + - const: qcom,dsi-phy-7nm-8150 These don't look like full SoC names (sm8150?) and it's ,-. > + > + reg: > +items: > + - description: dsi phy register set > + - description: dsi phy lane register set > + - description: dsi pll register set > + > + reg-names: > +items: > + - const: dsi_phy > + - const: dsi_phy_lane > + - const: dsi_pll > + > + vdds-supply: > +description: Phandle to 0.9V power supply regulator device node. > + > +required: > + - compatible > + - reg > + - reg-names > + - vdds-supply > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include > + #include > + > + dsi-phy@ae94400 { > + compatible = "qcom,dsi-phy-7nm-7280"; > + reg = <0x0ae94400 0x200>, > + <0x0ae94600 0x280>, > + <0x0ae94900 0x280>; > + reg-names = "dsi_phy", > + "dsi_phy_lane", > + "dsi_pll"; > + > + #clock-cells = <1>; > + #phy-cells = <0>; > + > + clocks = < DISP_CC_MDSS_AHB_CLK>, > + < RPMH_CXO_CLK>; > + clock-names = "iface", "ref"; > + > + vdds-supply = <_l10c_0p8>; > + }; > +... > -- > 2.7.4
Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY
On Mon, 31 May 2021 19:03:53 +0530, Rajeev Nandan wrote: > Add YAML schema for the device tree bindings for MSM 7nm DSI PHY driver. > > Cc: Jonathan Marek > Signed-off-by: Rajeev Nandan > --- > .../bindings/display/msm/dsi-phy-7nm.yaml | 68 > ++ > 1 file changed, 68 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/display/msm/dsi-phy-common.yaml' xargs: dt-doc-validate: exited with status 255; aborting Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.example.dts:19:18: fatal error: dt-bindings/clock/qcom,dispcc-sc7280.h: No such file or directory 19 | #include | ^~~~ compilation terminated. make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs make: *** [Makefile:1416: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1485686 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
[v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY
Add YAML schema for the device tree bindings for MSM 7nm DSI PHY driver. Cc: Jonathan Marek Signed-off-by: Rajeev Nandan --- .../bindings/display/msm/dsi-phy-7nm.yaml | 68 ++ 1 file changed, 68 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml new file mode 100644 index ..f17cfde --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml @@ -0,0 +1,68 @@ +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/dsi-phy-7nm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display DSI 7nm PHY + +maintainers: + - Rajeev Nandan + +allOf: + - $ref: dsi-phy-common.yaml# + +properties: + compatible: +oneOf: + - const: qcom,dsi-phy-7nm + - const: qcom,dsi-phy-7nm-7280 + - const: qcom,dsi-phy-7nm-8150 + + reg: +items: + - description: dsi phy register set + - description: dsi phy lane register set + - description: dsi pll register set + + reg-names: +items: + - const: dsi_phy + - const: dsi_phy_lane + - const: dsi_pll + + vdds-supply: +description: Phandle to 0.9V power supply regulator device node. + +required: + - compatible + - reg + - reg-names + - vdds-supply + +unevaluatedProperties: false + +examples: + - | + #include + #include + + dsi-phy@ae94400 { + compatible = "qcom,dsi-phy-7nm-7280"; + reg = <0x0ae94400 0x200>, + <0x0ae94600 0x280>, + <0x0ae94900 0x280>; + reg-names = "dsi_phy", + "dsi_phy_lane", + "dsi_pll"; + + #clock-cells = <1>; + #phy-cells = <0>; + + clocks = < DISP_CC_MDSS_AHB_CLK>, + < RPMH_CXO_CLK>; + clock-names = "iface", "ref"; + + vdds-supply = <_l10c_0p8>; + }; +... -- 2.7.4