On 19/07/2022 10:17, Tinghan Shen wrote: >>> + syscon@10006000 { >>> + compatible = "mediatek,scpsys", "syscon", "simple-mfd"; >> >> This should be a SoC-specific compatible (and filename). > > Ok. I think that you mean "mediatek,mt8195-scpsys". > I'll update it in next version. Yes. > >> >>> + reg = <0x10006000 0x100>; >>> + >>> + spm: power-controller { >> >> I think you created before less-portable, quite constrained bindings for >> power controller. You now require that mt8195-power-controller is always >> a child of some parent device which will share its regmap/MMIO with it. >> >> And what if in your next block there is no scpsys block and power >> controller is the scpsys alone? It's not possible with your bindings. > > Do you mean a power controller node that looks like this? > > scpsys: power-controller@10006000 { > compatible = "mediatek,mt6797-scpsys"; > #power-domain-cells = <1>; > > // ... > }; Yes, I mean, with an unit address. > >> >> Wouldn't it be better to assign some address space to the >> power-controller (now as an offset from scpsys)? > > Is this mean adding an offset after the node name? > > spm: power-controller@0 { This or above. I think it does not matter for the bindings - it's an implementation detail, whether you give to the child absolute SoC address or you give an bus-specific (scpsys) sub-address/offset. The point is that you have an unit address, thus in the future this could be a device node separate from scpsys. Best regards, Krzysztof