[PATCH 0/2] drm: bridge: Add TI tmds181 and sn65dp159 driver (RFC)

[PATCH 0/2] drm: bridge: Add TI tmds181 and sn65dp159 driver (RFC)

[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.

I'm fully aware that both bindings and driver require some more rework,
the bindings aren't complete yet and the driver needs some more #defines
for bitfields and such (the core code is over 5 years old).

My main question to the maintainers here: Is this a viable solution?
And if so, I'll do my best to document and clean it up further...



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   | 104 ++++
 drivers/gpu/drm/bridge/Kconfig                |  11 +
 drivers/gpu/drm/bridge/Makefile               |   1 +
 drivers/gpu/drm/bridge/ti-tmds181.c           | 512 ++++++++++++++++++
 4 files changed, 628 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 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.

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

 .../bindings/display/bridge/ti,tmds181.yaml   | 104 ++++++++++++++++++
 1 file changed, 104 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..87ffb556c4ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
@@ -0,0 +1,104 @@
+# 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/TMDS181
+
+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).
+
+  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
+
+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 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>
---

 drivers/gpu/drm/bridge/Kconfig      |  11 +
 drivers/gpu/drm/bridge/Makefile     |   1 +
 drivers/gpu/drm/bridge/ti-tmds181.c | 512 ++++++++++++++++++++++++++++
 3 files changed, 524 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..6fbbc13ddc10
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-tmds181.c
@@ -0,0 +1,512 @@
+// 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/module.h>
+#include <linux/kernel.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.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
+#define TMDS181_REG_EYESCAN	0xE
+
+enum tmds181_chip {
+	tmds181,
+	dp159,
+};
+
+struct tmds181_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct drm_bridge *next_bridge;
+	struct drm_bridge bridge;
+	enum tmds181_chip chip;
+};
+
+static inline struct tmds181_data *
+drm_bridge_to_tmds181_data(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct tmds181_data, bridge);
+}
+
+/* Set "apply" bit in control register after making changes */
+static int tmds181_apply_changes(struct tmds181_data *data)
+{
+	return regmap_write_bits(data->regmap,
+		TMDS181_REG_CTRLA, BIT(2), BIT(2));
+}
+
+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 4k 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 tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
+
+	/* Clear the PD_EN bit */
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9, BIT(3), 0);
+}
+
+static void tmds181_disable(struct drm_bridge *bridge)
+{
+	struct tmds181_data *data = drm_bridge_to_tmds181_data(bridge);
+
+	/* Set the PD_EN bit */
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9, BIT(3), BIT(3));
+}
+
+static const struct drm_bridge_funcs tmds181_bridge_funcs = {
+	.attach		= tmds181_attach,
+	.mode_valid	= tmds181_mode_valid,
+	.enable		= tmds181_enable,
+	.disable	= tmds181_disable,
+};
+
+static const char * const tmds181_modes[] = {
+	"redriver",
+	"auto1",
+	"auto2",
+	"retimer",
+};
+
+static ssize_t mode_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct tmds181_data *data = dev_get_drvdata(dev);
+	const char *equalizer;
+	u32 val;
+	int ret;
+
+	ret = regmap_read(data->regmap, TMDS181_REG_CTRLA, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val & BIT(4)) {
+		if (val & BIT(5))
+			equalizer = "eq-adaptive";
+		else
+			equalizer = "eq-fixed";
+	} else {
+		equalizer = "eq-disabled";
+	}
+
+	return scnprintf(buf, PAGE_SIZE, "%6s %s %s\n",
+			(val & BIT(7)) ? "sink" : "source",
+			tmds181_modes[val & 0x03],
+			equalizer);
+}
+
+static ssize_t mode_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct tmds181_data *data = dev_get_drvdata(dev);
+	u32 val;
+	int ret;
+	int i;
+
+	/* Strip trailing newline(s) for being user friendly */
+	while (len && buf[len] == '\n')
+		--len;
+
+	/* Need at least 4 actual characters */
+	if (len < 4)
+		return -EINVAL;
+
+	ret = regmap_read(data->regmap, TMDS181_REG_CTRLA, &val);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(tmds181_modes); ++i) {
+		if (strncmp(tmds181_modes[i], buf, len) == 0) {
+			val &= ~0x03;
+			val |= i;
+			break;
+		}
+	}
+
+	if (strncmp("sink", buf, len) == 0)
+		val |= BIT(7);
+
+	if (strncmp("source", buf, len) == 0)
+		val &= ~BIT(7);
+
+	if (strncmp("eq-", buf, 3) == 0) {
+		switch (buf[3]) {
+		case 'a': /* adaptive */
+			val |= BIT(4) | BIT(5);
+			break;
+		case 'f': /* fixed */
+			val |= BIT(4);
+			val &= ~BIT(5);
+			break;
+		case 'd': /* disable */
+			val &= ~(BIT(4) | BIT(5));
+			break;
+		}
+	}
+
+	/* Always set the "apply changes" bit */
+	val |= BIT(2);
+
+	ret = regmap_write(data->regmap, TMDS181_REG_CTRLA, val);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+/* termination for HDMI TX: 0=off, 1=150..300, 3=75..150 ohms */
+static ssize_t termination_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct tmds181_data *data = dev_get_drvdata(dev);
+	u32 val;
+	int ret;
+
+	ret = regmap_read(data->regmap, TMDS181_REG_CTRLB, &val);
+	if (ret < 0)
+		return ret;
+
+	val >>= 3;
+	val &= 0x03;
+
+	return scnprintf(buf, PAGE_SIZE, "%u\n", val);
+}
+
+static ssize_t termination_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct tmds181_data *data = dev_get_drvdata(dev);
+	u32 val;
+	unsigned long newval;
+	int ret;
+
+	ret = regmap_read(data->regmap, TMDS181_REG_CTRLB, &val);
+	if (ret < 0)
+		return ret;
+
+	ret = kstrtoul((const char *) buf, 10, &newval);
+	if (ret)
+		return ret;
+
+	if (newval > 3)
+		return -EINVAL;
+
+	val &= ~(0x03 << 3);
+	val |= newval << 3;
+
+	ret = regmap_write(data->regmap, TMDS181_REG_CTRLB, val);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+static DEVICE_ATTR_RW(mode);
+static DEVICE_ATTR_RW(termination);
+
+static struct attribute *tmds181_attrs[] = {
+	&dev_attr_mode.attr,
+	&dev_attr_termination.attr,
+	NULL,
+};
+
+static const struct attribute_group tmds181_attr_group = {
+	.attrs = tmds181_attrs,
+};
+
+static const u8 tmds181_id_tmds181[8] = "TMDS181 ";
+static const u8 tmds181_id_dp159[8]   = "DP159   ";
+
+static int tmds181_check_id(struct tmds181_data *data)
+{
+	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) {
+		dev_info(&data->client->dev, "Detected: TMDS181\n");
+		data->chip = tmds181;
+		return 0;
+	}
+
+	if (memcmp(buffer, tmds181_id_dp159, sizeof(buffer)) == 0) {
+		dev_info(&data->client->dev, "Detected: DP159\n");
+		data->chip = dp159;
+		return 0;
+	}
+
+	dev_err(&data->client->dev, "Unknown or wrong 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 0x15:
+	case 0x17 ... 0x1F:
+		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 = 0x20,
+	.volatile_reg = tmds181_regmap_is_volatile,
+};
+
+static int tmds181_probe(struct i2c_client *client)
+{
+	struct tmds181_data *data;
+	struct gpio_desc *oe_gpio;
+	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_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	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 */
+	ret = tmds181_check_id(data);
+	if (ret)
+		return ret;
+
+	/* We take care of power control, so disable the chips PM functions */
+	/* SIG_EN=0 PD_EN=1 HPD_AUTO_PWRDWN_DISABLE=1 I2C_DR_CTL=0b11*/
+	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9, 0x1f, 0x0f);
+
+	/* Apply configuration changes */
+	if (of_property_read_bool(client->dev.of_node, "source-mode"))
+		regmap_update_bits(data->regmap,
+				TMDS181_REG_CTRLA, BIT(7), 0);
+	if (of_property_read_bool(client->dev.of_node, "sink-mode"))
+		regmap_update_bits(data->regmap,
+				TMDS181_REG_CTRLA, BIT(7), BIT(7));
+	if (of_property_read_bool(client->dev.of_node, "redriver-mode"))
+		regmap_update_bits(data->regmap,
+				TMDS181_REG_CTRLA, 0x03, 0x00);
+	if (of_property_read_bool(client->dev.of_node, "retimer-mode"))
+		regmap_update_bits(data->regmap,
+				TMDS181_REG_CTRLA, 0x03, 0x03);
+	if (of_property_read_bool(client->dev.of_node, "adaptive-equalizer"))
+		regmap_update_bits(data->regmap,
+			TMDS181_REG_CTRLA, BIT(4) | BIT(5), BIT(4) | BIT(5));
+	if (of_property_read_bool(client->dev.of_node, "disable-equalizer"))
+		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA, BIT(4), 0);
+
+	switch (data->chip) {
+	case dp159:
+		/*  SLEW=0b00 Mode=HDMI DDC_TRAIN_SET=1 */
+		val = BIT(0);
+		/* Default slew rate is max */
+		param = 3;
+		if (!of_property_read_u32(client->dev.of_node,
+					"slew-rate", &param)) {
+			if (param > 3) {
+				dev_err(&client->dev, "invalid slew-rate\n");
+				return -EINVAL;
+			}
+			/* Implement 0 = slow, 3 = fast slew rate */
+			val = (3 - param) << 6;
+		}
+		if (of_property_read_bool(client->dev.of_node, "dvi-mode"))
+			val |= BIT(5);
+		break;
+	default:
+		/* DDC_DR_SEL=1 DDC_TRAIN_SETDISABLE=1 */
+		val = BIT(2) | BIT(0);
+		break;
+	}
+
+	/* termination for HDMI TX: 0=off, 1=150..300, 3=75..150 ohms */
+	if (!of_property_read_u32(client->dev.of_node, "termination", &param))
+		val |= ((param & 0x3) << 3);
+
+	ret = regmap_write(data->regmap, TMDS181_REG_CTRLB, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "regmap_write(B) failed\n");
+		return ret;
+	}
+
+	val = 0; /* Default for C register */
+	if (!of_property_read_u32(client->dev.of_node, "vswing-data", &param))
+		val |= (param << 5);
+	if (!of_property_read_u32(client->dev.of_node, "vswing-clk", &param))
+		val |= (param << 2);
+	/* Datasheet recommends HDMI_TWPST=0b01 for HDMI compliance */
+	if (of_property_read_bool(client->dev.of_node, "enable-de-emphasis"))
+		val |= 0x01;
+	ret = regmap_write(data->regmap, TMDS181_REG_CTRLC, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "regmap_write(C) failed\n");
+		return ret;
+	}
+
+	/* DIS_HDMI2_SWG: HDMI 1.x only, keep clock at full rate */
+	val = BIT(0);
+	if (!of_property_read_u32(client->dev.of_node, "eq-data", &param)) {
+		val |= (param << 3);
+		/* If defined, also force the "fixed" equalizer mode */
+		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA, BIT(5), 0);
+	}
+	if (!of_property_read_u32(client->dev.of_node, "eq-clk", &param)) {
+		val |= (param << 1);
+		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA, BIT(5), 0);
+	}
+	ret = regmap_write(data->regmap, TMDS181_REG_EQUALIZER, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "regmap_write(EQUALIZER) failed\n");
+		return ret;
+	}
+
+	ret = tmds181_apply_changes(data);
+	if (ret < 0) {
+		dev_err(&client->dev, "tmds181_apply_changes failed\n");
+		return ret;
+	}
+
+	ret = sysfs_create_group(&client->dev.kobj, &tmds181_attr_group);
+	if (ret)
+		dev_err(&client->dev, "sysfs_create_group error: %d\n", 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.funcs = &tmds181_bridge_funcs;
+	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", },
+	{ .compatible = "ti,sn65dp159", },
+	{}
+};
+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 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3999 bytes --]

