| Message ID | 1622468035-8453-2-git-send-email-rajeevny@codeaurora.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | drm/msm/dsi: Add display DSI support for SC7280 target | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
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 <jonathan@marek.ca> > Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org> > --- > .../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 <dt-bindings/clock/qcom,dispcc-sc7280.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 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.
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 <jonathan@marek.ca> > Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org> > --- > .../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 00000000..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 <rajeevny@codeaurora.org> > + > +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 <vendor>,<soc>-<block>. > + > + 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 <dt-bindings/clock/qcom,dispcc-sc7280.h> > + #include <dt-bindings/clock/qcom,rpmh.h> > + > + 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 = <&dispcc DISP_CC_MDSS_AHB_CLK>, > + <&rpmhcc RPMH_CXO_CLK>; > + clock-names = "iface", "ref"; > + > + vdds-supply = <&vreg_l10c_0p8>; > + }; > +... > -- > 2.7.4
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 > <vendor>,<soc>-<block>. 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
On 03-06-2021 01:32, rajeevny@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 >> <vendor>,<soc>-<block>. > > 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+dt@kernel.org" earlier in this thread. Please check my response to your review comments. Regarding your suggestion to use <vendor>,<soc>-<block> 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
On 6/16/21 1:50 AM, rajeevny@codeaurora.org wrote: > On 03-06-2021 01:32, rajeevny@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 >>> <vendor>,<soc>-<block>. >> >> 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+dt@kernel.org" earlier in this thread. > > Please check my response to your review comments. Regarding your > suggestion to use <vendor>,<soc>-<block> 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
On 17-06-2021 20:37, Jonathan Marek wrote: > On 6/16/21 1:50 AM, rajeevny@codeaurora.org wrote: >> On 03-06-2021 01:32, rajeevny@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 >>>> <vendor>,<soc>-<block>. >>> >>> 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+dt@kernel.org" earlier in this thread. >> >> Please check my response to your review comments. Regarding your >> suggestion to use <vendor>,<soc>-<block> 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-jonathan@marek.ca/ Thanks, Rajeev
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 00000000..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 <rajeevny@codeaurora.org> + +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 <dt-bindings/clock/qcom,dispcc-sc7280.h> + #include <dt-bindings/clock/qcom,rpmh.h> + + 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 = <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&rpmhcc RPMH_CXO_CLK>; + clock-names = "iface", "ref"; + + vdds-supply = <&vreg_l10c_0p8>; + }; +...
Add YAML schema for the device tree bindings for MSM 7nm DSI PHY driver. Cc: Jonathan Marek <jonathan@marek.ca> Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org> --- .../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