[PATCH v3 0/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[PATCH v3 0/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[Mike Looijmans]

In the past I've seen (and contributed to) hacks that model the chips as
phy or even (really!) clock drivers. Since the chip usually sits between
a signal that is (almost) HDMI and a HDMI connector, I decided to stop
lying and write it as a DRM bridge driver.

Our experience with these chips is that they work best under manual control
enabling them only once the signal is active. At low resolutions (under 4k),
the optimal setting is usually to only use redriver mode. Setting the
termination to 150-300 Ohms improves EMC performance at lower resolutions,
hence the driver enables 75-150 Ohms for HDMI2 modes and defaults to
150-300 Ohm termination for other modes.


Changes in v3:
Fix duplicate links
Add vcc-supply and vdd-supply
Fix missing type for ti,slew-rate
Lower-case hex values and use defines for EYESCAN registers
Remove equalizer code (unlikely to be used)
Remove attributes (no longer useful, undocumented)
Fix build for 6.17 kernel
Use devm_drm_bridge_alloc
Sort includes and add linux/bitfield.h
Check chip type and complain on mismatch

Changes in v2:
Document driver specific bindings like slew-rate and threshold
Use atomic_enable/disable
Use #defines for bit fields in registers
Allow HDMI 2 compliance
Filter modes on clock range
Use cross-over pixel frequency instead of manual overides
Devicetree bindings according to standards

Mike Looijmans (2):
  dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159
    bindings
  drm: bridge: Add TI tmds181 and sn65dp159 driver

 .../bindings/display/bridge/ti,tmds181.yaml   | 150 +++++++
 drivers/gpu/drm/bridge/Kconfig                |  11 +
 drivers/gpu/drm/bridge/Makefile               |   1 +
 drivers/gpu/drm/bridge/ti-tmds181.c           | 409 ++++++++++++++++++
 4 files changed, 571 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
 create mode 100644 drivers/gpu/drm/bridge/ti-tmds181.c

-- 
2.43.0

base-commit: 53e760d8949895390e256e723e7ee46618310361
branch: drm-ti-tmds181

Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

[PATCH v3 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Mike Looijmans]

Add DT binding document for TI TMDS181 and SN65DP159 HDMI retimers.

The two chips have similar register maps, but different applications
(source vs. sink).

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>

---

Changes in v3:
Fix duplicate links
Add vcc-supply and vdd-supply
Fix missing type for ti,slew-rate

Changes in v2:
Document driver specific bindings like slew-rate and threshold

 .../bindings/display/bridge/ti,tmds181.yaml   | 150 ++++++++++++++++++
 1 file changed, 150 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
new file mode 100644
index 000000000000..7433e694fae0
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
@@ -0,0 +1,150 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/ti,tmds181.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TMDS181 and SN65DP159 HDMI retimer/redriver chips
+
+maintainers:
+  - Mike Looijmans <mike.looijmans@topic.nl>
+
+description: |
+  Texas Instruments TMDS181 and SN65DP159 retimer and redriver chips.
+  https://www.ti.com/product/TMDS181
+  https://www.ti.com/product/SN65DP159
+
+properties:
+  compatible:
+    enum:
+      - ti,tmds181
+      - ti,sn65dp159
+
+  reg:
+    enum:
+      - 0x5b
+      - 0x5c
+      - 0x5d
+      - 0x5e
+
+  oe-gpios:
+    maxItems: 1
+    description: GPIO specifier for OE pin (active high).
+
+  vdd-supply:
+    description: Core power supply, 1.1V
+
+  vcc-supply:
+    description: IO power supply, 3.3V
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: Video port for HDMI (ish) input
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: Video port for HDMI output (panel or bridge)
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+    required:
+      - port@0
+      - port@1
+
+  ti,source-mode:
+    type: boolean
+    description:
+      Force chip to operate in "source" mode. Allows to use
+      a TMDS181 chip (which defaults to sink) as cable driver.
+
+  ti,sink-mode:
+    type: boolean
+    description:
+      Force chip to operate in "sink" mode. Allows to use
+      a DP159 chip (defaults to source) for incoming signals.
+
+  ti,retimer-threshold-hz:
+    minimum: 25000000
+    maximum: 600000000
+    default: 200000000
+    description:
+      Cross-over point. Up until this pixel clock frequency
+      the chip remains in the low-power redriver mode. Above
+      the threshold the chip should operate in retimer mode.
+
+  ti,dvi-mode:
+    type: boolean
+    description: Makes the DP159 chip operate in DVI mode.
+
+  ti,slew-rate:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 3
+    default: 3
+    description: Set slew rate, 0 is slowest, 3 is fastest.
+
+  ti,disable-equalizer:
+    type: boolean
+    description: Disable the equalizer (to save power).
+
+  ti,adaptive-equalizer:
+    type: boolean
+    description: Set the equalizer to adaptive mode.
+
+required:
+  - compatible
+  - reg
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bridge@5b {
+            compatible = "ti,sn65dp159";
+            reg = <0x5b>;
+
+            oe-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+
+                    endpoint {
+                        remote-endpoint = <&encoder_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+
+                    endpoint {
+                        remote-endpoint = <&hdmi_connector_in>;
+                    };
+                };
+            };
+        };
+    };
-- 
2.43.0


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

[PATCH v3 2/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[Mike Looijmans]

The tmds181 and sn65dp159 are "retimers" and hence can be considered
HDMI-to-HDMI bridges. Typical usage is to convert the output of an
FPGA into a valid HDMI signal, and it will typically be inserted
between an encoder and hdmi-connector.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---

Changes in v3:
Lower-case hex values and use defines for EYESCAN registers
Remove equalizer code (unlikely to be used)
Remove attributes (no longer useful, undocumented)
Fix build for 6.17 kernel
Use devm_drm_bridge_alloc
Sort includes and add linux/bitfield.h
Check chip type and complain on mismatch

Changes in v2:
Use atomic_enable/disable
Use #defines for bit fields in registers
Allow HDMI 2 compliance
Filter modes on clock range
Use cross-over pixel frequency instead of manual overides
Devicetree bindings according to standards

 drivers/gpu/drm/bridge/Kconfig      |  11 +
 drivers/gpu/drm/bridge/Makefile     |   1 +
 drivers/gpu/drm/bridge/ti-tmds181.c | 409 ++++++++++++++++++++++++++++
 3 files changed, 421 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-tmds181.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index b9e0ca85226a..753177fc9b50 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -430,6 +430,17 @@ config DRM_TI_SN65DSI86
 	help
 	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
 
+config DRM_TI_TMDS181
+        tristate "TI TMDS181 and SN65DP159 HDMI retimer bridge driver"
+	depends on OF
+	select DRM_KMS_HELPER
+	select REGMAP_I2C
+	help
+	  Enable this to support the TI TMDS181 and SN65DP159 HDMI retimers.
+	  The SN65DP159 provides output into a cable (source) whereas the
+	  TMDS181 is meant to forward a cable signal into a PCB (sink). Either
+	  can be set up as source or sink though.
+
 config DRM_TI_TPD12S015
 	tristate "TI TPD12S015 HDMI level shifter and ESD protection"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 245e8a27e3fc..f4b5089e903c 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_DRM_TI_SN65DSI83) += ti-sn65dsi83.o
 obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
 obj-$(CONFIG_DRM_TI_TDP158) += ti-tdp158.o
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
+obj-$(CONFIG_DRM_TI_TMDS181) += ti-tmds181.o
 obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o
 obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi.o
 obj-$(CONFIG_DRM_ITE_IT66121) += ite-it66121.o
