Re: [PATCH] phy: phy-mtk-hdmi: add new driver

[Mattijs Korpershoek <mkorpershoek@kernel.org>]

Hi Julien,

On Mon, Jul 27, 2026 at 11:28, Julien Stephan <jstephan@baylibre.com> wrote:

> Hello Mattijs,
>
> (used the new u-boot ml address u-boot@lists.u-boot-project.org and
> dropped the old denx one)
>
> Le mar. 21 juil. 2026 à 11:22, Mattijs Korpershoek
> <mkorpershoek@kernel.org> a écrit :
>>
>> Hi Julien,
>>
>> Thank you for the patch.
>>
>> On Fri, Jul 17, 2026 at 10:26, Julien Stephan <jstephan@baylibre.com> wrote:
>>
>> > Add a new driver for the HDMI PHY. The driver supports MT8188 and MT8195
>> > based SoCs. The driver is based on the corresponding kernel driver.
>>
>> What kernel version (exact commit, including the sha) has this been
>> ported from ?
>>
>> Mentioning it in the commit message will help reviewers to compare this
>> with the linux codebase.
>>
>> I can do a more detailed review once I know the linux base this has been
>> ported from.
>>
>
> Technically it's not a direct port from the upstream kernel driver.
> The code comes from an old downstream MediaTek code base.
> I guess it was polished a bit during kernel upstream, hence some
> variables names and functions are not 100% identical..

Ah, that make sense.

>
> So what would you like me to do? Send a v2 with a proper backport from
> kernel? Or being more explicit on the commit message is enough?
> Something like: "The driver is loosely based on the corresponding
> kernel driver at  45810d486bb4 phy: mediatek: add support for
> phy-mtk-hdmi-mt8195)

Ideally, we would do a backport from the Linux driver. That will make
future maintenance easier since we can continue porting fixes (when
relevant) from Linux when they arrive. That's probably a bit more work
though :(

If you want to keep this implementation, then I'd be more precise in the
commit message. Stating that it's based on a vendor driver and has been
polished up for U-Boot.

So I have a preference for doing the port from Linux, but I won't block
this if we just reword the commit message. Up to you to decide what
you'd like to do.

>
>> >
>> > Signed-off-by: Pavlo Yadvychuk <pyadvychuk@baylibre.com>
>> > Signed-off-by: Julien Stephan <jstephan@baylibre.com>
>> > ---
>>
>> [...]
>>
>> > +
>> > +     /* RG_HDMITXPLL_RESERVE[12:11] */
>> > +     mtk_hdmi_phy_mask(hdmi_phy, HDMI_1_PLL_CFG_1,
>> > +                       0x3 << RG_HDMITXPLL_RESERVE_BIT12_11_SHIFT,
>> > +                       RG_HDMITXPLL_RESERVE_BIT12_11);
>> > +
>> > +     /* TCL_EN */
>> > +     mtk_hdmi_phy_mask(hdmi_phy, HDMI_1_PLL_CFG_0,
>> > +                       0x1 << RG_HDMITXPLL_TCL_EN_SHIFT,
>> > +                       RG_HDMITXPLL_TCL_EN);
>> > +
>> > +     /* we should always read calibration impedance
>> > +      * from efuse, unless for debugging purposes.
>> > +      * This calibraion value is not board-dependent
>>
>> calibraion -> calibration
>>
>> > +      * so no SW adjustment required.
>> > +      */
>> > +     if (hdmi_phy->conf->efuse_sw_mode) {
>> > +             debug("efuse_sw_mode ENABLED!!!");
>> > +             mtk_hdmi_phy_mask(hdmi_phy, HDMI_CTL_1,
>> > +                               0x1f << RG_INTR_IMP_RG_MODE_SHIFT,
>> > +                               RG_INTR_IMP_RG_MODE);
>> > +     }
>> > +
>> > +     return 0;
>> > +}
>> > +
>>
>> [...]