On Mon, 8 Jul 2024 at 14:07, Marc Gonzalez <mgonzalez@xxxxxxxxxx> wrote: > > 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 > """ Add support for the HDMI PHY as present on the Qualcomm MSM8998 platform. The code is mostly c&p of the vendor code from msm-4.4, kernel.lnx.4.4.r38-rel. > > > >> 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) This (and the link to lore) ideally should be a part of the cover letter or the comment below '---' in the patch. > > > 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? That I won't check the math :-D > > > >> +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? vddio = vdda-phy (1.8V) vcca = vdda-pll (lower voltage) > 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 -- With best wishes Dmitry