On Tue, Aug 12, 2025 at 04:51:34PM +0200, Mike Looijmans wrote:
> Add DT binding document for TI TMDS181 and SN65DP159 HDMI retimers.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
> 
>  .../bindings/display/bridge/ti,tmds181.yaml   | 104 ++++++++++++++++++
>  1 file changed, 104 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..87ffb556c4ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
> @@ -0,0 +1,104 @@
> +# 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/TMDS181

These two links are the same.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,tmds181
> +      - ti,sn65dp159

The driver contains:
+	{ .compatible = "ti,tmds181", },
+	{ .compatible = "ti,sn65dp159", },
+	{}
so why is a fallback compatible not suitable here?

Otherwise, this looks fine to me.

> +
> +  reg:
> +    enum:
> +      - 0x5b
> +      - 0x5c
> +      - 0x5d
> +      - 0x5e
> +
> +  oe-gpios:
> +    maxItems: 1
> +    description: GPIO specifier for OE pin (active high).
> +
> +  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
> +
> +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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 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/20250812-225413
base:   53e760d8949895390e256e723e7ee46618310361
patch link:    https://lore.kernel.org/r/20250812145256.135645-3-mike.looijmans%40topic.nl
patch subject: [PATCH 2/2] drm: bridge: Add TI tmds181 and sn65dp159 driver
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20250813/202508131549.DaogZRF9-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 3769ce013be2879bf0b329c14a16f5cb766f26ce)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250813/202508131549.DaogZRF9-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/202508131549.DaogZRF9-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/ti-tmds181.c:273:41: warning: initializer-string for character array is too long, array size is 8 but initializer has size 9 (including the null terminating character); did you mean to use the 'nonstring' attribute? [-Wunterminated-string-initialization]
     273 | static const u8 tmds181_id_tmds181[8] = "TMDS181 ";
         |                                         ^~~~~~~~~~
   drivers/gpu/drm/bridge/ti-tmds181.c:274:41: warning: initializer-string for character array is too long, array size is 8 but initializer has size 9 (including the null terminating character); did you mean to use the 'nonstring' attribute? [-Wunterminated-string-initialization]
     274 | static const u8 tmds181_id_dp159[8]   = "DP159   ";
         |                                         ^~~~~~~~~~
   2 warnings generated.


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

   272	
 > 273	static const u8 tmds181_id_tmds181[8] = "TMDS181 ";
   274	static const u8 tmds181_id_dp159[8]   = "DP159   ";
   275	

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

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

