On Tue, 18 Jun 2019 at 19:42, Joseph Kogut <joseph.kogut@xxxxxxxxx> wrote: > > > > > > > > + reg = <0x11800000 0x5000>; > > > > > + interrupts = <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>, > > > > > + <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>, > > > > > + <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>; > > > > > + interrupt-names = "job", "mmu", "gpu"; > > > > > + clocks = <&clock CLK_G3D>; > > > > > + mali-supply = <&buck4_reg>; > > > > > > > > Please check if always-on property could be removed from buck4. > > > > > > I've checked, and this property can be removed safely. > > > > > > > Also, what about LDO27? It should be used as well (maybe through > > > > vendor-specific properties which would justify the need of new vendor > > > > compatible). > > > > > > > > > > I'm unsure how LDO27 is used, can you elaborate? > > > > It is supplying the VDD_G3DS (with a note "SRAM power"). I do not have > > any more data on it. However I did not check in vendor kernel for it. > > > > After checking (a fork of) the vendor sources [1], it seems to me that > this regulator is used for memory voltage related to Samsung's > Adaptive Supply Voltage, for which there is a pending patchset [2]. > > This seems to me to be out of the scope of this patchset, could you confirm? > > [1] https://github.com/kumajaya/android_kernel_samsung_universal5422/blob/ad41104d43e6470f8d4880d65b259dc7b903cc0d/arch/arm/mach-exynos/asv-exynos5422.c#L1052 > [2] https://lwn.net/Articles/784958/ Hi, Indeed the vendor sources suggest that voltage scaling of this regulator depends on ASV, not on frequency. However still the regulator is there in the hardware so it should be in the bindings as well (specific to Exynos). I guess the driver should also enable it but this is separate topic (adding per-platform quirks to Panfrost driver). Putting it to bindings also follows advice from line 12: https://elixir.bootlin.com/linux/v5.2-rc5/source/Documentation/devicetree/bindings/writing-bindings.txt#L12 Best regards, Krzysztof