diff --git a/drivers/gpu/drm/bridge/ti-tmds181.c b/drivers/gpu/drm/bridge/ti-tmds181.c
new file mode 100644
index 000000000000..8ac3eb808d5b
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-tmds181.c
@@ -0,0 +1,409 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI tmds181 and sn65dp159 HDMI redriver and retimer chips
+ *
+ * Copyright (C) 2018 - 2025 Topic Embedded Products <www.topic.nl>
+ *
+ * based on code
+ * Copyright (C) 2007 Hans Verkuil
+ * Copyright (C) 2016, 2017 Leon Woestenberg <leon@sidebranch.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
+
+MODULE_DESCRIPTION("I2C device driver for DP159 and TMDS181 redriver/retimer");
+MODULE_AUTHOR("Mike Looijmans");
+MODULE_LICENSE("GPL");
+
+#define TMDS181_REG_ID		0
+#define TMDS181_REG_REV		0x8
+#define TMDS181_REG_CTRL9	0x9
+/* Registers A and B have a volatile bit, but we don't use it, so cache is ok */
+#define TMDS181_REG_CTRLA	0xa
+#define TMDS181_REG_CTRLB	0xb
+#define TMDS181_REG_CTRLC	0xc
+#define TMDS181_REG_EQUALIZER	0xd
+/* EYESCAN registers don't appear to deserve separate names */
+#define TMDS181_REG_EYESCAN_E	0xe
+#define TMDS181_REG_EYESCAN_F	0xf
+#define TMDS181_REG_EYESCAN_15	0x15
+#define TMDS181_REG_EYESCAN_17	0x17
+#define TMDS181_REG_EYESCAN_1F	0x1f
+#define TMDS181_REG_AUX		0x20
+
+
+#define TMDS181_CTRL9_SIG_EN			BIT(4)
+#define TMDS181_CTRL9_PD_EN			BIT(3)
+#define TMDS181_CTRL9_HPD_AUTO_PWRDWN_DISABLE	BIT(2)
+#define TMDS181_CTRL9_I2C_DR_CTL		GENMASK(1, 0)
+
+#define TMDS181_CTRLA_MODE_SINK			BIT(7)
+#define TMDS181_CTRLA_HPDSNK_GATE_EN		BIT(6)
+#define TMDS181_CTRLA_EQ_ADA_EN			BIT(5)
+#define TMDS181_CTRLA_EQ_EN			BIT(4)
+#define TMDS181_CTRLA_AUX_BRG_EN		BIT(3)
+#define TMDS181_CTRLA_APPLY			BIT(2)
+#define TMDS181_CTRLA_DEV_FUNC_MODE		GENMASK(1, 0)
+
+#define TMDS181_CTRLB_SLEW_CTL			GENMASK(7, 6)
+#define TMDS181_CTRLB_HDMI_SEL_DVI		BIT(5)
+#define TMDS181_CTRLB_TX_TERM_CTL		GENMASK(4, 3)
+#define TMDS181_CTRLB_DDC_DR_SEL		BIT(2)
+#define TMDS181_CTRLB_TMDS_CLOCK_RATIO_STATUS	BIT(1)
+#define TMDS181_CTRLB_DDC_TRAIN_SET		BIT(0)
+
+#define TMDS181_CTRLB_TX_TERM_150_300_OHMS	1
+#define TMDS181_CTRLB_TX_TERM_75_150_OHMS	3
+
+#define TMDS181_CTRLC_VSWING_DATA		GENMASK(7, 5)
+#define TMDS181_CTRLC_VSWING_CLK		GENMASK(4, 2)
+#define TMDS181_CTRLC_HDMI_TWPST1		GENMASK(1, 0)
+
+#define TMDS181_EQ_DATA_LANE			GENMASK(5, 3)
+#define TMDS181_EQ_CLOCK_LANE			GENMASK(2, 1)
+#define TMDS181_EQ_DIS_HDMI2_SWG		BIT(0)
+
+/* Above this data rate HDMI2 standards apply (TX termination) */
+#define HDMI2_PIXEL_RATE_KHZ	340000
+
+enum tmds181_chip {
+	tmds181,
+	dp159,
+};
+
+struct tmds181_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct drm_bridge *next_bridge;
+	struct drm_bridge bridge;
+	u32 retimer_threshold_khz;
+};
+
+static inline struct tmds181_data *
+drm_bridge_to_tmds181_data(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct tmds181_data, bridge);
+}
+
+static int tmds181_attach(struct drm_bridge *bridge, struct drm_encoder *encoder,
+			  enum drm_bridge_attach_flags flags)
+{
+	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
+
+	return drm_bridge_attach(encoder, data->next_bridge, bridge, flags);
+}
+
+static enum drm_mode_status
+tmds181_mode_valid(struct drm_bridge *bridge, const struct drm_display_info *info,
+		   const struct drm_display_mode *mode)
+{
+	/* Clock limits: clk between 25 and 350 MHz, clk is 1/10 of bit clock */
+	if (mode->clock < 25000)
+		return MODE_CLOCK_LOW;
+
+	/* The actual HDMI clock (if provided) cannot exceed 350MHz */
+	if (mode->crtc_clock > 350000)
+		return MODE_CLOCK_HIGH;
+
+	/*
+	 * When in HDMI 2 mode, the clock is 1/40th of the bitrate. The limit is
+	 * then the data rate of 6Gbps, which would use a 600MHz pixel clock.
+	 */
+	if (mode->clock > 600000)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
+static void tmds181_enable(struct drm_bridge *bridge, struct drm_atomic_state *state)
+{
+	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
+	const struct drm_crtc_state *crtc_state;
+	const struct drm_display_mode *mode;
+	struct drm_connector *connector;
+	struct drm_crtc *crtc;
+	unsigned int val;
+
+	/*
+	 * Retrieve the CRTC adjusted mode. This requires a little dance to go
+	 * from the bridge to the encoder, to the connector and to the CRTC.
+	 */
+	connector = drm_atomic_get_new_connector_for_encoder(state,
+							     bridge->encoder);
+	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	mode = &crtc_state->adjusted_mode;
+
+	/* Set retimer/redriver mode based on pixel clock */
+	val = mode->clock > data->retimer_threshold_khz ? TMDS181_CTRLA_DEV_FUNC_MODE : 0;
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
+			   TMDS181_CTRLA_DEV_FUNC_MODE, val);
+
+	/* Configure TX termination based on pixel clock */
+	val = mode->clock > HDMI2_PIXEL_RATE_KHZ ?
+				TMDS181_CTRLB_TX_TERM_75_150_OHMS :
+				TMDS181_CTRLB_TX_TERM_150_300_OHMS;
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRLB,
+			   TMDS181_CTRLB_TX_TERM_CTL,
+			   FIELD_PREP(TMDS181_CTRLB_TX_TERM_CTL, val));
+
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9,
+			   TMDS181_CTRL9_PD_EN, 0);
+}
+
+static void tmds181_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
+{
+	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
+
+	/* Set the PD_EN bit */
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9,
+			   TMDS181_CTRL9_PD_EN, TMDS181_CTRL9_PD_EN);
+}
+
+static const struct drm_bridge_funcs tmds181_bridge_funcs = {
+	.attach		= tmds181_attach,
+	.mode_valid	= tmds181_mode_valid,
+	.atomic_enable	= tmds181_enable,
+	.atomic_disable	= tmds181_disable,
+
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+};
+
+static const u8 tmds181_id_tmds181[8] __nonstring = "TMDS181 ";
+static const u8 tmds181_id_dp159[8]   __nonstring = "DP159   ";
+
+static int tmds181_check_id(struct tmds181_data *data, enum tmds181_chip *chip)
+{
+	int ret;
+	int retry;
+	u8 buffer[8];
+
+	for (retry = 0; retry < 20; ++retry) {
+		ret = regmap_bulk_read(data->regmap, TMDS181_REG_ID, buffer,
+				       sizeof(buffer));
+		if (!ret)
+			break;
+
+		/* Compensate for very long OE power-up delays due to the cap */
+		usleep_range(5000, 10000);
+	}
+
+	if (ret) {
+		dev_err(&data->client->dev, "I2C read ID failed\n");
+		return ret;
+	}
+
+	if (memcmp(buffer, tmds181_id_tmds181, sizeof(buffer)) == 0) {
+		if (*chip != tmds181) {
+			dev_warn(&data->client->dev, "Detected: TMDS181\n");
+			*chip = tmds181;
+		}
+		return 0;
+	}
+
+	if (memcmp(buffer, tmds181_id_dp159, sizeof(buffer)) == 0) {
+		if (*chip != dp159) {
+			dev_warn(&data->client->dev, "Detected: DP159\n");
+			*chip = dp159;
+		}
+		return 0;
+	}
+
+	dev_err(&data->client->dev, "Unknown ID: %*pE\n", (int)sizeof(buffer), buffer);
+
+	return -ENODEV;
+}
+
+static bool tmds181_regmap_is_volatile(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	/* IBERT result and status registers, not used yet */
+	case TMDS181_REG_EYESCAN_15:
+	case TMDS181_REG_EYESCAN_17 ... TMDS181_REG_EYESCAN_1F:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config tmds181_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_RBTREE,
+	.max_register = TMDS181_REG_AUX,
+	.volatile_reg = tmds181_regmap_is_volatile,
+};
+
+static int tmds181_probe(struct i2c_client *client)
+{
+	struct tmds181_data *data;
+	struct gpio_desc *oe_gpio;
+	enum tmds181_chip chip;
+	int ret;
+	u32 param;
+	u8 val;
+
+	/* Check if the adapter supports the needed features */
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -EIO;
+
+	data = devm_drm_bridge_alloc(&client->dev, struct tmds181_data, bridge,
+				     &tmds181_bridge_funcs);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	data->client = client;
+	i2c_set_clientdata(client, data);
+	data->regmap = devm_regmap_init_i2c(client, &tmds181_regmap_config);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+
+	/* The "OE" pin acts as a reset */
+	oe_gpio = devm_gpiod_get_optional(&client->dev, "oe", GPIOD_OUT_LOW);
+	if (IS_ERR(oe_gpio)) {
+		ret = PTR_ERR(oe_gpio);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&client->dev, "failed to acquire 'oe' gpio\n");
+		return ret;
+	}
+	if (oe_gpio) {
+		/* Need at least 100us reset pulse */
+		usleep_range(100, 200);
+		gpiod_set_value_cansleep(oe_gpio, 1);
+	}
+
+	/* Reading the ID also provides enough time for the reset */
+	chip = (enum tmds181_chip)of_device_get_match_data(&client->dev);
+	ret = tmds181_check_id(data, &chip);
+	if (ret)
+		return ret;
+
+	/*
+	 * We take care of power control, so disable the chips PM functions, and
+	 * allow the DDC to run at 400kHz
+	 */
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9,
+			TMDS181_CTRL9_SIG_EN | TMDS181_CTRL9_PD_EN |
+			TMDS181_CTRL9_HPD_AUTO_PWRDWN_DISABLE |
+			TMDS181_CTRL9_I2C_DR_CTL,
+			TMDS181_CTRL9_PD_EN |
+			TMDS181_CTRL9_HPD_AUTO_PWRDWN_DISABLE |
+			TMDS181_CTRL9_I2C_DR_CTL);
+
+	/* Apply configuration changes */
+	if (of_property_read_bool(client->dev.of_node, "ti,source-mode"))
+		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
+				   TMDS181_CTRLA_MODE_SINK, 0);
+	if (of_property_read_bool(client->dev.of_node, "ti,sink-mode"))
+		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
+				   TMDS181_CTRLA_MODE_SINK, TMDS181_CTRLA_MODE_SINK);
+
+	/*
+	 * Using the automatic modes of the chip uses considerable power as it
+	 * will keep the PLL running at all times. So instead, define our own
+	 * threshold for the pixel rate. This also allows to use a sane default
+	 * of 200MHz pixel rate for the redriver-retimer crossover point, as the
+	 * modes below 3k don't show any benefit from the retimer.
+	 */
+	data->retimer_threshold_khz = 200000;
+	if (!of_property_read_u32(client->dev.of_node,
+				  "ti,retimer-threshold-hz", &param))
+		data->retimer_threshold_khz = param / 1000;
+
+	/* Default to low-power redriver mode */
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
+			   TMDS181_CTRLA_DEV_FUNC_MODE, 0x00);
+
+	if (of_property_read_bool(client->dev.of_node, "ti,adaptive-equalizer"))
+		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
+				   TMDS181_CTRLA_EQ_EN | TMDS181_CTRLA_EQ_ADA_EN,
+				   TMDS181_CTRLA_EQ_EN | TMDS181_CTRLA_EQ_ADA_EN);
+	if (of_property_read_bool(client->dev.of_node, "ti,disable-equalizer"))
+		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
+				   TMDS181_CTRLA_EQ_EN | TMDS181_CTRLA_EQ_ADA_EN,
+				   0);
+
+	switch (chip) {
+	case dp159:
+		val = 0;
+		if (!of_property_read_u32(client->dev.of_node,
+					  "ti,slew-rate", &param)) {
+			if (param > 3) {
+				dev_err(&client->dev, "invalid slew-rate\n");
+				return -EINVAL;
+			}
+			/* Implement 0 = slow, 3 = fast slew rate */
+			val = FIELD_PREP(TMDS181_CTRLB_SLEW_CTL, (3 - param));
+		}
+		if (of_property_read_bool(client->dev.of_node, "ti,dvi-mode"))
+			val |= TMDS181_CTRLB_HDMI_SEL_DVI;
+		break;
+	default:
+		val = TMDS181_CTRLB_DDC_DR_SEL;
+		break;
+	}
+
+	/* Default to low-speed termination */
+	val |= FIELD_PREP(TMDS181_CTRLB_TX_TERM_CTL, TMDS181_CTRLB_TX_TERM_150_300_OHMS);
+
+	ret = regmap_write(data->regmap, TMDS181_REG_CTRLB, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "regmap_write(B) failed\n");
+		return ret;
+	}
+
+	/* Find next bridge in chain */
+	data->next_bridge = devm_drm_of_get_bridge(&client->dev, client->dev.of_node, 1, 0);
+	if (IS_ERR(data->next_bridge))
+		return dev_err_probe(&client->dev, PTR_ERR(data->next_bridge),
+				     "Failed to find next bridge\n");
+
+	/* Register the bridge. */
+	data->bridge.of_node = client->dev.of_node;
+
+	return devm_drm_bridge_add(&client->dev, &data->bridge);
+}
+
+static const struct i2c_device_id tmds181_id[] = {
+	{ "tmds181", tmds181 },
+	{ "sn65dp159", dp159 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, tmds181_id);
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id tmds181_of_match[] = {
+	{ .compatible = "ti,tmds181", .data = (void *)tmds181 },
+	{ .compatible = "ti,sn65dp159", .data = (void *)dp159 },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tmds181_of_match);
+#endif
+
+static struct i2c_driver tmds181_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name	= "tmds181",
+		.of_match_table = of_match_ptr(tmds181_of_match),
+	},
+	.probe		= tmds181_probe,
+	.id_table	= tmds181_id,
+};
+
+module_i2c_driver(tmds181_driver);
-- 
2.43.0


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