[Mike Looijmans]

On 12-08-2025 19:58, Conor Dooley wrote:
> On Tue, Aug 12, 2025 at 04:51:34PM +0200, Mike Looijmans wrote:
>> Add DT binding document for TI TMDS181 and SN65DP159 HDMI retimers.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>>
>>   .../bindings/display/bridge/ti,tmds181.yaml   | 104 ++++++++++++++++++
>>   1 file changed, 104 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..87ffb556c4ad
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
>> @@ -0,0 +1,104 @@
>> +# 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/TMDS181
> These two links are the same.

Will fix in v3


>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,tmds181
>> +      - ti,sn65dp159
> The driver contains:
> +	{ .compatible = "ti,tmds181", },
> +	{ .compatible = "ti,sn65dp159", },
> +	{}
> so why is a fallback compatible not suitable here?

I don't understand the question. The two are slightly different chips, 
so it makes sense to describe that in the DT.

-- 
Mike Looijmans

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

[Krzysztof Kozlowski]

On 19/08/2025 09:46, Mike Looijmans wrote:
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,tmds181
>>> +      - ti,sn65dp159
>> The driver contains:
>> +	{ .compatible = "ti,tmds181", },
>> +	{ .compatible = "ti,sn65dp159", },
>> +	{}
>> so why is a fallback compatible not suitable here?
> 
> I don't understand the question. The two are slightly different chips, 

