On 28.09.2023 13:16, Dmitry Baryshkov wrote:
> Add support for HDMI PHY on Qualcomm MSM8974 / APQ8074 platforms.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
I only have a few style comments (and timers-howto.txt fixes)
[...]
> +#define HDMI_8974_VCO_MAX_FREQ 1800000000UL
> +#define HDMI_8974_VCO_MIN_FREQ 600000000UL
> +
> +#define HDMI_8974_COMMON_DIV 5
> +
> +static void qcom_uniphy_setup(void __iomem *base, unsigned int ref_freq,
> + bool sdm_mode,
> + bool ref_freq_mult_2,
> + bool dither,
> + unsigned int refclk_div,
> + unsigned int vco_freq)
> +{
> + unsigned int int_ref_freq = ref_freq * (ref_freq_mult_2 ? 2 : 1);
> + unsigned int div_in_freq = vco_freq / refclk_div;
> + unsigned int dc_offset = div_in_freq / int_ref_freq - 1;
> + unsigned int sdm_freq_seed;
> + unsigned int val;
> + unsigned int remain = div_in_freq - (dc_offset + 1) * int_ref_freq;
> + 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);
The ternary operator doesn't really improve readability here, imo
> +
> + writel((sdm_freq_seed >> 8) & 0xff, base + UNIPHY_PLL_SDM_CFG3);
> +
> + writel(sdm_freq_seed >> 16, base + UNIPHY_PLL_SDM_CFG4);
> +
> + ref_freq = ref_freq * 5 / 1000;
> + writel(ref_freq & 0xff, base + UNIPHY_PLL_CAL_CFG8);
> +
> + writel(ref_freq >> 8, base + UNIPHY_PLL_CAL_CFG9);
> +
> + vco_freq /= 1000;
> + writel(vco_freq & 0xff, base + UNIPHY_PLL_CAL_CFG10);
> +
> + writel(vco_freq >> 8, base + UNIPHY_PLL_CAL_CFG11);
> +}
> +
> +static unsigned long qcom_uniphy_recalc(void __iomem *base, unsigned long parent_rate)
> +{
> + unsigned long rate;
> + u32 refclk_cfg;
> + u32 dc_offset;
> + u64 fraq_n;
> + u32 val;
> +
> + refclk_cfg = readl(base + UNIPHY_PLL_REFCLK_CFG);
> + if (refclk_cfg & BIT(0))
Can we name this bit?
> + parent_rate *= 2;
> +
> + val = readl(base + UNIPHY_PLL_SDM_CFG0);
> + if (val & 0x40) {
BIT(6)? can we name it?
> + dc_offset = val & 0x3f;
GENMASK?
> + fraq_n = 0;
> + } else {
> + dc_offset = readl(base + UNIPHY_PLL_SDM_CFG1) & 0x3f;
GENMASK?
> + fraq_n = readl(base + UNIPHY_PLL_SDM_CFG2) |
> + (readl(base + UNIPHY_PLL_SDM_CFG3) << 8);
FIELD_PREP?
> + }
> +
> + rate = (dc_offset + 1) * parent_rate;
> + rate += mult_frac(fraq_n, parent_rate, 0x10000);
> +
> + rate *= (refclk_cfg >> 2) * 0x3 + 1;
3 would be more clear than 0x3 imo
[...]
> + udelay(50);
usleep_range
[...]
> + udelay(200);
usleep_range
> +
> + return 0;
> +}
> +
> +static int qcom_hdmi_msm8974_phy_pll_enable(struct qcom_hdmi_preqmp_phy *hdmi_phy)
> +{
> + int ret;
> + unsigned long status;
> +
> + /* Global enable */
> + hdmi_phy_write(hdmi_phy, REG_HDMI_8x74_GLB_CFG, 0x81);
> +
> + /* Power up power gen */
> + hdmi_phy_write(hdmi_phy, REG_HDMI_8x74_PD_CTRL0, 0x00);
> + udelay(350);
usleep_range
> +
> + /* PLL power up */
> + hdmi_pll_write(hdmi_phy, UNIPHY_PLL_GLB_CFG, 0x01);
> + udelay(5);
> +
> + /* Power up PLL LDO */
> + hdmi_pll_write(hdmi_phy, UNIPHY_PLL_GLB_CFG, 0x03);
> + udelay(350);
usleep_range
> +
> + /* PLL power up */
> + hdmi_pll_write(hdmi_phy, UNIPHY_PLL_GLB_CFG, 0x0f);
> + udelay(350);
usleep_range
> +
> + /* Poll for PLL ready status */
> + ret = readl_poll_timeout(hdmi_phy->pll_reg + UNIPHY_PLL_STATUS,
> + status, status & BIT(0),
magic bit name?
> + 100, 2000);
> + if (ret) {
> + dev_warn(hdmi_phy->dev, "HDMI PLL not ready\n");
> + goto err;
> + }
> +
> + udelay(350);
usleep_range
Konrad
