Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
backlight_3v3_regulator: backlight-3v3-regulator {compatible = "regulator-fixed";regulator-name = "backlight_3v3_regulator";regulator-min-microvolt = <3300000>;regulator-max-microvolt = <3300000>;gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;enable-active-high;pinctrl-names = "default";pinctrl-0 = <&edp_bl_power>;};
So I'm pretty sure that this is wrong and what you had on a previous patch was more correct. Specifically the PMIC's GPIO 7 truly _is_ an enable pin for the backlight. In the schematics I see it's named as "PMIC_EDP_BL_EN" and is essentially the same net as "EDP_BL_EN". This is distinct from the backlight _regulator_ that is named VREG_EDP_BP. I believe the VREG_EDP_BP is essentially sourced directly from PPVAR_SYS. That's how it works on herobrine and I believe that CRD is the same. You currently don't model ppvar_sys, but it's basically just a variable-voltage rail that could be provided somewhat directly from the battery or could be provided from Type C components. I believe that the panel backlight is designed to handle this fairly wide voltage range and it's done this way to get the best efficiency.
So personally I'd prefer if you do something like herobrine and model PPVAR_SYS. Then the backlight can use ppvar_sys as its regulator and you can go back to providing this as an "enable" pin for the backlight.
I know, technically it doesn't _really_ matter, but it's nice to model it more correctly.