Your driver says they are compatible. No one said the same, but compatible.

> so it makes sense to describe that in the DT.

Compatible devices should use fallback. There is plenty of examples (90%
of all binding files?) including example-schema describing this.

> 


Best regards,
Krzysztof

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

[Mike Looijmans]

On 19-08-2025 09:51, Krzysztof Kozlowski wrote:
> On 19/08/2025 09:46, Mike Looijmans wrote:
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - ti,tmds181
>>>> +      - ti,sn65dp159
>>> The driver contains:
>>> +	{ .compatible = "ti,tmds181", },
>>> +	{ .compatible = "ti,sn65dp159", },
>>> +	{}
>>> so why is a fallback compatible not suitable here?
>> I don't understand the question. The two are slightly different chips,
> Your driver says they are compatible. No one said the same, but compatible.
>
>> so it makes sense to describe that in the DT.
> Compatible devices should use fallback. There is plenty of examples (90%
> of all binding files?) including example-schema describing this.

Please help me out here, I'm happy to oblige, but I don't understand 
what you're asking.

To the best of my knowledge "fallback" compatible is when you write 
something like this in the device-tree:
    compatible = "st,m25p80", "jedec,spi-nor";
Which means that we can use the "jedec,spi-nor" driver if there's no 
specific match for "st,m25p80", correct?

I don't understand how that relates to your request, this is the first 
time I ever got this particular feedback. Looking at say the 
ti,sn65dsi83 driver, it does the same thing (supports the ti,sn65dsi83 
and ti,sn65dsi84).

Please explain or point me somewhere where I can find this?

>
>
> 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 2/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[Dmitry Baryshkov]

On Tue, Aug 12, 2025 at 04:51: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>
> ---
> 
>  drivers/gpu/drm/bridge/Kconfig      |  11 +
>  drivers/gpu/drm/bridge/Makefile     |   1 +
>  drivers/gpu/drm/bridge/ti-tmds181.c | 512 ++++++++++++++++++++++++++++
>  3 files changed, 524 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..6fbbc13ddc10
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-tmds181.c
> @@ -0,0 +1,512 @@
> +// 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/module.h>
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/delay.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
> +#define TMDS181_REG_EYESCAN	0xE

Usually it's recommended to use lowercase hex.

> +
> +enum tmds181_chip {
> +	tmds181,
> +	dp159,
> +};
> +
> +struct tmds181_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	struct drm_bridge *next_bridge;
> +	struct drm_bridge bridge;
> +	enum tmds181_chip chip;
> +};
> +

[...]

