Re: [PATCH 3/3] phy: phy-mtk-mipi-dsi: add new driver

[David Lechner <dlechner@baylibre.com>]

On 9/15/26 10:00 AM, Julien Stephan wrote:
> Add a new driver for the MIPI DSI PHY. The driver supports MT8183
> compatible SoCs. The driver is base on the corresponding kernel driver

s/base/based/

> commit 2ec9c210c1f9 ("phy: mediatek: Add support for MT8196 MIPI DSI PHY")
> 
> Few notes about the port from kernel:

s/Few/A few/

> - tested on Genio-350 which does not have nvmem-cells, so keep rt_code[]
>   zeroed and fallback to 0x210 has kernel does when no cell available.

s/has/as the/

>   Can be implemented later.
> - PLL is not registered as a clock provider:
>     - configure() = determine_rate+set_rate
>     - power_on() = clk_prepare_enable
>   configure() takes a struct phy_configure_opts_mipi_dphy and uses its
>   hs_clk_rate member, which carries the per-lane data rate the kernel
>   would have set on the "hs" clock.
> 

...

> +static int mtk_mipi_tx_pll_enable(struct mtk_mipi_tx *mipi_tx)
> +{
> +	void __iomem *base = mipi_tx->regs;
> +	unsigned int txdiv, txdiv0;
> +	u64 pcw;
> +
> +	dev_dbg(mipi_tx->dev, "enable: %llu bps\n", mipi_tx->data_rate);
> +
> +	if (mipi_tx->data_rate >= 2000000000) {
> +		txdiv = 1;
> +		txdiv0 = 0;
> +	} else if (mipi_tx->data_rate >= 1000000000) {
> +		txdiv = 2;
> +		txdiv0 = 1;
> +	} else if (mipi_tx->data_rate >= 500000000) {
> +		txdiv = 4;
> +		txdiv0 = 2;
> +	} else if (mipi_tx->data_rate > 250000000) {
> +		txdiv = 8;
> +		txdiv0 = 3;
> +	} else if (mipi_tx->data_rate >= 125000000) {
> +		txdiv = 16;
> +		txdiv0 = 4;
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	mtk_phy_clear_bits(base + MIPITX_PLL_CON4, RG_DSI_PLL_IBIAS);
> +
> +	mtk_phy_set_bits(base + MIPITX_PLL_PWR, AD_DSI_PLL_SDM_PWR_ON);
> +	mtk_phy_clear_bits(base + MIPITX_PLL_CON1, RG_DSI_PLL_EN);
> +	udelay(1);
> +	mtk_phy_clear_bits(base + MIPITX_PLL_PWR, AD_DSI_PLL_SDM_ISO_EN);
> +	pcw = div_u64(mipi_tx->data_rate * txdiv << 24, MIPITX_PLL_REF_RATE);

(mipi_tx->data_rate * txdiv) << 24

For those of us that can't remember order of operations.

> +	writel(pcw, base + MIPITX_PLL_CON0);
> +	mtk_phy_update_field(base + MIPITX_PLL_CON1, RG_DSI_PLL_POSDIV, txdiv0);
> +	mtk_phy_set_bits(base + MIPITX_PLL_CON1, RG_DSI_PLL_EN);
> +
> +	return 0;
> +}
> +

...

> +static void mtk_mipi_tx_power_on_signal(struct mtk_mipi_tx *mipi_tx)
> +{
> +	void __iomem *base = mipi_tx->regs;
> +
> +	/* BG_LPF_EN / BG_CORE_EN */
> +	writel(RG_DSI_PAD_TIEL_SEL | RG_DSI_BG_CORE_EN, base + MIPITX_LANE_CON);
> +	udelay(30);
> +	writel(RG_DSI_BG_CORE_EN | RG_DSI_BG_LPF_EN, base + MIPITX_LANE_CON);
> +
> +	/* Switch OFF each Lane */
> +	mtk_phy_clear_bits(base + MIPITX_D0_SW_CTL_EN, DSI_SW_CTL_EN);
> +	mtk_phy_clear_bits(base + MIPITX_D1_SW_CTL_EN, DSI_SW_CTL_EN);
> +	mtk_phy_clear_bits(base + MIPITX_D2_SW_CTL_EN, DSI_SW_CTL_EN);
> +	mtk_phy_clear_bits(base + MIPITX_D3_SW_CTL_EN, DSI_SW_CTL_EN);
> +	mtk_phy_clear_bits(base + MIPITX_CK_SW_CTL_EN, DSI_SW_CTL_EN);
> +
> +	mtk_phy_update_field(base + MIPITX_VOLTAGE_SEL, RG_DSI_HSTX_LDO_REF_SEL,
> +			     (mipi_tx->mipitx_drive - 3000) / 200);

Is this 3000 supposed to be MIPITX_DRIVE_MIN_UA?

> +
> +	mtk_mipi_tx_config_calibration_data(mipi_tx);
> +
> +	mtk_phy_set_bits(base + MIPITX_CK_CKMODE_EN, DSI_CK_CKMODE_EN);
> +}
> +

...

> +	/*
> +	 * The kernel reads the per-lane impedance trim codes from the
> +	 * "calibration-data" nvmem cell. No MediaTek board describes that cell

Technically, mipi_tx0: dsi-phy@11e50000 does in mt8183.dtsi

Not saying we need to implement it right now though. Just the comment isn't
100% accurate.

> +	 * in its device tree yet, so rt_code[] is left zeroed here and the
> +	 * SoC layer substitutes the same mid-scale default the kernel uses
> +	 * when the cell is absent.
> +	 */
> +
> +	return 0;
> +}
> +