Re: [PATCH v5 4/4] phy: qualcomm: add MSM8974 HDMI PHY support

[Konrad Dybcio]

On 3/14/26 6:06 AM, Dmitry Baryshkov wrote:
> From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> 
> Add support for HDMI PHY on Qualcomm MSM8974 / APQ8074 platforms.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> ---

[...]

> +	sdm_freq_seed = mult_frac(remain, 0x10000, int_ref_freq);
> +
> +	val = (ref_freq_mult_2 ? BIT(0) : 0) |
> +		((refclk_div - 1) << 2);
> +	writel(val, base + UNIPHY_PLL_REFCLK_CFG);
> +
> +	writel(sdm_mode ? 0 : 0x40 + dc_offset, base + UNIPHY_PLL_SDM_CFG0);
> +
> +	writel(dither ? 0x40 + dc_offset : 0, base + UNIPHY_PLL_SDM_CFG1);
> +
> +	writel(sdm_freq_seed & 0xff, base + UNIPHY_PLL_SDM_CFG2);

Some beautification (BIT(), FIELD_..(), defined magic values) would be
really nice to see.. although I'm not sure how much you can do with the
PLL registers..

[...]

> +	ref_freq = ref_freq * 5 / 1000;

mult_frac()

[...]

> +	rate = (dc_offset + 1) * parent_rate;
> +	rate += mult_frac(fraq_n, parent_rate, 0x10000);
> +
> +	rate *= (refclk_cfg >> 2) * 0x3 + 1;

Really strange calculation, but in the end this is (n * 0.75)+1 -
mult_frac()?

> +
> +	return rate;
> +}
> +
> +static const unsigned int qcom_hdmi_8974_divs[] = {1, 2, 4, 6};
> +
> +static unsigned long qcom_hdmi_8974_pll_recalc_rate(struct clk_hw *hw,
> +						    unsigned long parent_rate)
> +{
> +	struct qcom_hdmi_preqmp_phy *hdmi_phy = hw_clk_to_phy(hw);
> +	u32 div_idx = hdmi_pll_read(hdmi_phy, UNIPHY_PLL_POSTDIV1_CFG);
> +	unsigned long rate = qcom_uniphy_recalc(hdmi_phy->pll_reg, parent_rate);
> +
> +	return  rate / HDMI_8974_COMMON_DIV / qcom_hdmi_8974_divs[div_idx & 0x3];

nit: double space


> +}
> +
> +static int qcom_hdmi_8974_pll_determine_rate(struct clk_hw *hw,
> +					     struct clk_rate_request *req)
> +{
> +	req->rate = clamp(req->rate,
> +			  HDMI_8974_VCO_MIN_FREQ / HDMI_8974_COMMON_DIV / 6,
> +			  HDMI_8974_VCO_MAX_FREQ / HDMI_8974_COMMON_DIV / 1);

I don't know if it's a good direction, but maybe:

const unsigned long max_rate = HDMI_8974_VCO_MAX_FREQ / HDMI_8974_COMMON_DIV;

clamp(req->rate, max_rate / 6, max_rate)

?

[...]

> +static int qcom_hdmi_msm8974_phy_find_div(unsigned long long pixclk)
> +{
> +	int i;
> +	unsigned long long min_freq = HDMI_8974_VCO_MIN_FREQ / HDMI_8974_COMMON_DIV;

reverse-Christmas-tree?

> +
> +	if (pixclk > HDMI_8974_VCO_MAX_FREQ / HDMI_8974_COMMON_DIV)
> +		return -E2BIG;

include/uapi/asm-generic/errno-base.h
11:#define      E2BIG            7      /* Argument list too long */

-EINVAL?

Konrad