[PATCH v2] drm/msm: add msm8998 hdmi phy/pll support
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Mon Jul 8 05:49:23 PDT 2024
On Mon, 8 Jul 2024 at 14:07, Marc Gonzalez <mgonzalez at freebox.fr> 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 at freebox.fr>
> >>
> >> 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
More information about the linux-phy
mailing list