On 05/07/2024 16:34, Dmitry Baryshkov wrote: > On Thu, Jul 04, 2024 at 06:45:36PM GMT, Marc Gonzalez wrote: > >> From: Arnaud Vrac <avrac@xxxxxxxxxx> >> >> Ported from the downstream driver. > > Please write some sensible commit message. Here's an attempt at expanding the commit message: """ This code adds support for the HDMI PHY block in the MSM8998. It is a copy & paste of the vendor driver downstream: https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/clk/msm/mdss/mdss-hdmi-pll-8998.c """ >> drivers/gpu/drm/msm/Makefile | 1 + >> drivers/gpu/drm/msm/hdmi/hdmi.c | 1 + >> drivers/gpu/drm/msm/hdmi/hdmi.h | 8 + >> drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 5 + >> drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c | 789 +++++++++++++++++++++++++ >> drivers/gpu/drm/msm/registers/display/hdmi.xml | 89 +++ >> 6 files changed, 893 insertions(+) > > - Missing changelog - Rebase onto v6.10 - Move drivers/gpu/drm/msm/hdmi/hdmi.xml.h to drivers/gpu/drm/msm/registers/display/hdmi.xml - Add copyright attribution - Remove all dead/debug/temporary code > - Missing a pointer to bindings. Ideally bindings should come together with the driver. "qcom,hdmi-phy-8998" is defined in "HDMI TX support in msm8998" series (Acked by Rob Herring & Vinod Koul) > I'm not going to check the math, but it looks pretty close to what we > have for msm8996. What is the consequence of this? >> +static const char * const hdmi_phy_8998_reg_names[] = { >> + "vdda-pll", >> + "vdda-phy", > > Unless you have a strong reason to, please use vcca and vddio here, so > that we don't have unnecessary conditionals in schema. The vendor code uses vddio & vcca for msm8996; vdda-pll & vdda-phy for msm8998. Which is vcca? Which is vddio? https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8996-mdss-pll.dtsi https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-mdss-pll.dtsi#L121-172 Regards
