| Message ID | 20221110102834.8946-1-yunfei.dong@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] media: dt-bindings: media: mediatek: vcodec: Fix clock num not correctly | expand |
On Thu, Nov 10, 2022 at 06:28:32PM +0800, Yunfei Dong wrote: > mt8195 and mt8192 have different clock numbers, can't write 'clocks' and > 'clock-names' with const value. Not a compatible change. Explain why that is okay if it is. > > Move 'assigned-clocks' and 'assigned-clock-parents' to parent node. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > .../media/mediatek,vcodec-subdev-decoder.yaml | 119 +++++++++++------- > 1 file changed, 72 insertions(+), 47 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml > index c4f20acdc1f8..794012853834 100644 > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml > @@ -89,23 +89,33 @@ properties: > > ranges: true > > + clocks: > + minItems: 1 > + maxItems: 5 > + > + clock-names: > + minItems: 1 > + maxItems: 5 Why do both the parent and child have clocks? > + > + assigned-clocks: > + maxItems: 1 > + > + assigned-clock-parents: > + maxItems: 1 You can just drop assigned-clock properties. They are allowed in any node with 'clocks'. > + > # Required child node: > patternProperties: > - '^vcodec-lat@[0-9a-f]+$': > + '^vcodec-lat-soc@[0-9a-f]+$': > type: object > > properties: > compatible: > enum: > - - mediatek,mtk-vcodec-lat > - mediatek,mtk-vcodec-lat-soc > > reg: > maxItems: 1 > > - interrupts: > - maxItems: 1 > - Dropping interrupts? Not explained in the commit msg (why?). > iommus: > minItems: 1 > maxItems: 32 > @@ -114,22 +124,55 @@ patternProperties: > Refer to bindings/iommu/mediatek,iommu.yaml. > > clocks: > + minItems: 1 > maxItems: 5 > > clock-names: > - items: > - - const: sel > - - const: soc-vdec > - - const: soc-lat > - - const: vdec > - - const: top > + minItems: 1 > + maxItems: 5 We had names defined and now we don't. That's a step backwards. > > - assigned-clocks: > + power-domains: Adding power-domains? Rob
Hi Rob, Thanks for your comments. On Wed, 2022-11-16 at 11:29 -0600, Rob Herring wrote: > On Thu, Nov 10, 2022 at 06:28:32PM +0800, Yunfei Dong wrote: > > mt8195 and mt8192 have different clock numbers, can't write > > 'clocks' and > > 'clock-names' with const value. > > Not a compatible change. Explain why that is okay if it is. > This change is used for mt8195 platform for some architecture changed. Need to separate vcodec-lat with vcodec-lat-soc into different child node. At the same time, vcodec-lat-soc don't have interrupt, but having power domain and clks. > > > > Move 'assigned-clocks' and 'assigned-clock-parents' to parent node. > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > --- > > .../media/mediatek,vcodec-subdev-decoder.yaml | 119 +++++++++++--- > > ---- > > 1 file changed, 72 insertions(+), 47 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > decoder.yaml > > b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > decoder.yaml > > index c4f20acdc1f8..794012853834 100644 > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > subdev-decoder.yaml > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > subdev-decoder.yaml > > @@ -89,23 +89,33 @@ properties: > > > > ranges: true > > > > + clocks: > > + minItems: 1 > > + maxItems: 5 > > + > > + clock-names: > > + minItems: 1 > > + maxItems: 5 > > Why do both the parent and child have clocks? > If move assigned-clock-parents to child node, need to add 'ssigned- clock-parents' and 'assigned-clocks' for each child node. Only need to add one in parent node, child node no need to add if add 'ssigned- clock-parents' and 'assigned-clocks' in parent node. Adding 'assigned-clock-parents' and 'assigned-clocks' need to add 'clocks' and 'clock-names', or will check fail. > > + > > + assigned-clocks: > > + maxItems: 1 > > + > > + assigned-clock-parents: > > + maxItems: 1 > > You can just drop assigned-clock properties. They are allowed in any > node with 'clocks'. > Only need to add one in parent node, or need to add for each child node. > > + > > # Required child node: > > patternProperties: > > - '^vcodec-lat@[0-9a-f]+$': > > + '^vcodec-lat-soc@[0-9a-f]+$': > > type: object > > > > properties: > > compatible: > > enum: > > - - mediatek,mtk-vcodec-lat > > - mediatek,mtk-vcodec-lat-soc > > > > reg: > > maxItems: 1 > > > > - interrupts: > > - maxItems: 1 > > - > > Dropping interrupts? Not explained in the commit msg (why?). > vcodec-lat-soc no need interrupts, will add detail commit message in next patch. > > iommus: > > minItems: 1 > > maxItems: 32 > > @@ -114,22 +124,55 @@ patternProperties: > > Refer to bindings/iommu/mediatek,iommu.yaml. > > > > clocks: > > + minItems: 1 > > maxItems: 5 > > > > clock-names: > > - items: > > - - const: sel > > - - const: soc-vdec > > - - const: soc-lat > > - - const: vdec > > - - const: top > > + minItems: 1 > > + maxItems: 5 > > We had names defined and now we don't. That's a step backwards. > Mt8195/mt8192/mt8186/mt8188 have different clock number and clock names, so change it like this, do you have any other suggestion? > > > > - assigned-clocks: > > + power-domains: > > Adding power-domains? Vcodec-lat-soc need power domain and add one new child node vcodec-lat- soc. Best Regards, Yunfei Dong > > Rob
Hi Rob, Sorry to disturb you. Could you please help to check the comments in last mail when you are free? Best Regards, Yunfei Dong On Thu, 2022-11-17 at 10:16 +0800, yunfei.dong wrote: > Hi Rob, > > Thanks for your comments. > On Wed, 2022-11-16 at 11:29 -0600, Rob Herring wrote: > > On Thu, Nov 10, 2022 at 06:28:32PM +0800, Yunfei Dong wrote: > > > mt8195 and mt8192 have different clock numbers, can't write > > > 'clocks' and > > > 'clock-names' with const value. > > > > Not a compatible change. Explain why that is okay if it is. > > > > This change is used for mt8195 platform for some architecture > changed. > Need to separate vcodec-lat with vcodec-lat-soc into different child > node. > > At the same time, vcodec-lat-soc don't have interrupt, but having > power > domain and clks. > > > > > > Move 'assigned-clocks' and 'assigned-clock-parents' to parent > > > node. > > > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > > --- > > > .../media/mediatek,vcodec-subdev-decoder.yaml | 119 +++++++++++- > > > -- > > > ---- > > > 1 file changed, 72 insertions(+), 47 deletions(-) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > > decoder.yaml > > > b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > > decoder.yaml > > > index c4f20acdc1f8..794012853834 100644 > > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > subdev-decoder.yaml > > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > subdev-decoder.yaml > > > @@ -89,23 +89,33 @@ properties: > > > > > > ranges: true > > > > > > + clocks: > > > + minItems: 1 > > > + maxItems: 5 > > > + > > > + clock-names: > > > + minItems: 1 > > > + maxItems: 5 > > > > Why do both the parent and child have clocks? > > > > If move assigned-clock-parents to child node, need to add 'ssigned- > clock-parents' and 'assigned-clocks' for each child node. Only need > to > add one in parent node, child node no need to add if add 'ssigned- > clock-parents' and 'assigned-clocks' in parent node. > > Adding 'assigned-clock-parents' and 'assigned-clocks' need to add > 'clocks' and 'clock-names', or will check fail. > > > + > > > + assigned-clocks: > > > + maxItems: 1 > > > + > > > + assigned-clock-parents: > > > + maxItems: 1 > > > > You can just drop assigned-clock properties. They are allowed in > > any > > node with 'clocks'. > > > > Only need to add one in parent node, or need to add for each child > node. > > > + > > > # Required child node: > > > patternProperties: > > > - '^vcodec-lat@[0-9a-f]+$': > > > + '^vcodec-lat-soc@[0-9a-f]+$': > > > type: object > > > > > > properties: > > > compatible: > > > enum: > > > - - mediatek,mtk-vcodec-lat > > > - mediatek,mtk-vcodec-lat-soc > > > > > > reg: > > > maxItems: 1 > > > > > > - interrupts: > > > - maxItems: 1 > > > - > > > > Dropping interrupts? Not explained in the commit msg (why?). > > > > vcodec-lat-soc no need interrupts, will add detail commit message in > next patch. > > > iommus: > > > minItems: 1 > > > maxItems: 32 > > > @@ -114,22 +124,55 @@ patternProperties: > > > Refer to bindings/iommu/mediatek,iommu.yaml. > > > > > > clocks: > > > + minItems: 1 > > > maxItems: 5 > > > > > > clock-names: > > > - items: > > > - - const: sel > > > - - const: soc-vdec > > > - - const: soc-lat > > > - - const: vdec > > > - - const: top > > > + minItems: 1 > > > + maxItems: 5 > > > > We had names defined and now we don't. That's a step backwards. > > > > Mt8195/mt8192/mt8186/mt8188 have different clock number and clock > names, so change it like this, do you have any other suggestion? > > > > > > - assigned-clocks: > > > + power-domains: > > > > Adding power-domains? > > Vcodec-lat-soc need power domain and add one new child node vcodec- > lat- > soc. > > Best Regards, > Yunfei Dong > > > > Rob
diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml index c4f20acdc1f8..794012853834 100644 --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml @@ -89,23 +89,33 @@ properties: ranges: true + clocks: + minItems: 1 + maxItems: 5 + + clock-names: + minItems: 1 + maxItems: 5 + + assigned-clocks: + maxItems: 1 + + assigned-clock-parents: + maxItems: 1 + # Required child node: patternProperties: - '^vcodec-lat@[0-9a-f]+$': + '^vcodec-lat-soc@[0-9a-f]+$': type: object properties: compatible: enum: - - mediatek,mtk-vcodec-lat - mediatek,mtk-vcodec-lat-soc reg: maxItems: 1 - interrupts: - maxItems: 1 - iommus: minItems: 1 maxItems: 32 @@ -114,22 +124,55 @@ patternProperties: Refer to bindings/iommu/mediatek,iommu.yaml. clocks: + minItems: 1 maxItems: 5 clock-names: - items: - - const: sel - - const: soc-vdec - - const: soc-lat - - const: vdec - - const: top + minItems: 1 + maxItems: 5 - assigned-clocks: + power-domains: maxItems: 1 - assigned-clock-parents: + required: + - compatible + - reg + - iommus + - clocks + - clock-names + - power-domains + + additionalProperties: false + + '^vcodec-lat@[0-9a-f]+$': + type: object + + properties: + compatible: + enum: + - mediatek,mtk-vcodec-lat + + reg: + maxItems: 1 + + interrupts: maxItems: 1 + iommus: + minItems: 1 + maxItems: 32 + description: | + List of the hardware port in respective IOMMU block for current Socs. + Refer to bindings/iommu/mediatek,iommu.yaml. + + clocks: + minItems: 1 + maxItems: 5 + + clock-names: + minItems: 1 + maxItems: 5 + power-domains: maxItems: 1 @@ -139,8 +182,6 @@ patternProperties: - iommus - clocks - clock-names - - assigned-clocks - - assigned-clock-parents - power-domains additionalProperties: false @@ -166,15 +207,12 @@ patternProperties: Refer to bindings/iommu/mediatek,iommu.yaml. clocks: + minItems: 1 maxItems: 5 clock-names: - items: - - const: sel - - const: soc-vdec - - const: soc-lat - - const: vdec - - const: top + minItems: 1 + maxItems: 5 assigned-clocks: maxItems: 1 @@ -188,12 +226,9 @@ patternProperties: required: - compatible - reg - - interrupts - iommus - clocks - clock-names - - assigned-clocks - - assigned-clock-parents - power-domains additionalProperties: false @@ -205,17 +240,10 @@ required: - mediatek,scp - dma-ranges - ranges - -if: - properties: - compatible: - contains: - enum: - - mediatek,mtk-vcodec-lat - -then: - required: - - interrupts + - clocks + - clock-names + - assigned-clocks + - assigned-clock-parents additionalProperties: false @@ -241,6 +269,11 @@ examples: #size-cells = <2>; ranges = <0 0 0 0x16000000 0 0x40000>; reg = <0 0x16000000 0 0x1000>; /* VDEC_SYS */ + clocks = <&topckgen CLK_TOP_VDEC_SEL>, + <&topckgen CLK_TOP_MAINPLL_D4>; + clock-names = "sel", "top"; + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; + assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>; vcodec-lat@10000 { compatible = "mediatek,mtk-vcodec-lat"; reg = <0 0x10000 0 0x800>; @@ -253,14 +286,10 @@ examples: <&iommu0 M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>, <&iommu0 M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>, <&iommu0 M4U_PORT_L5_VDEC_UFO_ENC_EXT>; - clocks = <&topckgen CLK_TOP_VDEC_SEL>, - <&vdecsys_soc CLK_VDEC_SOC_VDEC>, + clocks = <&vdecsys_soc CLK_VDEC_SOC_VDEC>, <&vdecsys_soc CLK_VDEC_SOC_LAT>, - <&vdecsys_soc CLK_VDEC_SOC_LARB1>, - <&topckgen CLK_TOP_MAINPLL_D4>; + <&vdecsys_soc CLK_VDEC_SOC_LARB1>; clock-names = "sel", "soc-vdec", "soc-lat", "vdec", "top"; - assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; - assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>; power-domains = <&spm MT8192_POWER_DOMAIN_VDEC>; }; @@ -279,14 +308,10 @@ examples: <&iommu0 M4U_PORT_L4_VDEC_VLD2_EXT>, <&iommu0 M4U_PORT_L4_VDEC_AVC_MV_EXT>, <&iommu0 M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT>; - clocks = <&topckgen CLK_TOP_VDEC_SEL>, - <&vdecsys CLK_VDEC_VDEC>, + clocks = <&vdecsys CLK_VDEC_VDEC>, <&vdecsys CLK_VDEC_LAT>, - <&vdecsys CLK_VDEC_LARB1>, - <&topckgen CLK_TOP_MAINPLL_D4>; + <&vdecsys CLK_VDEC_LARB1>; clock-names = "sel", "soc-vdec", "soc-lat", "vdec", "top"; - assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; - assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>; power-domains = <&spm MT8192_POWER_DOMAIN_VDEC2>; }; };
mt8195 and mt8192 have different clock numbers, can't write 'clocks' and 'clock-names' with const value. Move 'assigned-clocks' and 'assigned-clock-parents' to parent node. Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> --- .../media/mediatek,vcodec-subdev-decoder.yaml | 119 +++++++++++------- 1 file changed, 72 insertions(+), 47 deletions(-)