> +
> +static const char * const tmds181_modes[] = {
> +	"redriver",
> +	"auto1",
> +	"auto2",
> +	"retimer",
> +};
> +
> +static ssize_t mode_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct tmds181_data *data = dev_get_drvdata(dev);
> +	const char *equalizer;
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, TMDS181_REG_CTRLA, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (val & BIT(4)) {
> +		if (val & BIT(5))
> +			equalizer = "eq-adaptive";
> +		else
> +			equalizer = "eq-fixed";
> +	} else {
> +		equalizer = "eq-disabled";
> +	}
> +
> +	return scnprintf(buf, PAGE_SIZE, "%6s %s %s\n",
> +			(val & BIT(7)) ? "sink" : "source",
> +			tmds181_modes[val & 0x03],
> +			equalizer);
> +}
> +
> +static ssize_t mode_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct tmds181_data *data = dev_get_drvdata(dev);
> +	u32 val;
> +	int ret;
> +	int i;
> +
> +	/* Strip trailing newline(s) for being user friendly */
> +	while (len && buf[len] == '\n')
> +		--len;
> +
> +	/* Need at least 4 actual characters */
> +	if (len < 4)
> +		return -EINVAL;
> +
> +	ret = regmap_read(data->regmap, TMDS181_REG_CTRLA, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(tmds181_modes); ++i) {
> +		if (strncmp(tmds181_modes[i], buf, len) == 0) {
> +			val &= ~0x03;
> +			val |= i;
> +			break;
> +		}
> +	}
> +
> +	if (strncmp("sink", buf, len) == 0)
> +		val |= BIT(7);
> +
> +	if (strncmp("source", buf, len) == 0)
> +		val &= ~BIT(7);
> +
> +	if (strncmp("eq-", buf, 3) == 0) {
> +		switch (buf[3]) {
> +		case 'a': /* adaptive */
> +			val |= BIT(4) | BIT(5);
> +			break;
> +		case 'f': /* fixed */
> +			val |= BIT(4);
> +			val &= ~BIT(5);
> +			break;
> +		case 'd': /* disable */
> +			val &= ~(BIT(4) | BIT(5));
> +			break;
> +		}
> +	}
> +
> +	/* Always set the "apply changes" bit */
> +	val |= BIT(2);
> +
> +	ret = regmap_write(data->regmap, TMDS181_REG_CTRLA, val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return len;
> +}
> +
> +/* termination for HDMI TX: 0=off, 1=150..300, 3=75..150 ohms */
> +static ssize_t termination_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct tmds181_data *data = dev_get_drvdata(dev);
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, TMDS181_REG_CTRLB, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	val >>= 3;
> +	val &= 0x03;
> +
> +	return scnprintf(buf, PAGE_SIZE, "%u\n", val);
> +}
> +
> +static ssize_t termination_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct tmds181_data *data = dev_get_drvdata(dev);
> +	u32 val;
> +	unsigned long newval;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, TMDS181_REG_CTRLB, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = kstrtoul((const char *) buf, 10, &newval);
> +	if (ret)
> +		return ret;
> +
> +	if (newval > 3)
> +		return -EINVAL;
> +
> +	val &= ~(0x03 << 3);
> +	val |= newval << 3;
> +
> +	ret = regmap_write(data->regmap, TMDS181_REG_CTRLB, val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return len;
> +}
> +
> +static DEVICE_ATTR_RW(mode);
> +static DEVICE_ATTR_RW(termination);

Undocumented ABI. Why are they configured at runtime rather than through
the DT / fwnode?

