[PATCH v1 00/11] Add ti948 and ti949 display bridge drivers

[PATCH v1 00/11] Add ti948 and ti949 display bridge drivers

[Michael Drake]

This patch series adds support for the ti948 and ti949 display
bridge devices.  They are both regmap i2c device drivers.

The ti949 converts HDMI video signals to FPD-Link III.
The ti948 converts FPD-Link III video signals to OpenLDI.

The drivers support PM suspend/resume, and rely on device tree /
ACPI nodes to set up any device dependency chain.  (ACPI requiring
the special DT namespace link device ID, PRP0001.)  The unified
device properties API is used to get board-specific config from
device tree / ACPI.

Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>

Michael Drake (11):
  dt-bindings: display/bridge: Add bindings for ti948
  ti948: i2c device driver for TI DS90UB948-Q1
  dt-bindings: display/bridge: Add config property for ti948
  ti948: Add support for configuration via device properties
  ti948: Add alive check function using schedule_delayed_work()
  ti948: Reconfigure in the alive check when device returns
  ti948: Add sysfs node for alive attribute
  dt-bindings: display/bridge: Add bindings for ti949
  ti949: i2c device driver for TI DS90UB949-Q1
  dt-bindings: display/bridge: Add config property for ti949
  ti949: Add support for configuration via device properties

 .../bindings/display/bridge/ti,ds90ub948.txt  |  45 ++
 .../bindings/display/bridge/ti,ds90ub949.txt  |  37 +
 drivers/gpu/drm/bridge/Kconfig                |  16 +
 drivers/gpu/drm/bridge/Makefile               |   2 +
 drivers/gpu/drm/bridge/ti948.c                | 689 ++++++++++++++++++
 drivers/gpu/drm/bridge/ti949.c                | 631 ++++++++++++++++
 6 files changed, 1420 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
 create mode 100644 drivers/gpu/drm/bridge/ti948.c
 create mode 100644 drivers/gpu/drm/bridge/ti949.c

-- 
2.20.1

[PATCH v1 01/11] dt-bindings: display/bridge: Add bindings for ti948

[Michael Drake]

Adds device tree bindings for:

  TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer

The device has the compatible string "ti,ds90ub948", and
and allows an arrray of strings to be provided as regulator
names to enable for operation of the device.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 .../bindings/display/bridge/ti,ds90ub948.txt  | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
new file mode 100644
index 000000000000..f9e86cb22900
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
@@ -0,0 +1,24 @@
+TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer
+=======================================================
+
+This is the binding for Texas Instruments DS90UB948-Q1 bridge deserializer.
+
+This device supports I2C only.
+
+Required properties:
+
+- compatible: "ti,ds90ub948"
+
+Optional properties:
+
+- regulators: List of regulator name strings to enable for operation of device.
+
+Example
+-------
+
+ti948: ds90ub948@0 {
+	compatible = "ti,ds90ub948";
+
+	regulators: "vcc",
+	            "vcc_disp";
+};
-- 
2.20.1

Re: [PATCH v1 01/11] dt-bindings: display/bridge: Add bindings for ti948

[Laurent Pinchart]

Hi Michael,

Thank you for the patch.

On Tue, Jun 11, 2019 at 03:04:02PM +0100, Michael Drake wrote:
> Adds device tree bindings for:
> 
>   TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer
> 
> The device has the compatible string "ti,ds90ub948", and
> and allows an arrray of strings to be provided as regulator

s/arrray/array/

> names to enable for operation of the device.
> 
> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
> Cc: Patrick Glaser <pglaser@tesla.com>
> Cc: Nate Case <ncase@tesla.com>
> ---
>  .../bindings/display/bridge/ti,ds90ub948.txt  | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
> new file mode 100644
> index 000000000000..f9e86cb22900
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
> @@ -0,0 +1,24 @@
> +TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer
> +=======================================================
> +
> +This is the binding for Texas Instruments DS90UB948-Q1 bridge deserializer.
> +
> +This device supports I2C only.

Then the DT node should be a child of its I2C controller, and have a reg
property.

> +
> +Required properties:
> +
> +- compatible: "ti,ds90ub948"
> +
> +Optional properties:
> +
> +- regulators: List of regulator name strings to enable for operation of device.

There's a standard binding for regulators, using *-supply properties.
Please use them.

You should also use the OF graph DT bindings to model the data
connections.

> +
> +Example
> +-------
> +
> +ti948: ds90ub948@0 {
> +	compatible = "ti,ds90ub948";
> +
> +	regulators: "vcc",
> +	            "vcc_disp";
> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 01/11] dt-bindings: display/bridge: Add bindings for ti948

[Michael Drake]

Hi Laurent,

On 11/06/2019 19:03, Laurent Pinchart wrote:
> Hi Michael,
> 
> Thank you for the patch.

My pleasure, and thank you for the feedback!  I'm sorry it's
taken me a while to respond to it.

> On Tue, Jun 11, 2019 at 03:04:02PM +0100, Michael Drake wrote:
>> Adds device tree bindings for:
>>
>>   TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer
>>
>> The device has the compatible string "ti,ds90ub948", and
>> and allows an arrray of strings to be provided as regulator
> 
> s/arrray/array/

Thanks.  Fixed for the next version.

>> names to enable for operation of the device.
>>
>> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
>> Cc: Patrick Glaser <pglaser@tesla.com>
>> Cc: Nate Case <ncase@tesla.com>
>> ---
>>  .../bindings/display/bridge/ti,ds90ub948.txt  | 24 +++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
>> new file mode 100644
>> index 000000000000..f9e86cb22900
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
>> @@ -0,0 +1,24 @@
>> +TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer
>> +=======================================================
>> +
>> +This is the binding for Texas Instruments DS90UB948-Q1 bridge deserializer.
>> +
>> +This device supports I2C only.
> 
> Then the DT node should be a child of its I2C controller, and have a reg
> property.

OK, thank you.  I'm not actually using DT, I'm using ACPI tables.

For DT, would this cause the device to get bound to the driver on
startup?

I tried adding a reg property with the value set to the device's
I2C address with device tree compatible properties, coupled with
the DT "compatible" string property, but that was insufficient to
bind the device to the driver.  At the moment, I'm using the ACPI
mechanism for achieving this.

So all I've done for this is add the "reg" property to the device
tree ti,ds90ub948.txt documentation file, in the Example section.

Is this what you were expecting?

For the other point, that the DT node should be a child of its I2C
controller, I've made the devices descendants of the I2C controller
in ACPI, but one of them is not a direct child.

The ti949 is a child of its I2C controller node, but the ti948
is a child of the ti949 node in the ACPI table.  I did this to
ensure that linux was aware of the dependency of the devices, so
that they would be brought up in the correct order and suspended
and resumed in the correct order.

>> +
>> +Required properties:
>> +
>> +- compatible: "ti,ds90ub948"
>> +
>> +Optional properties:
>> +
>> +- regulators: List of regulator name strings to enable for operation of device.
> 
> There's a standard binding for regulators, using *-supply properties.
> Please use them.

Thank you!  I've switched to using that, and it is less code.  :)

> You should also use the OF graph DT bindings to model the data
> connections.

I'm not clear on the implications of this from the driver code.
Does it need any code changes to the driver to support these
endpoints?  How are they used?

If it's just a documentation thing, I can easily do it, but
otherwise, I can't test Device Tree because we're using ACPI.

>> +
>> +Example
>> +-------
>> +
>> +ti948: ds90ub948@0 {
>> +	compatible = "ti,ds90ub948";
>> +
>> +	regulators: "vcc",
>> +	            "vcc_disp";
>> +};
> 

-- 
Michael Drake                 https://www.codethink.co.uk/

[PATCH v1 02/11] ti948: i2c device driver for TI DS90UB948-Q1

[Michael Drake]

This is a regmap i2c device driver for:

  TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer

It supports instantiation via device tree / ACPI table.

A list of regulators to enable for use of the device (e.g.
GPIOs for turning on power) may be provided as device tree
properties.  These are enabled on probe and PM resume.
They are disabled on remove and PM suspend.

Datasheet: http://www.ti.com/lit/ds/symlink/ds90ub948-q1.pdf

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 drivers/gpu/drm/bridge/Kconfig  |   8 +
 drivers/gpu/drm/bridge/Makefile |   1 +
 drivers/gpu/drm/bridge/ti948.c  | 509 ++++++++++++++++++++++++++++++++
 3 files changed, 518 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti948.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ee777469293a..149692dc8d48 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -81,6 +81,14 @@ config DRM_PARADE_PS8622
 	---help---
 	  Parade eDP-LVDS bridge chip driver.
 
+config DRM_TI948
+	tristate "TI DS90UB948-Q1 FPD-Link III to OpenLDI Deserializer"
+	select DRM_KMS_HELPER
+	select REGMAP_I2C
+	help
+	  This is a driver for the TI DS90UB948-Q1 device.
+	  It converts video signals from FPD-Link III to OpenLDI.
+
 config DRM_SIL_SII8620
 	tristate "Silicon Image SII8620 HDMI/MHL bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 4934fcf5a6f8..3fb37bbe10e1 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
 obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
+obj-$(CONFIG_DRM_TI948) += ti948.o
 obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
 obj-$(CONFIG_DRM_SII9234) += sii9234.o
diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
new file mode 100644
index 000000000000..c22252036bbe
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti948.c
@@ -0,0 +1,509 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer driver
+ *
+ * Copyright (C) 2019 Tesla Motors, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/regulator/consumer.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+
+/* Number of times to try checking for device on bringup. */
+#define TI948_DEVICE_ID_TRIES 10
+
+/**
+ * enum ti948_reg - TI948 registers.
+ *
+ * DS90UB948-Q1: 7.7: Table 11 DS90UB948-Q1 Registers
+ *
+ * http://www.ti.com/lit/ds/symlink/ds90ub948-q1.pdf
+ */
+enum ti948_reg {
+	TI948_REG_I2C_DEVICE_ID                 = 0x00,
+	TI948_REG_RESET                         = 0x01,
+	TI948_REG_GENERAL_CONFIGURATION_0       = 0x02,
+	TI948_REG_GENERAL_CONFIGURATION_1       = 0x03,
+	TI948_REG_BCC_WATCHDOG_CONTROL          = 0x04,
+	TI948_REG_I2C_CONTROL_1                 = 0x05,
+	TI948_REG_I2C_CONTROL_2                 = 0x06,
+	TI948_REG_REMOTE_ID                     = 0x07,
+	TI948_REG_SLAVEID_0                     = 0x08,
+	TI948_REG_SLAVEID_1                     = 0x09,
+	TI948_REG_SLAVEID_2                     = 0x0A,
+	TI948_REG_SLAVEID_3                     = 0x0B,
+	TI948_REG_SLAVEID_4                     = 0x0C,
+	TI948_REG_SLAVEID_5                     = 0x0D,
+	TI948_REG_SLAVEID_6                     = 0x0E,
+	TI948_REG_SLAVEID_7                     = 0x0F,
+	TI948_REG_SLAVEALIAS_0                  = 0x10,
+	TI948_REG_SLAVEALIAS_1                  = 0x11,
+	TI948_REG_SLAVEALIAS_2                  = 0x12,
+	TI948_REG_SLAVEALIAS_3                  = 0x13,
+	TI948_REG_SLAVEALIAS_4                  = 0x14,
+	TI948_REG_SLAVEALIAS_5                  = 0x15,
+	TI948_REG_SLAVEALIAS_6                  = 0x16,
+	TI948_REG_SLAVEALIAS_7                  = 0x17,
+	TI948_REG_MAILBOX_18                    = 0x18,
+	TI948_REG_MAILBOX_19                    = 0x19,
+	TI948_REG_GPIO_9_AND_GLOBAL_GPIO_CONFIG = 0x1A,
+	TI948_REG_FREQUENCY_COUNTER             = 0x1B,
+	TI948_REG_GENERAL_STATUS                = 0x1C,
+	TI948_REG_GPIO0_CONFIG                  = 0x1D,
+	TI948_REG_GPIO1_2_CONFIG                = 0x1E,
+	TI948_REG_GPIO3_CONFIG                  = 0x1F,
+	TI948_REG_GPIO5_6_CONFIG                = 0x20,
+	TI948_REG_GPIO7_8_CONFIG                = 0x21,
+	TI948_REG_DATAPATH_CONTROL              = 0x22,
+	TI948_REG_RX_MODE_STATUS                = 0x23,
+	TI948_REG_BIST_CONTROL                  = 0x24,
+	TI948_REG_BIST_ERROR_COUNT              = 0x25,
+	TI948_REG_SCL_HIGH_TIME                 = 0x26,
+	TI948_REG_SCL_LOW_TIME                  = 0x27,
+	TI948_REG_DATAPATH_CONTROL_2            = 0x28,
+	TI948_REG_FRC_CONTROL                   = 0x29,
+	TI948_REG_WHITE_BALANCE_CONTROL         = 0x2A,
+	TI948_REG_I2S_CONTROL                   = 0x2B,
+	TI948_REG_PCLK_TEST_MODE                = 0x2E,
+	TI948_REG_DUAL_RX_CTL                   = 0x34,
+	TI948_REG_AEQ_TEST                      = 0x35,
+	TI948_REG_MODE_SEL                      = 0x37,
+	TI948_REG_I2S_DIVSEL                    = 0x3A,
+	TI948_REG_EQ_STATUS                     = 0x3B,
+	TI948_REG_LINK_ERROR_COUNT              = 0x41,
+	TI948_REG_HSCC_CONTROL                  = 0x43,
+	TI948_REG_ADAPTIVE_EQ_BYPASS            = 0x44,
+	TI948_REG_ADAPTIVE_EQ_MIN_MAX           = 0x45,
+	TI948_REG_FPD_TX_MODE                   = 0x49,
+	TI948_REG_LVDS_CONTROL                  = 0x4B,
+	TI948_REG_CML_OUTPUT_CTL1               = 0x52,
+	TI948_REG_CML_OUTPUT_ENABLE             = 0x56,
+	TI948_REG_CML_OUTPUT_CTL2               = 0x57,
+	TI948_REG_CML_OUTPUT_CTL3               = 0x63,
+	TI948_REG_PGCTL                         = 0x64,
+	TI948_REG_PGCFG                         = 0x65,
+	TI948_REG_PGIA                          = 0x66,
+	TI948_REG_PGID                          = 0x67,
+	TI948_REG_PGDBG                         = 0x68,
+	TI948_REG_PGTSTDAT                      = 0x69,
+	TI948_REG_GPI_PIN_STATUS_1              = 0x6E,
+	TI948_REG_GPI_PIN_STATUS_2              = 0x6F,
+	TI948_REG_RX_ID0                        = 0xF0,
+	TI948_REG_RX_ID1                        = 0xF1,
+	TI948_REG_RX_ID2                        = 0xF2,
+	TI948_REG_RX_ID3                        = 0xF3,
+	TI948_REG_RX_ID4                        = 0xF4,
+	TI948_REG_RX_ID5                        = 0xF5
+};
+
+/**
+ * struct ti948_reg_val - ti948 register value
+ * @addr:     The address of the register
+ * @value:    The initial value of the register
+ */
+struct ti948_reg_val {
+	u8 addr;
+	u8 value;
+};
+
+/**
+ * struct ti948_ctx - ti948 driver context
+ * @i2c:         Handle for the device's i2c client.
+ * @regmap:      Handle for the device's regmap.
+ * @reg_names:   Array of regulator names, or NULL.
+ * @regs:        Array of regulators, or NULL.
+ * @reg_count:   Number of entries in reg_names and regs arrays.
+ */
+struct ti948_ctx {
+	struct i2c_client *i2c;
+	struct regmap *regmap;
+	const char **reg_names;
+	struct regulator **regs;
+	size_t reg_count;
+};
+
+static bool ti948_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case TI948_REG_I2C_DEVICE_ID     ... TI948_REG_I2S_CONTROL:
+		/*fall through*/
+	case TI948_REG_PCLK_TEST_MODE:
+		/*fall through*/
+	case TI948_REG_DUAL_RX_CTL       ... TI948_REG_AEQ_TEST:
+		/*fall through*/
+	case TI948_REG_MODE_SEL:
+		/*fall through*/
+	case TI948_REG_I2S_DIVSEL        ... TI948_REG_EQ_STATUS:
+		/*fall through*/
+	case TI948_REG_LINK_ERROR_COUNT:
+		/*fall through*/
+	case TI948_REG_HSCC_CONTROL      ... TI948_REG_ADAPTIVE_EQ_MIN_MAX:
+		/*fall through*/
+	case TI948_REG_FPD_TX_MODE:
+		/*fall through*/
+	case TI948_REG_LVDS_CONTROL:
+		/*fall through*/
+	case TI948_REG_CML_OUTPUT_CTL1:
+		/*fall through*/
+	case TI948_REG_CML_OUTPUT_ENABLE ... TI948_REG_CML_OUTPUT_CTL2:
+		/*fall through*/
+	case TI948_REG_CML_OUTPUT_CTL3   ... TI948_REG_PGTSTDAT:
+		/*fall through*/
+	case TI948_REG_GPI_PIN_STATUS_1  ... TI948_REG_GPI_PIN_STATUS_2:
+		/*fall through*/
+	case TI948_REG_RX_ID0            ... TI948_REG_RX_ID5:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool ti948_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case TI948_REG_I2C_DEVICE_ID     ... TI948_REG_FREQUENCY_COUNTER:
+		/*fall through*/
+	case TI948_REG_GPIO0_CONFIG      ... TI948_REG_BIST_CONTROL:
+		/*fall through*/
+	case TI948_REG_SCL_HIGH_TIME     ... TI948_REG_I2S_CONTROL:
+		/*fall through*/
+	case TI948_REG_PCLK_TEST_MODE:
+		/*fall through*/
+	case TI948_REG_DUAL_RX_CTL       ... TI948_REG_AEQ_TEST:
+		/*fall through*/
+	case TI948_REG_I2S_DIVSEL:
+		/*fall through*/
+	case TI948_REG_LINK_ERROR_COUNT:
+		/*fall through*/
+	case TI948_REG_HSCC_CONTROL      ... TI948_REG_ADAPTIVE_EQ_MIN_MAX:
+		/*fall through*/
+	case TI948_REG_FPD_TX_MODE:
+		/*fall through*/
+	case TI948_REG_LVDS_CONTROL:
+		/*fall through*/
+	case TI948_REG_CML_OUTPUT_CTL1:
+		/*fall through*/
+	case TI948_REG_CML_OUTPUT_ENABLE ... TI948_REG_CML_OUTPUT_CTL2:
+		/*fall through*/
+	case TI948_REG_CML_OUTPUT_CTL3   ... TI948_REG_PGDBG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config ti948_regmap_config = {
+	.val_bits      = 8,
+	.reg_bits      = 8,
+	.max_register  = TI948_REG_RX_ID5,
+	.readable_reg  = ti948_readable_reg,
+	.writeable_reg = ti948_writeable_reg,
+};
+
+static inline u8 ti948_device_address(struct ti948_ctx *ti948)
+{
+	return ti948->i2c->addr;
+}
+
+static inline u8 ti948_device_expected_id(struct ti948_ctx *ti948)
+{
+	return ti948_device_address(ti948) << 1;
+}
+
+static int ti948_device_check(struct ti948_ctx *ti948)
+{
+	unsigned int i2c_device_id;
+	int ret;
+
+	ret = regmap_read(ti948->regmap, TI948_REG_I2C_DEVICE_ID,
+			&i2c_device_id);
+	if (ret < 0) {
+		dev_err(&ti948->i2c->dev,
+				"Failed to read device id. (error %d)\n", ret);
+		return -ENODEV;
+	}
+
+	if (i2c_device_id != ti948_device_expected_id(ti948)) {
+		dev_err(&ti948->i2c->dev,
+				"Bad device ID: Got: %x, Expected: %x\n",
+				i2c_device_id, ti948_device_expected_id(ti948));
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+/**
+ * ti948_device_await() - Repeatedly check the ti948's device ID.
+ * @ti948: ti948 context to check.
+ *
+ * Repeatedly check the ti948's device ID to verify that the ti948 can be
+ * communicated with, and that it is the correct version.
+ *
+ * This function can also be used to wait until the ti948 is ready, after
+ * reset.
+ *
+ * Returns: 0 on success, negative on failure.
+ */
+static int ti948_device_await(struct ti948_ctx *ti948)
+{
+	int tries = 0;
+	int ret;
+
+	do {
+		ret = ti948_device_check(ti948);
+		if (ret < 0)
+			mdelay(1);
+		else
+			break;
+	} while (++tries < TI948_DEVICE_ID_TRIES);
+
+	if (tries >= TI948_DEVICE_ID_TRIES)
+		dev_err(&ti948->i2c->dev,
+				"Failed to read id after %d attempt(s)\n",
+				tries);
+
+	return ret;
+}
+
+static int ti948_power_on(struct ti948_ctx *ti948)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < ti948->reg_count; i++) {
+		dev_info(&ti948->i2c->dev, "Enabling %s regulator\n",
+				ti948->reg_names[i]);
+		ret = regulator_enable(ti948->regs[i]);
+		if (ret < 0) {
+			dev_err(&ti948->i2c->dev,
+					"Could not enable regulator %s: %d\n",
+					ti948->reg_names[i], ret);
+			return ret;
+		}
+
+		msleep(100);
+	}
+
+	ret = ti948_device_await(ti948);
+	if (ret != 0)
+		return ret;
+
+	msleep(500);
+
+	return 0;
+}
+
+static int ti948_power_off(struct ti948_ctx *ti948)
+{
+	int i;
+	int ret;
+
+	for (i = ti948->reg_count; i > 0; i--) {
+		dev_info(&ti948->i2c->dev, "Disabling %s regulator\n",
+				ti948->reg_names[i - 1]);
+		ret = regulator_disable(ti948->regs[i - 1]);
+		if (ret < 0) {
+			dev_err(&ti948->i2c->dev,
+					"Could not disable regulator %s: %d\n",
+					ti948->reg_names[i - 1], ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static inline struct ti948_ctx *ti948_ctx_from_dev(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+
+	return i2c_get_clientdata(client);
+}
+
+static int ti948_pm_resume(struct device *dev)
+{
+	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
+	int ret;
+
+	if (ti948 == NULL)
+		return 0;
+
+	ret = ti948_power_on(ti948);
+	if (ret != 0)
+		return ret;
+
+	return 0;
+}
+
+static int ti948_pm_suspend(struct device *dev)
+{
+	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
+
+	if (ti948 == NULL)
+		return 0;
+
+	return ti948_power_off(ti948);
+}
+
+static int ti948_get_regulator(struct device *dev, const char *id,
+		struct regulator **reg_out)
+{
+	struct regulator *reg;
+
+	reg = devm_regulator_get(dev, id);
+	if (IS_ERR(reg)) {
+		int ret = PTR_ERR(reg);
+
+		dev_err(dev, "Error requesting %s regulator: %d\n", id, ret);
+		return ret;
+	}
+
+	if (reg == NULL)
+		dev_warn(dev, "Could not get %s regulator\n", id);
+	else
+		dev_info(dev, "Got regulator %s\n", id);
+
+	*reg_out = reg;
+	return 0;
+}
+
+static int ti948_get_regulators(struct ti948_ctx *ti948)
+{
+	int i;
+	int ret;
+	size_t regulator_count;
+
+	ret = device_property_read_string_array(&ti948->i2c->dev,
+			"regulators", NULL, 0);
+	if (ret == -EINVAL ||
+	    ret == -ENODATA ||
+	    ret == 0) {
+		/* "regulators" property was either:
+		 *   - unset
+		 *   - valueless
+		 *   - set to empty list
+		 * Not an error; continue without regulators.
+		 */
+		dev_info(&ti948->i2c->dev,
+				"No regulators listed for device.\n");
+		return 0;
+
+	} else if (ret < 0) {
+		return ret;
+	}
+
+	regulator_count = ret;
+
+	ti948->reg_names = devm_kmalloc_array(&ti948->i2c->dev,
+			regulator_count, sizeof(*ti948->reg_names), GFP_KERNEL);
+	if (!ti948->reg_names)
+		return -ENOMEM;
+
+	ret = device_property_read_string_array(&ti948->i2c->dev, "regulators",
+			ti948->reg_names, regulator_count);
+	if (ret < 0)
+		return ret;
+
+	ti948->regs = devm_kmalloc_array(&ti948->i2c->dev,
+			regulator_count, sizeof(*ti948->regs), GFP_KERNEL);
+	if (!ti948->regs)
+		return -ENOMEM;
+
+	for (i = 0; i < regulator_count; i++) {
+		ret = ti948_get_regulator(&ti948->i2c->dev,
+				ti948->reg_names[i], &ti948->regs[i]);
+		if (ret != 0)
+			return ret;
+	}
+
+	ti948->reg_count = regulator_count;
+
+	return 0;
+}
+
+static int ti948_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct ti948_ctx *ti948;
+	int ret;
+
+	dev_info(&client->dev, "Begin probe (addr: %x)\n", client->addr);
+
+	ti948 = devm_kzalloc(&client->dev, sizeof(*ti948), GFP_KERNEL);
+	if (!ti948)
+		return -ENOMEM;
+
+	ti948->i2c = client;
+
+	ti948->regmap = devm_regmap_init_i2c(ti948->i2c, &ti948_regmap_config);
+	if (IS_ERR(ti948->regmap))
+		return PTR_ERR(ti948->regmap);
+
+	ret = ti948_get_regulators(ti948);
+	if (ret != 0)
+		return ret;
+
+	i2c_set_clientdata(client, ti948);
+
+	ret = ti948_pm_resume(&client->dev);
+	if (ret != 0)
+		return -EPROBE_DEFER;
+
+	dev_info(&ti948->i2c->dev, "End probe (addr: %x)\n", ti948->i2c->addr);
+
+	return 0;
+}
+
+static int ti948_remove(struct i2c_client *client)
+{
+	return ti948_pm_suspend(&client->dev);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id ti948_of_match[] = {
+	{ .compatible = "ti,ds90ub948" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ti948_of_match);
+#endif
+
+static const struct i2c_device_id ti948_i2c_id[] = {
+	{"ti948", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, ti948_i2c_id);
+
+static const struct dev_pm_ops ti948_pm_ops = {
+	.resume  = ti948_pm_resume,
+	.suspend = ti948_pm_suspend,
+};
+
+static struct i2c_driver ti948_driver = {
+	.driver = {
+		.name  = "ti948",
+		.owner = THIS_MODULE,
+		.pm    = &ti948_pm_ops,
+		.of_match_table = of_match_ptr(ti948_of_match),
+	},
+	.id_table = ti948_i2c_id,
+	.probe    = ti948_probe,
+	.remove   = ti948_remove,
+};
+module_i2c_driver(ti948_driver);
+
+MODULE_AUTHOR("Michael Drake <michael.drake@codethink.co.uk");
+MODULE_DESCRIPTION("TI DS90UB948-Q1 2K FPD-Link III to OpenLDI Deserializer driver");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1

[PATCH v1 03/11] dt-bindings: display/bridge: Add config property for ti948

[Michael Drake]

The config property can be used to provide an array of
register addresses and values to be written to configure
the device for the board.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 .../bindings/display/bridge/ti,ds90ub948.txt  | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
index f9e86cb22900..1e7033b0f3b7 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
@@ -12,6 +12,8 @@ Required properties:
 Optional properties:
 
 - regulators: List of regulator name strings to enable for operation of device.
+- config: List of <register address>,<value> pairs to be set to configure
+  device on powerup.  The register addresses and values are 8bit.
 
 Example
 -------
@@ -21,4 +23,23 @@ ti948: ds90ub948@0 {
 
 	regulators: "vcc",
 	            "vcc_disp";
+	config:
+	        /* set error count to max */
+	        <0x41>, <0x1f>,
+	        /* sets output mode, no change noticed */
+	        <0x49>, <0xe0>,
+	        /* speed up I2C, 0xE is around 480KHz */
+	        <0x26>, <0x0e>,
+	        /* speed up I2C, 0xE is around 480KHz */
+	        <0x27>, <0x0e>,
+	        /* sets GPIO0 as an input */
+	        <0x1D>, <0x13>,
+	        /* set GPIO2 high, backlight PWM (set to 0x50 for normal use) */
+	        <0x1E>, <0x50>,
+	        /* sets GPIO3 as an output with remote control for touch XRES */
+	        <0x1F>, <0x05>,
+	        /* set GPIO5 high, backlight enable on new display */
+	        <0x20>, <0x09>,
+	        /* set GPIO7 and GPIO8 high to enable touch power and prox sense */
+	        <0x21>, <0x91>;
 };
-- 
2.20.1

Re: [PATCH v1 03/11] dt-bindings: display/bridge: Add config property for ti948

[Laurent Pinchart]

Hi Michael,

Thank you for the patch.

On Tue, Jun 11, 2019 at 03:04:04PM +0100, Michael Drake wrote:
> The config property can be used to provide an array of
> register addresses and values to be written to configure
> the device for the board.

Please don't. DT describes the hardware (or more accurately the system),
it's not meant to store arbitrary configuration data. All the registers
specified below should instead be set by the driver based on a
combination of hardware description and information obtained at runtime.

> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
> Cc: Patrick Glaser <pglaser@tesla.com>
> Cc: Nate Case <ncase@tesla.com>
> ---
>  .../bindings/display/bridge/ti,ds90ub948.txt  | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
> index f9e86cb22900..1e7033b0f3b7 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
> @@ -12,6 +12,8 @@ Required properties:
>  Optional properties:
>  
>  - regulators: List of regulator name strings to enable for operation of device.
> +- config: List of <register address>,<value> pairs to be set to configure
> +  device on powerup.  The register addresses and values are 8bit.
>  
>  Example
>  -------
> @@ -21,4 +23,23 @@ ti948: ds90ub948@0 {
>  
>  	regulators: "vcc",
>  	            "vcc_disp";
> +	config:
> +	        /* set error count to max */
> +	        <0x41>, <0x1f>,
> +	        /* sets output mode, no change noticed */
> +	        <0x49>, <0xe0>,
> +	        /* speed up I2C, 0xE is around 480KHz */
> +	        <0x26>, <0x0e>,
> +	        /* speed up I2C, 0xE is around 480KHz */
> +	        <0x27>, <0x0e>,
> +	        /* sets GPIO0 as an input */
> +	        <0x1D>, <0x13>,
> +	        /* set GPIO2 high, backlight PWM (set to 0x50 for normal use) */
> +	        <0x1E>, <0x50>,
> +	        /* sets GPIO3 as an output with remote control for touch XRES */
> +	        <0x1F>, <0x05>,
> +	        /* set GPIO5 high, backlight enable on new display */
> +	        <0x20>, <0x09>,
> +	        /* set GPIO7 and GPIO8 high to enable touch power and prox sense */
> +	        <0x21>, <0x91>;
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 03/11] dt-bindings: display/bridge: Add config property for ti948

[Michael Drake]

Hi Laurent,

On 11/06/2019 19:07, Laurent Pinchart wrote:
> Hi Michael,
> 
> Thank you for the patch.

My pleasure, and thank you for the feedback!

> On Tue, Jun 11, 2019 at 03:04:04PM +0100, Michael Drake wrote:
>> The config property can be used to provide an array of
>> register addresses and values to be written to configure
>> the device for the board.
> 
> Please don't. DT describes the hardware (or more accurately the system),
> it's not meant to store arbitrary configuration data. All the registers
> specified below should instead be set by the driver based on a
> combination of hardware description and information obtained at runtime.

OK, understood.  I'll work on this.  For some of them
explicit firmware properties would be appropriate.

I'll go through it to ascertain what can be determined
at runtime.

>> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
>> Cc: Patrick Glaser <pglaser@tesla.com>
>> Cc: Nate Case <ncase@tesla.com>
>> ---
>>  .../bindings/display/bridge/ti,ds90ub948.txt  | 21 +++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
>> index f9e86cb22900..1e7033b0f3b7 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub948.txt
>> @@ -12,6 +12,8 @@ Required properties:
>>  Optional properties:
>>  
>>  - regulators: List of regulator name strings to enable for operation of device.
>> +- config: List of <register address>,<value> pairs to be set to configure
>> +  device on powerup.  The register addresses and values are 8bit.
>>  
>>  Example
>>  -------
>> @@ -21,4 +23,23 @@ ti948: ds90ub948@0 {
>>  
>>  	regulators: "vcc",
>>  	            "vcc_disp";
>> +	config:
>> +	        /* set error count to max */
>> +	        <0x41>, <0x1f>,
>> +	        /* sets output mode, no change noticed */
>> +	        <0x49>, <0xe0>,
>> +	        /* speed up I2C, 0xE is around 480KHz */
>> +	        <0x26>, <0x0e>,
>> +	        /* speed up I2C, 0xE is around 480KHz */
>> +	        <0x27>, <0x0e>,
>> +	        /* sets GPIO0 as an input */
>> +	        <0x1D>, <0x13>,
>> +	        /* set GPIO2 high, backlight PWM (set to 0x50 for normal use) */
>> +	        <0x1E>, <0x50>,
>> +	        /* sets GPIO3 as an output with remote control for touch XRES */
>> +	        <0x1F>, <0x05>,
>> +	        /* set GPIO5 high, backlight enable on new display */
>> +	        <0x20>, <0x09>,
>> +	        /* set GPIO7 and GPIO8 high to enable touch power and prox sense */
>> +	        <0x21>, <0x91>;
>>  };
> 

-- 
Michael Drake                 https://www.codethink.co.uk/

[PATCH v1 04/11] ti948: Add support for configuration via device properties

[Michael Drake]

This allows the device to be configured for the board in
device tree, or in ACPI tables.  The device node properties
can provide an array of register addresses and values to be
written to configure the device for the board.

The config is written to the device on probe and on PM resume.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 drivers/gpu/drm/bridge/ti948.c | 106 ++++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
index c22252036bbe..9cb37215f049 100644
--- a/drivers/gpu/drm/bridge/ti948.c
+++ b/drivers/gpu/drm/bridge/ti948.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/delay.h>
+#include <linux/slab.h>
 #include <linux/i2c.h>
 
 /* Number of times to try checking for device on bringup. */
@@ -122,6 +123,8 @@ struct ti948_reg_val {
  * struct ti948_ctx - ti948 driver context
  * @i2c:         Handle for the device's i2c client.
  * @regmap:      Handle for the device's regmap.
+ * @config:      Array of register values loaded from device properties.
+ * @config_len:  Number of entries in config.
  * @reg_names:   Array of regulator names, or NULL.
  * @regs:        Array of regulators, or NULL.
  * @reg_count:   Number of entries in reg_names and regs arrays.
@@ -129,6 +132,8 @@ struct ti948_reg_val {
 struct ti948_ctx {
 	struct i2c_client *i2c;
 	struct regmap *regmap;
+	struct ti948_reg_val *config;
+	size_t config_len;
 	const char **reg_names;
 	struct regulator **regs;
 	size_t reg_count;
@@ -212,6 +217,42 @@ static const struct regmap_config ti948_regmap_config = {
 	.writeable_reg = ti948_writeable_reg,
 };
 
+static int ti948_write_sequence(
+		struct ti948_ctx *ti948,
+		const struct ti948_reg_val *sequence,
+		u32 entries)
+{
+	int i;
+
+	for (i = 0; i < entries; i++) {
+		const struct ti948_reg_val *r = sequence + i;
+		int ret = regmap_write(ti948->regmap, r->addr, r->value);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ti948_write_config_seq(struct ti948_ctx *ti948)
+{
+	int ret;
+
+	if (ti948->config == NULL) {
+		dev_info(&ti948->i2c->dev, "No config for ti948 device\n");
+		return 0;
+	}
+
+	ret = ti948_write_sequence(ti948, ti948->config, ti948->config_len);
+	if (ret < 0)
+		return ret;
+
+	dev_info(&ti948->i2c->dev, "Successfully configured ti948\n");
+
+	return ret;
+}
+
 static inline u8 ti948_device_address(struct ti948_ctx *ti948)
 {
 	return ti948->i2c->addr;
@@ -345,7 +386,7 @@ static int ti948_pm_resume(struct device *dev)
 	if (ret != 0)
 		return ret;
 
-	return 0;
+	return ti948_write_config_seq(ti948);
 }
 
 static int ti948_pm_suspend(struct device *dev)
@@ -434,6 +475,65 @@ static int ti948_get_regulators(struct ti948_ctx *ti948)
 	return 0;
 }
 
+static int ti948_get_config(struct ti948_ctx *ti948)
+{
+	int i;
+	int ret;
+	u8 *config;
+	size_t config_len;
+
+	ret = device_property_read_u8_array(&ti948->i2c->dev,
+			"config", NULL, 0);
+	if (ret == -EINVAL ||
+	    ret == -ENODATA ||
+	    ret == 0) {
+		/* "config" property was either:
+		 *   - unset
+		 *   - valueless
+		 *   - set to empty list
+		 * Not an error; continue without config.
+		 */
+		dev_info(&ti948->i2c->dev, "No config defined for device.\n");
+		return 0;
+
+	} else if (ret < 0) {
+		return ret;
+	} else if (ret & 0x1) {
+		dev_err(&ti948->i2c->dev,
+			"Device property 'config' needs even entry count.\n");
+		return -EINVAL;
+	}
+
+	config_len = ret;
+
+	config = kmalloc_array(config_len, sizeof(*config), GFP_KERNEL);
+	if (!config)
+		return -ENOMEM;
+
+	ret = device_property_read_u8_array(&ti948->i2c->dev, "config",
+			config, config_len);
+	if (ret < 0) {
+		kfree(config);
+		return ret;
+	}
+
+	ti948->config = devm_kmalloc_array(&ti948->i2c->dev,
+			config_len / 2, sizeof(*ti948->config), GFP_KERNEL);
+	if (!ti948->config) {
+		kfree(config);
+		return -ENOMEM;
+	}
+
+	ti948->config_len = config_len / 2;
+	for (i = 0; i < config_len; i += 2) {
+		ti948->config[i / 2].addr = config[i];
+		ti948->config[i / 2].value = config[i + 1];
+	}
+	kfree(config);
+
+	return 0;
+}
+
 static int ti948_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
@@ -456,6 +556,10 @@ static int ti948_probe(struct i2c_client *client,
 	if (ret != 0)
 		return ret;
 
+	ret = ti948_get_config(ti948);
+	if (ret != 0)
+		return ret;
+
 	i2c_set_clientdata(client, ti948);
 
 	ret = ti948_pm_resume(&client->dev);
-- 
2.20.1

[PATCH v1 05/11] ti948: Add alive check function using schedule_delayed_work()

[Michael Drake]

This simply runs the function once every 5 seconds, while the
device is supposed to be active.  The alive check function is
currently simply a stub, that logs it has been called, and
re-inserts itself into the work queue.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 drivers/gpu/drm/bridge/ti948.c | 37 +++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
index 9cb37215f049..86daa3701b91 100644
--- a/drivers/gpu/drm/bridge/ti948.c
+++ b/drivers/gpu/drm/bridge/ti948.c
@@ -16,6 +16,7 @@
 
 #include <linux/regulator/consumer.h>
 #include <linux/of_device.h>
+#include <linux/workqueue.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/delay.h>
@@ -25,6 +26,9 @@
 /* Number of times to try checking for device on bringup. */
 #define TI948_DEVICE_ID_TRIES 10
 
+/* Alive check every 5 seconds. */
+#define TI948_ALIVE_CHECK_DELAY (5 * HZ)
+
 /**
  * enum ti948_reg - TI948 registers.
  *
@@ -374,9 +378,27 @@ static inline struct ti948_ctx *ti948_ctx_from_dev(struct device *dev)
 	return i2c_get_clientdata(client);
 }
 
+static inline struct ti948_ctx *delayed_work_to_ti948_ctx(
+		struct delayed_work *dwork)
+{
+	return container_of(dwork, struct ti948_ctx, alive_check);
+}
+
+static void ti948_alive_check(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct ti948_ctx *ti948 = delayed_work_to_ti948_ctx(dwork);
+
+	dev_info(&ti948->i2c->dev, "%s Alive check!\n", __func__);
+
+	/* Reschedule ourself for the next check. */
+	schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);
+}
+
 static int ti948_pm_resume(struct device *dev)
 {
 	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
+	bool scheduled;
 	int ret;
 
 	if (ti948 == NULL)
@@ -386,7 +408,18 @@ static int ti948_pm_resume(struct device *dev)
 	if (ret != 0)
 		return ret;
 
-	return ti948_write_config_seq(ti948);
+	ret = ti948_write_config_seq(ti948);
+	if (ret != 0)
+		return ret;
+
+	INIT_DELAYED_WORK(&ti948->alive_check, ti948_alive_check);
+
+	scheduled = schedule_delayed_work(
+			&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);
+	if (!scheduled)
+		dev_warn(&ti948->i2c->dev, "Alive check already scheduled\n");
+
+	return 0;
 }
 
 static int ti948_pm_suspend(struct device *dev)
@@ -396,6 +429,8 @@ static int ti948_pm_suspend(struct device *dev)
 	if (ti948 == NULL)
 		return 0;
 
+	cancel_delayed_work_sync(&ti948->alive_check);
+
 	return ti948_power_off(ti948);
 }
 
-- 
2.20.1

[PATCH v1 06/11] ti948: Reconfigure in the alive check when device returns

[Michael Drake]

If the alive check detects a transition to the alive state,
the device configuration is rewritten.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 drivers/gpu/drm/bridge/ti948.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
index 86daa3701b91..b5c766711c4b 100644
--- a/drivers/gpu/drm/bridge/ti948.c
+++ b/drivers/gpu/drm/bridge/ti948.c
@@ -132,6 +132,8 @@ struct ti948_reg_val {
  * @reg_names:   Array of regulator names, or NULL.
  * @regs:        Array of regulators, or NULL.
  * @reg_count:   Number of entries in reg_names and regs arrays.
+ * @alive_check: Context for the alive checking work item.
+ * @alive:       Whether the device is alive or not (alive_check).
  */
 struct ti948_ctx {
 	struct i2c_client *i2c;
@@ -141,6 +143,8 @@ struct ti948_ctx {
 	const char **reg_names;
 	struct regulator **regs;
 	size_t reg_count;
+	struct delayed_work alive_check;
+	bool alive;
 };
 
 static bool ti948_readable_reg(struct device *dev, unsigned int reg)
@@ -346,6 +350,8 @@ static int ti948_power_on(struct ti948_ctx *ti948)
 	if (ret != 0)
 		return ret;
 
+	ti948->alive = true;
+
 	msleep(500);
 
 	return 0;
@@ -356,6 +362,8 @@ static int ti948_power_off(struct ti948_ctx *ti948)
 	int i;
 	int ret;
 
+	ti948->alive = false;
+
 	for (i = ti948->reg_count; i > 0; i--) {
 		dev_info(&ti948->i2c->dev, "Disabling %s regulator\n",
 				ti948->reg_names[i - 1]);
@@ -388,8 +396,17 @@ static void ti948_alive_check(struct work_struct *work)
 {
 	struct delayed_work *dwork = to_delayed_work(work);
 	struct ti948_ctx *ti948 = delayed_work_to_ti948_ctx(dwork);
+	int ret = ti948_device_check(ti948);
 
-	dev_info(&ti948->i2c->dev, "%s Alive check!\n", __func__);
+	if (ti948->alive == false && ret == 0) {
+		dev_info(&ti948->i2c->dev, "Device has come back to life!\n");
+		ti948_write_config_seq(ti948);
+		ti948->alive = true;
+
+	} else if (ti948->alive == true && ret != 0) {
+		dev_info(&ti948->i2c->dev, "Device has stopped responding\n");
+		ti948->alive = false;
+	}
 
 	/* Reschedule ourself for the next check. */
 	schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);
-- 
2.20.1

Re: [PATCH v1 06/11] ti948: Reconfigure in the alive check when device returns

[Laurent Pinchart]

Hi Michael,

Thank you for the patch.

On Tue, Jun 11, 2019 at 03:04:07PM +0100, Michael Drake wrote:
> If the alive check detects a transition to the alive state,
> the device configuration is rewritten.

This seems like a big hack. You will have at the very least to explain
why this is needed, and why you can't configure the device in response
to drm_bridge operation calls.

> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
> Cc: Patrick Glaser <pglaser@tesla.com>
> Cc: Nate Case <ncase@tesla.com>
> ---
>  drivers/gpu/drm/bridge/ti948.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
> index 86daa3701b91..b5c766711c4b 100644
> --- a/drivers/gpu/drm/bridge/ti948.c
> +++ b/drivers/gpu/drm/bridge/ti948.c
> @@ -132,6 +132,8 @@ struct ti948_reg_val {
>   * @reg_names:   Array of regulator names, or NULL.
>   * @regs:        Array of regulators, or NULL.
>   * @reg_count:   Number of entries in reg_names and regs arrays.
> + * @alive_check: Context for the alive checking work item.
> + * @alive:       Whether the device is alive or not (alive_check).
>   */
>  struct ti948_ctx {
>  	struct i2c_client *i2c;
> @@ -141,6 +143,8 @@ struct ti948_ctx {
>  	const char **reg_names;
>  	struct regulator **regs;
>  	size_t reg_count;
> +	struct delayed_work alive_check;
> +	bool alive;
>  };
>  
>  static bool ti948_readable_reg(struct device *dev, unsigned int reg)
> @@ -346,6 +350,8 @@ static int ti948_power_on(struct ti948_ctx *ti948)
>  	if (ret != 0)
>  		return ret;
>  
> +	ti948->alive = true;
> +
>  	msleep(500);
>  
>  	return 0;
> @@ -356,6 +362,8 @@ static int ti948_power_off(struct ti948_ctx *ti948)
>  	int i;
>  	int ret;
>  
> +	ti948->alive = false;
> +
>  	for (i = ti948->reg_count; i > 0; i--) {
>  		dev_info(&ti948->i2c->dev, "Disabling %s regulator\n",
>  				ti948->reg_names[i - 1]);
> @@ -388,8 +396,17 @@ static void ti948_alive_check(struct work_struct *work)
>  {
>  	struct delayed_work *dwork = to_delayed_work(work);
>  	struct ti948_ctx *ti948 = delayed_work_to_ti948_ctx(dwork);
> +	int ret = ti948_device_check(ti948);
>  
> -	dev_info(&ti948->i2c->dev, "%s Alive check!\n", __func__);
> +	if (ti948->alive == false && ret == 0) {
> +		dev_info(&ti948->i2c->dev, "Device has come back to life!\n");
> +		ti948_write_config_seq(ti948);
> +		ti948->alive = true;
> +
> +	} else if (ti948->alive == true && ret != 0) {
> +		dev_info(&ti948->i2c->dev, "Device has stopped responding\n");
> +		ti948->alive = false;
> +	}
>  
>  	/* Reschedule ourself for the next check. */
>  	schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 06/11] ti948: Reconfigure in the alive check when device returns

[Michael Drake]

Hi Laurent,

On 11/06/2019 19:10, Laurent Pinchart wrote:
> Hi Michael,
> 
> Thank you for the patch.

My pleasure, and thank you for the feedback!

> On Tue, Jun 11, 2019 at 03:04:07PM +0100, Michael Drake wrote:
>> If the alive check detects a transition to the alive state,
>> the device configuration is rewritten.
> 
> This seems like a big hack. You will have at the very least to explain
> why this is needed, and why you can't configure the device in response
> to drm_bridge operation calls.

The ti948 device is situated inside the housing of the LCD
panel.  The ti949 is a normal i2c device on the system i2c
bus.  In order for the ti948 to appear on the system's i2c
bus, the ti949 must be powered up and configured.

The panel is connected to the system via the FPD-Link III.
If a connector for the wire to the display is unplugged and
re-inserted, then the ti948 will drop out and come back.

Application note AN-2173, "I2C Communication Over
FPD-LinkIII with Bidirectional Control Channel" describes
this:

  http://www.ti.com/lit/an/snla131a/snla131a.pdf

Perhaps I need to expand the commit message to explain this?

Alternatively is there a more standard way of dealing with
remotely connected i2c devices?

>> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
>> Cc: Patrick Glaser <pglaser@tesla.com>
>> Cc: Nate Case <ncase@tesla.com>
>> ---
>>  drivers/gpu/drm/bridge/ti948.c | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
>> index 86daa3701b91..b5c766711c4b 100644
>> --- a/drivers/gpu/drm/bridge/ti948.c
>> +++ b/drivers/gpu/drm/bridge/ti948.c
>> @@ -132,6 +132,8 @@ struct ti948_reg_val {
>>   * @reg_names:   Array of regulator names, or NULL.
>>   * @regs:        Array of regulators, or NULL.
>>   * @reg_count:   Number of entries in reg_names and regs arrays.
>> + * @alive_check: Context for the alive checking work item.
>> + * @alive:       Whether the device is alive or not (alive_check).
>>   */
>>  struct ti948_ctx {
>>  	struct i2c_client *i2c;
>> @@ -141,6 +143,8 @@ struct ti948_ctx {
>>  	const char **reg_names;
>>  	struct regulator **regs;
>>  	size_t reg_count;
>> +	struct delayed_work alive_check;
>> +	bool alive;
>>  };
>>  
>>  static bool ti948_readable_reg(struct device *dev, unsigned int reg)
>> @@ -346,6 +350,8 @@ static int ti948_power_on(struct ti948_ctx *ti948)
>>  	if (ret != 0)
>>  		return ret;
>>  
>> +	ti948->alive = true;
>> +
>>  	msleep(500);
>>  
>>  	return 0;
>> @@ -356,6 +362,8 @@ static int ti948_power_off(struct ti948_ctx *ti948)
>>  	int i;
>>  	int ret;
>>  
>> +	ti948->alive = false;
>> +
>>  	for (i = ti948->reg_count; i > 0; i--) {
>>  		dev_info(&ti948->i2c->dev, "Disabling %s regulator\n",
>>  				ti948->reg_names[i - 1]);
>> @@ -388,8 +396,17 @@ static void ti948_alive_check(struct work_struct *work)
>>  {
>>  	struct delayed_work *dwork = to_delayed_work(work);
>>  	struct ti948_ctx *ti948 = delayed_work_to_ti948_ctx(dwork);
>> +	int ret = ti948_device_check(ti948);
>>  
>> -	dev_info(&ti948->i2c->dev, "%s Alive check!\n", __func__);
>> +	if (ti948->alive == false && ret == 0) {
>> +		dev_info(&ti948->i2c->dev, "Device has come back to life!\n");
>> +		ti948_write_config_seq(ti948);
>> +		ti948->alive = true;
>> +
>> +	} else if (ti948->alive == true && ret != 0) {
>> +		dev_info(&ti948->i2c->dev, "Device has stopped responding\n");
>> +		ti948->alive = false;
>> +	}
>>  
>>  	/* Reschedule ourself for the next check. */
>>  	schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);
> 

-- 
Michael Drake                 https://www.codethink.co.uk/

[PATCH v1 07/11] ti948: Add sysfs node for alive attribute

[Michael Drake]

This may be used by userspace to determine the state
of the device.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 drivers/gpu/drm/bridge/ti948.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
index b5c766711c4b..b624eaeabb43 100644
--- a/drivers/gpu/drm/bridge/ti948.c
+++ b/drivers/gpu/drm/bridge/ti948.c
@@ -412,6 +412,16 @@ static void ti948_alive_check(struct work_struct *work)
 	schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);
 }
 
+static ssize_t alive_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%u\n", (unsigned int)ti948->alive);
+}
+
+static DEVICE_ATTR_RO(alive);
+
 static int ti948_pm_resume(struct device *dev)
 {
 	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
@@ -614,17 +624,31 @@ static int ti948_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, ti948);
 
+	ret = device_create_file(&client->dev, &dev_attr_alive);
+	if (ret) {
+		dev_err(&client->dev, "Could not create alive attr\n");
+		return ret;
+	}
+
 	ret = ti948_pm_resume(&client->dev);
-	if (ret != 0)
-		return -EPROBE_DEFER;
+	if (ret != 0) {
+		ret = -EPROBE_DEFER;
+		goto error;
+	}
 
 	dev_info(&ti948->i2c->dev, "End probe (addr: %x)\n", ti948->i2c->addr);
 
 	return 0;
+
+error:
+	device_remove_file(&client->dev, &dev_attr_alive);
+	return ret;
 }
 
 static int ti948_remove(struct i2c_client *client)
 {
+	device_remove_file(&client->dev, &dev_attr_alive);
+
 	return ti948_pm_suspend(&client->dev);
 }
 
-- 
2.20.1

Re: [PATCH v1 07/11] ti948: Add sysfs node for alive attribute

[Laurent Pinchart]

Hi Michael,

Thank you for the patch.

On Tue, Jun 11, 2019 at 03:04:08PM +0100, Michael Drake wrote:
> This may be used by userspace to determine the state
> of the device.

Why is this needed ? Userspace shouldn't even be aware that this device
exists.

> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
> Cc: Patrick Glaser <pglaser@tesla.com>
> Cc: Nate Case <ncase@tesla.com>
> ---
>  drivers/gpu/drm/bridge/ti948.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
> index b5c766711c4b..b624eaeabb43 100644
> --- a/drivers/gpu/drm/bridge/ti948.c
> +++ b/drivers/gpu/drm/bridge/ti948.c
> @@ -412,6 +412,16 @@ static void ti948_alive_check(struct work_struct *work)
>  	schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);
>  }
>  
> +static ssize_t alive_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%u\n", (unsigned int)ti948->alive);
> +}
> +
> +static DEVICE_ATTR_RO(alive);
> +
>  static int ti948_pm_resume(struct device *dev)
>  {
>  	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
> @@ -614,17 +624,31 @@ static int ti948_probe(struct i2c_client *client,
>  
>  	i2c_set_clientdata(client, ti948);
>  
> +	ret = device_create_file(&client->dev, &dev_attr_alive);
> +	if (ret) {
> +		dev_err(&client->dev, "Could not create alive attr\n");
> +		return ret;
> +	}
> +
>  	ret = ti948_pm_resume(&client->dev);
> -	if (ret != 0)
> -		return -EPROBE_DEFER;
> +	if (ret != 0) {
> +		ret = -EPROBE_DEFER;
> +		goto error;
> +	}
>  
>  	dev_info(&ti948->i2c->dev, "End probe (addr: %x)\n", ti948->i2c->addr);
>  
>  	return 0;
> +
> +error:
> +	device_remove_file(&client->dev, &dev_attr_alive);
> +	return ret;
>  }
>  
>  static int ti948_remove(struct i2c_client *client)
>  {
> +	device_remove_file(&client->dev, &dev_attr_alive);
> +
>  	return ti948_pm_suspend(&client->dev);
>  }
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 07/11] ti948: Add sysfs node for alive attribute

[Laurent Pinchart]

Hi Michael,

Thank you for the patch.

On Tue, Jun 11, 2019 at 03:04:08PM +0100, Michael Drake wrote:
> This may be used by userspace to determine the state
> of the device.

Why is this needed ? Userspace shouldn't even be aware that this device
exists.

> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
> Cc: Patrick Glaser <pglaser@tesla.com>
> Cc: Nate Case <ncase@tesla.com>
> ---
>  drivers/gpu/drm/bridge/ti948.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
> index b5c766711c4b..b624eaeabb43 100644
> --- a/drivers/gpu/drm/bridge/ti948.c
> +++ b/drivers/gpu/drm/bridge/ti948.c
> @@ -412,6 +412,16 @@ static void ti948_alive_check(struct work_struct *work)
>  	schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);
>  }
>  
> +static ssize_t alive_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%u\n", (unsigned int)ti948->alive);
> +}
> +
> +static DEVICE_ATTR_RO(alive);
> +
>  static int ti948_pm_resume(struct device *dev)
>  {
>  	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
> @@ -614,17 +624,31 @@ static int ti948_probe(struct i2c_client *client,
>  
>  	i2c_set_clientdata(client, ti948);
>  
> +	ret = device_create_file(&client->dev, &dev_attr_alive);
> +	if (ret) {
> +		dev_err(&client->dev, "Could not create alive attr\n");
> +		return ret;
> +	}
> +
>  	ret = ti948_pm_resume(&client->dev);
> -	if (ret != 0)
> -		return -EPROBE_DEFER;
> +	if (ret != 0) {
> +		ret = -EPROBE_DEFER;
> +		goto error;
> +	}
>  
>  	dev_info(&ti948->i2c->dev, "End probe (addr: %x)\n", ti948->i2c->addr);
>  
>  	return 0;
> +
> +error:
> +	device_remove_file(&client->dev, &dev_attr_alive);
> +	return ret;
>  }
>  
>  static int ti948_remove(struct i2c_client *client)
>  {
> +	device_remove_file(&client->dev, &dev_attr_alive);
> +
>  	return ti948_pm_suspend(&client->dev);
>  }
>  

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 07/11] ti948: Add sysfs node for alive attribute

[Michael Drake]

Hi Laurent,

On 11/06/2019 19:11, Laurent Pinchart wrote:
> Hi Michael,
> 
> Thank you for the patch.

My pleasure, and thank you for the feedback!

> On Tue, Jun 11, 2019 at 03:04:08PM +0100, Michael Drake wrote:
>> This may be used by userspace to determine the state
>> of the device.
> 
> Why is this needed ? Userspace shouldn't even be aware that this device
> exists.

The display (containing the ti948) could be unplugged.  (See my
response to the feedback on the previous commit in the series.)

If you can suggest a better or more standard way of doing this
I would be very happy to learn of it.

>> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
>> Cc: Patrick Glaser <pglaser@tesla.com>
>> Cc: Nate Case <ncase@tesla.com>
>> ---
>>  drivers/gpu/drm/bridge/ti948.c | 28 ++++++++++++++++++++++++++--
>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c
>> index b5c766711c4b..b624eaeabb43 100644
>> --- a/drivers/gpu/drm/bridge/ti948.c
>> +++ b/drivers/gpu/drm/bridge/ti948.c
>> @@ -412,6 +412,16 @@ static void ti948_alive_check(struct work_struct *work)
>>  	schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY);
>>  }
>>  
>> +static ssize_t alive_show(struct device *dev,
>> +		struct device_attribute *attr, char *buf)
>> +{
>> +	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
>> +
>> +	return scnprintf(buf, PAGE_SIZE, "%u\n", (unsigned int)ti948->alive);
>> +}
>> +
>> +static DEVICE_ATTR_RO(alive);
>> +
>>  static int ti948_pm_resume(struct device *dev)
>>  {
>>  	struct ti948_ctx *ti948 = ti948_ctx_from_dev(dev);
>> @@ -614,17 +624,31 @@ static int ti948_probe(struct i2c_client *client,
>>  
>>  	i2c_set_clientdata(client, ti948);
>>  
>> +	ret = device_create_file(&client->dev, &dev_attr_alive);
>> +	if (ret) {
>> +		dev_err(&client->dev, "Could not create alive attr\n");
>> +		return ret;
>> +	}
>> +
>>  	ret = ti948_pm_resume(&client->dev);
>> -	if (ret != 0)
>> -		return -EPROBE_DEFER;
>> +	if (ret != 0) {
>> +		ret = -EPROBE_DEFER;
>> +		goto error;
>> +	}
>>  
>>  	dev_info(&ti948->i2c->dev, "End probe (addr: %x)\n", ti948->i2c->addr);
>>  
>>  	return 0;
>> +
>> +error:
>> +	device_remove_file(&client->dev, &dev_attr_alive);
>> +	return ret;
>>  }
>>  
>>  static int ti948_remove(struct i2c_client *client)
>>  {
>> +	device_remove_file(&client->dev, &dev_attr_alive);
>> +
>>  	return ti948_pm_suspend(&client->dev);
>>  }
>>  
> 

-- 
Michael Drake                 https://www.codethink.co.uk/

[PATCH v1 08/11] dt-bindings: display/bridge: Add bindings for ti949

[Michael Drake]

Adds device tree bindings for:

  TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer

It supports instantiation via device tree / ACPI table.

The device has the compatible string "ti,ds90ub949", and
and allows an arrray of strings to be provided as regulator
names to enable for operation of the device.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 .../bindings/display/bridge/ti,ds90ub949.txt  | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
new file mode 100644
index 000000000000..3ba3897d5e81
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
@@ -0,0 +1,24 @@
+TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer
+============================================================
+
+This is the binding for Texas Instruments DS90UB949-Q1 bridge serializer.
+
+This device supports I2C only.
+
+Required properties:
+
+- compatible: "ti,ds90ub949"
+
+Optional properties:
+
+- regulators: List of regulator name strings to enable for operation of device.
+
+Example
+-------
+
+ti949: ds90ub949@0 {
+	compatible = "ti,ds90ub949";
+
+	regulators: "vcc",
+	            "vcc_hdmi";
+};
-- 
2.20.1

Re: [PATCH v1 08/11] dt-bindings: display/bridge: Add bindings for ti949

[Laurent Pinchart]

Hi Michael,

Thank you for the patch.

On Tue, Jun 11, 2019 at 03:04:09PM +0100, Michael Drake wrote:
> Adds device tree bindings for:
> 
>   TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer
> 
> It supports instantiation via device tree / ACPI table.
> 
> The device has the compatible string "ti,ds90ub949", and
> and allows an arrray of strings to be provided as regulator
> names to enable for operation of the device.

All the comments I made regarding the ds90ub948 DT bindings apply here
too. Same for the comments related to the driver, they apply to the
subsequent patches in this series.

> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
> Cc: Patrick Glaser <pglaser@tesla.com>
> Cc: Nate Case <ncase@tesla.com>
> ---
>  .../bindings/display/bridge/ti,ds90ub949.txt  | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
> new file mode 100644
> index 000000000000..3ba3897d5e81
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
> @@ -0,0 +1,24 @@
> +TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer
> +============================================================
> +
> +This is the binding for Texas Instruments DS90UB949-Q1 bridge serializer.
> +
> +This device supports I2C only.
> +
> +Required properties:
> +
> +- compatible: "ti,ds90ub949"
> +
> +Optional properties:
> +
> +- regulators: List of regulator name strings to enable for operation of device.
> +
> +Example
> +-------
> +
> +ti949: ds90ub949@0 {
> +	compatible = "ti,ds90ub949";
> +
> +	regulators: "vcc",
> +	            "vcc_hdmi";
> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 08/11] dt-bindings: display/bridge: Add bindings for ti949

[Michael Drake]

Hi Laurent,

On 11/06/2019 19:13, Laurent Pinchart wrote:
> Hi Michael,
> 
> Thank you for the patch.

My pleasure, and thank you for the feedback!

> On Tue, Jun 11, 2019 at 03:04:09PM +0100, Michael Drake wrote:
>> Adds device tree bindings for:
>>
>>   TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer
>>
>> It supports instantiation via device tree / ACPI table.
>>
>> The device has the compatible string "ti,ds90ub949", and
>> and allows an arrray of strings to be provided as regulator
>> names to enable for operation of the device.
> 
> All the comments I made regarding the ds90ub948 DT bindings apply here
> too. Same for the comments related to the driver, they apply to the
> subsequent patches in this series.

OK, understood.

Thank you very much for the review.

>> Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
>> Cc: Patrick Glaser <pglaser@tesla.com>
>> Cc: Nate Case <ncase@tesla.com>
>> ---
>>  .../bindings/display/bridge/ti,ds90ub949.txt  | 24 +++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
>> new file mode 100644
>> index 000000000000..3ba3897d5e81
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
>> @@ -0,0 +1,24 @@
>> +TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer
>> +============================================================
>> +
>> +This is the binding for Texas Instruments DS90UB949-Q1 bridge serializer.
>> +
>> +This device supports I2C only.
>> +
>> +Required properties:
>> +
>> +- compatible: "ti,ds90ub949"
>> +
>> +Optional properties:
>> +
>> +- regulators: List of regulator name strings to enable for operation of device.
>> +
>> +Example
>> +-------
>> +
>> +ti949: ds90ub949@0 {
>> +	compatible = "ti,ds90ub949";
>> +
>> +	regulators: "vcc",
>> +	            "vcc_hdmi";
>> +};
> 

-- 
Michael Drake                 https://www.codethink.co.uk/

[PATCH v1 09/11] ti949: i2c device driver for TI DS90UB949-Q1

[Michael Drake]

This is a regmap i2c device driver for:

  TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer

It supports instantiation via device tree / ACPI table.

A list of regulators to enable for use of the device (e.g.
GPIOs for turning on power) may be provided as device tree
properties.  These are enabled on probe and PM resume.
They are disabled on remove and PM suspend.

Datasheet: http://www.ti.com/lit/ds/symlink/ds90ub949-q1.pdf

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 drivers/gpu/drm/bridge/Kconfig  |   8 +
 drivers/gpu/drm/bridge/Makefile |   1 +
 drivers/gpu/drm/bridge/ti949.c  | 511 ++++++++++++++++++++++++++++++++
 3 files changed, 520 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti949.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 149692dc8d48..fbc0eddda8f0 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -89,6 +89,14 @@ config DRM_TI948
 	  This is a driver for the TI DS90UB948-Q1 device.
 	  It converts video signals from FPD-Link III to OpenLDI.
 
+config DRM_TI949
+	tristate "TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer"
+	select DRM_KMS_HELPER
+	select REGMAP_I2C
+	help
+	  This is a driver for the TI DS90UB949-Q1 device.
+	  It converts video signals from HDMI to FPD-Link III.
+
 config DRM_SIL_SII8620
 	tristate "Silicon Image SII8620 HDMI/MHL bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 3fb37bbe10e1..1788b5efe234 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
 obj-$(CONFIG_DRM_TI948) += ti948.o
+obj-$(CONFIG_DRM_TI949) += ti949.o
 obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
 obj-$(CONFIG_DRM_SII9234) += sii9234.o
diff --git a/drivers/gpu/drm/bridge/ti949.c b/drivers/gpu/drm/bridge/ti949.c
new file mode 100644
index 000000000000..04618ca5f25e
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti949.c
@@ -0,0 +1,511 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer driver
+ *
+ * Copyright (C) 2019 Tesla Motors, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/regulator/consumer.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+
+/* Number of times to try checking for device on bringup. */
+#define TI949_DEVICE_ID_TRIES 10
+
+/**
+ * enum ti949_reg - TI949 registers.
+ *
+ * DS90UB949-Q1: 8.6 Register Maps: Table 10. Serial Control Bus Registers
+ *
+ * http://www.ti.com/lit/ds/snls452/snls452.pdf
+ */
+enum ti949_reg {
+	TI949_REG_I2C_DEVICE_ID                         = 0x00,
+	TI949_REG_RESET                                 = 0x01,
+	TI949_REG_GENERAL_CONFIGURATION                 = 0x03,
+	TI949_REG_MODE_SELECT                           = 0x04,
+	TI949_REG_I2C_CONTROL                           = 0x05,
+	TI949_REG_DES_ID                                = 0x06,
+	TI949_REG_SLAVE_ID                              = 0x07,
+	TI949_REG_SLAVE_ALIAS                           = 0x08,
+	TI949_REG_CRC_ERRORS_A                          = 0x0A,
+	TI949_REG_CRC_ERRORS_B                          = 0x0B,
+	TI949_REG_GENERAL_STATUS                        = 0x0C,
+	TI949_REG_GPIO0_CONFIGURATION                   = 0x0D,
+	TI949_REG_GPIO1_AND_2_CONFIGURATION             = 0x0E,
+	TI949_REG_GPIO3_CONFIGURATION                   = 0x0F,
+	TI949_REG_GPIO5_REG_AND_GPIO6_REG_CONFIGURATION = 0x10,
+	TI949_REG_GPIO7_REG_AND_GPIO8_REG_CONFIGURATION = 0x11,
+	TI949_REG_DATA_PATH_CONTROL                     = 0x12,
+	TI949_REG_GENERAL_PURPOSE_CONTROL               = 0x13,
+	TI949_REG_BIST_CONTROL                          = 0x14,
+	TI949_REG_I2C_VOLTAGE_SELECT                    = 0x15,
+	TI949_REG_BCC_WATCHDOG_CONTROL                  = 0x16,
+	TI949_REG_I2C_CONTROL2                          = 0x17,
+	TI949_REG_SCL_HIGH_TIME                         = 0x18,
+	TI949_REG_SCL_LOW_TIME                          = 0x19,
+	TI949_REG_DATA_PATH_CONTROL_2                   = 0x1A,
+	TI949_REG_BIST_BC_ERROR_COUNT                   = 0x1B,
+	TI949_REG_GPIO_PIN_STATUS_1                     = 0x1C,
+	TI949_REG_GPIO_PIN_STATUS_2                     = 0x1D,
+	TI949_REG_TRANSMITTER_PORT_SELECT               = 0x1E,
+	TI949_REG_FREQUENCY_COUNTER                     = 0x1F,
+	TI949_REG_DESERIALIZER_CAPABILITIES_1           = 0x20,
+	TI949_REG_DESERIALIZER_CAPABILITIES_2           = 0x21,
+	TI949_REG_LINK_DETECT_CONTROL                   = 0x26,
+	TI949_REG_SCLK_CTRL                             = 0x30,
+	TI949_REG_AUDIO_CTS0                            = 0x31,
+	TI949_REG_AUDIO_CTS1                            = 0x32,
+	TI949_REG_AUDIO_CTS2                            = 0x33,
+	TI949_REG_AUDIO_N0                              = 0x34,
+	TI949_REG_AUDIO_N1                              = 0x35,
+	TI949_REG_AUDIO_N2_COEFF                        = 0x36,
+	TI949_REG_CLK_CLEAN_STS                         = 0x37,
+	TI949_REG_ANA_IA_CNTL                           = 0x40,
+	TI949_REG_ANA_IA_ADDR                           = 0x41,
+	TI949_REG_ANA_IA_CTL                            = 0x42,
+	TI949_REG_APB_CTL                               = 0x48,
+	TI949_REG_APB_ADR0                              = 0x49,
+	TI949_REG_APB_ADR1                              = 0x4A,
+	TI949_REG_APB_DATA0                             = 0x4B,
+	TI949_REG_APB_DATA1                             = 0x4C,
+	TI949_REG_APB_DATA2                             = 0x4D,
+	TI949_REG_APB_DATA3                             = 0x4E,
+	TI949_REG_BRIDGE_CTL                            = 0x4F,
+	TI949_REG_BRIDGE_STS                            = 0x50,
+	TI949_REG_EDID_ID                               = 0x51,
+	TI949_REG_EDID_CFG0                             = 0x52,
+	TI949_REG_EDID_CFG1                             = 0x53,
+	TI949_REG_BRIDGE_CFG                            = 0x54,
+	TI949_REG_AUDIO_CFG                             = 0x55,
+	TI949_REG_DUAL_STS                              = 0x5A,
+	TI949_REG_DUAL_CTL1                             = 0x5B,
+	TI949_REG_DUAL_CTL2                             = 0x5C,
+	TI949_REG_FREQ_LOW                              = 0x5D,
+	TI949_REG_FREQ_HIGH                             = 0x5E,
+	TI949_REG_HDMI_FREQUENCY                        = 0x5F,
+	TI949_REG_SPI_TIMING1                           = 0x60,
+	TI949_REG_SPI_TIMING2                           = 0x61,
+	TI949_REG_SPI_CONFIG                            = 0x62,
+	TI949_REG_PATTERN_GENERATOR_CONTROL             = 0x64,
+	TI949_REG_PATTERN_GENERATOR_CONFIGURATION       = 0x65,
+	TI949_REG_PGIA                                  = 0x66,
+	TI949_REG_PGID                                  = 0x67,
+	TI949_REG_SLAVE_ID1                             = 0x70,
+	TI949_REG_SLAVE_ID2                             = 0x71,
+	TI949_REG_SLAVE_ID3                             = 0x72,
+	TI949_REG_SLAVE_ID4                             = 0x73,
+	TI949_REG_SLAVE_ID5                             = 0x74,
+	TI949_REG_SLAVE_ID6                             = 0x75,
+	TI949_REG_SLAVE_ID7                             = 0x76,
+	TI949_REG_SLAVE_ALIAS1                          = 0x77,
+	TI949_REG_SLAVE_ALIAS2                          = 0x78,
+	TI949_REG_SLAVE_ALIAS3                          = 0x79,
+	TI949_REG_SLAVE_ALIAS4                          = 0x7A,
+	TI949_REG_SLAVE_ALIAS5                          = 0x7B,
+	TI949_REG_SLAVE_ALIAS6                          = 0x7C,
+	TI949_REG_SLAVE_ALIAS7                          = 0x7D,
+	TI949_REG_ICR                                   = 0xC6,
+	TI949_REG_ISR                                   = 0xC7,
+	TI949_REG_TX_ID_0                               = 0xF0,
+	TI949_REG_TX_ID_1                               = 0xF1,
+	TI949_REG_TX_ID_2                               = 0xF2,
+	TI949_REG_TX_ID_3                               = 0xF3,
+	TI949_REG_TX_ID_4                               = 0xF4,
+	TI949_REG_TX_ID_5                               = 0xF5,
+};
+
+/**
+ * struct ti949_ctx - ti949 driver context
+ * @i2c:         Handle for the device's i2c client.
+ * @regmap:      Handle for the device's regmap.
+ * @reg_names:   Array of regulator names, or NULL.
+ * @regs:        Array of regulators, or NULL.
+ * @reg_count:   Number of entries in reg_names and regs arrays.
+ */
+struct ti949_ctx {
+	struct i2c_client *i2c;
+	struct regmap *regmap;
+	const char **reg_names;
+	struct regulator **regs;
+	size_t reg_count;
+};
+
+static bool ti949_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case TI949_REG_I2C_DEVICE_ID ... TI949_REG_DESERIALIZER_CAPABILITIES_2:
+		/*fall through*/
+	case TI949_REG_LINK_DETECT_CONTROL:
+		/*fall through*/
+	case TI949_REG_SCLK_CTRL     ... TI949_REG_CLK_CLEAN_STS:
+		/*fall through*/
+	case TI949_REG_ANA_IA_CNTL   ... TI949_REG_ANA_IA_CTL:
+		/*fall through*/
+	case TI949_REG_APB_CTL       ... TI949_REG_AUDIO_CFG:
+		/*fall through*/
+	case TI949_REG_DUAL_STS      ... TI949_REG_SPI_CONFIG:
+		/*fall through*/
+	case TI949_REG_PATTERN_GENERATOR_CONTROL ... TI949_REG_PGID:
+		/*fall through*/
+	case TI949_REG_SLAVE_ID1     ... TI949_REG_SLAVE_ALIAS7:
+		/*fall through*/
+	case TI949_REG_ICR           ... TI949_REG_ISR:
+		/*fall through*/
+	case TI949_REG_TX_ID_0       ... TI949_REG_TX_ID_5:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool ti949_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case TI949_REG_I2C_DEVICE_ID       ... TI949_REG_SLAVE_ALIAS:
+		/*fall through*/
+	case TI949_REG_GPIO0_CONFIGURATION ... TI949_REG_DATA_PATH_CONTROL:
+		/*fall through*/
+	case TI949_REG_BIST_CONTROL        ... TI949_REG_DATA_PATH_CONTROL_2:
+		/*fall through*/
+	case TI949_REG_TRANSMITTER_PORT_SELECT ...
+			TI949_REG_DESERIALIZER_CAPABILITIES_2:
+		/*fall through*/
+	case TI949_REG_LINK_DETECT_CONTROL:
+		/*fall through*/
+	case TI949_REG_SCLK_CTRL           ... TI949_REG_AUDIO_N2_COEFF:
+		/*fall through*/
+	case TI949_REG_ANA_IA_CNTL         ... TI949_REG_ANA_IA_CTL:
+		/*fall through*/
+	case TI949_REG_APB_CTL             ... TI949_REG_BRIDGE_CTL:
+		/*fall through*/
+	case TI949_REG_EDID_ID             ... TI949_REG_AUDIO_CFG:
+		/*fall through*/
+	case TI949_REG_DUAL_CTL1           ... TI949_REG_SPI_CONFIG:
+		/*fall through*/
+	case TI949_REG_PATTERN_GENERATOR_CONTROL ... TI949_REG_PGID:
+		/*fall through*/
+	case TI949_REG_SLAVE_ID1           ... TI949_REG_SLAVE_ALIAS7:
+		/*fall through*/
+	case TI949_REG_ICR:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config ti949_regmap_config = {
+	.val_bits      = 8,
+	.reg_bits      = 8,
+	.max_register  = TI949_REG_TX_ID_5,
+	.readable_reg  = ti949_readable_reg,
+	.writeable_reg = ti949_writeable_reg,
+};
+
+static inline u8 ti949_device_address(struct ti949_ctx *ti949)
+{
+	return ti949->i2c->addr;
+}
+
+static inline u8 ti949_device_expected_id(struct ti949_ctx *ti949)
+{
+	return ti949_device_address(ti949) << 1;
+}
+
+static int ti949_device_check(struct ti949_ctx *ti949)
+{
+	unsigned int i2c_device_id;
+	int ret;
+
+	ret = regmap_read(ti949->regmap, TI949_REG_I2C_DEVICE_ID,
+			&i2c_device_id);
+	if (ret < 0) {
+		dev_err(&ti949->i2c->dev,
+				"Failed to read device id. (error %d)\n", ret);
+		return -ENODEV;
+	}
+
+	if (i2c_device_id != ti949_device_expected_id(ti949)) {
+		dev_err(&ti949->i2c->dev,
+				"Bad device ID: Got: %x, Expected: %x\n",
+				i2c_device_id, ti949_device_expected_id(ti949));
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+/**
+ * ti949_device_await() - Repeatedly check the ti949's device ID.
+ * @ti949: ti949 context to check.
+ *
+ * Repeatedly check the ti949's device ID to verify that the ti949 can be
+ * communicated with, and that it is the correct version.
+ *
+ * This function can also be used to wait until the ti949 is ready, after
+ * reset.
+ *
+ * Returns: 0 on success, negative on failure.
+ */
+static int ti949_device_await(struct ti949_ctx *ti949)
+{
+	int tries = 0;
+	int ret;
+
+	do {
+		ret = ti949_device_check(ti949);
+		if (ret < 0)
+			mdelay(1);
+		else
+			break;
+	} while (++tries < TI949_DEVICE_ID_TRIES);
+
+	if (tries >= TI949_DEVICE_ID_TRIES)
+		dev_err(&ti949->i2c->dev,
+				"Failed to read id after %d attempt(s)\n",
+				tries);
+
+	return ret;
+}
+
+static int ti949_power_on(struct ti949_ctx *ti949)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < ti949->reg_count; i++) {
+		dev_info(&ti949->i2c->dev, "Enabling %s regulator\n",
+				ti949->reg_names[i]);
+		ret = regulator_enable(ti949->regs[i]);
+		if (ret < 0) {
+			dev_err(&ti949->i2c->dev,
+					"Could not enable regulator %s: %d\n",
+					ti949->reg_names[i], ret);
+			return ret;
+		}
+
+		msleep(100);
+	}
+
+	ret = ti949_device_await(ti949);
+	if (ret != 0)
+		return ret;
+
+	msleep(500);
+
+	return 0;
+}
+
+static int ti949_power_off(struct ti949_ctx *ti949)
+{
+	int i;
+	int ret;
+
+	for (i = ti949->reg_count; i > 0; i--) {
+		dev_info(&ti949->i2c->dev, "Disabling %s regulator\n",
+				ti949->reg_names[i - 1]);
+		ret = regulator_disable(ti949->regs[i - 1]);
+		if (ret < 0) {
+			dev_err(&ti949->i2c->dev,
+					"Could not disable regulator %s: %d\n",
+					ti949->reg_names[i - 1], ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static inline struct ti949_ctx *ti949_ctx_from_dev(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+
+	return i2c_get_clientdata(client);
+}
+
+static int ti949_pm_resume(struct device *dev)
+{
+	struct ti949_ctx *ti949 = ti949_ctx_from_dev(dev);
+	int ret;
+
+	if (ti949 == NULL)
+		return 0;
+
+	ret = ti949_power_on(ti949);
+	if (ret != 0)
+		return ret;
+
+	return 0;
+}
+
+static int ti949_pm_suspend(struct device *dev)
+{
+	struct ti949_ctx *ti949 = ti949_ctx_from_dev(dev);
+
+	if (ti949 == NULL)
+		return 0;
+
+	return ti949_power_off(ti949);
+}
+
+static int ti949_get_regulator(struct device *dev, const char *id,
+		struct regulator **reg_out)
+{
+	struct regulator *reg;
+
+	reg = devm_regulator_get(dev, id);
+	if (IS_ERR(reg)) {
+		int ret = PTR_ERR(reg);
+
+		dev_err(dev, "Error requesting %s regulator: %d\n", id, ret);
+		return ret;
+	}
+
+	if (reg == NULL)
+		dev_warn(dev, "Could not get %s regulator\n", id);
+	else
+		dev_info(dev, "Got regulator %s\n", id);
+
+	*reg_out = reg;
+	return 0;
+}
+
+static int ti949_get_regulators(struct ti949_ctx *ti949)
+{
+	int i;
+	int ret;
+	size_t regulator_count;
+
+	ret = device_property_read_string_array(&ti949->i2c->dev,
+			"regulators", NULL, 0);
+	if (ret == -EINVAL ||
+	    ret == -ENODATA ||
+	    ret == 0) {
+		/* "regulators" property was either:
+		 *   - unset
+		 *   - valueless
+		 *   - set to empty list
+		 * Not an error; continue without regulators.
+		 */
+		dev_info(&ti949->i2c->dev,
+				"No regulators listed for device.\n");
+		return 0;
+
+	} else if (ret < 0) {
+		return ret;
+	}
+
+	regulator_count = ret;
+
+	ti949->reg_names = devm_kmalloc_array(&ti949->i2c->dev,
+			regulator_count, sizeof(*ti949->reg_names), GFP_KERNEL);
+	if (!ti949->reg_names)
+		return -ENOMEM;
+
+	ret = device_property_read_string_array(&ti949->i2c->dev, "regulators",
+			ti949->reg_names, regulator_count);
+	if (ret < 0)
+		return ret;
+
+	ti949->regs = devm_kmalloc_array(&ti949->i2c->dev,
+			regulator_count, sizeof(*ti949->regs), GFP_KERNEL);
+	if (!ti949->regs)
+		return -ENOMEM;
+
+	for (i = 0; i < regulator_count; i++) {
+		ret = ti949_get_regulator(&ti949->i2c->dev,
+				ti949->reg_names[i], &ti949->regs[i]);
+		if (ret != 0)
+			return ret;
+	}
+
+	ti949->reg_count = regulator_count;
+
+	return 0;
+}
+
+static int ti949_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct ti949_ctx *ti949;
+	int ret;
+
+	dev_info(&client->dev, "Begin probe (addr: %x)\n", client->addr);
+
+	ti949 = devm_kzalloc(&client->dev, sizeof(*ti949), GFP_KERNEL);
+	if (!ti949)
+		return -ENOMEM;
+
+	ti949->i2c = client;
+
+	ti949->regmap = devm_regmap_init_i2c(ti949->i2c, &ti949_regmap_config);
+	if (IS_ERR(ti949->regmap))
+		return PTR_ERR(ti949->regmap);
+
+	ret = ti949_get_regulators(ti949);
+	if (ret != 0)
+		return ret;
+
+	i2c_set_clientdata(client, ti949);
+
+	ret = ti949_pm_resume(&client->dev);
+	if (ret != 0)
+		return -EPROBE_DEFER;
+
+	dev_info(&ti949->i2c->dev, "End probe (addr: %x)\n", ti949->i2c->addr);
+
+	return 0;
+}
+
+static int ti949_remove(struct i2c_client *client)
+{
+	return ti949_pm_suspend(&client->dev);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id ti949_of_match[] = {
+	{ .compatible = "ti,ds90ub949" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ti949_of_match);
+#endif
+
+static const struct i2c_device_id ti949_i2c_id[] = {
+	{"ti949", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, ti949_i2c_id);
+
+static const struct dev_pm_ops ti949_pm_ops = {
+	.resume  = ti949_pm_resume,
+	.suspend = ti949_pm_suspend,
+};
+
+static struct i2c_driver ti949_driver = {
+	.driver = {
+		.name  = "ti949",
+		.owner = THIS_MODULE,
+		.pm    = &ti949_pm_ops,
+		.of_match_table = of_match_ptr(ti949_of_match),
+	},
+	.id_table = ti949_i2c_id,
+	.probe    = ti949_probe,
+	.remove   = ti949_remove,
+};
+module_i2c_driver(ti949_driver);
+
+MODULE_AUTHOR("Michael Drake <michael.drake@codethink.co.uk");
+MODULE_DESCRIPTION("TI DS90UB949-Q1 1080p HDMI to FPD-Link III bridge serializer driver");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1

[PATCH v1 10/11] dt-bindings: display/bridge: Add config property for ti949

[Michael Drake]

The config property can be used to provide an array of
register addresses and values to be written to configure
the device for the board.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 .../bindings/display/bridge/ti,ds90ub949.txt        | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
index 3ba3897d5e81..b1e38d732f17 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90ub949.txt
@@ -12,6 +12,8 @@ Required properties:
 Optional properties:
 
 - regulators: List of regulator name strings to enable for operation of device.
+- config: List of <register address>,<value> pairs to be set to configure
+  device on powerup.  The register addresses and values are 8bit.
 
 Example
 -------
@@ -21,4 +23,15 @@ ti949: ds90ub949@0 {
 
 	regulators: "vcc",
 	            "vcc_hdmi";
+	config:
+	        /* GPIO0 is an output with remote value */
+	        <0x0D>, <0x25>,
+	        /* GPIO3 is an input for XRES */
+	        <0x0F>, <0x03>,
+	        /* GPIO2 is an input for backlight PWM */
+	        <0x0E>, <0x30>,
+	        /* Enables forward channel I2C pass through */
+	        <0x17>, <0x9e>,
+	        /* Enables PORT1 registers I2C access */
+	        <0x1E>, <0x04>;
 };
-- 
2.20.1

[PATCH v1 11/11] ti949: Add support for configuration via device properties

[Michael Drake]

This allows the device to be configured for the board in
device tree, or in ACPI tables.  The device node properties
can provide an array of register addresses and values to be
written to configure the device for the board.

The config is written to the device on probe and on PM resume.

Signed-off-by: Michael Drake <michael.drake@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 drivers/gpu/drm/bridge/ti949.c | 120 +++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti949.c b/drivers/gpu/drm/bridge/ti949.c
index 04618ca5f25e..57dcecd10ace 100644
--- a/drivers/gpu/drm/bridge/ti949.c
+++ b/drivers/gpu/drm/bridge/ti949.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/delay.h>
+#include <linux/slab.h>
 #include <linux/i2c.h>
 
 /* Number of times to try checking for device on bringup. */
@@ -127,10 +128,22 @@ enum ti949_reg {
 	TI949_REG_TX_ID_5                               = 0xF5,
 };
 
+/**
+ * struct ti949_reg_val - ti949 register value
+ * @addr:     The address of the register
+ * @value:    The initial value of the register
+ */
+struct ti949_reg_val {
+	u8 addr;
+	u8 value;
+};
+
 /**
  * struct ti949_ctx - ti949 driver context
  * @i2c:         Handle for the device's i2c client.
  * @regmap:      Handle for the device's regmap.
+ * @config:      Array of register values loaded from device properties.
+ * @config_len:  Number of entries in config.
  * @reg_names:   Array of regulator names, or NULL.
  * @regs:        Array of regulators, or NULL.
  * @reg_count:   Number of entries in reg_names and regs arrays.
@@ -138,6 +151,8 @@ enum ti949_reg {
 struct ti949_ctx {
 	struct i2c_client *i2c;
 	struct regmap *regmap;
+	struct ti949_reg_val *config;
+	size_t config_len;
 	const char **reg_names;
 	struct regulator **regs;
 	size_t reg_count;
@@ -214,6 +229,42 @@ static const struct regmap_config ti949_regmap_config = {
 	.writeable_reg = ti949_writeable_reg,
 };
 
+static int ti949_write_sequence(
+		struct ti949_ctx *ti949,
+		const struct ti949_reg_val *sequence,
+		u32 entries)
+{
+	int i;
+
+	for (i = 0; i < entries; i++) {
+		const struct ti949_reg_val *r = sequence + i;
+		int ret = regmap_write(ti949->regmap, r->addr, r->value);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ti949_write_config_seq(struct ti949_ctx *ti949)
+{
+	int ret;
+
+	if (ti949->config == NULL) {
+		dev_info(&ti949->i2c->dev, "No config for ti949 device\n");
+		return 0;
+	}
+
+	ret = ti949_write_sequence(ti949, ti949->config, ti949->config_len);
+	if (ret < 0)
+		return ret;
+
+	dev_info(&ti949->i2c->dev, "Successfully configured ti949\n");
+
+	return ret;
+}
+
 static inline u8 ti949_device_address(struct ti949_ctx *ti949)
 {
 	return ti949->i2c->addr;
@@ -347,6 +398,12 @@ static int ti949_pm_resume(struct device *dev)
 	if (ret != 0)
 		return ret;
 
+	ret = ti949_write_config_seq(ti949);
+	if (ret != 0)
+		return ret;
+
+	/* Extend 200ms after ti949 init for display HW tolerance. */
+	msleep(200);
 	return 0;
 }
 
@@ -436,6 +493,65 @@ static int ti949_get_regulators(struct ti949_ctx *ti949)
 	return 0;
 }
 
+static int ti949_get_config(struct ti949_ctx *ti949)
+{
+	int i;
+	int ret;
+	u8 *config;
+	size_t config_len;
+
+	ret = device_property_read_u8_array(&ti949->i2c->dev,
+			"config", NULL, 0);
+	if (ret == -EINVAL ||
+	    ret == -ENODATA ||
+	    ret == 0) {
+		/* "config" property was either:
+		 *   - unset
+		 *   - valueless
+		 *   - set to empty list
+		 * Not an error; continue without config.
+		 */
+		dev_info(&ti949->i2c->dev, "No config defined for device.\n");
+		return 0;
+
+	} else if (ret < 0) {
+		return ret;
+	} else if (ret & 0x1) {
+		dev_err(&ti949->i2c->dev,
+			"Device property 'config' needs even entry count.\n");
+		return -EINVAL;
+	}
+
+	config_len = ret;
+
+	config = kmalloc_array(config_len, sizeof(*config), GFP_KERNEL);
+	if (!config)
+		return -ENOMEM;
+
+	ret = device_property_read_u8_array(&ti949->i2c->dev, "config",
+			config, config_len);
+	if (ret < 0) {
+		kfree(config);
+		return ret;
+	}
+
+	ti949->config = devm_kmalloc_array(&ti949->i2c->dev,
+			config_len / 2, sizeof(*ti949->config), GFP_KERNEL);
+	if (!ti949->config) {
+		kfree(config);
+		return -ENOMEM;
+	}
+
+	ti949->config_len = config_len / 2;
+	for (i = 0; i < config_len; i += 2) {
+		ti949->config[i / 2].addr = config[i];
+		ti949->config[i / 2].value = config[i + 1];
+	}
+	kfree(config);
+
+	return 0;
+}
+
 static int ti949_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
@@ -458,6 +574,10 @@ static int ti949_probe(struct i2c_client *client,
 	if (ret != 0)
 		return ret;
 
+	ret = ti949_get_config(ti949);
+	if (ret != 0)
+		return ret;
+
 	i2c_set_clientdata(client, ti949);
 
 	ret = ti949_pm_resume(&client->dev);
-- 
2.20.1