[PATCH v6 0/5] phy: Add the StarFive JH7110 Innosilicon HDMI PHY

[PATCH v6 0/5] phy: Add the StarFive JH7110 Innosilicon HDMI PHY

[Michal Wilczynski]

This series adds the HDMI PHY driver for the StarFive JH7110 and shares
the pre-PLL code it has in common with the Rockchip RK3328 Innosilicon
HDMI PHY.

Up to v5 it was part of the JH7110 display series [1]. It is split out
as Vinod suggested, so it can go through linux-phy on its own; the rest
continues as v6 of "drm: starfive: jh7110: Enable display subsystem".
Nothing here depends on the display series. The display series depends
on this one: its HDMI subsystem binding references the PHY binding from
patch 1, and it needs the PHY driver at runtime.

[1] https://lore.kernel.org/all/20260929-jh7110-clean-send-v5-0-82b4d8e3c6c7@samsung.com/

The JH7110 has the same Innosilicon PHY as the RK3328, offset by 0x100
because it sits behind the HDMI controller in a shared register block.
Patch 2 factors out the pre-PLL config format, table lookup,
determine_rate, recalc_rate and the pre-PLL programming; patch 3 moves
Rockchip onto it; patch 4 adds the JH7110 driver. Pixel clock tables,
post-PLL and analog config stay SoC specific.

Patch 3 should be a no-op for Rockchip - same writes, same order, same
values - and RK3228, whose pre-PLL is at different addresses, keeps its
own register code and shares only the lookup. I have no Rockchip
hardware, so it is build tested only (arm and riscv). A Tested-by would
help.

Patch 5 adds a MAINTAINERS entry for the binding and the driver, as the
other JH7110 PHY drivers have.

Testing
=======

Tested on a VisionFive 2 v1.3B together with the display series, using
modetest: all 42 modes the sink advertises work, with pixel clocks from
25.175 MHz up to 297 MHz.

Every commit builds for riscv and arm.

Changes in v6:
- Split out of the JH7110 display series (Vinod)
- inno_read() returns the regmap_read() error and recalc_rate checks it,
  instead of a failed read turning into a zero divisor (Vinod)
- Bound the pre-PLL and post-PLL table walks with the table size instead
  of relying on a sentinel (Vinod)
- Lowercase hex in the JH7110 pixel clock table (Vinod)
- New patch 5: a MAINTAINERS entry for the JH7110 HDMI PHY, moved out of
  the display series' entry
- Rebased onto v7.3, collected tags
- Link to v5: https://lore.kernel.org/all/20260929-jh7110-clean-send-v5-0-82b4d8e3c6c7@samsung.com/

---
Michal Wilczynski (5):
      dt-bindings: phy: Add starfive,jh7110-inno-hdmi-phy
      phy: Add common Innosilicon HDMI PHY helpers
      phy: rockchip: inno-hdmi: Use the common Innosilicon PHY helpers
      phy: starfive: Add jh7110-inno-hdmi-phy driver
      MAINTAINERS: Add StarFive JH7110 HDMI PHY entry

 .../phy/starfive,jh7110-inno-hdmi-phy.yaml         |  49 ++
 MAINTAINERS                                        |   6 +
 drivers/phy/Kconfig                                |   8 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-inno-hdmi.c                        | 358 +++++++++++++
 drivers/phy/rockchip/Kconfig                       |   1 +
 drivers/phy/rockchip/phy-rockchip-inno-hdmi.c      | 169 +-----
 drivers/phy/starfive/Kconfig                       |  20 +
 drivers/phy/starfive/Makefile                      |   1 +
 drivers/phy/starfive/phy-jh7110-inno-hdmi.c        | 587 +++++++++++++++++++++
 include/linux/phy/inno-hdmi-phy.h                  |  88 +++
 11 files changed, 1144 insertions(+), 144 deletions(-)
---
base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
change-id: 20261028-jh7110-inno-hdmi-phy-83e5cfa643e3

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 1/5] dt-bindings: phy: Add starfive,jh7110-inno-hdmi-phy

[Michal Wilczynski]

Add the dt-binding for the StarFive JH7110 Innosilicon HDMI PHY.

This device is a child of the starfive,jh7110-hdmi-subsystem node. It
functions as both a PHY provider for the controller and as a clock
provider for the variable pixel clock (hdmi_pclk), which it generates
from its refoclk.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../phy/starfive,jh7110-inno-hdmi-phy.yaml         | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/starfive,jh7110-inno-hdmi-phy.yaml b/Documentation/devicetree/bindings/phy/starfive,jh7110-inno-hdmi-phy.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..6417553e0f5ba295b0451301ca7c1a25534993a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/starfive,jh7110-inno-hdmi-phy.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/starfive,jh7110-inno-hdmi-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 Innosilicon INNO HDMI PHY
+
+maintainers:
+  - Michal Wilczynski <m.wilczynski@samsung.com>
+
+description:
+  The PHY portion of the StarFive JH7110 INNO HDMI IP.
+
+properties:
+  compatible:
+    const: starfive,jh7110-inno-hdmi-phy
+
+  clocks:
+    maxItems: 1
+    description: Reference oscillator.
+
+  '#clock-cells':
+    const: 0
+
+  '#phy-cells':
+    const: 0
+
+required:
+  - compatible
+  - clocks
+  - '#clock-cells'
+  - '#phy-cells'
+
+additionalProperties: false
+
+examples:
+  # Shown as a child of the HDMI subsystem node; see
+  # Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-hdmi-subsystem.yaml
+  # for the full node.
+  - |
+    phy {
+        compatible = "starfive,jh7110-inno-hdmi-phy";
+        #clock-cells = <0>;
+        clocks = <&xin24m>;
+        #phy-cells = <0>;
+    };
+
+...

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 2/5] phy: Add common Innosilicon HDMI PHY helpers

[Michal Wilczynski]

The Innosilicon HDMI PHY IP is used by several SoCs. They differ in
where the PHY register block sits in the register space and in which
pixel clocks they support, but the pre-PLL programming sequence and the
layout of its registers are the same.

Add a small library holding that shared part: the pre-PLL configuration
table format, a lookup, clk_ops determine_rate and recalc_rate helpers,
and the pre-PLL register programming. Callers pass a regmap, a register
offset for the PHY block, and their own pixel clock table and its
size.

No driver uses it yet; the users are converted separately.

Reviewed-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Joshua Peisach <jpeisach@ubuntu.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Byron Stanoszek <gandalf@winds.org>
Tested-by: OOHehir <oo.hehir@gmail.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/phy/Kconfig               |   8 +
 drivers/phy/Makefile              |   1 +
 drivers/phy/phy-inno-hdmi.c       | 358 ++++++++++++++++++++++++++++++++++++++
 include/linux/phy/inno-hdmi-phy.h |  88 ++++++++++
 4 files changed, 455 insertions(+)

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 19f3b7d12b7d492d16000fa52483dc5002590d7a..0e973b3b02a30704d339db9f15d4695bcbec1c75 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -168,6 +168,14 @@ config PHY_XGENE
 	help
 	  This option enables support for APM X-Gene SoC multi-purpose PHY.
 
+config PHY_INNO_HDMI
+	tristate
+	depends on GENERIC_PHY
+	select REGMAP
+	help
+	  Common helpers for the Innosilicon HDMI PHY, shared by the SoC
+	  drivers that instantiate it. Selected by those drivers.
+
 source "drivers/phy/allwinner/Kconfig"
 source "drivers/phy/amlogic/Kconfig"
 source "drivers/phy/apple/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index d7aa516bcc49e7662c9c9db421833fe591dfbc05..e93c473dfc6d837ed6ba540f0b0204a0d68d25bf 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_PHY_COMMON_PROPS)		+= phy-common-props.o
 obj-$(CONFIG_PHY_COMMON_PROPS_TEST)	+= phy-common-props-test.o
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
 obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY)	+= phy-core-mipi-dphy.o
+obj-$(CONFIG_PHY_INNO_HDMI)		+= phy-inno-hdmi.o
 obj-$(CONFIG_PHY_AIROHA_PCIE)		+= phy-airoha-pcie.o
 obj-$(CONFIG_PHY_CAN_TRANSCEIVER)	+= phy-can-transceiver.o
 obj-$(CONFIG_PHY_ECONET_PCIE)		+= phy-econet-pcie.o