> +
> +static struct attribute *tmds181_attrs[] = {
> +	&dev_attr_mode.attr,
> +	&dev_attr_termination.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group tmds181_attr_group = {
> +	.attrs = tmds181_attrs,
> +};
> +
> +static const u8 tmds181_id_tmds181[8] = "TMDS181 ";
> +static const u8 tmds181_id_dp159[8]   = "DP159   ";
> +
> +static int tmds181_check_id(struct tmds181_data *data)
> +{
> +	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) {
> +		dev_info(&data->client->dev, "Detected: TMDS181\n");
> +		data->chip = tmds181;
> +		return 0;
> +	}
> +
> +	if (memcmp(buffer, tmds181_id_dp159, sizeof(buffer)) == 0) {
> +		dev_info(&data->client->dev, "Detected: DP159\n");
> +		data->chip = dp159;
> +		return 0;
> +	}
> +
> +	dev_err(&data->client->dev, "Unknown or wrong 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 0x15:
> +	case 0x17 ... 0x1F:
> +		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 = 0x20,
> +	.volatile_reg = tmds181_regmap_is_volatile,
> +};
> +
> +static int tmds181_probe(struct i2c_client *client)
> +{
> +	struct tmds181_data *data;
> +	struct gpio_desc *oe_gpio;
> +	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_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	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 */
> +	ret = tmds181_check_id(data);
> +	if (ret)
> +		return ret;
> +
> +	/* We take care of power control, so disable the chips PM functions */
> +	/* SIG_EN=0 PD_EN=1 HPD_AUTO_PWRDWN_DISABLE=1 I2C_DR_CTL=0b11*/
> +	regmap_update_bits(data->regmap, TMDS181_REG_CTRL9, 0x1f, 0x0f);
> +
> +	/* Apply configuration changes */
> +	if (of_property_read_bool(client->dev.of_node, "source-mode"))

Undocumented DT properties. Please document them in DT bindings.

> +		regmap_update_bits(data->regmap,
> +				TMDS181_REG_CTRLA, BIT(7), 0);
> +	if (of_property_read_bool(client->dev.of_node, "sink-mode"))
> +		regmap_update_bits(data->regmap,
> +				TMDS181_REG_CTRLA, BIT(7), BIT(7));
> +	if (of_property_read_bool(client->dev.of_node, "redriver-mode"))
> +		regmap_update_bits(data->regmap,
> +				TMDS181_REG_CTRLA, 0x03, 0x00);
> +	if (of_property_read_bool(client->dev.of_node, "retimer-mode"))
> +		regmap_update_bits(data->regmap,
> +				TMDS181_REG_CTRLA, 0x03, 0x03);
> +	if (of_property_read_bool(client->dev.of_node, "adaptive-equalizer"))
> +		regmap_update_bits(data->regmap,
> +			TMDS181_REG_CTRLA, BIT(4) | BIT(5), BIT(4) | BIT(5));
> +	if (of_property_read_bool(client->dev.of_node, "disable-equalizer"))
> +		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA, BIT(4), 0);
> +
> +	switch (data->chip) {
> +	case dp159:
> +		/*  SLEW=0b00 Mode=HDMI DDC_TRAIN_SET=1 */
> +		val = BIT(0);
> +		/* Default slew rate is max */
> +		param = 3;
> +		if (!of_property_read_u32(client->dev.of_node,
> +					"slew-rate", &param)) {
> +			if (param > 3) {
> +				dev_err(&client->dev, "invalid slew-rate\n");
> +				return -EINVAL;
> +			}
> +			/* Implement 0 = slow, 3 = fast slew rate */
> +			val = (3 - param) << 6;
> +		}
> +		if (of_property_read_bool(client->dev.of_node, "dvi-mode"))
> +			val |= BIT(5);
> +		break;
> +	default:
> +		/* DDC_DR_SEL=1 DDC_TRAIN_SETDISABLE=1 */
> +		val = BIT(2) | BIT(0);
> +		break;
> +	}
> +
> +	/* termination for HDMI TX: 0=off, 1=150..300, 3=75..150 ohms */
> +	if (!of_property_read_u32(client->dev.of_node, "termination", &param))
> +		val |= ((param & 0x3) << 3);
> +
> +	ret = regmap_write(data->regmap, TMDS181_REG_CTRLB, val);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "regmap_write(B) failed\n");
> +		return ret;
> +	}
> +
> +	val = 0; /* Default for C register */
> +	if (!of_property_read_u32(client->dev.of_node, "vswing-data", &param))
> +		val |= (param << 5);
> +	if (!of_property_read_u32(client->dev.of_node, "vswing-clk", &param))
> +		val |= (param << 2);
> +	/* Datasheet recommends HDMI_TWPST=0b01 for HDMI compliance */
> +	if (of_property_read_bool(client->dev.of_node, "enable-de-emphasis"))
> +		val |= 0x01;
> +	ret = regmap_write(data->regmap, TMDS181_REG_CTRLC, val);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "regmap_write(C) failed\n");
> +		return ret;
> +	}
> +
> +	/* DIS_HDMI2_SWG: HDMI 1.x only, keep clock at full rate */
> +	val = BIT(0);
> +	if (!of_property_read_u32(client->dev.of_node, "eq-data", &param)) {
> +		val |= (param << 3);
> +		/* If defined, also force the "fixed" equalizer mode */
> +		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA, BIT(5), 0);
> +	}
> +	if (!of_property_read_u32(client->dev.of_node, "eq-clk", &param)) {
> +		val |= (param << 1);
> +		regmap_update_bits(data->regmap, TMDS181_REG_CTRLA, BIT(5), 0);
> +	}
> +	ret = regmap_write(data->regmap, TMDS181_REG_EQUALIZER, val);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "regmap_write(EQUALIZER) failed\n");
> +		return ret;
> +	}
> +
> +	ret = tmds181_apply_changes(data);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "tmds181_apply_changes failed\n");
> +		return ret;
> +	}
> +
> +	ret = sysfs_create_group(&client->dev.kobj, &tmds181_attr_group);
> +	if (ret)
> +		dev_err(&client->dev, "sysfs_create_group error: %d\n", 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.funcs = &tmds181_bridge_funcs;
> +	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", },
> +	{ .compatible = "ti,sn65dp159", },
> +	{}
> +};
> +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

-- 
With best wishes
Dmitry

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

[Mike Looijmans]

On 19-08-2025 12:26, Dmitry Baryshkov wrote:
> On Tue, Aug 12, 2025 at 04:51: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>
>> ---
>>
>>   drivers/gpu/drm/bridge/Kconfig      |  11 +
>>   drivers/gpu/drm/bridge/Makefile     |   1 +
>>   drivers/gpu/drm/bridge/ti-tmds181.c | 512 ++++++++++++++++++++++++++++
>>   3 files changed, 524 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..6fbbc13ddc10
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/ti-tmds181.c
>> @@ -0,0 +1,512 @@
>> +// 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/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/i2c.h>
>> +#include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/regmap.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/delay.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
>> +#define TMDS181_REG_EYESCAN	0xE
> Usually it's recommended to use lowercase hex.