Re: [PATCH v3 2/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[kernel test robot]

Hi Mike,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 53e760d8949895390e256e723e7ee46618310361]

url:    https://github.com/intel-lab-lkp/linux/commits/Mike-Looijmans/dt-bindings-drm-bridge-ti-tmds181-Add-TI-TMDS181-and-SN65DP159-bindings/20250820-224316
base:   53e760d8949895390e256e723e7ee46618310361
patch link:    https://lore.kernel.org/r/20250820144128.17603-3-mike.looijmans%40topic.nl
patch subject: [PATCH v3 2/2] drm: bridge: Add TI tmds181 and sn65dp159 driver
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20250821/202508211421.aYwuLvvk-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250821/202508211421.aYwuLvvk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508211421.aYwuLvvk-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/ti-tmds181.c:292:9: warning: cast to smaller integer type 'enum tmds181_chip' from 'const void *' [-Wvoid-pointer-to-enum-cast]
     292 |         chip = (enum tmds181_chip)of_device_get_match_data(&client->dev);
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +292 drivers/gpu/drm/bridge/ti-tmds181.c

   252	
   253	static int tmds181_probe(struct i2c_client *client)
   254	{
   255		struct tmds181_data *data;
   256		struct gpio_desc *oe_gpio;
   257		enum tmds181_chip chip;
   258		int ret;
   259		u32 param;
   260		u8 val;
   261	
   262		/* Check if the adapter supports the needed features */
   263		if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
   264			return -EIO;
   265	
   266		data = devm_drm_bridge_alloc(&client->dev, struct tmds181_data, bridge,
   267					     &tmds181_bridge_funcs);
   268		if (IS_ERR(data))
   269			return PTR_ERR(data);
   270	
   271		data->client = client;
   272		i2c_set_clientdata(client, data);
   273		data->regmap = devm_regmap_init_i2c(client, &tmds181_regmap_config);
   274		if (IS_ERR(data->regmap))
   275			return PTR_ERR(data->regmap);
   276	
   277		/* The "OE" pin acts as a reset */
   278		oe_gpio = devm_gpiod_get_optional(&client->dev, "oe", GPIOD_OUT_LOW);
   279		if (IS_ERR(oe_gpio)) {
   280			ret = PTR_ERR(oe_gpio);
   281			if (ret != -EPROBE_DEFER)
   282				dev_err(&client->dev, "failed to acquire 'oe' gpio\n");
   283			return ret;
   284		}
   285		if (oe_gpio) {
   286			/* Need at least 100us reset pulse */
   287			usleep_range(100, 200);
   288			gpiod_set_value_cansleep(oe_gpio, 1);
   289		}
   290	
   291		/* Reading the ID also provides enough time for the reset */
 > 292		chip = (enum tmds181_chip)of_device_get_match_data(&client->dev);
   293		ret = tmds181_check_id(data, &chip);
   294		if (ret)
   295			return ret;
   296	
   297		/*
   298		 * We take care of power control, so disable the chips PM functions, and
   299		 * allow the DDC to run at 400kHz
   300		 */
   301		regmap_update_bits(data->regmap, TMDS181_REG_CTRL9,
   302				TMDS181_CTRL9_SIG_EN | TMDS181_CTRL9_PD_EN |
   303				TMDS181_CTRL9_HPD_AUTO_PWRDWN_DISABLE |
   304				TMDS181_CTRL9_I2C_DR_CTL,
   305				TMDS181_CTRL9_PD_EN |
   306				TMDS181_CTRL9_HPD_AUTO_PWRDWN_DISABLE |
   307				TMDS181_CTRL9_I2C_DR_CTL);
   308	
   309		/* Apply configuration changes */
   310		if (of_property_read_bool(client->dev.of_node, "ti,source-mode"))
   311			regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
   312					   TMDS181_CTRLA_MODE_SINK, 0);
   313		if (of_property_read_bool(client->dev.of_node, "ti,sink-mode"))
   314			regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
   315					   TMDS181_CTRLA_MODE_SINK, TMDS181_CTRLA_MODE_SINK);
   316	
   317		/*
   318		 * Using the automatic modes of the chip uses considerable power as it
   319		 * will keep the PLL running at all times. So instead, define our own
   320		 * threshold for the pixel rate. This also allows to use a sane default
   321		 * of 200MHz pixel rate for the redriver-retimer crossover point, as the
   322		 * modes below 3k don't show any benefit from the retimer.
   323		 */
   324		data->retimer_threshold_khz = 200000;
   325		if (!of_property_read_u32(client->dev.of_node,
   326					  "ti,retimer-threshold-hz", &param))
   327			data->retimer_threshold_khz = param / 1000;
   328	
   329		/* Default to low-power redriver mode */
   330		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
   331				   TMDS181_CTRLA_DEV_FUNC_MODE, 0x00);
   332	
   333		if (of_property_read_bool(client->dev.of_node, "ti,adaptive-equalizer"))
   334			regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
   335					   TMDS181_CTRLA_EQ_EN | TMDS181_CTRLA_EQ_ADA_EN,
   336					   TMDS181_CTRLA_EQ_EN | TMDS181_CTRLA_EQ_ADA_EN);
   337		if (of_property_read_bool(client->dev.of_node, "ti,disable-equalizer"))
   338			regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
   339					   TMDS181_CTRLA_EQ_EN | TMDS181_CTRLA_EQ_ADA_EN,
   340					   0);
   341	
   342		switch (chip) {
   343		case dp159:
   344			val = 0;
   345			if (!of_property_read_u32(client->dev.of_node,
   346						  "ti,slew-rate", &param)) {
   347				if (param > 3) {
   348					dev_err(&client->dev, "invalid slew-rate\n");
   349					return -EINVAL;
   350				}
   351				/* Implement 0 = slow, 3 = fast slew rate */
   352				val = FIELD_PREP(TMDS181_CTRLB_SLEW_CTL, (3 - param));
   353			}
   354			if (of_property_read_bool(client->dev.of_node, "ti,dvi-mode"))
   355				val |= TMDS181_CTRLB_HDMI_SEL_DVI;
   356			break;
   357		default:
   358			val = TMDS181_CTRLB_DDC_DR_SEL;
   359			break;
   360		}
   361	
   362		/* Default to low-speed termination */
   363		val |= FIELD_PREP(TMDS181_CTRLB_TX_TERM_CTL, TMDS181_CTRLB_TX_TERM_150_300_OHMS);
   364	
   365		ret = regmap_write(data->regmap, TMDS181_REG_CTRLB, val);
   366		if (ret < 0) {
   367			dev_err(&client->dev, "regmap_write(B) failed\n");
   368			return ret;
   369		}
   370	
   371		/* Find next bridge in chain */
   372		data->next_bridge = devm_drm_of_get_bridge(&client->dev, client->dev.of_node, 1, 0);
   373		if (IS_ERR(data->next_bridge))
   374			return dev_err_probe(&client->dev, PTR_ERR(data->next_bridge),
   375					     "Failed to find next bridge\n");
   376	
   377		/* Register the bridge. */
   378		data->bridge.of_node = client->dev.of_node;
   379	
   380		return devm_drm_bridge_add(&client->dev, &data->bridge);
   381	}
   382	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 2/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[Krzysztof Kozlowski]

