On 20/06/2023 14:46, Nícolas F. R. A. Prado wrote: > On Tue, Jun 20, 2023 at 10:12:14AM +0200, Krzysztof Kozlowski wrote: >> On 20/06/2023 02:03, Nícolas F. R. A. Prado wrote: >>> The binding expects the first register space to be VDEC_SYS. But on >>> mt8183, which uses the stateless decoders, this space is used only for >>> controlling clocks and resets, which are better described as separate >>> clock-controller and reset-controller nodes. >>> >>> In fact, in mt8173's devicetree there are already such separate >>> clock-controller nodes, which cause duplicate addresses between the >>> vdecsys node and the vcodec node. But for this SoC, since the stateful >>> decoder code makes other uses of the VDEC_SYS register space, it's not >>> straightforward to remove it. >>> >>> In order to avoid the same address conflict to happen on mt8183, >>> since the only current use of the VDEC_SYS register space in >>> the driver is to read the status of a hardware controlled clock, remove >>> the VDEC_SYS register space from the binding and describe an extra >>> syscon that will be used to directly check the hardware status. >>> >>> Also add reg-names to be able to tell that this new register schema is >>> used, so the driver can keep backward compatibility. >>> >>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx> >>> >>> --- >>> I dropped the tags from this commit since a syscon is now used instead >>> of an extra clock. >>> >>> Changes in v3: >>> - Removed the active clock >>> - Added a mediatek,vdecsys syscon property >>> >>> Changes in v2: >>> - Merged with patch 1 (media: dt-bindings: mediatek,vcodec: Allow single >>> clock for mt8183) to avoid changing number of clocks twice >>> - Added maxItems to reg-names >>> - Constrained clocks for each compatible >>> - Reordered properties for each compatible >>> >>> .../media/mediatek,vcodec-decoder.yaml | 30 +++++++++++++++++++ >>> 1 file changed, 30 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml >>> index 1e56ece44aee..2f625c50bbfe 100644 >>> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml >>> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml >>> @@ -21,8 +21,13 @@ properties: >>> - mediatek,mt8183-vcodec-dec >>> >>> reg: >>> + minItems: 11 >>> maxItems: 12 >>> >>> + reg-names: >>> + minItems: 11 >>> + maxItems: 11 >> >> maxItems: 12 >> >>> + >>> interrupts: >>> maxItems: 1 >>> >>> @@ -60,6 +65,10 @@ properties: >>> description: >>> Describes point to scp. >>> >>> + mediatek,vdecsys: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: Phandle to the vdecsys syscon node. >>> + >>> required: >>> - compatible >>> - reg >>> @@ -79,8 +88,26 @@ allOf: >>> then: >>> required: >>> - mediatek,scp >>> + - mediatek,vdecsys >>> >>> properties: >>> + reg: >>> + maxItems: 11 >>> + >>> + reg-names: >>> + items: >>> + - const: misc >>> + - const: ld >>> + - const: top >>> + - const: cm >>> + - const: ad >>> + - const: av >>> + - const: pp >>> + - const: hwd >>> + - const: hwq >>> + - const: hwb >>> + - const: hwg >>> + >>> clocks: >>> minItems: 1 >>> maxItems: 1 >>> @@ -101,6 +128,9 @@ allOf: >>> - mediatek,vpu >>> >>> properties: >>> + reg: >>> + minItems: 12 >> >> >> What about reg-names here? They should be also defined and in sync with >> regs. > > That's intentional. As described in the commit message, mt8173 will keep having > the VDEC_SYS iospace, while mt8183 won't. And we use the presence of reg-names > to tell them apart. > > So, mt8173 has 12 regs, no reg-names and no syscon, while mt8183 has 11 regs, > with reg-names and the syscon. reg-names is not the way to tell apart variants. Compatible is. If you add reg-names for one variant, it's expected to have it defined for other as well, because the order of items in reg is always fixed. Best regards, Krzysztof