Will change in v3


>> ...
>> +	/* Apply configuration changes */
>> +	if (of_property_read_bool(client->dev.of_node, "source-mode"))
> Undocumented DT properties. Please document them in DT bindings.

Damn, this is the wrong version of the file, this looks like it's just 
v1 again. I'll post v3 ASAP.


> ...


-- 
Mike Looijmans

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]

On Tue, Aug 19, 2025 at 10:26:15AM +0200, Mike Looijmans wrote:
> On 19-08-2025 09:51, Krzysztof Kozlowski wrote:
> > On 19/08/2025 09:46, Mike Looijmans wrote:
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    enum:
> > > > > +      - ti,tmds181
> > > > > +      - ti,sn65dp159
> > > > The driver contains:
> > > > +	{ .compatible = "ti,tmds181", },
> > > > +	{ .compatible = "ti,sn65dp159", },
> > > > +	{}
> > > > so why is a fallback compatible not suitable here?
> > > I don't understand the question. The two are slightly different chips,
> > Your driver says they are compatible. No one said the same, but compatible.
> > 
> > > so it makes sense to describe that in the DT.
> > Compatible devices should use fallback. There is plenty of examples (90%
> > of all binding files?) including example-schema describing this.
> 
> Please help me out here, I'm happy to oblige, but I don't understand what
> you're asking.
> 
> To the best of my knowledge "fallback" compatible is when you write
> something like this in the device-tree:
>    compatible = "st,m25p80", "jedec,spi-nor";
> Which means that we can use the "jedec,spi-nor" driver if there's no
> specific match for "st,m25p80", correct?
> 
> I don't understand how that relates to your request, this is the first time
> I ever got this particular feedback. Looking at say the ti,sn65dsi83 driver,
> it does the same thing (supports the ti,sn65dsi83 and ti,sn65dsi84).
> 
> Please explain or point me somewhere where I can find this?

Devices that are supersets of, or functionally identical to, others should
use fallback compatibles. The driver treats these devices as functionally
identical to one another when it comes to match data (as there is none)
so you need to either use a fallback compatible or explain in your
commit message why one is not suitable here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Krzysztof Kozlowski]

On 19/08/2025 10:26, Mike Looijmans wrote:
> On 19-08-2025 09:51, Krzysztof Kozlowski wrote:
>> On 19/08/2025 09:46, Mike Looijmans wrote:
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - ti,tmds181
>>>>> +      - ti,sn65dp159
>>>> The driver contains:
>>>> +	{ .compatible = "ti,tmds181", },
>>>> +	{ .compatible = "ti,sn65dp159", },
>>>> +	{}
>>>> so why is a fallback compatible not suitable here?
>>> I don't understand the question. The two are slightly different chips,
>> Your driver says they are compatible. No one said the same, but compatible.
>>
>>> so it makes sense to describe that in the DT.
>> Compatible devices should use fallback. There is plenty of examples (90%
>> of all binding files?) including example-schema describing this.
> 
> Please help me out here, I'm happy to oblige, but I don't understand 
> what you're asking.
> 
> To the best of my knowledge "fallback" compatible is when you write 
> something like this in the device-tree:
>     compatible = "st,m25p80", "jedec,spi-nor";
> Which means that we can use the "jedec,spi-nor" driver if there's no 
> specific match for "st,m25p80", correct?

Yes.

> 
> I don't understand how that relates to your request, this is the first 
> time I ever got this particular feedback. Looking at say the 
> ti,sn65dsi83 driver, it does the same thing (supports the ti,sn65dsi83 
> and ti,sn65dsi84).
> 
> Please explain or point me somewhere where I can find this?
I already pointed out to example-schema.

Also, e.g. first file in iio/adc:
adi,ad4000.yaml


Best regards,
Krzysztof

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

[Mike Looijmans]

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
On 20-08-2025 08:44, Krzysztof Kozlowski wrote:
> On 19/08/2025 10:26, Mike Looijmans wrote:
>> On 19-08-2025 09:51, Krzysztof Kozlowski wrote:
>>> On 19/08/2025 09:46, Mike Looijmans wrote:
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    enum:
>>>>>> +      - ti,tmds181
>>>>>> +      - ti,sn65dp159
>>>>> The driver contains:
>>>>> +	{ .compatible = "ti,tmds181", },
>>>>> +	{ .compatible = "ti,sn65dp159", },
>>>>> +	{}
>>>>> so why is a fallback compatible not suitable here?
>>>> I don't understand the question. The two are slightly different chips,
>>> Your driver says they are compatible. No one said the same, but compatible.
>>>
>>>> so it makes sense to describe that in the DT.
>>> Compatible devices should use fallback. There is plenty of examples (90%
>>> of all binding files?) including example-schema describing this.
>> Please help me out here, I'm happy to oblige, but I don't understand
>> what you're asking.
>>
>> To the best of my knowledge "fallback" compatible is when you write
>> something like this in the device-tree:
>>      compatible = "st,m25p80", "jedec,spi-nor";
>> Which means that we can use the "jedec,spi-nor" driver if there's no
>> specific match for "st,m25p80", correct?
> Yes.
>
>> I don't understand how that relates to your request, this is the first
>> time I ever got this particular feedback. Looking at say the
>> ti,sn65dsi83 driver, it does the same thing (supports the ti,sn65dsi83
>> and ti,sn65dsi84).
>>
>> Please explain or point me somewhere where I can find this?
> I already pointed out to example-schema.
>
> Also, e.g. first file in iio/adc:
> adi,ad4000.yaml
>
I think I get it. Instead of having compatibles "a" and "b" the driver only 
supports "a" in its match table, and the devicetree entry must be either 
compatible="a"; or compatible="b","a". Using compatible="b"; would be disallowed.

I actually planned (I have implemented it locally already for v3) for the 
driver to check the chip type and complain if it doesn't match the devicetree. 
If the wrong device is there, the most likely cause is that the input and 
output buses got mixed up. That would also justify having separate 
compatibles, right?

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

[Krzysztof Kozlowski]

On 20/08/2025 11:37, Mike Looijmans wrote:
> 
> 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 fix your email client not to attach such top signature.

...

>>
>> Also, e.g. first file in iio/adc:
>> adi,ad4000.yaml
>>
> I think I get it. Instead of having compatibles "a" and "b" the driver only 
> supports "a" in its match table, and the devicetree entry must be either 
> compatible="a"; or compatible="b","a". Using compatible="b"; would be disallowed.
> 
> I actually planned (I have implemented it locally already for v3) for the 
> driver to check the chip type and complain if it doesn't match the devicetree. 
> If the wrong device is there, the most likely cause is that the input and 
> output buses got mixed up. That would also justify having separate 

I don't understand why. I don't get what is input and output bus.

Either devices are compatible or not.

If you can check which device you have via registers, then usually they
are compatible.

Best regards,
Krzysztof

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

[Mike Looijmans]

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
On 20-08-2025 13:35, Krzysztof Kozlowski wrote:
> On 20/08/2025 11:37, Mike Looijmans wrote:
> Please fix your email client not to attach such top signature.
>
> ...

It's actually the company IT that does that, but I'll keep doing my best to 
prevent it (though sometimes it kicks in anyway).


>
>>> Also, e.g. first file in iio/adc:
>>> adi,ad4000.yaml
>>>
>> I think I get it. Instead of having compatibles "a" and "b" the driver only
>> supports "a" in its match table, and the devicetree entry must be either
>> compatible="a"; or compatible="b","a". Using compatible="b"; would be disallowed.
>>
>> I actually planned (I have implemented it locally already for v3) for the
>> driver to check the chip type and complain if it doesn't match the devicetree.
>> If the wrong device is there, the most likely cause is that the input and
>> output buses got mixed up. That would also justify having separate
> I don't understand why. I don't get what is input and output bus.
>
> Either devices are compatible or not.
>
> If you can check which device you have via registers, then usually they
> are compatible.
>
They have almost the same registers, but they are intended for opposite sides 
of the HDMI cable. The DP159 variant expects to drive signals into the cable, 
while the TDMS181 expects to receive its input from the cable. If you 
encounter a TMDS181 where you expected the other, something's wrong with your 
board (the board manufacturer placed the wrong chip) or your devicetree (e.g. 
you mixed up the I2C addresses of the receiver and sender). In such case, the 
chip will happily do its thing, but the system may not work as expected.

Valid setups:

CRTC -> pcb -> DP159 -> cable -> hdmi-monitor

CRTC -> pcb -> cable -> TMDS181 -> hdmi-to-lvds -> panel

CRTC -> pcb -> DP159 -> cable -> TMDS181 -> hdmi-to-lvds -> panel

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

On Wed, Aug 20, 2025 at 11:37:24AM +0200, Mike Looijmans wrote:

> I actually planned (I have implemented it locally already for v3) for the
> driver to check the chip type and complain if it doesn't match the
> devicetree. If the wrong device is there, the most likely cause is that the
> input and output buses got mixed up. That would also justify having separate
> compatibles, right?

It's not the kernel's job to verify the devicetree, it should be assumed
to be correct. You're ensuring that when another compatible device
arrives later on that a new string in a binding will not be sufficient
for the device to be supported and driver changes will be required to
make it work.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]