On Wed, Aug 20, 2025 at 04:40:35PM +0200, Mike Looijmans wrote:
> The tmds181 and sn65dp159 are "retimers" and hence can be considered
> HDMI-to-HDMI bridges. Typical usage is to convert the output of an
> FPGA into a valid HDMI signal, and it will typically be inserted
> between an encoder and hdmi-connector.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
> 
> Changes in v3:
> Lower-case hex values and use defines for EYESCAN registers
> Remove equalizer code (unlikely to be used)
> Remove attributes (no longer useful, undocumented)
> Fix build for 6.17 kernel
> Use devm_drm_bridge_alloc
> Sort includes and add linux/bitfield.h
> Check chip type and complain on mismatch
> 
> Changes in v2:
> Use atomic_enable/disable
> Use #defines for bit fields in registers
> Allow HDMI 2 compliance
> Filter modes on clock range
> Use cross-over pixel frequency instead of manual overides
> Devicetree bindings according to standards
> 
>  drivers/gpu/drm/bridge/Kconfig      |  11 +
>  drivers/gpu/drm/bridge/Makefile     |   1 +
>  drivers/gpu/drm/bridge/ti-tmds181.c | 409 ++++++++++++++++++++++++++++
>  3 files changed, 421 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/ti-tmds181.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index b9e0ca85226a..753177fc9b50 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -430,6 +430,17 @@ config DRM_TI_SN65DSI86
>  	help
>  	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
>  
> +config DRM_TI_TMDS181
> +        tristate "TI TMDS181 and SN65DP159 HDMI retimer bridge driver"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	select REGMAP_I2C
> +	help
> +	  Enable this to support the TI TMDS181 and SN65DP159 HDMI retimers.
> +	  The SN65DP159 provides output into a cable (source) whereas the
> +	  TMDS181 is meant to forward a cable signal into a PCB (sink). Either
> +	  can be set up as source or sink though.
> +
>  config DRM_TI_TPD12S015
>  	tristate "TI TPD12S015 HDMI level shifter and ESD protection"
>  	depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 245e8a27e3fc..f4b5089e903c 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_DRM_TI_SN65DSI83) += ti-sn65dsi83.o
>  obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>  obj-$(CONFIG_DRM_TI_TDP158) += ti-tdp158.o
>  obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> +obj-$(CONFIG_DRM_TI_TMDS181) += ti-tmds181.o
>  obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o
>  obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi.o
>  obj-$(CONFIG_DRM_ITE_IT66121) += ite-it66121.o
> diff --git a/drivers/gpu/drm/bridge/ti-tmds181.c b/drivers/gpu/drm/bridge/ti-tmds181.c
> new file mode 100644
> index 000000000000..8ac3eb808d5b
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-tmds181.c
> @@ -0,0 +1,409 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI tmds181 and sn65dp159 HDMI redriver and retimer chips
> + *
> + * Copyright (C) 2018 - 2025 Topic Embedded Products <www.topic.nl>
> + *
> + * based on code
> + * Copyright (C) 2007 Hans Verkuil
> + * Copyright (C) 2016, 2017 Leon Woestenberg <leon@sidebranch.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_print.h>
> +#include <drm/drm_probe_helper.h>
> +
> +MODULE_DESCRIPTION("I2C device driver for DP159 and TMDS181 redriver/retimer");
> +MODULE_AUTHOR("Mike Looijmans");
> +MODULE_LICENSE("GPL");
> +
> +#define TMDS181_REG_ID		0
> +#define TMDS181_REG_REV		0x8
> +#define TMDS181_REG_CTRL9	0x9
> +/* Registers A and B have a volatile bit, but we don't use it, so cache is ok */
> +#define TMDS181_REG_CTRLA	0xa
> +#define TMDS181_REG_CTRLB	0xb
> +#define TMDS181_REG_CTRLC	0xc
> +#define TMDS181_REG_EQUALIZER	0xd
> +/* EYESCAN registers don't appear to deserve separate names */
> +#define TMDS181_REG_EYESCAN_E	0xe
> +#define TMDS181_REG_EYESCAN_F	0xf
> +#define TMDS181_REG_EYESCAN_15	0x15
> +#define TMDS181_REG_EYESCAN_17	0x17
> +#define TMDS181_REG_EYESCAN_1F	0x1f
> +#define TMDS181_REG_AUX		0x20
> +
> +
> +#define TMDS181_CTRL9_SIG_EN			BIT(4)
> +#define TMDS181_CTRL9_PD_EN			BIT(3)
> +#define TMDS181_CTRL9_HPD_AUTO_PWRDWN_DISABLE	BIT(2)
> +#define TMDS181_CTRL9_I2C_DR_CTL		GENMASK(1, 0)
> +
> +#define TMDS181_CTRLA_MODE_SINK			BIT(7)
> +#define TMDS181_CTRLA_HPDSNK_GATE_EN		BIT(6)
> +#define TMDS181_CTRLA_EQ_ADA_EN			BIT(5)
> +#define TMDS181_CTRLA_EQ_EN			BIT(4)
> +#define TMDS181_CTRLA_AUX_BRG_EN		BIT(3)
> +#define TMDS181_CTRLA_APPLY			BIT(2)
> +#define TMDS181_CTRLA_DEV_FUNC_MODE		GENMASK(1, 0)
> +
> +#define TMDS181_CTRLB_SLEW_CTL			GENMASK(7, 6)
> +#define TMDS181_CTRLB_HDMI_SEL_DVI		BIT(5)
> +#define TMDS181_CTRLB_TX_TERM_CTL		GENMASK(4, 3)
> +#define TMDS181_CTRLB_DDC_DR_SEL		BIT(2)
> +#define TMDS181_CTRLB_TMDS_CLOCK_RATIO_STATUS	BIT(1)
> +#define TMDS181_CTRLB_DDC_TRAIN_SET		BIT(0)
> +
> +#define TMDS181_CTRLB_TX_TERM_150_300_OHMS	1
> +#define TMDS181_CTRLB_TX_TERM_75_150_OHMS	3
> +
> +#define TMDS181_CTRLC_VSWING_DATA		GENMASK(7, 5)
> +#define TMDS181_CTRLC_VSWING_CLK		GENMASK(4, 2)
> +#define TMDS181_CTRLC_HDMI_TWPST1		GENMASK(1, 0)
> +
> +#define TMDS181_EQ_DATA_LANE			GENMASK(5, 3)
> +#define TMDS181_EQ_CLOCK_LANE			GENMASK(2, 1)
> +#define TMDS181_EQ_DIS_HDMI2_SWG		BIT(0)
> +
> +/* Above this data rate HDMI2 standards apply (TX termination) */
> +#define HDMI2_PIXEL_RATE_KHZ	340000
> +
> +enum tmds181_chip {
> +	tmds181,
> +	dp159,
> +};
> +
> +struct tmds181_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	struct drm_bridge *next_bridge;
> +	struct drm_bridge bridge;
> +	u32 retimer_threshold_khz;
> +};
> +
> +static inline struct tmds181_data *
> +drm_bridge_to_tmds181_data(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct tmds181_data, bridge);
> +}
> +
> +static int tmds181_attach(struct drm_bridge *bridge, struct drm_encoder *encoder,
> +			  enum drm_bridge_attach_flags flags)
> +{
> +	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
> +
> +	return drm_bridge_attach(encoder, data->next_bridge, bridge, flags);
> +}
> +
> +static enum drm_mode_status
> +tmds181_mode_valid(struct drm_bridge *bridge, const struct drm_display_info *info,
> +		   const struct drm_display_mode *mode)
> +{
> +	/* Clock limits: clk between 25 and 350 MHz, clk is 1/10 of bit clock */
> +	if (mode->clock < 25000)
> +		return MODE_CLOCK_LOW;
> +
> +	/* The actual HDMI clock (if provided) cannot exceed 350MHz */
> +	if (mode->crtc_clock > 350000)
> +		return MODE_CLOCK_HIGH;
> +
> +	/*
> +	 * When in HDMI 2 mode, the clock is 1/40th of the bitrate. The limit is
> +	 * then the data rate of 6Gbps, which would use a 600MHz pixel clock.
> +	 */
> +	if (mode->clock > 600000)
> +		return MODE_CLOCK_HIGH;
> +
> +	return MODE_OK;
> +}
> +
> +static void tmds181_enable(struct drm_bridge *bridge, struct drm_atomic_state *state)
> +{
> +	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
> +	const struct drm_crtc_state *crtc_state;
> +	const struct drm_display_mode *mode;
> +	struct drm_connector *connector;
> +	struct drm_crtc *crtc;
> +	unsigned int val;
> +
> +	/*
> +	 * Retrieve the CRTC adjusted mode. This requires a little dance to go
> +	 * from the bridge to the encoder, to the connector and to the CRTC.
> +	 */
> +	connector = drm_atomic_get_new_connector_for_encoder(state,
> +							     bridge->encoder);
> +	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	mode = &crtc_state->adjusted_mode;
> +
> +	/* Set retimer/redriver mode based on pixel clock */
> +	val = mode->clock > data->retimer_threshold_khz ? TMDS181_CTRLA_DEV_FUNC_MODE : 0;
> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
> +			   TMDS181_CTRLA_DEV_FUNC_MODE, val);
> +
> +	/* Configure TX termination based on pixel clock */
> +	val = mode->clock > HDMI2_PIXEL_RATE_KHZ ?
> +				TMDS181_CTRLB_TX_TERM_75_150_OHMS :
> +				TMDS181_CTRLB_TX_TERM_150_300_OHMS;
> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRLB,
> +			   TMDS181_CTRLB_TX_TERM_CTL,
> +			   FIELD_PREP(TMDS181_CTRLB_TX_TERM_CTL, val));
> +
> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9,
> +			   TMDS181_CTRL9_PD_EN, 0);
> +}
> +
> +static void tmds181_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
> +{
> +	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
> +
> +	/* Set the PD_EN bit */
> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9,
> +			   TMDS181_CTRL9_PD_EN, TMDS181_CTRL9_PD_EN);
> +}
> +
> +static const struct drm_bridge_funcs tmds181_bridge_funcs = {
> +	.attach		= tmds181_attach,
> +	.mode_valid	= tmds181_mode_valid,
> +	.atomic_enable	= tmds181_enable,
> +	.atomic_disable	= tmds181_disable,
> +
> +	.atomic_reset = drm_atomic_helper_bridge_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +};
> +
> +static const u8 tmds181_id_tmds181[8] __nonstring = "TMDS181 ";
> +static const u8 tmds181_id_dp159[8]   __nonstring = "DP159   ";
> +
> +static int tmds181_check_id(struct tmds181_data *data, enum tmds181_chip *chip)
> +{
> +	int ret;
> +	int retry;
> +	u8 buffer[8];
> +
> +	for (retry = 0; retry < 20; ++retry) {
> +		ret = regmap_bulk_read(data->regmap, TMDS181_REG_ID, buffer,
> +				       sizeof(buffer));
> +		if (!ret)
> +			break;
> +
> +		/* Compensate for very long OE power-up delays due to the cap */
> +		usleep_range(5000, 10000);
> +	}
> +
> +	if (ret) {
> +		dev_err(&data->client->dev, "I2C read ID failed\n");
> +		return ret;
> +	}
> +
> +	if (memcmp(buffer, tmds181_id_tmds181, sizeof(buffer)) == 0) {
> +		if (*chip != tmds181) {
> +			dev_warn(&data->client->dev, "Detected: TMDS181\n");
> +			*chip = tmds181;
> +		}
> +		return 0;
> +	}
> +
> +	if (memcmp(buffer, tmds181_id_dp159, sizeof(buffer)) == 0) {
> +		if (*chip != dp159) {
> +			dev_warn(&data->client->dev, "Detected: DP159\n");
> +			*chip = dp159;
> +		}
> +		return 0;
> +	}
> +
> +	dev_err(&data->client->dev, "Unknown ID: %*pE\n", (int)sizeof(buffer), buffer);
> +
> +	return -ENODEV;
> +}
> +
> +static bool tmds181_regmap_is_volatile(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	/* IBERT result and status registers, not used yet */
> +	case TMDS181_REG_EYESCAN_15:
> +	case TMDS181_REG_EYESCAN_17 ... TMDS181_REG_EYESCAN_1F:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct regmap_config tmds181_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_RBTREE,
> +	.max_register = TMDS181_REG_AUX,
> +	.volatile_reg = tmds181_regmap_is_volatile,
> +};
> +
> +static int tmds181_probe(struct i2c_client *client)
> +{
> +	struct tmds181_data *data;
> +	struct gpio_desc *oe_gpio;
> +	enum tmds181_chip chip;
> +	int ret;
> +	u32 param;
> +	u8 val;
> +
> +	/* Check if the adapter supports the needed features */
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +		return -EIO;
> +
> +	data = devm_drm_bridge_alloc(&client->dev, struct tmds181_data, bridge,
> +				     &tmds181_bridge_funcs);
> +	if (IS_ERR(data))
> +		return PTR_ERR(data);
> +
> +	data->client = client;
> +	i2c_set_clientdata(client, data);
> +	data->regmap = devm_regmap_init_i2c(client, &tmds181_regmap_config);
> +	if (IS_ERR(data->regmap))
> +		return PTR_ERR(data->regmap);
> +
> +	/* The "OE" pin acts as a reset */
> +	oe_gpio = devm_gpiod_get_optional(&client->dev, "oe", GPIOD_OUT_LOW);
> +	if (IS_ERR(oe_gpio)) {
> +		ret = PTR_ERR(oe_gpio);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&client->dev, "failed to acquire 'oe' gpio\n");

Heh, nice...

> +		return ret;
> +	}