diff --git a/drivers/phy/phy-inno-hdmi.c b/drivers/phy/phy-inno-hdmi.c
new file mode 100644
index 0000000000000000000000000000000000000000..6900d4d83e46489e4bc47c5688421a3c0b9a2370
--- /dev/null
+++ b/drivers/phy/phy-inno-hdmi.c
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Common helpers for the Innosilicon HDMI PHY.
+ *
+ * Copyright (c) 2017 Rockchip Electronics Co. Ltd.
+ * Copyright (c) 2025 Samsung Electronics Co., Ltd.
+ *
+ * Author: Zheng Yang <zhengyang@rock-chips.com>
+ * Author: Michal Wilczynski <m.wilczynski@samsung.com>
+ *
+ * Derived from drivers/phy/rockchip/phy-rockchip-inno-hdmi.c.
+ *
+ * The same PHY IP is used by several SoCs, which differ in where the PHY
+ * register block sits and in the pixel clock table they support, but share
+ * the pre-PLL programming sequence.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/iopoll.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include <linux/phy/inno-hdmi-phy.h>
+
+#define INNO_PRE_PLL_CONTROL			0xa0
+#define INNO_PRE_PLL_POWER_DOWN			BIT(0)
+#define INNO_PCLK_VCO_DIV_5_MASK		BIT(1)
+#define INNO_PCLK_VCO_DIV_5(x)			FIELD_PREP(BIT(1), x)
+
+#define INNO_PRE_PLL_DIV_1			0xa1
+#define INNO_PRE_PLL_PRE_DIV_MASK		GENMASK(5, 0)
+#define INNO_PRE_PLL_PRE_DIV(x)			FIELD_PREP(GENMASK(5, 0), x)
+
+#define INNO_PRE_PLL_DIV_2			0xa2
+#define INNO_SPREAD_SPECTRUM_MOD_DISABLE	BIT(6)
+#define INNO_PRE_PLL_FRAC_DIV_DISABLE		FIELD_PREP(GENMASK(5, 4), 3)
+#define INNO_PRE_PLL_FB_DIV_11_8_MASK		GENMASK(3, 0)
+#define INNO_PRE_PLL_FB_DIV_11_8(x)		FIELD_PREP(GENMASK(3, 0), (x) >> 8)
+
+#define INNO_PRE_PLL_DIV_3			0xa3
+#define INNO_PRE_PLL_FB_DIV_7_0(x)		FIELD_PREP(GENMASK(7, 0), x)
+
+#define INNO_PRE_PLL_TMDSCLK_DIV		0xa4
+#define INNO_PRE_PLL_TMDSCLK_DIV_C(x)		FIELD_PREP(GENMASK(1, 0), x)
+#define INNO_PRE_PLL_TMDSCLK_DIV_B(x)		FIELD_PREP(GENMASK(3, 2), x)
+#define INNO_PRE_PLL_TMDSCLK_DIV_A(x)		FIELD_PREP(GENMASK(5, 4), x)
+
+#define INNO_PCLK_DIV_AB			0xa5
+#define INNO_PCLK_DIV_B_MASK			GENMASK(6, 5)
+#define INNO_PCLK_DIV_B(x)			FIELD_PREP(GENMASK(6, 5), x)
+#define INNO_PCLK_DIV_A_MASK			GENMASK(4, 0)
+#define INNO_PCLK_DIV_A(x)			FIELD_PREP(GENMASK(4, 0), x)
+
+#define INNO_PCLK_DIV_CD			0xa6
+#define INNO_PCLK_DIV_C(x)			FIELD_PREP(GENMASK(6, 5), x)
+#define INNO_PCLK_DIV_D_MASK			GENMASK(4, 0)
+#define INNO_PCLK_DIV_D(x)			FIELD_PREP(GENMASK(4, 0), x)
+
+#define INNO_PRE_PLL_LOCK_STATUS		0xa9
+#define INNO_PRE_PLL_LOCK			BIT(0)
+
+#define INNO_PRE_PLL_FRAC_DIV_23_16		0xd1
+#define INNO_PRE_PLL_FRAC_DIV_15_8		0xd2
+#define INNO_PRE_PLL_FRAC_DIV_7_0		0xd3
+#define INNO_PRE_PLL_FRAC_DIV(x)		FIELD_PREP(GENMASK(7, 0), x)
+
+#define INNO_FRAC_DIV_WIDTH			24
+
+static unsigned int inno_reg(const struct inno_hdmi_phy_pre_pll *pll,
+			     unsigned int reg)
+{
+	return (pll->offset + reg) * 4;
+}
+
+static void inno_write(const struct inno_hdmi_phy_pre_pll *pll,
+		       unsigned int reg, u8 val)
+{
+	regmap_write(pll->regmap, inno_reg(pll, reg), val);
+}
+
+static int inno_read(const struct inno_hdmi_phy_pre_pll *pll, unsigned int reg,
+		     unsigned int *val)
+{
+	return regmap_read(pll->regmap, inno_reg(pll, reg), val);
+}
+
+static void inno_update_bits(const struct inno_hdmi_phy_pre_pll *pll,
+			     unsigned int reg, u8 mask, u8 val)
+{
+	regmap_update_bits(pll->regmap, inno_reg(pll, reg), mask, val);
+}
+
+/**
+ * inno_hdmi_phy_pre_pll_lookup - find the settings for a pixel clock
+ * @pll: pre-PLL instance
+ * @pixclock: requested pixel clock, in Hz
+ * @tmdsclock: requested TMDS clock, in Hz
+ *
+ * Return: the matching table entry, or an ERR_PTR if the PHY cannot generate
+ * the requested combination.
+ */
+const struct inno_hdmi_phy_pre_pll_config *
+inno_hdmi_phy_pre_pll_lookup(const struct inno_hdmi_phy_pre_pll *pll,
+			     unsigned long pixclock, unsigned long tmdsclock)
+{
+	unsigned int i;
+
+	for (i = 0; i < pll->table_size; i++) {
+		const struct inno_hdmi_phy_pre_pll_config *cfg = &pll->table[i];
+
+		if (!cfg->pixclock)
+			continue;
+
+		if (cfg->pixclock == pixclock && cfg->tmdsclock == tmdsclock)
+			return cfg;
+	}
+
+	return ERR_PTR(-EINVAL);
+}
+EXPORT_SYMBOL_GPL(inno_hdmi_phy_pre_pll_lookup);
+
+/**
+ * inno_hdmi_phy_pre_pll_determine_rate - clk_ops.determine_rate helper
+ * @pll: pre-PLL instance
+ * @req: rate request, updated with the rate the PHY would produce
+ *
+ * The PHY can only generate the pixel clocks described by its table, so a
+ * request that does not appear there is rejected rather than rounded.
+ *
+ * Return: 0 on success, -EINVAL if the rate is not supported.
+ */
+int inno_hdmi_phy_pre_pll_determine_rate(const struct inno_hdmi_phy_pre_pll *pll,
+					 struct clk_rate_request *req)
+{
+	unsigned long rate = rounddown(req->rate, 1000);
+	unsigned int i;
+
+	for (i = 0; i < pll->table_size; i++) {
+		const struct inno_hdmi_phy_pre_pll_config *cfg = &pll->table[i];
+
+		if (!cfg->pixclock)
+			continue;
+
+		if (cfg->pixclock == rate) {
+			req->rate = cfg->pixclock;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(inno_hdmi_phy_pre_pll_determine_rate);
+
+/**
+ * inno_hdmi_phy_pre_pll_recalc_rate - clk_ops.recalc_rate helper
+ * @pll: pre-PLL instance
+ * @parent_rate: rate of the reference clock, in Hz
+ *
+ * Return: the pixel clock the pre-PLL is currently programmed for, in Hz.
+ */
+unsigned long
+inno_hdmi_phy_pre_pll_recalc_rate(const struct inno_hdmi_phy_pre_pll *pll,
+				  unsigned long parent_rate)
+{
+	unsigned int control, div_1, div_2, div_3, pclk_div_ab, pclk_div_cd;
+	unsigned int frac_div_7_0, frac_div_15_8, frac_div_23_16, div;
+	u8 nd, no_a, no_b, no_d;
+	unsigned long frac, rate;
+	u64 vco, vco_rate;
+	u16 nf;
+	int ret;
+
+	ret = inno_read(pll, INNO_PRE_PLL_CONTROL, &control);
+	if (ret)
+		goto err;
+	ret = inno_read(pll, INNO_PRE_PLL_DIV_1, &div_1);
+	if (ret)
+		goto err;
+	ret = inno_read(pll, INNO_PRE_PLL_DIV_2, &div_2);
+	if (ret)
+		goto err;
+	ret = inno_read(pll, INNO_PRE_PLL_DIV_3, &div_3);
+	if (ret)
+		goto err;
+	ret = inno_read(pll, INNO_PCLK_DIV_AB, &pclk_div_ab);
+	if (ret)
+		goto err;
+	ret = inno_read(pll, INNO_PCLK_DIV_CD, &pclk_div_cd);
+	if (ret)
+		goto err;
+
+	nd = div_1 & INNO_PRE_PLL_PRE_DIV_MASK;
+	nf = (div_2 & INNO_PRE_PLL_FB_DIV_11_8_MASK) << 8;
+	nf |= div_3;
+	vco = parent_rate * nf;
+
+	if (!(div_2 & INNO_PRE_PLL_FRAC_DIV_DISABLE)) {
+		ret = inno_read(pll, INNO_PRE_PLL_FRAC_DIV_7_0, &frac_div_7_0);
+		if (ret)
+			goto err;
+		ret = inno_read(pll, INNO_PRE_PLL_FRAC_DIV_15_8, &frac_div_15_8);
+		if (ret)
+			goto err;
+		ret = inno_read(pll, INNO_PRE_PLL_FRAC_DIV_23_16,
+				&frac_div_23_16);
+		if (ret)
+			goto err;
+
+		frac = frac_div_7_0 | (frac_div_15_8 << 8) |
+		       (frac_div_23_16 << 16);
+		vco += DIV_ROUND_CLOSEST(parent_rate * frac,
+					 BIT(INNO_FRAC_DIV_WIDTH));
+	}
+
+	vco_rate = vco;
+
+	if (control & INNO_PCLK_VCO_DIV_5_MASK) {
+		div = nd * 5;
+	} else {
+		no_a = pclk_div_ab & INNO_PCLK_DIV_A_MASK;
+		no_b = FIELD_GET(INNO_PCLK_DIV_B_MASK, pclk_div_ab) + 2;
+		no_d = pclk_div_cd & INNO_PCLK_DIV_D_MASK;
+
+		div = nd * (no_a == 1 ? no_b : no_a) * no_d * 2;
+	}
+
+	/*
+	 * An unprogrammed pre-PLL reads its dividers back as zero, which would
+	 * leave do_div() dividing by it.
+	 */
+	if (!div) {
+		dev_dbg(pll->dev, "pre-PLL dividers read back as zero\n");
+		return 0;
+	}
+
+	do_div(vco, div);
+
+	rate = DIV_ROUND_CLOSEST((unsigned long)vco, 1000) * 1000;
+
+	/* The VCO is only in spec between 1.4 and 3.2 GHz, so log it. */
+	dev_dbg(pll->dev, "%s rate %lu vco %llu\n", __func__, rate, vco_rate);
+
+	return rate;
+
+err:
+	dev_err(pll->dev, "failed to read the pre-PLL registers: %d\n", ret);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(inno_hdmi_phy_pre_pll_recalc_rate);
+
+/**
+ * inno_hdmi_phy_pre_pll_configure - program the pre-PLL dividers
+ * @pll: pre-PLL instance
+ * @cfg: settings to program
+ *
+ * The caller is responsible for powering the pre-PLL down beforehand and back
+ * up afterwards, and for waiting for it to lock.
+ */
+void inno_hdmi_phy_pre_pll_configure(const struct inno_hdmi_phy_pre_pll *pll,
+				     const struct inno_hdmi_phy_pre_pll_config *cfg)
+{
+	u8 val;
+
+	inno_update_bits(pll, INNO_PRE_PLL_CONTROL, INNO_PCLK_VCO_DIV_5_MASK,
+			 INNO_PCLK_VCO_DIV_5(cfg->vco_div_5_en));
+	inno_write(pll, INNO_PRE_PLL_DIV_1, INNO_PRE_PLL_PRE_DIV(cfg->prediv));
+
+	val = INNO_SPREAD_SPECTRUM_MOD_DISABLE;
+	if (!cfg->fracdiv)
+		val |= INNO_PRE_PLL_FRAC_DIV_DISABLE;
+
+	inno_write(pll, INNO_PRE_PLL_DIV_2,
+		   INNO_PRE_PLL_FB_DIV_11_8(cfg->fbdiv) | val);
+	inno_write(pll, INNO_PRE_PLL_DIV_3,
+		   INNO_PRE_PLL_FB_DIV_7_0(cfg->fbdiv));
+
+	inno_write(pll, INNO_PCLK_DIV_AB,
+		   INNO_PCLK_DIV_A(cfg->pclk_div_a) |
+		   INNO_PCLK_DIV_B(cfg->pclk_div_b));
+	inno_write(pll, INNO_PCLK_DIV_CD,
+		   INNO_PCLK_DIV_C(cfg->pclk_div_c) |
+		   INNO_PCLK_DIV_D(cfg->pclk_div_d));
+
+	inno_write(pll, INNO_PRE_PLL_TMDSCLK_DIV,
+		   INNO_PRE_PLL_TMDSCLK_DIV_A(cfg->tmds_div_a) |
+		   INNO_PRE_PLL_TMDSCLK_DIV_B(cfg->tmds_div_b) |
+		   INNO_PRE_PLL_TMDSCLK_DIV_C(cfg->tmds_div_c));
+
+	inno_write(pll, INNO_PRE_PLL_FRAC_DIV_7_0,
+		   INNO_PRE_PLL_FRAC_DIV(cfg->fracdiv));
+	inno_write(pll, INNO_PRE_PLL_FRAC_DIV_15_8,
+		   INNO_PRE_PLL_FRAC_DIV(cfg->fracdiv >> 8));
+	inno_write(pll, INNO_PRE_PLL_FRAC_DIV_23_16,
+		   INNO_PRE_PLL_FRAC_DIV(cfg->fracdiv >> 16));
+}
+EXPORT_SYMBOL_GPL(inno_hdmi_phy_pre_pll_configure);
+
+/**
+ * inno_hdmi_phy_pre_pll_power_down - power the pre-PLL down or up
+ * @pll: pre-PLL instance
+ * @power_down: true to power down, false to power up
+ */
+void inno_hdmi_phy_pre_pll_power_down(const struct inno_hdmi_phy_pre_pll *pll,
+				      bool power_down)
+{
+	inno_update_bits(pll, INNO_PRE_PLL_CONTROL, INNO_PRE_PLL_POWER_DOWN,
+			 power_down ? INNO_PRE_PLL_POWER_DOWN : 0);
+}
+EXPORT_SYMBOL_GPL(inno_hdmi_phy_pre_pll_power_down);
+
+/**
+ * inno_hdmi_phy_pre_pll_is_locked - report whether the pre-PLL has locked
+ * @pll: pre-PLL instance
+ *
+ * Return: true if the pre-PLL has locked, false if it has not or if the lock
+ * status could not be read.
+ */
+bool inno_hdmi_phy_pre_pll_is_locked(const struct inno_hdmi_phy_pre_pll *pll)
+{
+	unsigned int val;
+	int ret;
+
+	ret = inno_read(pll, INNO_PRE_PLL_LOCK_STATUS, &val);
+	if (ret)
+		return false;
+
+	return val & INNO_PRE_PLL_LOCK;
+}
+EXPORT_SYMBOL_GPL(inno_hdmi_phy_pre_pll_is_locked);
+
+/**
+ * inno_hdmi_phy_pre_pll_wait_locked - wait for the pre-PLL to lock
+ * @pll: pre-PLL instance
+ * @timeout_us: how long to wait, in microseconds
+ *
+ * Return: 0 once locked, -ETIMEDOUT otherwise.
+ */
+int inno_hdmi_phy_pre_pll_wait_locked(const struct inno_hdmi_phy_pre_pll *pll,
+				      unsigned int timeout_us)
+{
+	unsigned int val;
+
+	return regmap_read_poll_timeout(pll->regmap,
+					inno_reg(pll, INNO_PRE_PLL_LOCK_STATUS),
+					val, val & INNO_PRE_PLL_LOCK,
+					1000, timeout_us);
+}
+EXPORT_SYMBOL_GPL(inno_hdmi_phy_pre_pll_wait_locked);
+
+MODULE_DESCRIPTION("Innosilicon HDMI PHY common helpers");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/phy/inno-hdmi-phy.h b/include/linux/phy/inno-hdmi-phy.h
new file mode 100644
index 0000000000000000000000000000000000000000..3fe0d965da1f32fc2ac8f871167c0c17045f4642
--- /dev/null
+++ b/include/linux/phy/inno-hdmi-phy.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Common helpers for the Innosilicon HDMI PHY.
+ *
+ * Copyright (c) 2017 Rockchip Electronics Co. Ltd.
+ * Copyright (c) 2025 Samsung Electronics Co., Ltd.
+ *
+ * Author: Zheng Yang <zhengyang@rock-chips.com>
+ * Author: Michal Wilczynski <m.wilczynski@samsung.com>
+ *
+ * Derived from drivers/phy/rockchip/phy-rockchip-inno-hdmi.c.
+ */
+
+#ifndef __PHY_INNO_HDMI_H__
+#define __PHY_INNO_HDMI_H__
+
+#include <linux/types.h>
+
+struct clk_rate_request;
+struct regmap;
+
+/**
+ * struct inno_hdmi_phy_pre_pll_config - pre-PLL settings for one pixel clock
+ * @pixclock: pixel clock this entry describes, in Hz
+ * @tmdsclock: TMDS clock this entry describes, in Hz
+ * @prediv: pre-PLL reference divider
+ * @fbdiv: pre-PLL feedback divider
+ * @tmds_div_a: TMDS clock divider A
+ * @tmds_div_b: TMDS clock divider B
+ * @tmds_div_c: TMDS clock divider C
+ * @pclk_div_a: pixel clock divider A
+ * @pclk_div_b: pixel clock divider B
+ * @pclk_div_c: pixel clock divider C
+ * @pclk_div_d: pixel clock divider D
+ * @vco_div_5_en: divide the VCO output by five
+ * @fracdiv: pre-PLL fractional divider, zero to use integer mode
+ */
+struct inno_hdmi_phy_pre_pll_config {
+	unsigned long pixclock;
+	unsigned long tmdsclock;
+	u8 prediv;
+	u16 fbdiv;
+	u8 tmds_div_a;
+	u8 tmds_div_b;
+	u8 tmds_div_c;
+	u8 pclk_div_a;
+	u8 pclk_div_b;
+	u8 pclk_div_c;
+	u8 pclk_div_d;
+	u8 vco_div_5_en;
+	u32 fracdiv;
+};
+
+/**
+ * struct inno_hdmi_phy_pre_pll - a pre-PLL instance
+ * @dev: device used for log messages
+ * @regmap: register map covering the PHY registers
+ * @offset: offset of the PHY register block, in units of the register index.
+ *	    Zero where the PHY block starts the register space, 0x100 on the
+ *	    StarFive JH7110 where it is preceded by the HDMI controller.
+ * @table: pixel clock table
+ * @table_size: number of entries in @table, including any sentinel
+ */
+struct inno_hdmi_phy_pre_pll {
+	struct device *dev;
+	struct regmap *regmap;
+	unsigned int offset;
+	const struct inno_hdmi_phy_pre_pll_config *table;
+	unsigned int table_size;
+};
+
+const struct inno_hdmi_phy_pre_pll_config *
+inno_hdmi_phy_pre_pll_lookup(const struct inno_hdmi_phy_pre_pll *pll,
+			     unsigned long pixclock, unsigned long tmdsclock);
+int inno_hdmi_phy_pre_pll_determine_rate(const struct inno_hdmi_phy_pre_pll *pll,
+					 struct clk_rate_request *req);
+unsigned long
+inno_hdmi_phy_pre_pll_recalc_rate(const struct inno_hdmi_phy_pre_pll *pll,
+				  unsigned long parent_rate);
+void inno_hdmi_phy_pre_pll_configure(const struct inno_hdmi_phy_pre_pll *pll,
+				     const struct inno_hdmi_phy_pre_pll_config *cfg);
+void inno_hdmi_phy_pre_pll_power_down(const struct inno_hdmi_phy_pre_pll *pll,
+				      bool power_down);
+bool inno_hdmi_phy_pre_pll_is_locked(const struct inno_hdmi_phy_pre_pll *pll);
+int inno_hdmi_phy_pre_pll_wait_locked(const struct inno_hdmi_phy_pre_pll *pll,
+				      unsigned int timeout_us);
+
+#endif /* __PHY_INNO_HDMI_H__ */

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 2/5] phy: Add common Innosilicon HDMI PHY helpers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: inno-hdmi: 32-bit overflow in inno_hdmi_phy_pre_pll_recalc_rate()
--

commit ddb60d944975dab0de39c50bd23d32e884d239ae
Author: Michal Wilczynski <m.wilczynski@samsung.com>

phy: Add common Innosilicon HDMI PHY helpers

This commit introduces a shared helper library for the Innosilicon HDMI PHY
pre-PLL configuration, including table lookup, clock rate calculation helpers,
and register programming.

> diff --git a/drivers/phy/phy-inno-hdmi.c b/drivers/phy/phy-inno-hdmi.c
> new file mode 100644
> index 0000000000000..6900d4d83e464
> --- /dev/null
> +++ b/drivers/phy/phy-inno-hdmi.c
[ ... ]
> +unsigned long
> +inno_hdmi_phy_pre_pll_recalc_rate(const struct inno_hdmi_phy_pre_pll *pll,
> +				  unsigned long parent_rate)
> +{
> +	unsigned int control, div_1, div_2, div_3, pclk_div_ab, pclk_div_cd;
> +	unsigned int frac_div_7_0, frac_div_15_8, frac_div_23_16, div;
> +	u8 nd, no_a, no_b, no_d;
> +	unsigned long frac, rate;
> +	u64 vco, vco_rate;
> +	u16 nf;
> +	int ret;
[ ... ]
> +	nd = div_1 & INNO_PRE_PLL_PRE_DIV_MASK;
> +	nf = (div_2 & INNO_PRE_PLL_FB_DIV_11_8_MASK) << 8;
> +	nf |= div_3;
> +	vco = parent_rate * nf;
> +
> +	if (!(div_2 & INNO_PRE_PLL_FRAC_DIV_DISABLE)) {
> +		ret = inno_read(pll, INNO_PRE_PLL_FRAC_DIV_7_0, &frac_div_7_0);
> +		if (ret)
> +			goto err;
> +		ret = inno_read(pll, INNO_PRE_PLL_FRAC_DIV_15_8, &frac_div_15_8);
> +		if (ret)
> +			goto err;
> +		ret = inno_read(pll, INNO_PRE_PLL_FRAC_DIV_23_16,
> +				&frac_div_23_16);
> +		if (ret)
> +			goto err;
> +
> +		frac = frac_div_7_0 | (frac_div_15_8 << 8) |
> +		       (frac_div_23_16 << 16);
> +		vco += DIV_ROUND_CLOSEST(parent_rate * frac,
> +					 BIT(INNO_FRAC_DIV_WIDTH));
> +	}

[Severity: Medium]
Can the VCO calculations in inno_hdmi_phy_pre_pll_recalc_rate() in
drivers/phy/phy-inno-hdmi.c overflow on 32-bit architectures?

Both parent_rate and frac have type unsigned long, while nf is a u16 and vco
is a u64. On 32-bit targets where unsigned long is 32 bits wide,
parent_rate * nf is evaluated in 32-bit arithmetic before being widened to
u64. With a typical 24 MHz reference clock, any feedback divider nf >= 179
exceeds U32_MAX, whereas the 12-bit feedback divider reaches up to 757 in the
Innosilicon pre-PLL configuration tables.

Similarly, when the fractional divider is enabled, parent_rate * frac and
DIV_ROUND_CLOSEST() operate on 32-bit unsigned long values before adding to
vco. For a 24 MHz parent_rate and a 24-bit frac value, parent_rate * frac
requires up to 49 bits and overflows 32-bit unsigned long whenever frac >= 179.

Should parent_rate be cast to u64 in parent_rate * nf, and should the
fractional calculation use DIV_ROUND_CLOSEST_ULL((u64)parent_rate * frac,
BIT(INNO_FRAC_DIV_WIDTH)) to avoid 32-bit truncation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-jh7110-inno-hdmi-phy-v6-0-f3379684d97e@samsung.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 3/5] phy: rockchip: inno-hdmi: Use the common Innosilicon PHY helpers

[Michal Wilczynski]

The RK3328 pre-PLL programming, its table lookup and its recalc_rate and
determine_rate implementations are the generic Innosilicon ones, so drop
the local copies and use the shared helpers instead.

The RK3228 pre-PLL sits at different register addresses, so it keeps its
own register level code and only shares the table lookup.

The now unused RK3328 pre-PLL register macros go with it, as does the
local pre-PLL config lookup wrapper; both call sites already have the
TMDS clock to hand and call inno_hdmi_phy_pre_pll_lookup() directly.

The RK3328 recalc_rate used to log the pre-PLL output as "vco". The
shared helper logs it too, and reports the actual VCO frequency
alongside the rate, which is the value with the 1.4-3.2 GHz constraint.

The register writes, their order and the values written are unchanged.
No functional change intended.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Tested-by: Byron Stanoszek <gandalf@winds.org>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/phy/rockchip/Kconfig                  |   1 +
 drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 169 ++++----------------------
 2 files changed, 26 insertions(+), 144 deletions(-)

diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
index 14698571b607590dc0e7a29f3b39eb9a5d4e3d25..9732c5be87717dc3ddbea1c60080952336a07af9 100644
--- a/drivers/phy/rockchip/Kconfig
+++ b/drivers/phy/rockchip/Kconfig
@@ -34,6 +34,7 @@ config PHY_ROCKCHIP_INNO_HDMI
 	depends on COMMON_CLK
 	depends on HAS_IOMEM
 	select GENERIC_PHY
+	select PHY_INNO_HDMI
 	help
 	  Enable this to support the Rockchip Innosilicon HDMI PHY.
 
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
index c3d257a0f4a65b4c475ed7246568dd463b46ed86..809583a7030627832794ec0b08295549616f8f28 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/phy/inno-hdmi-phy.h>
 #include <linux/phy/phy.h>
 #include <linux/slab.h>
 
@@ -130,42 +131,14 @@
 #define RK3328_INT_VSS_AGND_ESD_DET			BIT(1)
 #define RK3328_INT_AGND_VSS_ESD_DET			BIT(0)
 /* REG: 0xa0 */
-#define RK3328_PCLK_VCO_DIV_5_MASK			BIT(1)
-#define RK3328_PCLK_VCO_DIV_5(x)			UPDATE(x, 1, 1)
 #define RK3328_PRE_PLL_POWER_DOWN			BIT(0)
 /* REG: 0xa1 */
-#define RK3328_PRE_PLL_PRE_DIV_MASK			GENMASK(5, 0)
-#define RK3328_PRE_PLL_PRE_DIV(x)			UPDATE(x, 5, 0)
 /* REG: 0xa2 */
-/* unset means center spread */
-#define RK3328_SPREAD_SPECTRUM_MOD_DOWN			BIT(7)
-#define RK3328_SPREAD_SPECTRUM_MOD_DISABLE		BIT(6)
-#define RK3328_PRE_PLL_FRAC_DIV_DISABLE			UPDATE(3, 5, 4)
-#define RK3328_PRE_PLL_FB_DIV_11_8_MASK			GENMASK(3, 0)
-#define RK3328_PRE_PLL_FB_DIV_11_8(x)			UPDATE((x) >> 8, 3, 0)
 /* REG: 0xa3 */
-#define RK3328_PRE_PLL_FB_DIV_7_0(x)			UPDATE(x, 7, 0)
 /* REG: 0xa4*/
-#define RK3328_PRE_PLL_TMDSCLK_DIV_C_MASK		GENMASK(1, 0)
-#define RK3328_PRE_PLL_TMDSCLK_DIV_C(x)			UPDATE(x, 1, 0)
-#define RK3328_PRE_PLL_TMDSCLK_DIV_B_MASK		GENMASK(3, 2)
-#define RK3328_PRE_PLL_TMDSCLK_DIV_B(x)			UPDATE(x, 3, 2)
-#define RK3328_PRE_PLL_TMDSCLK_DIV_A_MASK		GENMASK(5, 4)
-#define RK3328_PRE_PLL_TMDSCLK_DIV_A(x)			UPDATE(x, 5, 4)
 /* REG: 0xa5 */
-#define RK3328_PRE_PLL_PCLK_DIV_B_SHIFT			5
-#define RK3328_PRE_PLL_PCLK_DIV_B_MASK			GENMASK(6, 5)
-#define RK3328_PRE_PLL_PCLK_DIV_B(x)			UPDATE(x, 6, 5)
-#define RK3328_PRE_PLL_PCLK_DIV_A_MASK			GENMASK(4, 0)
-#define RK3328_PRE_PLL_PCLK_DIV_A(x)			UPDATE(x, 4, 0)
 /* REG: 0xa6 */
-#define RK3328_PRE_PLL_PCLK_DIV_C_SHIFT			5
-#define RK3328_PRE_PLL_PCLK_DIV_C_MASK			GENMASK(6, 5)
-#define RK3328_PRE_PLL_PCLK_DIV_C(x)			UPDATE(x, 6, 5)
-#define RK3328_PRE_PLL_PCLK_DIV_D_MASK			GENMASK(4, 0)
-#define RK3328_PRE_PLL_PCLK_DIV_D(x)			UPDATE(x, 4, 0)
 /* REG: 0xa9 */
-#define RK3328_PRE_PLL_LOCK_STATUS			BIT(0)
 /* REG: 0xaa */
 #define RK3328_POST_PLL_POST_DIV_ENABLE			GENMASK(3, 2)
 #define RK3328_POST_PLL_REFCLK_SEL_TMDS			BIT(1)
@@ -218,11 +191,8 @@
 #define RK3328_TMDS_TERM_RESIST_1000			BIT(1)
 #define RK3328_TMDS_TERM_RESIST_2000			BIT(0)
 /* REG: 0xd1 */
-#define RK3328_PRE_PLL_FRAC_DIV_23_16(x)		UPDATE((x) >> 16, 7, 0)
 /* REG: 0xd2 */
-#define RK3328_PRE_PLL_FRAC_DIV_15_8(x)			UPDATE((x) >> 8, 7, 0)
 /* REG: 0xd3 */
-#define RK3328_PRE_PLL_FRAC_DIV_7_0(x)			UPDATE(x, 7, 0)
 
 struct inno_hdmi_phy_drv_data;
 
@@ -240,6 +210,8 @@ struct inno_hdmi_phy {
 	const struct inno_hdmi_phy_drv_data *plat_data;
 	int chip_version;
 
+	struct inno_hdmi_phy_pre_pll pre_pll;
+
 	/* clk provider */
 	struct clk_hw hw;
 	struct clk *phyclk;
@@ -248,21 +220,6 @@ struct inno_hdmi_phy {
 	unsigned long opts_tmds_char_rate;
 };
 
-struct pre_pll_config {
-	unsigned long pixclock;
-	unsigned long tmdsclock;
-	u8 prediv;
-	u16 fbdiv;
-	u8 tmds_div_a;
-	u8 tmds_div_b;
-	u8 tmds_div_c;
-	u8 pclk_div_a;
-	u8 pclk_div_b;
-	u8 pclk_div_c;
-	u8 pclk_div_d;
-	u8 vco_div_5_en;
-	u32 fracdiv;
-};
 
 struct post_pll_config {
 	unsigned long tmdsclock;
@@ -291,7 +248,7 @@ struct inno_hdmi_phy_drv_data {
 	const struct phy_config		*phy_cfg_table;
 };
 
-static const struct pre_pll_config pre_pll_cfg_table[] = {
+static const struct inno_hdmi_phy_pre_pll_config pre_pll_cfg_table[] = {
 	{ 25175000,  25175000,  3,  125, 3, 1, 1,  1, 3, 3,  4, 0, 0xe00000},
 	{ 25175000,  31468750,  1,   41, 0, 3, 3,  1, 3, 3,  4, 0, 0xf5554f},
 	{ 27000000,  27000000,  1,   36, 0, 3, 3,  1, 2, 3,  4, 0,      0x0},
@@ -603,7 +560,7 @@ static irqreturn_t inno_hdmi_phy_rk3328_irq(int irq, void *dev_id)
 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;
+	const struct inno_hdmi_phy_pre_pll_config *cfg = pre_pll_cfg_table;
 	unsigned long tmdsclock;
 
 	if (!(mode == PHY_MODE_HDMI && submode == PHY_HDMI_MODE_TMDS))
@@ -717,23 +674,6 @@ static const struct phy_ops inno_hdmi_phy_ops = {
 	.validate = inno_hdmi_phy_validate,
 };
 
-static const
-struct pre_pll_config *inno_hdmi_phy_get_pre_pll_cfg(struct inno_hdmi_phy *inno,
-						     unsigned long rate)
-{
-	const struct pre_pll_config *cfg = pre_pll_cfg_table;
-	unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
-
-	for (; cfg->pixclock != 0; cfg++)
-		if (cfg->pixclock == rate && cfg->tmdsclock == tmdsclock)
-			break;
-
-	if (cfg->pixclock == 0)
-		return ERR_PTR(-EINVAL);
-
-	return cfg;
-}
-
 static int inno_hdmi_phy_rk3228_clk_is_prepared(struct clk_hw *hw)
 {
 	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
@@ -797,7 +737,7 @@ unsigned long inno_hdmi_phy_rk3228_clk_recalc_rate(struct clk_hw *hw,
 static int inno_hdmi_phy_rk3228_clk_determine_rate(struct clk_hw *hw,
 						   struct clk_rate_request *req)
 {
-	const struct pre_pll_config *cfg = pre_pll_cfg_table;
+	const struct inno_hdmi_phy_pre_pll_config *cfg = pre_pll_cfg_table;
 
 	req->rate = (req->rate / 1000) * 1000;
 
@@ -818,7 +758,7 @@ static int inno_hdmi_phy_rk3228_clk_set_rate(struct clk_hw *hw,
 					     unsigned long parent_rate)
 {
 	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
-	const struct pre_pll_config *cfg;
+	const struct inno_hdmi_phy_pre_pll_config *cfg;
 	unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
 	u32 v;
 	int ret;
@@ -829,7 +769,7 @@ static int inno_hdmi_phy_rk3228_clk_set_rate(struct clk_hw *hw,
 	if (inno->pixclock == rate && inno->tmdsclock == tmdsclock)
 		return 0;
 
-	cfg = inno_hdmi_phy_get_pre_pll_cfg(inno, rate);
+	cfg = inno_hdmi_phy_pre_pll_lookup(&inno->pre_pll, rate, tmdsclock);
 	if (IS_ERR(cfg))
 		return PTR_ERR(cfg);
 
@@ -915,39 +855,9 @@ unsigned long inno_hdmi_phy_rk3328_clk_recalc_rate(struct clk_hw *hw,
 						   unsigned long parent_rate)
 {
 	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
-	unsigned long frac;
-	u8 nd, no_a, no_b, no_d;
-	u64 vco;
-	u16 nf;
-
-	nd = inno_read(inno, 0xa1) & RK3328_PRE_PLL_PRE_DIV_MASK;
-	nf = ((inno_read(inno, 0xa2) & RK3328_PRE_PLL_FB_DIV_11_8_MASK) << 8);
-	nf |= inno_read(inno, 0xa3);
-	vco = parent_rate * nf;
-
-	if (!(inno_read(inno, 0xa2) & RK3328_PRE_PLL_FRAC_DIV_DISABLE)) {
-		frac = inno_read(inno, 0xd3) |
-		       (inno_read(inno, 0xd2) << 8) |
-		       (inno_read(inno, 0xd1) << 16);
-		vco += DIV_ROUND_CLOSEST(parent_rate * frac, (1 << 24));
-	}
-
-	if (inno_read(inno, 0xa0) & RK3328_PCLK_VCO_DIV_5_MASK) {
-		do_div(vco, nd * 5);
-	} else {
-		no_a = inno_read(inno, 0xa5) & RK3328_PRE_PLL_PCLK_DIV_A_MASK;
-		no_b = inno_read(inno, 0xa5) & RK3328_PRE_PLL_PCLK_DIV_B_MASK;
-		no_b >>= RK3328_PRE_PLL_PCLK_DIV_B_SHIFT;
-		no_b += 2;
-		no_d = inno_read(inno, 0xa6) & RK3328_PRE_PLL_PCLK_DIV_D_MASK;
-
-		do_div(vco, (nd * (no_a == 1 ? no_b : no_a) * no_d * 2));
-	}
 
-	inno->pixclock = DIV_ROUND_CLOSEST((unsigned long)vco, 1000) * 1000;
-
-	dev_dbg(inno->dev, "%s rate %lu vco %llu\n",
-		__func__, inno->pixclock, vco);
+	inno->pixclock = inno_hdmi_phy_pre_pll_recalc_rate(&inno->pre_pll,
+							   parent_rate);
 
 	return inno->pixclock;
 }
@@ -955,20 +865,9 @@ unsigned long inno_hdmi_phy_rk3328_clk_recalc_rate(struct clk_hw *hw,
 static int inno_hdmi_phy_rk3328_clk_determine_rate(struct clk_hw *hw,
 						   struct clk_rate_request *req)
 {
-	const struct pre_pll_config *cfg = pre_pll_cfg_table;
-
-	req->rate = (req->rate / 1000) * 1000;
-
-	for (; cfg->pixclock != 0; cfg++)
-		if (cfg->pixclock == req->rate)
-			break;
-
-	if (cfg->pixclock == 0)
-		return -EINVAL;
-
-	req->rate = cfg->pixclock;
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
 
-	return 0;
+	return inno_hdmi_phy_pre_pll_determine_rate(&inno->pre_pll, req);
 }
 
 static int inno_hdmi_phy_rk3328_clk_set_rate(struct clk_hw *hw,
@@ -976,9 +875,8 @@ static int inno_hdmi_phy_rk3328_clk_set_rate(struct clk_hw *hw,
 					     unsigned long parent_rate)
 {
 	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
-	const struct pre_pll_config *cfg;
+	const struct inno_hdmi_phy_pre_pll_config *cfg;
 	unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
-	u32 val;
 	int ret;
 
 	dev_dbg(inno->dev, "%s rate %lu tmdsclk %lu\n",
@@ -987,39 +885,15 @@ static int inno_hdmi_phy_rk3328_clk_set_rate(struct clk_hw *hw,
 	if (inno->pixclock == rate && inno->tmdsclock == tmdsclock)
 		return 0;
 
-	cfg = inno_hdmi_phy_get_pre_pll_cfg(inno, rate);
+	cfg = inno_hdmi_phy_pre_pll_lookup(&inno->pre_pll, rate, tmdsclock);
 	if (IS_ERR(cfg))
 		return PTR_ERR(cfg);
 
-	inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN,
-			 RK3328_PRE_PLL_POWER_DOWN);
+	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, true);
+	inno_hdmi_phy_pre_pll_configure(&inno->pre_pll, cfg);
+	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, false);
 
-	/* Configure pre-pll */
-	inno_update_bits(inno, 0xa0, RK3328_PCLK_VCO_DIV_5_MASK,
-			 RK3328_PCLK_VCO_DIV_5(cfg->vco_div_5_en));
-	inno_write(inno, 0xa1, RK3328_PRE_PLL_PRE_DIV(cfg->prediv));
-
-	val = RK3328_SPREAD_SPECTRUM_MOD_DISABLE;
-	if (!cfg->fracdiv)
-		val |= RK3328_PRE_PLL_FRAC_DIV_DISABLE;
-	inno_write(inno, 0xa2, RK3328_PRE_PLL_FB_DIV_11_8(cfg->fbdiv) | val);
-	inno_write(inno, 0xa3, RK3328_PRE_PLL_FB_DIV_7_0(cfg->fbdiv));
-	inno_write(inno, 0xa5, RK3328_PRE_PLL_PCLK_DIV_A(cfg->pclk_div_a) |
-		   RK3328_PRE_PLL_PCLK_DIV_B(cfg->pclk_div_b));
-	inno_write(inno, 0xa6, RK3328_PRE_PLL_PCLK_DIV_C(cfg->pclk_div_c) |
-		   RK3328_PRE_PLL_PCLK_DIV_D(cfg->pclk_div_d));
-	inno_write(inno, 0xa4, RK3328_PRE_PLL_TMDSCLK_DIV_C(cfg->tmds_div_c) |
-		   RK3328_PRE_PLL_TMDSCLK_DIV_A(cfg->tmds_div_a) |
-		   RK3328_PRE_PLL_TMDSCLK_DIV_B(cfg->tmds_div_b));
-	inno_write(inno, 0xd3, RK3328_PRE_PLL_FRAC_DIV_7_0(cfg->fracdiv));
-	inno_write(inno, 0xd2, RK3328_PRE_PLL_FRAC_DIV_15_8(cfg->fracdiv));
-	inno_write(inno, 0xd1, RK3328_PRE_PLL_FRAC_DIV_23_16(cfg->fracdiv));
-
-	inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN, 0);
-
-	/* Wait for Pre-PLL lock */
-	ret = inno_poll(inno, 0xa9, val, val & RK3328_PRE_PLL_LOCK_STATUS,
-			1000, 10000);
+	ret = inno_hdmi_phy_pre_pll_wait_locked(&inno->pre_pll, 10000);
 	if (ret) {
 		dev_err(inno->dev, "Pre-PLL locking failed\n");
 		return ret;
@@ -1418,6 +1292,13 @@ static int inno_hdmi_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(inno->regmap))
 		return PTR_ERR(inno->regmap);
 
+	/* The PHY block starts the register space on these SoCs. */
+	inno->pre_pll.dev = inno->dev;
+	inno->pre_pll.regmap = inno->regmap;
+	inno->pre_pll.offset = 0;
+	inno->pre_pll.table = pre_pll_cfg_table;
+	inno->pre_pll.table_size = ARRAY_SIZE(pre_pll_cfg_table);
+
 	/* only the newer rk3328 hdmiphy has an interrupt */
 	inno->irq = platform_get_irq(pdev, 0);
 	if (inno->irq > 0) {

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 4/5] phy: starfive: Add jh7110-inno-hdmi-phy driver

[Michal Wilczynski]

Add the HDMI PHY driver for the StarFive JH7110.

This driver binds to the starfive,jh7110-inno-hdmi-phy node and
gets its regmap from the parent. Its only clock input is the reference
oscillator (xin24m); it does not depend on voutcrg.

This driver provides two main functions:
 - Clock Provider: It registers clk_ops to provide the variable pixel
   clock. The .set_rate operation configures the Pre-PLL registers
   (0x1a0+) based on the requested rate.

 - PHY Provider: It registers phy_ops for the controller. The .power_on
   op configures and enables the Post-PLL and other analog blocks (BIAS,
   LDO, Serializer, etc.).

The clock ops reach registers in the window shared with the HDMI
controller, and access to that window is gated by the controller's
system clock, which only the sibling controller driver holds. The clock
framework walks every registered clock regardless of which drivers have
bound, so deliberately do not implement .is_prepared, which lets the
framework fall back to the software prepare count, and return the cached
rate from .recalc_rate whenever the clock is not prepared. Without this,
clk_disable_unused() reads a PHY register with that clock gated and the
resulting stalled bus transaction wedges the CPU hard enough that it
stops responding to NMIs. It is reproducible by building the bridge
driver as a module so that the controller has not bound by then.

The JH7110 instantiates the same Innosilicon IP as the Rockchip RK3328,
with the PHY register block shifted by 0x100 because it sits behind the
HDMI controller in the shared register space. The pre-PLL programming
therefore comes from the common Innosilicon helpers; this driver adds
the JH7110 pixel clock table, the post-PLL and the analog configuration.

Co-developed-by: Dominique Belhachemi <db@domibel.de>
Signed-off-by: Dominique Belhachemi <db@domibel.de>
Tested-by: Byron Stanoszek <gandalf@winds.org>
Tested-by: OOHehir <oo.hehir@gmail.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/phy/starfive/Kconfig                |  20 +
 drivers/phy/starfive/Makefile               |   1 +
 drivers/phy/starfive/phy-jh7110-inno-hdmi.c | 587 ++++++++++++++++++++++++++++
 3 files changed, 608 insertions(+)

diff --git a/drivers/phy/starfive/Kconfig b/drivers/phy/starfive/Kconfig
index d0cdd7cb4a13de22ff643c89a79d99cce57284d7..1da417ffbf224c13224e42a3ce322e3d0c272824 100644
--- a/drivers/phy/starfive/Kconfig
+++ b/drivers/phy/starfive/Kconfig
@@ -25,6 +25,26 @@ config PHY_STARFIVE_JH7110_DPHY_TX
 	  system. If M is selected, the module will be called
 	  phy-jh7110-dphy-tx.ko.
 
+config PHY_STARFIVE_JH7110_INNO_HDMI
+	tristate "Starfive JH7110 INNO HDMI PHY"
+	select PHY_INNO_HDMI
+	depends on COMMON_CLK
+	select GENERIC_PHY
+	help
+	  This option enables the driver for the analog HDMI PHY (Physical
+	  Layer) on the StarFive JH7110 SoC.
+
+	  This driver binds to a child node of the 'starfive,jh7110-hdmi-subsystem'
+	  parent driver and gets its register map from that parent.
+
+	  It is responsible for two main functions:
+	  1. PHY Provider: It provides standard PHY operations (.power_on,
+	     .power_off) for the HDMI controller (bridge) driver. This
+	     involves configuring the Post-PLL and analog TMDS blocks.
+	  2. Clock Provider: It registers as a clock provider to supply the
+	     variable pixel clock (hdmi_pclk) to the HDMI controller and
+	     the VOUT subsystem, which it generates using the Pre-PLL.
+
 config PHY_STARFIVE_JH7110_PCIE
 	tristate "Starfive JH7110 PCIE 2.0/USB 3.0 PHY support"
 	depends on HAS_IOMEM
diff --git a/drivers/phy/starfive/Makefile b/drivers/phy/starfive/Makefile
index eedc4a6fec156320c99ac0a0da609083b6a6a695..e7b13f00880b500f933f21b6037384d5c6884e3e 100644
--- a/drivers/phy/starfive/Makefile
+++ b/drivers/phy/starfive/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_PHY_STARFIVE_JH7110_DPHY_RX)	+= phy-jh7110-dphy-rx.o
 obj-$(CONFIG_PHY_STARFIVE_JH7110_DPHY_TX)	+= phy-jh7110-dphy-tx.o
+obj-$(CONFIG_PHY_STARFIVE_JH7110_INNO_HDMI)	+= phy-jh7110-inno-hdmi.o
 obj-$(CONFIG_PHY_STARFIVE_JH7110_PCIE)		+= phy-jh7110-pcie.o
 obj-$(CONFIG_PHY_STARFIVE_JH7110_USB)		+= phy-jh7110-usb.o
diff --git a/drivers/phy/starfive/phy-jh7110-inno-hdmi.c b/drivers/phy/starfive/phy-jh7110-inno-hdmi.c
new file mode 100644
index 0000000000000000000000000000000000000000..ebda363e50a3e3b5e01629590e68fae4aae4ae32
--- /dev/null
+++ b/drivers/phy/starfive/phy-jh7110-inno-hdmi.c
@@ -0,0 +1,587 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017 Rockchip Electronics Co. Ltd.
+ * Copyright (c) 2025 Samsung Electronics Co., Ltd.
+ *
+ * Author: Zheng Yang <zhengyang@rock-chips.com>
+ * Author: Michal Wilczynski <m.wilczynski@samsung.com>
+ *
+ * The register layout and programming sequence derive from
+ * drivers/phy/rockchip/phy-rockchip-inno-hdmi.c; the JH7110 places the
+ * same Innosilicon PHY block at a 0x100 register offset.
+ *
+ * This driver handles the PHY portion of the StarFive Innosilicon HDMI IP,
+ * which is part of a monolithic HDMI block. It provides the variable pixel
+ * clock (from the Pre-PLL) and the PHY operations (for the Post-PLL/analog).
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/phy/inno-hdmi-phy.h>
+#include <linux/phy/phy.h>
+#include <linux/slab.h>
+
+/*
+ * StarFive (JH7110) Innosilicon HDMI PHY Register Definitions
+ */
+
+/* REG: 0x1aa */
+#define STF_INNO_POST_PLL_DIV_1		0x1aa
+#define STF_INNO_POST_PLL_POST_DIV_ENABLE	GENMASK(3, 2)
+#define STF_INNO_POST_PLL_REFCLK_SEL_TMDS	BIT(1)
+#define STF_INNO_POST_PLL_POWER_DOWN		BIT(0)
+
+/* REG: 0x1ab */
+#define STF_INNO_POST_PLL_DIV_2		0x1ab
+#define STF_INNO_POST_PLL_PRE_DIV(x)		FIELD_PREP(GENMASK(5, 0), x)
+#define STF_INNO_POST_PLL_FB_DIV_8(x)		FIELD_PREP(BIT(7), (x) >> 8)
+
+/* REG: 0x1ac */
+#define STF_INNO_POST_PLL_DIV_3		0x1ac
+#define STF_INNO_POST_PLL_FB_DIV_7_0(x)	FIELD_PREP(GENMASK(7, 0), x)
+
+/* REG: 0x1ad */
+#define STF_INNO_POST_PLL_DIV_4		0x1ad
+#define STF_INNO_POST_PLL_POST_DIV_MASK	GENMASK(1, 0)
+
+/* REG: 0x1af */
+#define STF_INNO_POST_PLL_LOCK_STATUS		0x1af
+#define STF_INNO_POST_PLL_LOCK		BIT(0)
+
+/* REG: 0x1b0 */
+#define STF_INNO_BIAS_CONTROL			0x1b0
+#define STF_INNO_BIAS_ENABLE			BIT(2)
+
+/* REG: 0x1b2 */
+#define STF_INNO_TMDS_CONTROL			0x1b2
+#define STF_INNO_TMDS_CLK_DRIVER_EN		BIT(3)
+#define STF_INNO_TMDS_D2_DRIVER_EN		BIT(2)
+#define STF_INNO_TMDS_D1_DRIVER_EN		BIT(1)
+#define STF_INNO_TMDS_D0_DRIVER_EN		BIT(0)
+#define STF_INNO_TMDS_DRIVER_ENABLE		(STF_INNO_TMDS_CLK_DRIVER_EN | \
+						 STF_INNO_TMDS_D2_DRIVER_EN | \
+						 STF_INNO_TMDS_D1_DRIVER_EN | \
+						 STF_INNO_TMDS_D0_DRIVER_EN)
+
+/* REG: 0x1b4 */
+#define STF_INNO_LDO_CONTROL			0x1b4
+#define STF_INNO_LDO_ENABLE			(BIT(2) | BIT(1) | BIT(0))
+
+/* REG: 0x1be */
+#define STF_INNO_SERIALIER_CONTROL		0x1be
+#define STF_INNO_SERIALIER_ENABLE		(BIT(6) | BIT(5) | BIT(4) | BIT(0))
+
+/* REG: 0x1cc */
+#define STF_INNO_RX_CONTROL			0x1cc
+#define STF_INNO_RX_ENABLE			(BIT(3) | BIT(2) | BIT(1) | BIT(0))
+
+/*
+ * These tables are copied from the monolithic driver.
+ * They match the Rockchip PHY driver tables.
+ */
+
+struct post_pll_config {
+	unsigned long tmdsclock;
+	u8 prediv;
+	u16 fbdiv;
+	u8 postdiv;
+	u8 post_div_en;
+};
+
+static const struct inno_hdmi_phy_pre_pll_config pre_pll_cfg_table[] = {
+	{ 25175000, 25175000, 1, 100, 2, 3, 3, 12, 3, 3, 4, 0, 0xf55555 },
+	{ 25200000, 25200000, 1, 100, 2, 3, 3, 12, 3, 3, 4, 0, 0 },
+	{ 27000000, 27000000, 1, 90, 3, 2, 2, 10, 3, 3, 4, 0, 0 },
+	{ 27027000, 27027000, 1, 90, 3, 2, 2, 10, 3, 3, 4, 0, 0x170a3d },
+	{ 28320000, 28320000, 1, 28, 2, 1, 1, 3, 0, 3, 4, 0, 0x51eb85 },
+	{ 30240000, 30240000, 1, 30, 2, 1, 1, 3, 0, 3, 4, 0, 0x3d70a3 },
+	{ 31500000, 31500000, 1, 31, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
+	{ 33750000, 33750000, 1, 33, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
+	{ 36000000, 36000000, 1, 36, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 40000000, 40000000, 1, 80, 2, 2, 2, 12, 2, 2, 2, 0, 0 },
+	{ 46970000, 46970000, 1, 46, 2, 1, 1, 3, 0, 3, 4, 0, 0xf851eb },
+	{ 49500000, 49500000, 1, 49, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
+	{ 49000000, 49000000, 1, 49, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 50000000, 50000000, 1, 50, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 54000000, 54000000, 1, 54, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 54054000, 54054000, 1, 54, 2, 1, 1, 3, 0, 3, 4, 0, 0x0dd2f1 },
+	{ 57284000, 57284000, 1, 57, 2, 1, 1, 3, 0, 3, 4, 0, 0x48b439 },
+	{ 58230000, 58230000, 1, 58, 2, 1, 1, 3, 0, 3, 4, 0, 0x3ae147 },
+	{ 59341000, 59341000, 1, 59, 2, 1, 1, 3, 0, 3, 4, 0, 0x574bc6 },
+	{ 59400000, 59400000, 1, 99, 3, 1, 1, 1, 3, 3, 4, 0, 0 },
+	{ 65000000, 65000000, 1, 130, 2, 2, 2, 12, 0, 2, 2, 0, 0 },
+	{ 68250000, 68250000, 1, 68, 2, 1, 1, 3, 0, 3, 4, 0, 0x3fffff },
+	{ 71000000, 71000000, 1, 71, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 74176000, 74176000, 1, 98, 1, 2, 2, 1, 2, 3, 4, 0, 0xe6ae6b },
+	{ 74250000, 74250000, 1, 99, 1, 2, 2, 1, 2, 3, 4, 0, 0 },
+	{ 75000000, 75000000, 1, 75, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 78750000, 78750000, 1, 78, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
+	{ 79500000, 79500000, 1, 79, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
+	{ 83500000, 83500000, 2, 167, 2, 1, 1, 1, 0, 0, 6, 0, 0 },
+	{ 83500000, 104375000, 1, 104, 2, 1, 1, 1, 1, 0, 5, 0, 0x600000 },
+	{ 85500000, 85500000, 1, 85, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
+	{ 85750000, 85750000, 1, 85, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
+	{ 85800000, 85800000, 1, 85, 2, 1, 1, 3, 0, 3, 4, 0, 0xcccccc },
+	{ 88750000, 88750000, 1, 88, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
+	{ 89910000, 89910000, 1, 89, 2, 1, 1, 3, 0, 3, 4, 0, 0xe8f5c1 },
+	{ 90000000, 90000000, 1, 90, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 101000000, 101000000, 1, 101, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 102250000, 102250000, 1, 102, 2, 1, 1, 3, 0, 3, 4, 0, 0x3fffff },
+	{ 106500000, 106500000, 1, 106, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
+	{ 108000000, 108000000, 1, 90, 3, 0, 0, 5, 0, 2, 2, 0, 0 },
+	{ 119000000, 119000000, 1, 119, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 131481000, 131481000, 1, 131, 2, 1, 1, 3, 0, 3, 4, 0, 0x7b22d1 },
+	{ 135000000, 135000000, 1, 135, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 136750000, 136750000, 1, 136, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
+	{ 147180000, 147180000, 1, 147, 2, 1, 1, 3, 0, 3, 4, 0, 0x2e147a },
+	{ 148352000, 148352000, 1, 98, 1, 1, 1, 1, 2, 2, 2, 0, 0xe6ae6b },
+	{ 148500000, 148500000, 1, 99, 1, 1, 1, 1, 2, 2, 2, 0, 0 },
+	{ 154000000, 154000000, 1, 154, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 156000000, 156000000, 1, 156, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 157000000, 157000000, 1, 157, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 162000000, 162000000, 1, 162, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 174250000, 174250000, 1, 145, 3, 0, 0, 5, 0, 2, 2, 0, 0x355555 },
+	{ 174500000, 174500000, 1, 174, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
+	{ 174570000, 174570000, 1, 174, 2, 1, 1, 3, 0, 3, 4, 0, 0x91eb84 },
+	{ 175500000, 175500000, 1, 175, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
+	{ 185590000, 185590000, 1, 185, 2, 1, 1, 3, 0, 3, 4, 0, 0x970a3c },
+	{ 187000000, 187000000, 1, 187, 2, 1, 1, 3, 0, 3, 4, 0, 0 },
+	{ 201000000, 201000000, 1, 134, 1, 1, 1, 1, 2, 2, 2, 0, 0 },
+	{ 235690000, 235690000, 1, 235, 2, 1, 1, 3, 0, 3, 4, 0, 0xb0a3d6 },
+	{ 241500000, 241500000, 1, 161, 1, 1, 1, 4, 0, 2, 2, 0, 0 },
+	{ 241700000, 241700000, 1, 241, 2, 1, 1, 3, 0, 3, 4, 0, 0xb33332 },
+	{ 262750000, 262750000, 1, 262, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
+	{ 296500000, 296500000, 1, 296, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
+	{ 296703000, 296703000, 1, 98, 0, 1, 1, 1, 0, 2, 2, 0, 0xe6ae6b },
+	{ 297000000, 297000000, 1, 99, 0, 1, 1, 1, 0, 2, 2, 0, 0 },
+	{ 594000000, 594000000, 1, 99, 0, 2, 0, 1, 0, 1, 1, 0, 0 },
+	{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
+};
+
+static const struct post_pll_config post_pll_cfg_table[] = {
+	{ 25200000, 1, 80, 13, 3 },
+	{ 27000000, 1, 40, 11, 3 },
+	{ 27027000, 1, 40, 11, 3 },
+	{ 33750000, 1, 40, 11, 3 },
+	{ 49000000, 1, 20, 1, 3 },
+	{ 65000000, 1, 20, 1, 3 },
+	{ 74250000, 1, 20, 1, 3 },
+	{ 88750000, 1, 20, 1, 3 },
+	{ 108000000, 1, 20, 1, 3 },
+	{ 148500000, 1, 20, 1, 3 },
+	{ 162000000, 1, 20, 1, 3 },
+	{ 174250000, 1, 20, 1, 3 },
+	{ 187000000, 1, 20, 1, 3 },
+	{ 241700000, 1, 20, 1, 3 },
+	{ 297000000, 4, 20, 0, 0 },
+	{ 594000000, 4, 20, 0, 0 }, /* postpll_postdiv_en = 0 */
+};
+
+struct starfive_hdmi_phy {
+	struct device *dev;
+	struct regmap *regmap;
+	struct phy *phy;
+	struct clk *refoclk;
+
+	struct inno_hdmi_phy_pre_pll pre_pll;
+
+	struct clk_hw hw;
+	struct clk *phyclk;
+	unsigned long pixclock;
+	unsigned long tmdsclock;
+};
+
+static inline void inno_write(struct starfive_hdmi_phy *inno, u32 reg, u8 val)
+{
+	regmap_write(inno->regmap, reg * 4, val);
+}
+
+static inline u8 inno_read(struct starfive_hdmi_phy *inno, u32 reg)
+{
+	u32 val;
+
+	regmap_read(inno->regmap, reg * 4, &val);
+	return val;
+}
+
+static inline void inno_update_bits(struct starfive_hdmi_phy *inno, u16 reg,
+				    u8 mask, u8 val)
+{
+	regmap_update_bits(inno->regmap, reg * 4, mask, val);
+}
+
+#define inno_poll(inno, reg, val, cond, sleep_us, timeout_us) \
+	regmap_read_poll_timeout((inno)->regmap, (reg) * 4, val, cond, \
+				 sleep_us, timeout_us)
+
+static inline struct starfive_hdmi_phy *to_starfive_hdmi_phy(struct clk_hw *hw)
+{
+	return container_of(hw, struct starfive_hdmi_phy, hw);
+}
+
+static int starfive_hdmi_phy_clk_prepare(struct clk_hw *hw)
+{
+	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
+	int ret;
+
+	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, false);
+
+	if (inno_hdmi_phy_pre_pll_is_locked(&inno->pre_pll))
+		return 0;
+
+	ret = inno_hdmi_phy_pre_pll_wait_locked(&inno->pre_pll, 100000);
+	if (ret) {
+		dev_err(inno->dev, "Timeout waiting for pre-PLL lock\n");
+		inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, true);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void starfive_hdmi_phy_clk_unprepare(struct clk_hw *hw)
+{
+	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
+
+	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, true);
+	inno->pixclock = 0; /* Invalidate cached rate */
+}
+
+static unsigned long starfive_hdmi_phy_clk_recalc_rate(struct clk_hw *hw,
+						       unsigned long parent_rate)
+{
+	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
+
+	/*
+	 * Reading the PLL back requires the HDMI controller's system clock,
+	 * which the sibling controller driver holds and this driver cannot
+	 * request without creating a probe cycle with voutcrg. The clock
+	 * framework walks every registered clock regardless of which drivers
+	 * have bound, so fall back to the cached rate whenever this clock is
+	 * not prepared; an unclocked access to the shared register window
+	 * wedges the bus hard enough that the CPU stops taking NMIs.
+	 */
+	if (!clk_hw_is_prepared(hw))
+		return inno->pixclock;
+
+	inno->pixclock = inno_hdmi_phy_pre_pll_recalc_rate(&inno->pre_pll,
+							   parent_rate);
+
+	return inno->pixclock;
+}
+
+static int starfive_hdmi_phy_clk_determine_rate(struct clk_hw *hw,
+						struct clk_rate_request *req)
+{
+	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
+
+	return inno_hdmi_phy_pre_pll_determine_rate(&inno->pre_pll, req);
+}
+
+static int starfive_hdmi_phy_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+					  unsigned long parent_rate)
+{
+	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
+	const struct inno_hdmi_phy_pre_pll_config *cfg;
+
+	/*
+	 * The JH7110 only drives 8bpc, so the TMDS clock always matches the
+	 * pixel clock.
+	 */
+	cfg = inno_hdmi_phy_pre_pll_lookup(&inno->pre_pll, rate, rate);
+	if (IS_ERR(cfg))
+		return PTR_ERR(cfg);
+
+	dev_dbg(inno->dev, "%s rate %lu tmdsclk %lu\n",
+		__func__, rate, cfg->tmdsclock);
+
+	if (inno->pixclock == rate && inno->tmdsclock == cfg->tmdsclock)
+		return 0;
+
+	inno_update_bits(inno, STF_INNO_BIAS_CONTROL,
+			 STF_INNO_BIAS_ENABLE, STF_INNO_BIAS_ENABLE);
+	inno_write(inno, STF_INNO_RX_CONTROL, STF_INNO_RX_ENABLE);
+
+	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, true);
+	inno_hdmi_phy_pre_pll_configure(&inno->pre_pll, cfg);
+	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, false);
+
+	inno->pixclock = rate;
+	inno->tmdsclock = cfg->tmdsclock;
+
+	return 0;
+}
+
+static const struct clk_ops starfive_hdmi_phy_clk_ops = {
+	.prepare = starfive_hdmi_phy_clk_prepare,
+	.unprepare = starfive_hdmi_phy_clk_unprepare,
+	.recalc_rate = starfive_hdmi_phy_clk_recalc_rate,
+	.determine_rate = starfive_hdmi_phy_clk_determine_rate,
+	.set_rate = starfive_hdmi_phy_clk_set_rate,
+};
+
+static void starfive_hdmi_phy_power_down(struct starfive_hdmi_phy *inno)
+{
+	inno_write(inno, STF_INNO_TMDS_CONTROL, 0x00);
+	inno_write(inno, STF_INNO_SERIALIER_CONTROL, 0x00);
+	inno_write(inno, STF_INNO_LDO_CONTROL, 0x00);
+	inno_update_bits(inno, STF_INNO_BIAS_CONTROL,
+			 STF_INNO_BIAS_ENABLE, 0x00);
+	inno_write(inno, STF_INNO_RX_CONTROL, 0x00);
+
+	/* Power down Post-PLL */
+	inno_update_bits(inno, STF_INNO_POST_PLL_DIV_1,
+			 STF_INNO_POST_PLL_POWER_DOWN,
+			 STF_INNO_POST_PLL_POWER_DOWN);
+
+	inno->tmdsclock = 0;
+	inno->pixclock = 0;
+}
+
+static int starfive_hdmi_phy_power_on(struct phy *phy)
+{
+	struct starfive_hdmi_phy *inno = phy_get_drvdata(phy);
+	const struct post_pll_config *cfg = NULL;
+	const struct inno_hdmi_phy_pre_pll_config *pre_cfg;
+	unsigned long tmdsclock;
+	u8 reg_1ad_value;
+	u8 reg_1aa_value;
+	unsigned int i;
+
+	u32 v;
+	int ret;
+
+	tmdsclock = clk_get_rate(inno->phyclk);
+	pre_cfg = inno_hdmi_phy_pre_pll_lookup(&inno->pre_pll, tmdsclock,
+					       tmdsclock);
+	if (IS_ERR(pre_cfg)) {
+		dev_err(inno->dev, "no pre-PLL config for current pixel clock\n");
+		return PTR_ERR(pre_cfg);
+	}
+	tmdsclock = pre_cfg->tmdsclock;
+	inno->tmdsclock = tmdsclock;
+
+	inno_update_bits(inno, STF_INNO_BIAS_CONTROL,
+			 STF_INNO_BIAS_ENABLE, STF_INNO_BIAS_ENABLE);
+	inno_write(inno, STF_INNO_RX_CONTROL, STF_INNO_RX_ENABLE);
+
+	/* Find Post-PLL config */
+	for (i = 0; i < ARRAY_SIZE(post_pll_cfg_table); i++) {
+		if (tmdsclock <= post_pll_cfg_table[i].tmdsclock) {
+			cfg = &post_pll_cfg_table[i];
+			break;
+		}
+	}
+
+	if (!cfg) {
+		dev_err(inno->dev, "Failed to find Post-PLL config\n");
+		ret = -EINVAL;
+		goto err_power_down;
+	}
+	dev_dbg(inno->dev, "Inno HDMI PHY Power On: pixclk %lu, tmdsclk %lu\n",
+		inno->pixclock, tmdsclock);
+
+	reg_1ad_value = cfg->post_div_en ? cfg->postdiv : 0x00;
+	reg_1aa_value = cfg->post_div_en ? 0x0e : 0x02;
+
+	/*
+	 * Pre-PLL is already prepared and running at inno->pixclock
+	 * via the clk_set_rate and prepare calls from the controller/bridge.
+	 * Now, configure and enable the Post-PLL and TMDS outputs.
+	 */
+
+	inno_write(inno, STF_INNO_POST_PLL_DIV_2,
+		   STF_INNO_POST_PLL_PRE_DIV(cfg->prediv));
+	inno_write(inno, STF_INNO_POST_PLL_DIV_3, cfg->fbdiv & 0xff);
+	inno_write(inno, STF_INNO_POST_PLL_DIV_4, reg_1ad_value);
+
+	/* Power up Post-PLL */
+	inno_write(inno, STF_INNO_POST_PLL_DIV_1, reg_1aa_value);
+
+	/* Wait for post PLL lock */
+	ret = inno_poll(inno, STF_INNO_POST_PLL_LOCK_STATUS, v,
+			v & STF_INNO_POST_PLL_LOCK, 1000, 100000);
+	if (ret) {
+		dev_err(inno->dev, "Post-PLL locking failed\n");
+		goto err_power_down;
+	}
+
+	inno_write(inno, STF_INNO_LDO_CONTROL, STF_INNO_LDO_ENABLE);
+	inno_write(inno, STF_INNO_SERIALIER_CONTROL,
+		   STF_INNO_SERIALIER_ENABLE);
+	inno_write(inno, STF_INNO_TMDS_CONTROL, 0x8f);
+
+	return 0;
+
+	/*
+	 * The PHY core leaves power_count at zero when .power_on fails and
+	 * never calls .power_off, so undo the blocks enabled above here.
+	 */
+err_power_down:
+	starfive_hdmi_phy_power_down(inno);
+	return ret;
+}
+
+static int starfive_hdmi_phy_power_off(struct phy *phy)
+{
+	struct starfive_hdmi_phy *inno = phy_get_drvdata(phy);
+
+	dev_dbg(inno->dev, "Inno HDMI PHY Power Off\n");
+
+	starfive_hdmi_phy_power_down(inno);
+
+	return 0;
+}
+
+static const struct phy_ops starfive_hdmi_phy_ops = {
+	.owner = THIS_MODULE,
+	.power_on = starfive_hdmi_phy_power_on,
+	.power_off = starfive_hdmi_phy_power_off,
+};
+
+static int starfive_hdmi_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device *parent = dev->parent;
+	struct starfive_hdmi_phy *inno;
+	struct phy_provider *phy_provider;
+	struct regmap *regmap;
+	struct clk_init_data init = {};
+	const char *refoclk_name;
+	int ret;
+
+	inno = devm_kzalloc(dev, sizeof(*inno), GFP_KERNEL);
+	if (!inno)
+		return -ENOMEM;
+
+	inno->dev = dev;
+
+	/* Get the regmap from the parent device */
+	regmap = dev_get_regmap(parent, NULL);
+	if (!regmap) {
+		dev_err(dev, "Failed to get parent regmap\n");
+		return -ENODEV;
+	}
+	inno->regmap = regmap;
+
+	/*
+	 * The PHY block sits behind the HDMI controller in the shared register
+	 * space, so the common pre-PLL helpers need a 0x100 register offset.
+	 */
+	inno->pre_pll.dev = dev;
+	inno->pre_pll.regmap = regmap;
+	inno->pre_pll.offset = 0x100;
+	inno->pre_pll.table = pre_pll_cfg_table;
+	inno->pre_pll.table_size = ARRAY_SIZE(pre_pll_cfg_table);
+
+	/* Get the input reference clock */
+	inno->refoclk = devm_clk_get(inno->dev, NULL);
+	if (IS_ERR(inno->refoclk)) {
+		ret = PTR_ERR(inno->refoclk);
+		dev_err(inno->dev, "failed to get oscillator-ref clock: %d\n",
+			ret);
+		return ret;
+	}
+
+	/* We must prepare/enable refoclk here so .set_rate/.recalc_rate work */
+	ret = clk_prepare_enable(inno->refoclk);
+	if (ret) {
+		dev_err(dev, "Failed to enable refoclk: %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, inno);
+
+	/* Initialize and register the clock provider */
+	refoclk_name = __clk_get_name(inno->refoclk);
+	init.parent_names = &refoclk_name;
+	init.num_parents = 1;
+	init.flags = 0;
+	init.name = "hdmi_pclk";
+	init.ops = &starfive_hdmi_phy_clk_ops;
+
+	inno->hw.init = &init;
+	inno->phyclk = devm_clk_register(dev, &inno->hw);
+	if (IS_ERR(inno->phyclk)) {
+		ret = PTR_ERR(inno->phyclk);
+		dev_err(dev, "Failed to register clock provider: %d\n", ret);
+		goto err_disable_refoclk;
+	}
+
+	ret = of_clk_add_provider(dev->of_node, of_clk_src_simple_get, inno->phyclk);
+	if (ret) {
+		dev_err(dev, "Failed to add clock provider: %d\n", ret);
+		goto err_disable_refoclk;
+	}
+
+	/*
+	 * Do not program a default rate here. The PHY registers live in the
+	 * window gated by the controller's system clock, which this driver
+	 * cannot hold without creating a probe cycle with voutcrg, and
+	 * .set_rate writes to them. Leave the hardware alone until the
+	 * controller drives it from its .enable hook.
+	 */
+
+	/* Create and register the PHY provider */
+	inno->phy = devm_phy_create(inno->dev, NULL, &starfive_hdmi_phy_ops);
+	if (IS_ERR(inno->phy)) {
+		ret = PTR_ERR(inno->phy);
+		dev_err(inno->dev, "failed to create HDMI PHY: %d\n", ret);
+		goto err_del_clk_provider;
+	}
+
+	phy_set_drvdata(inno->phy, inno);
+
+	phy_provider = devm_of_phy_provider_register(inno->dev,
+						     of_phy_simple_xlate);
+	ret = PTR_ERR_OR_ZERO(phy_provider);
+	if (ret)
+		goto err_del_clk_provider;
+
+	return 0;
+
+err_del_clk_provider:
+	of_clk_del_provider(dev->of_node);
+err_disable_refoclk:
+	clk_disable_unprepare(inno->refoclk);
+	return ret;
+}
+
+static void starfive_hdmi_phy_remove(struct platform_device *pdev)
+{
+	struct starfive_hdmi_phy *inno = platform_get_drvdata(pdev);
+
+	of_clk_del_provider(pdev->dev.of_node);
+	clk_disable_unprepare(inno->refoclk);
+}
+
+static const struct of_device_id starfive_hdmi_phy_of_match[] = {
+	{ .compatible = "starfive,jh7110-inno-hdmi-phy", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, starfive_hdmi_phy_of_match);
+
+static struct platform_driver starfive_hdmi_phy_driver = {
+	.probe = starfive_hdmi_phy_probe,
+	.remove = starfive_hdmi_phy_remove,
+	.driver = {
+		.name = "starfive-inno-hdmi-phy",
+		.of_match_table = starfive_hdmi_phy_of_match,
+	},
+};
+module_platform_driver(starfive_hdmi_phy_driver);
+
+MODULE_AUTHOR("Michal Wilczynski <m.wilczynski@samsung.com>");
+MODULE_DESCRIPTION("StarFive JH7110 Innosilicon HDMI PHY Driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 4/5] phy: starfive: Add jh7110-inno-hdmi-phy driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] phy: starfive: hdmi: fix rate cache and BIAS/RX state across disable/enable
- [High] phy: starfive: jh7110-inno-hdmi: invalid pre_pll_cfg_table divider values
- [Medium] phy: starfive: jh7110-inno-hdmi: broken Pre-PLL power and lock sequencing in clk_ops
- [Low] phy: starfive: unused inno_read() helper and register macros in hdmi phy
--

commit de90e248a2b1a2d110bed58fd3d5de2e75a01d3d
Author: Michal Wilczynski <m.wilczynski@samsung.com>

phy: starfive: Add jh7110-inno-hdmi-phy driver

This commit adds the Innosilicon HDMI PHY driver for the StarFive JH7110 SoC.
The driver acts as both a clock provider for the variable pixel clock via the
Pre-PLL and a PHY provider configuring the Post-PLL and analog blocks.

> diff --git a/drivers/phy/starfive/phy-jh7110-inno-hdmi.c b/drivers/phy/starfive/phy-jh7110-inno-hdmi.c
> new file mode 100644
> index 0000000000000..ebda363e50a3e
> --- /dev/null
> +++ b/drivers/phy/starfive/phy-jh7110-inno-hdmi.c
[ ... ]
> +/* REG: 0x1aa */
> +#define STF_INNO_POST_PLL_DIV_1		0x1aa
> +#define STF_INNO_POST_PLL_POST_DIV_ENABLE	GENMASK(3, 2)
> +#define STF_INNO_POST_PLL_REFCLK_SEL_TMDS	BIT(1)
> +#define STF_INNO_POST_PLL_POWER_DOWN		BIT(0)
> +
> +/* REG: 0x1ab */
> +#define STF_INNO_POST_PLL_DIV_2		0x1ab
> +#define STF_INNO_POST_PLL_PRE_DIV(x)		FIELD_PREP(GENMASK(5, 0), x)
> +#define STF_INNO_POST_PLL_FB_DIV_8(x)		FIELD_PREP(BIT(7), (x) >> 8)
> +
> +/* REG: 0x1ac */
> +#define STF_INNO_POST_PLL_DIV_3		0x1ac
> +#define STF_INNO_POST_PLL_FB_DIV_7_0(x)	FIELD_PREP(GENMASK(7, 0), x)
> +
> +/* REG: 0x1ad */
> +#define STF_INNO_POST_PLL_DIV_4		0x1ad
> +#define STF_INNO_POST_PLL_POST_DIV_MASK	GENMASK(1, 0)
> +
> +/* REG: 0x1af */
> +#define STF_INNO_POST_PLL_LOCK_STATUS		0x1af
> +#define STF_INNO_POST_PLL_LOCK		BIT(0)
> +
> +/* REG: 0x1b0 */
> +#define STF_INNO_BIAS_CONTROL			0x1b0
> +#define STF_INNO_BIAS_ENABLE			BIT(2)
> +
> +/* REG: 0x1b2 */
> +#define STF_INNO_TMDS_CONTROL			0x1b2
> +#define STF_INNO_TMDS_CLK_DRIVER_EN		BIT(3)
> +#define STF_INNO_TMDS_D2_DRIVER_EN		BIT(2)
> +#define STF_INNO_TMDS_D1_DRIVER_EN		BIT(1)
> +#define STF_INNO_TMDS_D0_DRIVER_EN		BIT(0)
> +#define STF_INNO_TMDS_DRIVER_ENABLE		(STF_INNO_TMDS_CLK_DRIVER_EN | \
> +						 STF_INNO_TMDS_D2_DRIVER_EN | \
> +						 STF_INNO_TMDS_D1_DRIVER_EN | \
> +						 STF_INNO_TMDS_D0_DRIVER_EN)
[ ... ]
> +static const struct inno_hdmi_phy_pre_pll_config pre_pll_cfg_table[] = {
> +	{ 25175000, 25175000, 1, 100, 2, 3, 3, 12, 3, 3, 4, 0, 0xf55555 },
> +	{ 25200000, 25200000, 1, 100, 2, 3, 3, 12, 3, 3, 4, 0, 0 },
> +	{ 27000000, 27000000, 1, 90, 3, 2, 2, 10, 3, 3, 4, 0, 0 },
> +	{ 27027000, 27027000, 1, 90, 3, 2, 2, 10, 3, 3, 4, 0, 0x170a3d },
> +	{ 28320000, 28320000, 1, 28, 2, 1, 1, 3, 0, 3, 4, 0, 0x51eb85 },
> +	{ 30240000, 30240000, 1, 30, 2, 1, 1, 3, 0, 3, 4, 0, 0x3d70a3 },
> +	{ 31500000, 31500000, 1, 31, 2, 1, 1, 3, 0, 3, 4, 0, 0x7fffff },
> +	{ 33750000, 33750000, 1, 33, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
[ ... ]
> +	{ 78750000, 78750000, 1, 78, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
[ ... ]
> +	{ 85750000, 85750000, 1, 85, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
> +	{ 85800000, 85800000, 1, 85, 2, 1, 1, 3, 0, 3, 4, 0, 0xcccccc },
> +	{ 88750000, 88750000, 1, 88, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
[ ... ]
> +	{ 136750000, 136750000, 1, 136, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },
[ ... ]
> +	{ 262750000, 262750000, 1, 262, 2, 1, 1, 3, 0, 3, 4, 0, 0xcfffff },

[Severity: High]
Do these eight entries in pre_pll_cfg_table generate the wrong pixel clock
and cause starfive_hdmi_phy_power_on() to fail with -EINVAL?

With the 24 MHz reference clock:
- 25175000 has div = 1 * 12 * 4 * 2 = 96, so 24 MHz * (100 + 0xf55555 / 2^24)
  / 96 yields 25,239,583 Hz, which inno_hdmi_phy_pre_pll_recalc_rate()
  recalculates as 25240000 Hz.
- 25200000 has div = 96 and fracdiv = 0, so 24 MHz * 100 / 96 yields
  25000000 Hz.
- 33750000, 78750000, 85750000, 88750000, 136750000, and 262750000 have
  div = 1 * 3 * 4 * 2 = 24 and fracdiv = 0xcfffff (~0.8125 instead of
  0xc00000 = 0.75), which adds 812.5 kHz instead of 750 kHz and recalculates
  to 33812000, 78812000, 85812000, 88812000, 136812000, and 262812000 Hz.

When hdmi_pclk is prepared and starfive_hdmi_phy_clk_recalc_rate() runs:

    clk_change_rate()
      clk_recalc()
        starfive_hdmi_phy_clk_recalc_rate()
          inno_hdmi_phy_pre_pll_recalc_rate()

the clock framework caches the recalculated rate in core->rate. When
starfive_hdmi_phy_power_on() then calls:

    tmdsclock = clk_get_rate(inno->phyclk);
    pre_cfg = inno_hdmi_phy_pre_pll_lookup(&inno->pre_pll, tmdsclock,
                                           tmdsclock);

inno_hdmi_phy_pre_pll_lookup() requires an exact match against
cfg->pixclock in pre_pll_cfg_table, which fails with -EINVAL for all eight
modes.

[ ... ]
> +static inline void inno_write(struct starfive_hdmi_phy *inno, u32 reg, u8 val)
> +{
> +	regmap_write(inno->regmap, reg * 4, val);
> +}
> +
> +static inline u8 inno_read(struct starfive_hdmi_phy *inno, u32 reg)
> +{
> +	u32 val;
> +
> +	regmap_read(inno->regmap, reg * 4, &val);
> +	return val;
> +}

[Severity: Low]
Is inno_read() needed here? It is never called in this file, and the
register bit macros STF_INNO_POST_PLL_POST_DIV_ENABLE,
STF_INNO_POST_PLL_REFCLK_SEL_TMDS, STF_INNO_POST_PLL_FB_DIV_8,
STF_INNO_POST_PLL_FB_DIV_7_0, STF_INNO_POST_PLL_POST_DIV_MASK, and
STF_INNO_TMDS_DRIVER_ENABLE defined at the top of the file are also unused
(while starfive_hdmi_phy_power_on() open-codes 0x0e, 0x02, cfg->fbdiv & 0xff,
and 0x8f), which triggers compiler warnings under W=2 builds.

[ ... ]
> +static int starfive_hdmi_phy_clk_prepare(struct clk_hw *hw)
> +{
> +	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
> +	int ret;
> +
> +	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, false);
> +
> +	if (inno_hdmi_phy_pre_pll_is_locked(&inno->pre_pll))
> +		return 0;
> +
> +	ret = inno_hdmi_phy_pre_pll_wait_locked(&inno->pre_pll, 100000);
> +	if (ret) {
> +		dev_err(inno->dev, "Timeout waiting for pre-PLL lock\n");
> +		inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, true);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void starfive_hdmi_phy_clk_unprepare(struct clk_hw *hw)
> +{
> +	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
> +
> +	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, true);
> +	inno->pixclock = 0; /* Invalidate cached rate */
> +}
> +
> +static unsigned long starfive_hdmi_phy_clk_recalc_rate(struct clk_hw *hw,
> +						       unsigned long parent_rate)
> +{
> +	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
> +
> +	/*
> +	 * Reading the PLL back requires the HDMI controller's system clock,
> +	 * which the sibling controller driver holds and this driver cannot
> +	 * request without creating a probe cycle with voutcrg. The clock
> +	 * framework walks every registered clock regardless of which drivers
> +	 * have bound, so fall back to the cached rate whenever this clock is
> +	 * not prepared; an unclocked access to the shared register window
> +	 * wedges the bus hard enough that the CPU stops taking NMIs.
> +	 */
> +	if (!clk_hw_is_prepared(hw))
> +		return inno->pixclock;
> +
> +	inno->pixclock = inno_hdmi_phy_pre_pll_recalc_rate(&inno->pre_pll,
> +							   parent_rate);
> +
> +	return inno->pixclock;
> +}
[ ... ]
> +static int starfive_hdmi_phy_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> +					  unsigned long parent_rate)
> +{
> +	struct starfive_hdmi_phy *inno = to_starfive_hdmi_phy(hw);
> +	const struct inno_hdmi_phy_pre_pll_config *cfg;
> +
> +	/*
> +	 * The JH7110 only drives 8bpc, so the TMDS clock always matches the
> +	 * pixel clock.
> +	 */
> +	cfg = inno_hdmi_phy_pre_pll_lookup(&inno->pre_pll, rate, rate);
> +	if (IS_ERR(cfg))
> +		return PTR_ERR(cfg);
> +
> +	dev_dbg(inno->dev, "%s rate %lu tmdsclk %lu\n",
> +		__func__, rate, cfg->tmdsclock);
> +
> +	if (inno->pixclock == rate && inno->tmdsclock == cfg->tmdsclock)
> +		return 0;
> +
> +	inno_update_bits(inno, STF_INNO_BIAS_CONTROL,
> +			 STF_INNO_BIAS_ENABLE, STF_INNO_BIAS_ENABLE);
> +	inno_write(inno, STF_INNO_RX_CONTROL, STF_INNO_RX_ENABLE);
> +
> +	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, true);
> +	inno_hdmi_phy_pre_pll_configure(&inno->pre_pll, cfg);
> +	inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, false);
> +
> +	inno->pixclock = rate;
> +	inno->tmdsclock = cfg->tmdsclock;
> +
> +	return 0;
> +}

[Severity: Medium]
Should starfive_hdmi_phy_clk_set_rate() wait for the Pre-PLL to lock or keep
it powered down when the clock is unprepared?

In starfive_hdmi_phy_clk_set_rate(), the driver powers down the Pre-PLL,
configures it, and powers it back up via
inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, false), returning 0 without
calling inno_hdmi_phy_pre_pll_wait_locked().

If clk_set_rate() is called while hdmi_pclk is already prepared,
starfive_hdmi_phy_clk_prepare() is not called again because prepare_count is
already non-zero, so nothing waits for the Pre-PLL to lock before
starfive_hdmi_phy_power_on() configures and enables the Post-PLL (which uses
the Pre-PLL TMDS clock as its reference).

Conversely, if clk_set_rate() is called while hdmi_pclk is unprepared,
starfive_hdmi_phy_clk_set_rate() leaves the Pre-PLL, STF_INNO_BIAS_ENABLE,
and STF_INNO_RX_ENABLE powered on in hardware while prepare_count is 0.

> +
> +static const struct clk_ops starfive_hdmi_phy_clk_ops = {
> +	.prepare = starfive_hdmi_phy_clk_prepare,
> +	.unprepare = starfive_hdmi_phy_clk_unprepare,
> +	.recalc_rate = starfive_hdmi_phy_clk_recalc_rate,
> +	.determine_rate = starfive_hdmi_phy_clk_determine_rate,
> +	.set_rate = starfive_hdmi_phy_clk_set_rate,
> +};
> +
> +static void starfive_hdmi_phy_power_down(struct starfive_hdmi_phy *inno)
> +{
> +	inno_write(inno, STF_INNO_TMDS_CONTROL, 0x00);
> +	inno_write(inno, STF_INNO_SERIALIER_CONTROL, 0x00);
> +	inno_write(inno, STF_INNO_LDO_CONTROL, 0x00);
> +	inno_update_bits(inno, STF_INNO_BIAS_CONTROL,
> +			 STF_INNO_BIAS_ENABLE, 0x00);
> +	inno_write(inno, STF_INNO_RX_CONTROL, 0x00);
> +
> +	/* Power down Post-PLL */
> +	inno_update_bits(inno, STF_INNO_POST_PLL_DIV_1,
> +			 STF_INNO_POST_PLL_POWER_DOWN,
> +			 STF_INNO_POST_PLL_POWER_DOWN);
> +
> +	inno->tmdsclock = 0;
> +	inno->pixclock = 0;
> +}

[Severity: High]
What happens when HDMI output is disabled and then re-enabled at the same
pixel clock rate?

When the display is disabled:
1. starfive_hdmi_phy_power_off() -> starfive_hdmi_phy_power_down() clears
   STF_INNO_BIAS_ENABLE and STF_INNO_RX_CONTROL to 0, and sets
   inno->tmdsclock = 0 and inno->pixclock = 0.
2. clk_disable_unprepare() -> starfive_hdmi_phy_clk_unprepare() powers down
   the Pre-PLL and sets inno->pixclock = 0.

However, clk_core_unprepare() in the clock framework does not clear
core->rate, so the clock framework still caches the previous rate R.

When the display is re-enabled at the same rate R:
1. clk_set_rate(phyclk, R) sees rate == clk_core_get_rate_nolock(core) in
   clk_core_set_rate_nolock() and returns 0 early without calling
   starfive_hdmi_phy_clk_set_rate().
2. clk_prepare_enable(phyclk) calls starfive_hdmi_phy_clk_prepare() before
   phy_power_on(), powering up the Pre-PLL and polling
   inno_hdmi_phy_pre_pll_wait_locked() while STF_INNO_BIAS_ENABLE and
   STF_INNO_RX_ENABLE are still disabled at 0 (and without reprogramming the
   Pre-PLL if the shared register block was reset).

Additionally, if a clock tree walk invokes starfive_hdmi_phy_clk_recalc_rate()
while phyclk is unprepared after inno->pixclock was zeroed, it returns 0 and
overwrites core->rate with 0. If the display is then re-enabled without a new
clk_set_rate() call, clk_get_rate(inno->phyclk) in
starfive_hdmi_phy_power_on() returns 0 and fails
inno_hdmi_phy_pre_pll_lookup() with -EINVAL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-jh7110-inno-hdmi-phy-v6-0-f3379684d97e@samsung.com?part=4

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 5/5] MAINTAINERS: Add StarFive JH7110 HDMI PHY entry

[Michal Wilczynski]

Add an entry for the StarFive JH7110 HDMI PHY binding and driver, next
to the other JH7110 PHY drivers.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 782389bd5cd3d9e0117ebdfae707e1f61cb3e772..9e42d1705fcb3fa46325761a19c6b7244c18b7c4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26106,6 +26106,12 @@ S:	Supported
 F:	Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-tx.yaml
 F:	drivers/phy/starfive/phy-jh7110-dphy-tx.c
 
+STARFIVE JH7110 HDMI PHY DRIVER
+M:	Michal Wilczynski <m.wilczynski@samsung.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/phy/starfive,jh7110-inno-hdmi-phy.yaml
+F:	drivers/phy/starfive/phy-jh7110-inno-hdmi.c
+
 STARFIVE JH7110 MMC/SD/SDIO DRIVER
 M:	William Qiu <william.qiu@starfivetech.com>
 S:	Supported

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy