Re: [PATCH v3 1/2] phy: rockchip: inno-hdmi: Add configure() and validate() ops

[Jonas Karlman <jonas@kwiboo.se>]

On 5/17/2026 6:54 PM, Vinod Koul wrote:
> On 15-05-26, 23:04, Jonas Karlman wrote:
>> Hi,
>>
>> On 5/15/2026 9:55 PM, Jonas Karlman wrote:
>>> The commit 10ed34d6eaaf ("phy: Add HDMI configuration options")
>>> introduced a way for HDMI PHYs to be configured through the generic
>>> phy_configure() function.
>>>
>>> This driver derives the TMDS character rate from the pixel clock and the
>>> PHY bus width setting. However, no in-tree consumer of this PHY has ever
>>> called phy_set_bus_width() to change the TMDS character rate as only
>>> 8-bit RGB output is supported by the HDMI display driver.
>>>
>>> Add configure() and validate() ops to allow consumers to configure the
>>> TMDS character rate using phy_configure(). Fallback to the deprecated
>>> way of using the PHY bus width to configure the TMDS character rate.
>>>
>>> A typical call chain during DRM modeset on a RK3328 device:
>>>
>>>   dw_hdmi_rockchip_encoder_atomic_check():
>>>   - inno_hdmi_phy_validate(): pixclock 148500000 tmdsclock 594000000
>>>
>>>   dw_hdmi_rockchip_encoder_atomic_mode_set():
>>>   - inno_hdmi_phy_configure(): pixclock 148500000
>>>     - inno_hdmi_phy_validate(): pixclock 148500000 tmdsclock 594000000
>>>
>>>   vop_crtc_atomic_enable():
>>>   - inno_hdmi_phy_rk3328_clk_set_rate(): rate 594000000 tmdsclk 594000000
>>>   - inno_hdmi_phy_rk3328_clk_set_rate(): pixclock 594000000 tmdsclock 594000000
>>>   - inno_hdmi_phy_rk3328_clk_recalc_rate(): pixclock 594000000 vco 594000000
>>>
>>>   dw_hdmi_rockchip_encoder_enable():
>>>   - inno_hdmi_phy_power_on(): Inno HDMI PHY Power On
>>>   - inno_hdmi_phy_rk3328_clk_set_rate(): rate 594000000 tmdsclk 594000000
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>> Changes in v3:
>>> - Change validate() ops to only validate tmdsclock
>>> - Add comments about expected consumer usage
>>> - Update commit message with a typical call chain
>>> Changes in v2:
>>> - Add validate() ops to validate that the TMDS rate is supported
>>> - Split out parts that remove the old workaround into a separate patch
>>>
>>> Patch "drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set()"
>>> at [1] adds phy_validate() and phy_configure() calls for this HDMI PHY.
>>>
>>> [1] https://lore.kernel.org/dri-devel/20260510183114.1248840-10-jonas@kwiboo.se/
>>> ---
>>>  drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 60 ++++++++++++++++++-
>>>  1 file changed, 59 insertions(+), 1 deletion(-)
>>
>> [snip]
>>
>>> +static int inno_hdmi_phy_validate(struct phy *phy, enum phy_mode mode,
>>> +				  int submode, union phy_configure_opts *opts)
>>> +{
>>> +	const struct pre_pll_config *cfg = pre_pll_cfg_table;
>>> +	unsigned long tmdsclock;
>>> +
>>> +	if (!(mode == PHY_MODE_HDMI && submode == PHY_HDMI_MODE_TMDS))
>>> +		return -EINVAL;
>>> +
>>> +	if (!opts->hdmi.tmds_char_rate || opts->hdmi.tmds_char_rate > 594000000)
>>> +		return -EINVAL;
>>
>> Sashiko reasoning log pointed out that a consumer of phy_validate() or
>> phy_configure() can make a call with opts=NULL, so I may likely send a
>> v4 of this series to fix such possible NULL pointer dereference here.
> 
> Yes please

I will send a v4 that includes a NULL check, thanks.

> 
>>
>> Or is that something that possible should be checked before phy core
>> calls the .validate()/.configure() ops?
> 
> The opts is an optional argument depending upon the mode to be valid. So
> we cant do that in the core...
> 
>>
>> Multiple other phy .configure() ops seem to dereference opts members
>> without any type of opts NULL check. (next-20260508)
> 
> We should fix these
> 
> 
> PS: Any reason why you are sending encypted emails?

Hopefully you find a way to read this :-)

My email provider tries to be smart and encrypts any outgoing mail when
there is a PGP key published for a recipient in a Web Key Directory,
something that kernel.org has [2].

I raised an issue with the provider and was hoping that the change they
made [3] would fix the issue, and it looks like your published PGP key
is not sign-only and is in fact encryption-capable.

Will open a new issue and ask them to add some way to fully disable use
of automatic encryption on outgoing mails.

[2] https://www.kernel.org/category/signatures.html
[3] https://github.com/forwardemail/forwardemail.net/commit/88def5438a8da678252c234e876d3cdd469a8202

Regards,
Jonas