On 7.08.2023 16:04, Krzysztof Kozlowski wrote: > On 07/08/2023 14:41, Konrad Dybcio wrote: >> On 5.08.2023 21:29, Krzysztof Kozlowski wrote: >>> On 04/08/2023 22:09, Konrad Dybcio wrote: >>>> Both of these SoCs implement an IRIS2 block, with SC8280XP being able >>>> to clock it a bit higher. >>>> >>> >>> ... >>> >>>> + >>>> + iommus: >>>> + maxItems: 1 >>>> + >>>> + video-decoder: >>>> + type: object >>>> + >>>> + properties: >>>> + compatible: >>>> + const: venus-decoder >>> >>> That's not how compatibles are constructed... missing vendor prefix, SoC >>> or IP block name. >>> >>>> + >>>> + required: >>>> + - compatible >>>> + >>>> + additionalProperties: false >>> >>> Why do you need this child node? Child nodes without properties are >>> usually useless. >> For both comments: I aligned with what was there.. >> >> The driver abuses these compats to probe enc/dec submodules, even though >> every Venus implementation (to my knowledge) is implicitly enc/dec capable.. > > Holy crap, I see... > >> >> Perhaps a bigger clean-up is due. I guess I could just create the venc/vdec >> devices from the venus core probe and get rid of this fake stuff? > > Few devices (qcom,msm8996-venus.yaml, sdm660, sdm845) have clocks there, > so we actually could stay with these subnodes, just correct the > compatibles to a list with correct prefixes: > > qcom,sc8280xp-venus-decoder + qcom,venus-decoder Hm.. looks like pre-845-v2 (with the v2 being "v2 binding" and not "v2 chip" or "v2 hardware") these were used to look up clocks but then they were moved to the root node. I am not quite sure if it makes sense to distinguish e.g. sc8280xp-venus-decoder within sc8280xp-venus.. Perhaps deprecating the "8916 way" (clocks under subnodes), adding some boilerplate to look up clocks/pds in both places and converting everybody to the "7180 way" way of doing things (clocks under venus), and then getting rid of venus encoder/decoder completely (by calling device creation from venus probe) would be better. WDYT? Konrad