...

> +
> +static const struct i2c_device_id tmds181_id[] = {
> +	{ "tmds181", tmds181 },
> +	{ "sn65dp159", dp159 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, tmds181_id);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id tmds181_of_match[] = {
> +	{ .compatible = "ti,tmds181", .data = (void *)tmds181 },
> +	{ .compatible = "ti,sn65dp159", .data = (void *)dp159 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, tmds181_of_match);
> +#endif
> +
> +static struct i2c_driver tmds181_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,

Nice coincidence - this stars in one of my talks on OSSE
(https://sched.co/25VoV) as my litmus test for crazy old, vendor code.
Please come to the session if you are around or just check the slides
afterwards.

The open-coding dev_err_probe() is another great example.

Best regards,
Krzysztof

Re: [PATCH v3 2/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[Mike Looijmans]

On 21-08-2025 09:36, Krzysztof Kozlowski wrote:
> On Wed, Aug 20, 2025 at 04:40:35PM +0200, Mike Looijmans wrote:
>> The tmds181 and sn65dp159 are "retimers" and hence can be considered
>> HDMI-to-HDMI bridges. Typical usage is to convert the output of an
>> FPGA into a valid HDMI signal, and it will typically be inserted
>> between an encoder and hdmi-connector.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>>
>> Changes in v3:
>> Lower-case hex values and use defines for EYESCAN registers
>> Remove equalizer code (unlikely to be used)
>> Remove attributes (no longer useful, undocumented)
>> Fix build for 6.17 kernel
>> Use devm_drm_bridge_alloc
>> Sort includes and add linux/bitfield.h
>> Check chip type and complain on mismatch
>>
>> Changes in v2:
>> Use atomic_enable/disable
>> Use #defines for bit fields in registers
>> Allow HDMI 2 compliance
>> Filter modes on clock range
>> Use cross-over pixel frequency instead of manual overides
>> Devicetree bindings according to standards
>>
>>   drivers/gpu/drm/bridge/Kconfig      |  11 +
>>   drivers/gpu/drm/bridge/Makefile     |   1 +
>>   drivers/gpu/drm/bridge/ti-tmds181.c | 409 ++++++++++++++++++++++++++++
>>   3 files changed, 421 insertions(+)
>>   create mode 100644 drivers/gpu/drm/bridge/ti-tmds181.c
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index b9e0ca85226a..753177fc9b50 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -430,6 +430,17 @@ config DRM_TI_SN65DSI86
>>   	help
>>   	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
>>   
>> +config DRM_TI_TMDS181
>> +        tristate "TI TMDS181 and SN65DP159 HDMI retimer bridge driver"
>> +	depends on OF
>> +	select DRM_KMS_HELPER
>> +	select REGMAP_I2C
>> +	help
>> +	  Enable this to support the TI TMDS181 and SN65DP159 HDMI retimers.
>> +	  The SN65DP159 provides output into a cable (source) whereas the
>> +	  TMDS181 is meant to forward a cable signal into a PCB (sink). Either
>> +	  can be set up as source or sink though.
>> +
>>   config DRM_TI_TPD12S015
>>   	tristate "TI TPD12S015 HDMI level shifter and ESD protection"
>>   	depends on OF
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index 245e8a27e3fc..f4b5089e903c 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -39,6 +39,7 @@ obj-$(CONFIG_DRM_TI_SN65DSI83) += ti-sn65dsi83.o
>>   obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>>   obj-$(CONFIG_DRM_TI_TDP158) += ti-tdp158.o
>>   obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
>> +obj-$(CONFIG_DRM_TI_TMDS181) += ti-tmds181.o
>>   obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o
>>   obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi.o
>>   obj-$(CONFIG_DRM_ITE_IT66121) += ite-it66121.o
>> diff --git a/drivers/gpu/drm/bridge/ti-tmds181.c b/drivers/gpu/drm/bridge/ti-tmds181.c
>> new file mode 100644
>> index 000000000000..8ac3eb808d5b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/ti-tmds181.c
>> @@ -0,0 +1,409 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * TI tmds181 and sn65dp159 HDMI redriver and retimer chips
>> + *
>> + * Copyright (C) 2018 - 2025 Topic Embedded Products <www.topic.nl>
>> + *
>> + * based on code
>> + * Copyright (C) 2007 Hans Verkuil
>> + * Copyright (C) 2016, 2017 Leon Woestenberg <leon@sidebranch.com>
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_bridge.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_print.h>
>> +#include <drm/drm_probe_helper.h>
>> +
>> +MODULE_DESCRIPTION("I2C device driver for DP159 and TMDS181 redriver/retimer");
>> +MODULE_AUTHOR("Mike Looijmans");
>> +MODULE_LICENSE("GPL");
>> +
>> +#define TMDS181_REG_ID		0
>> +#define TMDS181_REG_REV		0x8
>> +#define TMDS181_REG_CTRL9	0x9
>> +/* Registers A and B have a volatile bit, but we don't use it, so cache is ok */
>> +#define TMDS181_REG_CTRLA	0xa
>> +#define TMDS181_REG_CTRLB	0xb
>> +#define TMDS181_REG_CTRLC	0xc
>> +#define TMDS181_REG_EQUALIZER	0xd
>> +/* EYESCAN registers don't appear to deserve separate names */
>> +#define TMDS181_REG_EYESCAN_E	0xe
>> +#define TMDS181_REG_EYESCAN_F	0xf
>> +#define TMDS181_REG_EYESCAN_15	0x15
>> +#define TMDS181_REG_EYESCAN_17	0x17
>> +#define TMDS181_REG_EYESCAN_1F	0x1f
>> +#define TMDS181_REG_AUX		0x20
>> +
>> +
>> +#define TMDS181_CTRL9_SIG_EN			BIT(4)
>> +#define TMDS181_CTRL9_PD_EN			BIT(3)
>> +#define TMDS181_CTRL9_HPD_AUTO_PWRDWN_DISABLE	BIT(2)
>> +#define TMDS181_CTRL9_I2C_DR_CTL		GENMASK(1, 0)
>> +
>> +#define TMDS181_CTRLA_MODE_SINK			BIT(7)
>> +#define TMDS181_CTRLA_HPDSNK_GATE_EN		BIT(6)
>> +#define TMDS181_CTRLA_EQ_ADA_EN			BIT(5)
>> +#define TMDS181_CTRLA_EQ_EN			BIT(4)
>> +#define TMDS181_CTRLA_AUX_BRG_EN		BIT(3)
>> +#define TMDS181_CTRLA_APPLY			BIT(2)
>> +#define TMDS181_CTRLA_DEV_FUNC_MODE		GENMASK(1, 0)
>> +
>> +#define TMDS181_CTRLB_SLEW_CTL			GENMASK(7, 6)
>> +#define TMDS181_CTRLB_HDMI_SEL_DVI		BIT(5)
>> +#define TMDS181_CTRLB_TX_TERM_CTL		GENMASK(4, 3)
>> +#define TMDS181_CTRLB_DDC_DR_SEL		BIT(2)
>> +#define TMDS181_CTRLB_TMDS_CLOCK_RATIO_STATUS	BIT(1)
>> +#define TMDS181_CTRLB_DDC_TRAIN_SET		BIT(0)
>> +
>> +#define TMDS181_CTRLB_TX_TERM_150_300_OHMS	1
>> +#define TMDS181_CTRLB_TX_TERM_75_150_OHMS	3
>> +
>> +#define TMDS181_CTRLC_VSWING_DATA		GENMASK(7, 5)
>> +#define TMDS181_CTRLC_VSWING_CLK		GENMASK(4, 2)
>> +#define TMDS181_CTRLC_HDMI_TWPST1		GENMASK(1, 0)
>> +
>> +#define TMDS181_EQ_DATA_LANE			GENMASK(5, 3)
>> +#define TMDS181_EQ_CLOCK_LANE			GENMASK(2, 1)
>> +#define TMDS181_EQ_DIS_HDMI2_SWG		BIT(0)
>> +
>> +/* Above this data rate HDMI2 standards apply (TX termination) */
>> +#define HDMI2_PIXEL_RATE_KHZ	340000
>> +
>> +enum tmds181_chip {
>> +	tmds181,
>> +	dp159,
>> +};
>> +
>> +struct tmds181_data {
>> +	struct i2c_client *client;
>> +	struct regmap *regmap;
>> +	struct drm_bridge *next_bridge;
>> +	struct drm_bridge bridge;
>> +	u32 retimer_threshold_khz;
>> +};
>> +
>> +static inline struct tmds181_data *
>> +drm_bridge_to_tmds181_data(struct drm_bridge *bridge)
>> +{
>> +	return container_of(bridge, struct tmds181_data, bridge);
>> +}
>> +
>> +static int tmds181_attach(struct drm_bridge *bridge, struct drm_encoder *encoder,
>> +			  enum drm_bridge_attach_flags flags)
>> +{
>> +	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
>> +
>> +	return drm_bridge_attach(encoder, data->next_bridge, bridge, flags);
>> +}
>> +
>> +static enum drm_mode_status
>> +tmds181_mode_valid(struct drm_bridge *bridge, const struct drm_display_info *info,
>> +		   const struct drm_display_mode *mode)
>> +{
>> +	/* Clock limits: clk between 25 and 350 MHz, clk is 1/10 of bit clock */
>> +	if (mode->clock < 25000)
>> +		return MODE_CLOCK_LOW;
>> +
>> +	/* The actual HDMI clock (if provided) cannot exceed 350MHz */
>> +	if (mode->crtc_clock > 350000)
>> +		return MODE_CLOCK_HIGH;
>> +
>> +	/*
>> +	 * When in HDMI 2 mode, the clock is 1/40th of the bitrate. The limit is
>> +	 * then the data rate of 6Gbps, which would use a 600MHz pixel clock.
>> +	 */
>> +	if (mode->clock > 600000)
>> +		return MODE_CLOCK_HIGH;
>> +
>> +	return MODE_OK;
>> +}
>> +
>> +static void tmds181_enable(struct drm_bridge *bridge, struct drm_atomic_state *state)
>> +{
>> +	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
>> +	const struct drm_crtc_state *crtc_state;
>> +	const struct drm_display_mode *mode;
>> +	struct drm_connector *connector;
>> +	struct drm_crtc *crtc;
>> +	unsigned int val;
>> +
>> +	/*
>> +	 * Retrieve the CRTC adjusted mode. This requires a little dance to go
>> +	 * from the bridge to the encoder, to the connector and to the CRTC.
>> +	 */
>> +	connector = drm_atomic_get_new_connector_for_encoder(state,
>> +							     bridge->encoder);
>> +	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
>> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>> +	mode = &crtc_state->adjusted_mode;
>> +
>> +	/* Set retimer/redriver mode based on pixel clock */
>> +	val = mode->clock > data->retimer_threshold_khz ? TMDS181_CTRLA_DEV_FUNC_MODE : 0;
>> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRLA,
>> +			   TMDS181_CTRLA_DEV_FUNC_MODE, val);
>> +
>> +	/* Configure TX termination based on pixel clock */
>> +	val = mode->clock > HDMI2_PIXEL_RATE_KHZ ?
>> +				TMDS181_CTRLB_TX_TERM_75_150_OHMS :
>> +				TMDS181_CTRLB_TX_TERM_150_300_OHMS;
>> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRLB,
>> +			   TMDS181_CTRLB_TX_TERM_CTL,
>> +			   FIELD_PREP(TMDS181_CTRLB_TX_TERM_CTL, val));
>> +
>> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9,
>> +			   TMDS181_CTRL9_PD_EN, 0);
>> +}
>> +
>> +static void tmds181_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
>> +{
>> +	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
>> +
>> +	/* Set the PD_EN bit */
>> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9,
>> +			   TMDS181_CTRL9_PD_EN, TMDS181_CTRL9_PD_EN);
>> +}
>> +
>> +static const struct drm_bridge_funcs tmds181_bridge_funcs = {
>> +	.attach		= tmds181_attach,
>> +	.mode_valid	= tmds181_mode_valid,
>> +	.atomic_enable	= tmds181_enable,
>> +	.atomic_disable	= tmds181_disable,
>> +
>> +	.atomic_reset = drm_atomic_helper_bridge_reset,
>> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>> +};
>> +
>> +static const u8 tmds181_id_tmds181[8] __nonstring = "TMDS181 ";
>> +static const u8 tmds181_id_dp159[8]   __nonstring = "DP159   ";
>> +
>> +static int tmds181_check_id(struct tmds181_data *data, enum tmds181_chip *chip)
>> +{
>> +	int ret;
>> +	int retry;
>> +	u8 buffer[8];
>> +
>> +	for (retry = 0; retry < 20; ++retry) {
>> +		ret = regmap_bulk_read(data->regmap, TMDS181_REG_ID, buffer,
>> +				       sizeof(buffer));
>> +		if (!ret)
>> +			break;
>> +
>> +		/* Compensate for very long OE power-up delays due to the cap */
>> +		usleep_range(5000, 10000);
>> +	}
>> +
>> +	if (ret) {
>> +		dev_err(&data->client->dev, "I2C read ID failed\n");
>> +		return ret;
>> +	}
>> +
>> +	if (memcmp(buffer, tmds181_id_tmds181, sizeof(buffer)) == 0) {
>> +		if (*chip != tmds181) {
>> +			dev_warn(&data->client->dev, "Detected: TMDS181\n");
>> +			*chip = tmds181;
>> +		}
>> +		return 0;
>> +	}
>> +
>> +	if (memcmp(buffer, tmds181_id_dp159, sizeof(buffer)) == 0) {
>> +		if (*chip != dp159) {
>> +			dev_warn(&data->client->dev, "Detected: DP159\n");
>> +			*chip = dp159;
>> +		}
>> +		return 0;
>> +	}
>> +
>> +	dev_err(&data->client->dev, "Unknown ID: %*pE\n", (int)sizeof(buffer), buffer);
>> +
>> +	return -ENODEV;
>> +}
>> +
>> +static bool tmds181_regmap_is_volatile(struct device *dev, unsigned int reg)
>> +{
>> +	switch (reg) {
>> +	/* IBERT result and status registers, not used yet */
>> +	case TMDS181_REG_EYESCAN_15:
>> +	case TMDS181_REG_EYESCAN_17 ... TMDS181_REG_EYESCAN_1F:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +static const struct regmap_config tmds181_regmap_config = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +	.cache_type = REGCACHE_RBTREE,
>> +	.max_register = TMDS181_REG_AUX,
>> +	.volatile_reg = tmds181_regmap_is_volatile,
>> +};
>> +
>> +static int tmds181_probe(struct i2c_client *client)
>> +{
>> +	struct tmds181_data *data;
>> +	struct gpio_desc *oe_gpio;
>> +	enum tmds181_chip chip;
>> +	int ret;
>> +	u32 param;
>> +	u8 val;
>> +
>> +	/* Check if the adapter supports the needed features */
>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>> +		return -EIO;
>> +
>> +	data = devm_drm_bridge_alloc(&client->dev, struct tmds181_data, bridge,
>> +				     &tmds181_bridge_funcs);
>> +	if (IS_ERR(data))
>> +		return PTR_ERR(data);
>> +
>> +	data->client = client;
>> +	i2c_set_clientdata(client, data);
>> +	data->regmap = devm_regmap_init_i2c(client, &tmds181_regmap_config);
>> +	if (IS_ERR(data->regmap))
>> +		return PTR_ERR(data->regmap);
>> +
>> +	/* The "OE" pin acts as a reset */
>> +	oe_gpio = devm_gpiod_get_optional(&client->dev, "oe", GPIOD_OUT_LOW);
>> +	if (IS_ERR(oe_gpio)) {
>> +		ret = PTR_ERR(oe_gpio);
>> +		if (ret != -EPROBE_DEFER)
>> +			dev_err(&client->dev, "failed to acquire 'oe' gpio\n");
> Heh, nice...

Glad to be of service, though it does make me feel old.


>
>> +		return ret;
>> +	}
> ...
>
>> +
>> +static const struct i2c_device_id tmds181_id[] = {
>> +	{ "tmds181", tmds181 },
>> +	{ "sn65dp159", dp159 },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, tmds181_id);
>> +
>> +#if IS_ENABLED(CONFIG_OF)
>> +static const struct of_device_id tmds181_of_match[] = {
>> +	{ .compatible = "ti,tmds181", .data = (void *)tmds181 },
>> +	{ .compatible = "ti,sn65dp159", .data = (void *)dp159 },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, tmds181_of_match);
>> +#endif
>> +
>> +static struct i2c_driver tmds181_driver = {
>> +	.driver = {
>> +		.owner = THIS_MODULE,
> Nice coincidence - this stars in one of my talks on OSSE
> (https://sched.co/25VoV) as my litmus test for crazy old, vendor code.
> Please come to the session if you are around or just check the slides
> afterwards.

Oh, Amsterdam this year. Hadn't spotted that in time, or I would have 
been there!

I added coccinelle to my toolkit (would make a good addition to patman). 
Indeed would have prevented the old-skool I2C stuff that I've been 
lugging along in drivers for a decade...

I'll fix those in v4.

And yeah, the real issue is that I didn't submit this driver seven years 
ago. In my defense, the other guys also didn't.

For your talk, maybe add a tip on how to avoid the verbose output.
make C=1 CHECK=scripts/coccicheck drivers/gpu/drm/bridge/ti-tmds181.o
I get a gazillion lines of "/usr/bin/spatch -D report ..." and it's 
difficult to spot the warnings. Tried adding "V=0" but that didn't make 
a difference, piping through "grep :" helped a bundle.


> The open-coding dev_err_probe() is another great example.
>
> Best regards,
> Krzysztof
>

-- 
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Re: [PATCH v3 2/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[Krzysztof Kozlowski]

On 22/08/2025 17:16, Mike Looijmans wrote:
>>> +
>>> +static struct i2c_driver tmds181_driver = {
>>> +	.driver = {
>>> +		.owner = THIS_MODULE,
>> Nice coincidence - this stars in one of my talks on OSSE
>> (https://sched.co/25VoV) as my litmus test for crazy old, vendor code.
>> Please come to the session if you are around or just check the slides
>> afterwards.
> 
> Oh, Amsterdam this year. Hadn't spotted that in time, or I would have 
> been there!
> 
> I added coccinelle to my toolkit (would make a good addition to patman). 
> Indeed would have prevented the old-skool I2C stuff that I've been 
> lugging along in drivers for a decade...
> 
> I'll fix those in v4.
> 
> And yeah, the real issue is that I didn't submit this driver seven years 
> ago. In my defense, the other guys also didn't.
> 
> For your talk, maybe add a tip on how to avoid the verbose output.
> make C=1 CHECK=scripts/coccicheck drivers/gpu/drm/bridge/ti-tmds181.o
> I get a gazillion lines of "/usr/bin/spatch -D report ..." and it's 
> difficult to spot the warnings. Tried adding "V=0" but that didn't make 
> a difference, piping through "grep :" helped a bundle.

make coccicheck M=drivers/gpu/drm/bridge/ DEBUG_FILE=cocci.err



Best regards,
Krzysztof