diff mbox

[v3,1/2] drm/bridge: add Silicon Image SiI9234 driver

Message ID 1504620099-20024-2-git-send-email-m.purski@samsung.com (mailing list archive)
State Superseded
Headers show

Commit Message

Maciej Purski Sept. 5, 2017, 2:01 p.m. UTC
SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
It is controlled via I2C bus. Its interaction with other
devices in video pipeline is performed mainly on HW level.
The only interaction it does on device driver level is
filtering-out unsupported video modes, it exposes drm_bridge
interface to perform this operation.

This patch is based on the code refactored by Tomasz Stanislawski
<t.stanislaws@samsung.com>, which was initially developed by:
Adam Hampson <ahampson@sta.samsung.com>
Erik Gilling <konkers@android.com>
Shankar Bandal <shankar.b@samsung.com>
Dharam Kumar <dharam.kr@samsung.com>

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 .../devicetree/bindings/display/bridge/sii9234.txt |  34 +
 drivers/gpu/drm/bridge/Kconfig                     |   8 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/sii9234.c                   | 993 +++++++++++++++++++++
 4 files changed, 1036 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/sii9234.txt
 create mode 100644 drivers/gpu/drm/bridge/sii9234.c

Comments

Krzysztof Kozlowski Sept. 8, 2017, 5:05 p.m. UTC | #1
On Tue, Sep 05, 2017 at 04:01:38PM +0200, Maciej Purski wrote:
> SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
> It is controlled via I2C bus. Its interaction with other
> devices in video pipeline is performed mainly on HW level.
> The only interaction it does on device driver level is
> filtering-out unsupported video modes, it exposes drm_bridge
> interface to perform this operation.
> 
> This patch is based on the code refactored by Tomasz Stanislawski
> <t.stanislaws@samsung.com>, which was initially developed by:
> Adam Hampson <ahampson@sta.samsung.com>
> Erik Gilling <konkers@android.com>
> Shankar Bandal <shankar.b@samsung.com>
> Dharam Kumar <dharam.kr@samsung.com>
> 
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../devicetree/bindings/display/bridge/sii9234.txt |  34 +
>  drivers/gpu/drm/bridge/Kconfig                     |   8 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/sii9234.c                   | 993 +++++++++++++++++++++
>  4 files changed, 1036 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/sii9234.txt
>  create mode 100644 drivers/gpu/drm/bridge/sii9234.c
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
> new file mode 100644
> index 0000000..3ce7413
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
> @@ -0,0 +1,34 @@
> +Silicon Image SiI9234 HDMI/MHL bridge bindings
> +
> +Required properties:
> +	- compatible : "sil,sii9234".
> +	- reg : I2C address for TPI interface, use 0x39
> +	- avcc33-supply : MHL/USB Switch Supply Voltage (3.3V)
> +	- iovcc18-supply : I/O Supply Voltage (1.8V)
> +	- avcc12-supply : TMDS Analog Supply Voltage (1.2V)
> +	- cvcc12-supply : Digital Core Supply Voltage (1.2V)
> +	- interrupts, interrupt-parent: interrupt specifier of INT pin
> +	- reset-gpios: gpio specifier of RESET pin (active low)
> +	- video interfaces: Device node can contain video interface port
> +			    node for HDMI encoder according to [1].
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +	sii9234@39 {
> +		compatible = "sil,sii9234";
> +		reg = <0x39>;
> +		avcc33-supply = <&vcc33mhl>;
> +		iovcc18-supply = <&vcc18mhl>;
> +		avcc12-supply = <&vsil12>;
> +		cvcc12-supply = <&vsil12>;
> +		reset-gpios = <&gpf3 4 GPIO_ACTIVE_LOW>;
> +		interrupt-parent = <&gpf3>;
> +		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		port {
> +			mhl_to_hdmi: endpoint {
> +				remote-endpoint = <&hdmi_to_mhl>;
> +			};
> +		};
> +	};
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index adf9ae0..9dba16fd 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -84,6 +84,14 @@ config DRM_SII902X
>  	---help---
>  	  Silicon Image sii902x bridge chip driver.
>  
> +config DRM_SII9234
> +	tristate "Silicon Image SII9234 HDMI/MHL bridge"
> +	depends on OF
> +	---help---
> +	  Say Y here if you want support for the MHL interface.
> +	  It is an I2C driver, that detects connection of MHL bridge
> +	  and starts encapsulation of HDMI signal.
> +
>  config DRM_TOSHIBA_TC358767
>  	tristate "Toshiba TC358767 eDP bridge"
>  	depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index defcf1e..e3d5eb0 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>  obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>  obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>  obj-$(CONFIG_DRM_SII902X) += sii902x.o
> +obj-$(CONFIG_DRM_SII9234) += sii9234.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
> new file mode 100644
> index 0000000..b70d69a
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/sii9234.c
> @@ -0,0 +1,993 @@
> +/*
> + * Copyright (C) 2017 Samsung Electronics
> + *
> + * Authors:
> + *    Tomasz Stanislawski <t.stanislaws@samsung.com>
> + *    Maciej Purski <m.purski@samsung.com>
> + *
> + * Based on sii9234 driver created by:
> + *    Adam Hampson <ahampson@sta.samsung.com>
> + *    Erik Gilling <konkers@android.com>
> + *    Shankar Bandal <shankar.b@samsung.com>
> + *    Dharam Kumar <dharam.kr@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program
> + *
> + */
> +#include <drm/bridge/mhl.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_edid.h>
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +#define CBUS_DEVCAP_OFFSET		0x80
> +
> +#define SII9234_MHL_VERSION		0x11
> +#define SII9234_SCRATCHPAD_SIZE		0x10
> +#define SII9234_INT_STAT_SIZE		0x33
> +
> +#define BIT_TMDS_CCTRL_TMDS_OE		BIT(4)
> +#define MHL_HPD_OUT_OVR_EN		BIT(4)
> +#define MHL_HPD_OUT_OVR_VAL		BIT(5)
> +#define MHL_INIT_TIMEOUT		0x0C
> +
> +/* MHL Tx registers and bits */
> +#define MHL_TX_SRST                     0x05
> +#define MHL_TX_SYSSTAT_REG              0x09
> +#define MHL_TX_INTR1_REG                0x71
> +#define MHL_TX_INTR4_REG                0x74
> +#define MHL_TX_INTR1_ENABLE_REG         0x75
> +#define MHL_TX_INTR4_ENABLE_REG         0x78
> +#define MHL_TX_INT_CTRL_REG             0x79
> +#define MHL_TX_TMDS_CCTRL               0x80
> +#define MHL_TX_DISC_CTRL1_REG           0x90
> +#define MHL_TX_DISC_CTRL2_REG           0x91
> +#define MHL_TX_DISC_CTRL3_REG           0x92
> +#define MHL_TX_DISC_CTRL4_REG		0x93
> +#define MHL_TX_DISC_CTRL5_REG           0x94
> +#define MHL_TX_DISC_CTRL6_REG           0x95
> +#define MHL_TX_DISC_CTRL7_REG           0x96
> +#define MHL_TX_DISC_CTRL8_REG		0x97
> +#define MHL_TX_STAT2_REG                0x99

                                 ^^^^^^^^
Sometimes tabs, sometimes spaces. Please make it consistent: tabs.


> +#define MHL_TX_MHLTX_CTL1_REG           0xA0
> +#define MHL_TX_MHLTX_CTL2_REG           0xA1
> +#define MHL_TX_MHLTX_CTL4_REG           0xA3
> +#define MHL_TX_MHLTX_CTL6_REG           0xA5
> +#define MHL_TX_MHLTX_CTL7_REG           0xA6
> +
> +
> +#define RSEN_STATUS			BIT(2)
> +#define HPD_CHANGE_INT			BIT(6)
> +#define RSEN_CHANGE_INT			BIT(5)
> +#define RGND_READY_INT			BIT(6)
> +#define VBUS_LOW_INT			BIT(5)
> +#define CBUS_LKOUT_INT			BIT(4)
> +#define MHL_DISC_FAIL_INT		BIT(3)
> +#define MHL_EST_INT			BIT(2)
> +#define HPD_CHANGE_INT_MASK		BIT(6)
> +#define RSEN_CHANGE_INT_MASK		BIT(5)
> +
> +#define RGND_READY_MASK			BIT(6)
> +#define CBUS_LKOUT_MASK			BIT(4)
> +#define MHL_DISC_FAIL_MASK		BIT(3)
> +#define MHL_EST_MASK			BIT(2)
> +
> +#define SKIP_GND			BIT(6)
> +
> +#define ATT_THRESH_SHIFT		0x04
> +#define ATT_THRESH_MASK			(0x03 << ATT_THRESH_SHIFT)
> +#define USB_D_OEN			BIT(3)
> +#define DEGLITCH_TIME_MASK		0x07
> +#define DEGLITCH_TIME_2MS		0
> +#define DEGLITCH_TIME_4MS		1
> +#define DEGLITCH_TIME_8MS		2
> +#define DEGLITCH_TIME_16MS		3
> +#define DEGLITCH_TIME_40MS		4
> +#define DEGLITCH_TIME_50MS		5
> +#define DEGLITCH_TIME_60MS		6
> +#define DEGLITCH_TIME_128MS		7
> +
> +#define USB_D_OVR			BIT(7)
> +#define USB_ID_OVR			BIT(6)
> +#define DVRFLT_SEL			BIT(5)
> +#define BLOCK_RGND_INT			BIT(4)
> +#define SKIP_DEG			BIT(3)
> +#define CI2CA_POL			BIT(2)
> +#define CI2CA_WKUP			BIT(1)
> +#define SINGLE_ATT			BIT(0)
> +
> +#define USB_D_ODN			BIT(5)
> +#define VBUS_CHECK			BIT(2)
> +#define RGND_INTP_MASK			0x03
> +#define RGND_INTP_OPEN			0
> +#define RGND_INTP_2K			1
> +#define RGND_INTP_1K			2
> +#define RGND_INTP_SHORT			3
> +
> +/* HDMI registers */
> +#define HDMI_RX_TMDS0_CCTRL1_REG	0x10
> +#define HDMI_RX_TMDS_CLK_EN_REG		0x11
> +#define HDMI_RX_TMDS_CH_EN_REG		0x12
> +#define HDMI_RX_PLL_CALREFSEL_REG	0x17
> +#define HDMI_RX_PLL_VCOCAL_REG		0x1A
> +#define HDMI_RX_EQ_DATA0_REG		0x22
> +#define HDMI_RX_EQ_DATA1_REG		0x23
> +#define HDMI_RX_EQ_DATA2_REG		0x24
> +#define HDMI_RX_EQ_DATA3_REG		0x25
> +#define HDMI_RX_EQ_DATA4_REG		0x26
> +#define HDMI_RX_TMDS_ZONE_CTRL_REG	0x4C
> +#define HDMI_RX_TMDS_MODE_CTRL_REG	0x4D
> +
> +/* CBUS registers */
> +#define CBUS_INT_STATUS_1_REG		0x08
> +#define CBUS_INTR1_ENABLE_REG		0x09
> +#define CBUS_MSC_REQ_ABORT_REASON_REG	0x0D
> +#define CBUS_INT_STATUS_2_REG		0x1E
> +#define CBUS_INTR2_ENABLE_REG		0x1F
> +#define CBUS_LINK_CONTROL_2_REG		0x31
> +#define CBUS_MHL_STATUS_REG_0		0xB0
> +#define CBUS_MHL_STATUS_REG_1		0xB1
> +
> +#define BIT_CBUS_RESET			BIT(3)
> +#define SET_HPD_DOWNSTREAM		BIT(6)
> +
> +/* TPI registers */
> +#define TPI_DPD_REG			0x3D
> +
> +/* timeouts */

Timeout in what kind of units?

> +#define T_SRC_VBUS_CBUS_TO_STABLE	200
> +#define T_SRC_CBUS_FLOAT		100
> +#define T_SRC_CBUS_DEGLITCH		2
> +#define T_SRC_RXSENSE_DEGLITCH		110
> +#define MHL1_MAX_CLK			75000
> +
> +/* I2C addresses */

Rather useless comment.

> +#define I2C_TPI_ADDR			0x3D
> +#define I2C_HDMI_ADDR			0x49
> +#define I2C_CBUS_ADDR			0x64
> +
> +enum sii9234_state {
> +	ST_OFF,
> +	ST_D3,
> +	ST_RGND_INIT,
> +	ST_RGND_1K,
> +	ST_RSEN_HIGH,
> +	ST_MHL_ESTABLISHED,
> +	ST_FAILURE_DISCOVERY,
> +	ST_FAILURE,
> +};
> +
> +struct sii9234 {
> +	struct i2c_client *client[4];
> +	struct drm_bridge bridge;
> +	struct device *dev;
> +	struct gpio_desc *gpio_reset;
> +	int i2c_error;
> +	struct regulator_bulk_data supplies[4];
> +
> +	enum sii9234_state state;
> +	struct mutex lock;
> +};
> +
> +enum sii9234_client_id {
> +	I2C_MHL,
> +	I2C_TPI,
> +	I2C_HDMI,
> +	I2C_CBUS,
> +};
> +
> +static char *sii9234_client_name[] = {

static const char *

> +	[I2C_MHL] = "MHL",
> +	[I2C_TPI] = "TPI",
> +	[I2C_HDMI] = "HDMI",
> +	[I2C_CBUS] = "CBUS",
> +};
> +
> +static int sii9234_writeb(struct sii9234 *ctx, int id, int offset,
> +			    int value)
> +{
> +	int ret;
> +	struct i2c_client *client = ctx->client[id];
> +
> +	if (ctx->i2c_error)
> +		return ctx->i2c_error;
> +
> +	ret = i2c_smbus_write_byte_data(client, offset, value);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "writeb: %4s[0x%02x] <- 0x%02x\n",
> +			sii9234_client_name[id], offset, value);
> +	ctx->i2c_error = ret;
> +
> +	return ret;
> +}
> +
> +static int sii9234_writebm(struct sii9234 *ctx, int id, int offset,
> +			    int value, int mask)
> +{
> +	int ret;
> +	struct i2c_client *client = ctx->client[id];
> +
> +	if (ctx->i2c_error)
> +		return ctx->i2c_error;
> +
> +	ret = i2c_smbus_write_byte(client, offset);
> +	if (ret < 0) {
> +		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
> +			sii9234_client_name[id], offset, value);
> +		ctx->i2c_error = ret;
> +		return ret;
> +	}
> +
> +	ret = i2c_smbus_read_byte(client);
> +	if (ret < 0) {
> +		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
> +			sii9234_client_name[id], offset, value);
> +		ctx->i2c_error = ret;
> +		return ret;
> +	}
> +
> +	value = (value & mask) | (ret & ~mask);
> +
> +	ret = i2c_smbus_write_byte_data(client, offset, value);
> +	if (ret < 0) {
> +		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
> +			sii9234_client_name[id], offset, value);
> +		ctx->i2c_error = ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int sii9234_readb(struct sii9234 *ctx, int id, int offset)
> +{
> +	int ret;
> +	struct i2c_client *client = ctx->client[id];
> +
> +	if (ctx->i2c_error)
> +		return ctx->i2c_error;
> +
> +	ret = i2c_smbus_write_byte(client, offset);
> +	if (ret < 0) {
> +		dev_err(ctx->dev, "readb: %4s[0x%02x]\n",
> +			sii9234_client_name[id], offset);
> +		ctx->i2c_error = ret;
> +		return ret;
> +	}
> +
> +	ret = i2c_smbus_read_byte(client);
> +	if (ret < 0) {
> +		dev_err(ctx->dev, "readb: %4s[0x%02x]\n",
> +			sii9234_client_name[id], offset);
> +		ctx->i2c_error = ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int sii9234_clear_error(struct sii9234 *ctx)
> +{
> +	int ret = ctx->i2c_error;
> +
> +	ctx->i2c_error = 0;
> +
> +	return ret;
> +}
> +
> +#define mhl_tx_writeb(sii9234, offset, value) \
> +	sii9234_writeb(sii9234, I2C_MHL, offset, value)
> +#define mhl_tx_writebm(sii9234, offset, value, mask) \
> +	sii9234_writebm(sii9234, I2C_MHL, offset, value, mask)
> +#define mhl_tx_readb(sii9234, offset) \
> +	sii9234_readb(sii9234, I2C_MHL, offset)
> +#define cbus_writeb(sii9234, offset, value) \
> +	sii9234_writeb(sii9234, I2C_CBUS, offset, value)
> +#define cbus_writebm(sii9234, offset, value, mask) \
> +	sii9234_writebm(sii9234, I2C_CBUS, offset, value, mask)
> +#define cbus_readb(sii9234, offset) \
> +	sii9234_readb(sii9234, I2C_CBUS, offset)
> +#define hdmi_writeb(sii9234, offset, value) \
> +	sii9234_writeb(sii9234, I2C_HDMI, offset, value)
> +#define hdmi_writebm(sii9234, offset, value, mask) \
> +	sii9234_writebm(sii9234, I2C_HDMI, offset, value, mask)
> +#define hdmi_readb(sii9234, offset) \
> +	sii9234_readb(sii9234, I2C_HDMI, offset)
> +#define tpi_writeb(sii9234, offset, value) \
> +	sii9234_writeb(sii9234, I2C_TPI, offset, value)
> +#define tpi_writebm(sii9234, offset, value, mask) \
> +	sii9234_writebm(sii9234, I2C_TPI, offset, value, mask)
> +#define tpi_readb(sii9234, offset) \
> +	sii9234_readb(sii9234, I2C_TPI, offset)
> +
> +static u8 sii9234_tmds_control(struct sii9234 *ctx, bool enable)
> +{
> +	mhl_tx_writebm(ctx, MHL_TX_TMDS_CCTRL, enable ? ~0 : 0,
> +						BIT_TMDS_CCTRL_TMDS_OE);
> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, enable ? ~0 : 0,
> +				MHL_HPD_OUT_OVR_EN | MHL_HPD_OUT_OVR_VAL);
> +	return sii9234_clear_error(ctx);
> +}
> +
> +static int sii9234_cbus_reset(struct sii9234 *ctx)
> +{
> +	int i;
> +
> +	mhl_tx_writebm(ctx, MHL_TX_SRST, ~0, BIT_CBUS_RESET);
> +	msleep(T_SRC_CBUS_DEGLITCH);
> +	mhl_tx_writebm(ctx, MHL_TX_SRST, 0, BIT_CBUS_RESET);
> +
> +	for (i = 0; i < 4; i++) {
> +		/* Enable WRITE_STAT interrupt for writes to all
> +		 * 4 MSC Status registers.
> +		 */
> +		cbus_writeb(ctx, 0xE0 + i, 0xF2);
> +		/* Enable SET_INT interrupt for writes to all
> +		 * 4 MSC Interrupt registers.
> +		 */

Comments with leading /*:
/*
 * foo bar
 * bar foo
 */

Please run checkpatch and fix the warnings everywhere.

> +		cbus_writeb(ctx, 0xF0 + i, 0xF2);
> +	}
> +
> +	return sii9234_clear_error(ctx);
> +}
> +
> +/* Require to chek mhl imformation of samsung in cbus_init_register */
> +static int sii9234_cbus_init(struct sii9234 *ctx)
> +{
> +	cbus_writeb(ctx, 0x07, 0xF2);
> +	cbus_writeb(ctx, 0x40, 0x03);
> +	cbus_writeb(ctx, 0x42, 0x06);
> +	cbus_writeb(ctx, 0x36, 0x0C);
> +	cbus_writeb(ctx, 0x3D, 0xFD);
> +	cbus_writeb(ctx, 0x1C, 0x01);
> +	cbus_writeb(ctx, 0x1D, 0x0F);
> +	cbus_writeb(ctx, 0x44, 0x02);
> +	/* Setup our devcap */
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEV_STATE, 0x00);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_MHL_VERSION,
> +							SII9234_MHL_VERSION);

Please align indentation with parenthesis.

> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_CAT,
> +							MHL_DCAP_CAT_SOURCE);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_ADOPTER_ID_H, 0x01);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_ADOPTER_ID_L, 0x41);
> +
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_VID_LINK_MODE,
> +			MHL_DCAP_VID_LINK_RGB444 | MHL_DCAP_VID_LINK_YCBCR444);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_VIDEO_TYPE,
> +							MHL_DCAP_VT_GRAPHICS);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_LOG_DEV_MAP,
> +							MHL_DCAP_LD_GUI);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_BANDWIDTH, 0x0F);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_FEATURE_FLAG,
> +		    MHL_DCAP_FEATURE_RCP_SUPPORT | MHL_DCAP_FEATURE_RAP_SUPPORT
> +		   | MHL_DCAP_FEATURE_SP_SUPPORT);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEVICE_ID_H, 0x0);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEVICE_ID_L, 0x0);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_SCRATCHPAD_SIZE,
> +						SII9234_SCRATCHPAD_SIZE);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_INT_STAT_SIZE,
> +						SII9234_INT_STAT_SIZE);
> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_RESERVED, 0);
> +	cbus_writebm(ctx, 0x31, 0x0C, 0x0C);
> +	cbus_writeb(ctx, 0x30, 0x01);
> +	cbus_writebm(ctx, 0x3C, 0x30, 0x38);
> +	cbus_writebm(ctx, 0x22, 0x0D, 0x0F);
> +	cbus_writebm(ctx, 0x2E, 0x15, 0x15);
> +	cbus_writeb(ctx, CBUS_INTR1_ENABLE_REG, 0);
> +	cbus_writeb(ctx, CBUS_INTR2_ENABLE_REG, 0);
> +
> +	return sii9234_clear_error(ctx);
> +}
> +
> +static void force_usb_id_switch_open(struct sii9234 *ctx)
> +{
> +	/* Disable CBUS discovery */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, 0, 0x01);
> +	/* Force USB ID switch to open */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, USB_ID_OVR);
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL3_REG, ~0, 0x86);
> +	/* Force upstream HPD to 0 when not in MHL mode. */
> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 0x30);
> +}
> +
> +static void release_usb_id_switch_open(struct sii9234 *ctx)
> +{
> +	msleep(T_SRC_CBUS_FLOAT);
> +	/* Clear USB ID switch to open */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, 0, USB_ID_OVR);
> +	/* Enable CBUS discovery */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, ~0, 0x01);
> +}
> +
> +static int sii9234_power_init(struct sii9234 *ctx)
> +{
> +	/* Force the SiI9234 into the D0 state. */
> +	tpi_writeb(ctx, TPI_DPD_REG, 0x3F);
> +	/* Enable TxPLL Clock */
> +	hdmi_writeb(ctx, HDMI_RX_TMDS_CLK_EN_REG, 0x01);
> +	/* Enable Tx Clock Path & Equalizer */
> +	hdmi_writeb(ctx, HDMI_RX_TMDS_CH_EN_REG, 0x15);
> +	/* Power Up TMDS */
> +	mhl_tx_writeb(ctx, 0x08, 0x35);
> +	return sii9234_clear_error(ctx);
> +}
> +
> +static int sii9234_hdmi_init(struct sii9234 *ctx)
> +{
> +	hdmi_writeb(ctx, HDMI_RX_TMDS0_CCTRL1_REG, 0xC1);
> +	hdmi_writeb(ctx, HDMI_RX_PLL_CALREFSEL_REG, 0x03);
> +	hdmi_writeb(ctx, HDMI_RX_PLL_VCOCAL_REG, 0x20);
> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA0_REG, 0x8A);
> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA1_REG, 0x6A);
> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA2_REG, 0xAA);
> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA3_REG, 0xCA);
> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA4_REG, 0xEA);
> +	hdmi_writeb(ctx, HDMI_RX_TMDS_ZONE_CTRL_REG, 0xA0);
> +	hdmi_writeb(ctx, HDMI_RX_TMDS_MODE_CTRL_REG, 0x00);
> +	mhl_tx_writeb(ctx, MHL_TX_TMDS_CCTRL, 0x34);
> +	hdmi_writeb(ctx, 0x45, 0x44);
> +	hdmi_writeb(ctx, 0x31, 0x0A);
> +	hdmi_writeb(ctx, HDMI_RX_TMDS0_CCTRL1_REG, 0xC1);
> +
> +	return sii9234_clear_error(ctx);
> +}
> +
> +static int sii9234_mhl_tx_ctl_int(struct sii9234 *ctx)
> +{
> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL1_REG, 0xD0);
> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL2_REG, 0xFC);
> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL4_REG, 0xEB);
> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL7_REG, 0x0C);
> +
> +	return sii9234_clear_error(ctx);
> +}
> +
> +static int sii9234_reset(struct sii9234 *ctx)
> +{
> +	int ret;
> +
> +	sii9234_clear_error(ctx);
> +
> +	ret = sii9234_power_init(ctx);
> +	if (ret < 0)
> +		return ret;
> +	ret = sii9234_cbus_reset(ctx);
> +	if (ret < 0)
> +		return ret;
> +	ret = sii9234_hdmi_init(ctx);
> +	if (ret < 0)
> +		return ret;
> +	ret = sii9234_mhl_tx_ctl_int(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable HDCP Compliance safety */
> +	mhl_tx_writeb(ctx, 0x2B, 0x01);
> +	/* CBUS discovery cycle time for each drive and float = 150us */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, 0x04, 0x06);
> +	/* Clear bit 6 (reg_skip_rgnd) */
> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL2_REG, (1 << 7) /* Reserved */
> +		| 2 << ATT_THRESH_SHIFT | DEGLITCH_TIME_50MS);
> +	/* Changed from 66 to 65 for 94[1:0] = 01 = 5k reg_cbusmhl_pup_sel
> +	 * 1.8V CBUS VTH & GND threshold
> +	 * to meet CTS 3.3.7.2 spec
> +	 */
> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL5_REG, 0x77);
> +	cbus_writebm(ctx, CBUS_LINK_CONTROL_2_REG, ~0, MHL_INIT_TIMEOUT);
> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL6_REG, 0xA0);
> +	/* RGND & single discovery attempt (RGND blocking) */
> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL6_REG, BLOCK_RGND_INT |
> +			       DVRFLT_SEL | SINGLE_ATT);
> +	/* Use VBUS path of discovery state machine */
> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL8_REG, 0);
> +	/* 0x92[3] sets the CBUS / ID switch */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, USB_ID_OVR);
> +	/* To allow RGND engine to operate correctly.
> +	 * When moving the chip from D2 to D0 (power up, init regs)
> +	 * the values should be
> +	 * 94[1:0] = 01  reg_cbusmhl_pup_sel[1:0] should be set for 5k
> +	 * 93[7:6] = 10  reg_cbusdisc_pup_sel[1:0] should be
> +	 * set for 10k (default)
> +	 * 93[5:4] = 00  reg_cbusidle_pup_sel[1:0] = open (default)
> +	 */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL3_REG, ~0, 0x86);
> +	/* Change from CC to 8C to match 5K
> +	 * to meet CTS 3.3.72 spec
> +	 */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, ~0, 0x8C);
> +	/* Configure the interrupt as active high */
> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 0x06);
> +
> +	msleep(25);
> +
> +	/* Release usb_id switch */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, 0,  USB_ID_OVR);
> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL1_REG, 0x27);
> +
> +	ret = sii9234_clear_error(ctx);
> +	if (ret < 0)
> +		return ret;
> +	ret = sii9234_cbus_init(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable Auto soft reset on SCDT = 0 */
> +	mhl_tx_writeb(ctx, 0x05, 0x04);
> +	/* HDMI Transcode mode enable */
> +	mhl_tx_writeb(ctx, 0x0D, 0x1C);
> +	mhl_tx_writeb(ctx, MHL_TX_INTR4_ENABLE_REG,
> +			       RGND_READY_MASK | CBUS_LKOUT_MASK |
> +			       MHL_DISC_FAIL_MASK | MHL_EST_MASK);
> +	mhl_tx_writeb(ctx, MHL_TX_INTR1_ENABLE_REG, 0x60);
> +
> +	/* This point is very important before measure RGND impedance */
> +	force_usb_id_switch_open(ctx);
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, 0, 0xF0);
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL5_REG, 0, 0x03);
> +	release_usb_id_switch_open(ctx);
> +
> +	/* Force upstream HPD to 0 when not in MHL mode */
> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 1 << 5);
> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, ~0, 1 << 4);
> +
> +	return sii9234_clear_error(ctx);
> +}
> +
> +static int sii9234_goto_d3(struct sii9234 *ctx)
> +{
> +	int ret;
> +
> +	dev_dbg(ctx->dev, "sii9234: detection started d3\n");
> +
> +	ret = sii9234_reset(ctx);
> +	if (ret < 0)
> +		goto exit;
> +
> +	hdmi_writeb(ctx, 0x01, 0x03);
> +	tpi_writebm(ctx, TPI_DPD_REG, 0, 1);
> +	/* I2C above is expected to fail because power goes down */
> +	sii9234_clear_error(ctx);
> +
> +	ctx->state = ST_D3;
> +
> +	return 0;
> + exit:
> +	dev_err(ctx->dev, "%s failed\n", __func__);
> +	return -1;
> +}
> +
> +static int sii9234_hw_on(struct sii9234 *ctx)
> +{
> +	return regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +}
> +
> +static void sii9234_hw_off(struct sii9234 *ctx)
> +{
> +	gpiod_set_value(ctx->gpio_reset, 1);
> +	usleep_range(10000, 20000);

These are big enough for msleep, unless precise wakeup within 10-20 ms
is required?

> +	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +}
> +
> +static void sii9234_hw_reset(struct sii9234 *ctx)
> +{
> +	gpiod_set_value(ctx->gpio_reset, 1);
> +	usleep_range(10000, 20000);

Same.

> +	gpiod_set_value(ctx->gpio_reset, 0);
> +}
> +
> +static void sii9234_cable_in(struct sii9234 *ctx)
> +{
> +	int ret;
> +
> +	mutex_lock(&ctx->lock);
> +	if (ctx->state != ST_OFF)
> +		goto unlock;
> +	ret = sii9234_hw_on(ctx);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	sii9234_hw_reset(ctx);
> +	sii9234_goto_d3(ctx);
> +	/* To avoid irq storm, when hw is in meta state */
> +	enable_irq(to_i2c_client(ctx->dev)->irq);
> +
> +unlock:
> +	mutex_unlock(&ctx->lock);
> +}
> +
> +static void sii9234_cable_out(struct sii9234 *ctx)
> +{
> +	mutex_lock(&ctx->lock);
> +
> +	if (ctx->state == ST_OFF)
> +		goto unlock;
> +
> +	disable_irq(to_i2c_client(ctx->dev)->irq);
> +	tpi_writeb(ctx, TPI_DPD_REG, 0);
> +	/* Turn on&off hpd festure for only QCT HDMI */
> +	sii9234_hw_off(ctx);
> +
> +	ctx->state = ST_OFF;
> +
> +unlock:
> +	mutex_unlock(&ctx->lock);
> +}
> +
> +static enum sii9234_state sii9234_rgnd_ready_irq(struct sii9234 *ctx)
> +{
> +	int value;
> +
> +	if (ctx->state == ST_D3) {
> +		int ret;
> +
> +		dev_dbg(ctx->dev, "RGND_READY_INT\n");
> +		sii9234_hw_reset(ctx);
> +
> +		ret = sii9234_reset(ctx);
> +		if (ret < 0) {
> +			dev_err(ctx->dev, "sii9234_reset() failed\n");
> +			return ST_FAILURE;
> +		}
> +
> +		return ST_RGND_INIT;
> +	}
> +
> +	/* Got interrupt in inappropriate state */
> +	if (ctx->state != ST_RGND_INIT)
> +		return ST_FAILURE;
> +
> +	value = mhl_tx_readb(ctx, MHL_TX_STAT2_REG);
> +	if (sii9234_clear_error(ctx))
> +		return ST_FAILURE;
> +
> +	if ((value & RGND_INTP_MASK) != RGND_INTP_1K) {
> +		dev_warn(ctx->dev, "RGND is not 1k\n");
> +		return ST_RGND_INIT;
> +	}
> +	dev_dbg(ctx->dev, "RGND 1K!!\n");
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, ~0, 0x8C);
> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL5_REG, 0x77);
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, 0x05);
> +	if (sii9234_clear_error(ctx))
> +		return ST_FAILURE;
> +
> +	usleep_range(T_SRC_VBUS_CBUS_TO_STABLE * USEC_PER_MSEC,
> +		     T_SRC_VBUS_CBUS_TO_STABLE * USEC_PER_MSEC);

usleep for 200ms?

Pretty long... maybe msleep()?

> +
> +	return ST_RGND_1K;
> +}
> +
> +static enum sii9234_state sii9234_mhl_established(struct sii9234 *ctx)
> +{
> +	dev_dbg(ctx->dev, "mhl est interrupt\n");
> +
> +	/* Discovery override */
> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL1_REG, 0x10);
> +	/* Increase DDC translation layer timer (byte mode) */
> +	cbus_writeb(ctx, 0x07, 0x32);
> +	cbus_writebm(ctx, 0x44, ~0, 1 << 1);
> +	/* Keep the discovery enabled. Need RGND interrupt */
> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, ~0, 1);
> +	mhl_tx_writeb(ctx, MHL_TX_INTR1_ENABLE_REG,
> +	       RSEN_CHANGE_INT_MASK | HPD_CHANGE_INT_MASK);
> +
> +	if (sii9234_clear_error(ctx))
> +		return ST_FAILURE;
> +
> +	return ST_MHL_ESTABLISHED;
> +}
> +
> +static enum sii9234_state sii9234_hpd_change(struct sii9234 *ctx)
> +{
> +	int value;
> +
> +	value = cbus_readb(ctx, CBUS_MSC_REQ_ABORT_REASON_REG);
> +	if (sii9234_clear_error(ctx))
> +		return ST_FAILURE;
> +
> +	if (value & SET_HPD_DOWNSTREAM) {
> +		/* Downstream HPD High, Enable TMDS */
> +		sii9234_tmds_control(ctx, true);
> +	} else {
> +		/* Downstream HPD Low, Disable TMDS */
> +		sii9234_tmds_control(ctx, false);
> +	}
> +
> +	return ctx->state;
> +}
> +
> +static enum sii9234_state sii9234_rsen_change(struct sii9234 *ctx)
> +{
> +	int value;
> +
> +	/* Work_around code to handle wrong interrupt */
> +	if (ctx->state != ST_RGND_1K) {
> +		dev_err(ctx->dev, "RSEN_HIGH without RGND_1K\n");
> +		return ST_FAILURE;
> +	}
> +	value = mhl_tx_readb(ctx, MHL_TX_SYSSTAT_REG);
> +	if (value < 0)
> +		return ST_FAILURE;
> +
> +	if (value & RSEN_STATUS) {
> +		dev_dbg(ctx->dev, "MHL cable connected.. RSEN High\n");
> +		return ST_RSEN_HIGH;
> +	}
> +	dev_dbg(ctx->dev, "RSEN lost\n");
> +	/* Once RSEN loss is confirmed,we need to check
> +	 * based on cable status and chip power status,whether
> +	 * it is SINK Loss(HDMI cable not connected, TV Off)
> +	 * or MHL cable disconnection
> +	 * TODO: Define the below mhl_disconnection()
> +	 */
> +	msleep(T_SRC_RXSENSE_DEGLITCH);
> +	value = mhl_tx_readb(ctx, MHL_TX_SYSSTAT_REG);
> +	if (value < 0)
> +		return ST_FAILURE;
> +	dev_dbg(ctx->dev, "sys_stat: %x\n", value);
> +
> +	if (value & RSEN_STATUS) {
> +		dev_dbg(ctx->dev, "RSEN recovery\n");
> +		return ST_RSEN_HIGH;
> +	}
> +	dev_dbg(ctx->dev, "RSEN Really LOW\n");
> +	/* To meet CTS 3.3.22.2 spec */
> +	sii9234_tmds_control(ctx, false);
> +	force_usb_id_switch_open(ctx);
> +	release_usb_id_switch_open(ctx);
> +
> +	return ST_FAILURE;
> +}
> +
> +static irqreturn_t sii9234_irq_thread(int irq, void *data)
> +{
> +	struct sii9234 *ctx = data;
> +	int intr1, intr4;
> +	int intr1_en, intr4_en;
> +	int cbus_intr1, cbus_intr2;
> +
> +	dev_dbg(ctx->dev, "%s\n", __func__);
> +
> +	mutex_lock(&ctx->lock);
> +
> +	intr1 = mhl_tx_readb(ctx, MHL_TX_INTR1_REG);
> +	intr4 = mhl_tx_readb(ctx, MHL_TX_INTR4_REG);
> +	intr1_en = mhl_tx_readb(ctx, MHL_TX_INTR1_ENABLE_REG);
> +	intr4_en = mhl_tx_readb(ctx, MHL_TX_INTR4_ENABLE_REG);
> +	cbus_intr1 = cbus_readb(ctx, CBUS_INT_STATUS_1_REG);
> +	cbus_intr2 = cbus_readb(ctx, CBUS_INT_STATUS_2_REG);
> +
> +	if (sii9234_clear_error(ctx))
> +		goto done;
> +
> +	dev_dbg(ctx->dev, "irq %02x/%02x %02x/%02x %02x/%02x\n",
> +		 intr1, intr1_en, intr4, intr4_en, cbus_intr1, cbus_intr2);
> +
> +	if (intr4 & RGND_READY_INT)
> +		ctx->state = sii9234_rgnd_ready_irq(ctx);
> +	if (intr1 & RSEN_CHANGE_INT)
> +		ctx->state = sii9234_rsen_change(ctx);
> +	if (intr4 & MHL_EST_INT)
> +		ctx->state = sii9234_mhl_established(ctx);
> +	if (intr1 & HPD_CHANGE_INT)
> +		ctx->state = sii9234_hpd_change(ctx);
> +	if (intr4 & CBUS_LKOUT_INT)
> +		ctx->state = ST_FAILURE;
> +	if (intr4 & MHL_DISC_FAIL_INT)
> +		ctx->state = ST_FAILURE_DISCOVERY;
> +
> + done:
> +	/* Clean interrupt status and pending flags */
> +	mhl_tx_writeb(ctx, MHL_TX_INTR1_REG, intr1);
> +	mhl_tx_writeb(ctx, MHL_TX_INTR4_REG, intr4);
> +	cbus_writeb(ctx, CBUS_MHL_STATUS_REG_0, 0xFF);
> +	cbus_writeb(ctx, CBUS_MHL_STATUS_REG_1, 0xFF);
> +	cbus_writeb(ctx, CBUS_INT_STATUS_1_REG, cbus_intr1);
> +	cbus_writeb(ctx, CBUS_INT_STATUS_2_REG, cbus_intr2);
> +
> +	sii9234_clear_error(ctx);
> +
> +	if (ctx->state == ST_FAILURE) {
> +		dev_dbg(ctx->dev, "try to reset after failure\n");
> +		sii9234_hw_reset(ctx);
> +		sii9234_goto_d3(ctx);
> +	}
> +
> +	if (ctx->state == ST_FAILURE_DISCOVERY) {
> +		dev_err(ctx->dev, "discovery failed, no power for MHL?\n");
> +		tpi_writebm(ctx, TPI_DPD_REG, 0, 1);
> +		ctx->state = ST_D3;
> +	}
> +
> +	mutex_unlock(&ctx->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +
> +static int sii9234_init_resources(struct sii9234 *ctx,
> +	struct i2c_client *client)
> +{
> +	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> +	int ret;
> +
> +	if (!ctx->dev->of_node) {
> +		dev_err(ctx->dev, "not DT device\n");
> +		return -ENODEV;
> +	}
> +
> +	ctx->gpio_reset = devm_gpiod_get(ctx->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->gpio_reset)) {
> +		dev_err(ctx->dev, "failed to get reset gpio from DT\n");
> +		return PTR_ERR(ctx->gpio_reset);
> +	}
> +
> +	ctx->supplies[0].supply = "cvcc12";
> +	ctx->supplies[1].supply = "avcc33";
> +	ctx->supplies[2].supply = "iovcc18";
> +	ctx->supplies[3].supply = "avcc12";
> +	ret = devm_regulator_bulk_get(ctx->dev, 4, ctx->supplies);
> +	if (ret) {
> +		dev_err(ctx->dev, "regulator_bulk failed\n");
> +		return ret;
> +	}
> +
> +	ctx->client[I2C_MHL] = client;
> +
> +	ctx->client[I2C_TPI] = i2c_new_dummy(adapter, I2C_TPI_ADDR);
> +	if (!ctx->client[I2C_TPI]) {
> +		dev_err(ctx->dev, "failed to create TPI client\n");
> +		return -ENODEV;
> +	}
> +
> +	ctx->client[I2C_HDMI] = i2c_new_dummy(adapter, I2C_HDMI_ADDR);
> +	if (!ctx->client[I2C_HDMI]) {
> +		dev_err(ctx->dev, "failed to create HDMI RX client\n");
> +		goto fail_tpi;
> +	}
> +
> +	ctx->client[I2C_CBUS] = i2c_new_dummy(adapter, I2C_CBUS_ADDR);
> +	if (!ctx->client[I2C_CBUS]) {
> +		dev_err(ctx->dev, "failed to create CBUS client\n");
> +		goto fail_hdmi;
> +	}

You are using i2c_smbus_* calls. Why not converting to regmap_i2c? It is
preferred interface.

> +
> +	return 0;
> +
> +fail_hdmi:
> +	i2c_unregister_device(ctx->client[I2C_HDMI]);
> +fail_tpi:
> +	i2c_unregister_device(ctx->client[I2C_TPI]);
> +
> +	return -ENODEV;
> +}
> +
> +static void sii9234_deinit_resources(struct sii9234 *ctx)
> +{
> +	i2c_unregister_device(ctx->client[I2C_CBUS]);
> +	i2c_unregister_device(ctx->client[I2C_HDMI]);
> +	i2c_unregister_device(ctx->client[I2C_TPI]);
> +}
> +
> +static inline struct sii9234 *bridge_to_sii9234(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct sii9234, bridge);
> +}
> +
> +static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
> +					const struct drm_display_mode *mode)
> +{
> +	if (mode->clock > MHL1_MAX_CLK)
> +		return MODE_CLOCK_HIGH;
> +
> +	return MODE_OK;
> +}
> +
> +static const struct drm_bridge_funcs sii9234_bridge_funcs = {
> +	.mode_valid = sii9234_mode_valid,
> +};
> +
> +static int sii9234_probe(struct i2c_client *client,
> +					      const struct i2c_device_id *id)
> +{
> +	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> +	struct sii9234 *ctx;
> +	struct device *dev = &client->dev;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->dev = dev;
> +	mutex_init(&ctx->lock);
> +
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> +		dev_err(dev, "I2C adapter lacks SMBUS feature\n");
> +		return -EIO;
> +	}
> +
> +	if (!client->irq) {
> +		dev_err(dev, "no irq provided\n");
> +		return -EINVAL;
> +	}
> +
> +	irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
> +	ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +					sii9234_irq_thread,
> +					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> +					"sii9234", ctx);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to install IRQ handler\n");
> +		return ret;
> +	}

You are setting up interrupts before initialization of I2C. Can the
interrupt happen in this short window?

> +
> +	ret = sii9234_init_resources(ctx, client);
> +	if (ret < 0)
> +		return ret;
> +
> +	i2c_set_clientdata(client, ctx);
> +
> +	ctx->bridge.funcs = &sii9234_bridge_funcs;
> +	ctx->bridge.of_node = dev->of_node;
> +	drm_bridge_add(&ctx->bridge);
> +
> +	sii9234_cable_in(ctx);
> +
> +	return 0;
> +}
> +
> +static int sii9234_remove(struct i2c_client *client)
> +{
> +	struct sii9234 *ctx = i2c_get_clientdata(client);
> +
> +	sii9234_cable_out(ctx);
> +	drm_bridge_remove(&ctx->bridge);
> +	sii9234_deinit_resources(ctx);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sii9234_dt_match[] = {
> +	{ .compatible = "sil,sii9234" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, sii9234_dt_match);
> +
> +static const struct i2c_device_id sii9234_id[] = {
> +	{ "SII9234", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, sii9234_id);
> +
> +static struct i2c_driver sii9234_driver = {
> +	.driver = {
> +		.name	= "sii9234",
> +		.of_match_table = sii9234_dt_match,
> +	},
> +	.probe		= sii9234_probe,
> +	.remove		= sii9234_remove,
> +	.id_table = sii9234_id,

Some assignments above are indented, some not...

Best regards,
Krzysztof

> +};
> +
> +module_i2c_driver(sii9234_driver);
> +MODULE_LICENSE("GPL");
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maciej Purski Sept. 11, 2017, 11:42 a.m. UTC | #2
Hi Krzysztof,
thank you for your comments.

On 08.09.2017 at 19:05, Krzysztof Kozlowski wrote:

> On Tue, Sep 05, 2017 at 04:01:38PM +0200, Maciej Purski wrote:
>> SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
>> It is controlled via I2C bus. Its interaction with other
>> devices in video pipeline is performed mainly on HW level.
>> The only interaction it does on device driver level is
>> filtering-out unsupported video modes, it exposes drm_bridge
>> interface to perform this operation.
>>
>> This patch is based on the code refactored by Tomasz Stanislawski
>> <t.stanislaws@samsung.com>, which was initially developed by:
>> Adam Hampson <ahampson@sta.samsung.com>
>> Erik Gilling <konkers@android.com>
>> Shankar Bandal <shankar.b@samsung.com>
>> Dharam Kumar <dharam.kr@samsung.com>
>>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>>   .../devicetree/bindings/display/bridge/sii9234.txt |  34 +
>>   drivers/gpu/drm/bridge/Kconfig                     |   8 +
>>   drivers/gpu/drm/bridge/Makefile                    |   1 +
>>   drivers/gpu/drm/bridge/sii9234.c                   | 993 +++++++++++++++++++++
>>   4 files changed, 1036 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/display/bridge/sii9234.txt
>>   create mode 100644 drivers/gpu/drm/bridge/sii9234.c
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
>> new file mode 100644
>> index 0000000..3ce7413
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
>> @@ -0,0 +1,34 @@
>> +Silicon Image SiI9234 HDMI/MHL bridge bindings
>> +
>> +Required properties:
>> +	- compatible : "sil,sii9234".
>> +	- reg : I2C address for TPI interface, use 0x39
>> +	- avcc33-supply : MHL/USB Switch Supply Voltage (3.3V)
>> +	- iovcc18-supply : I/O Supply Voltage (1.8V)
>> +	- avcc12-supply : TMDS Analog Supply Voltage (1.2V)
>> +	- cvcc12-supply : Digital Core Supply Voltage (1.2V)
>> +	- interrupts, interrupt-parent: interrupt specifier of INT pin
>> +	- reset-gpios: gpio specifier of RESET pin (active low)
>> +	- video interfaces: Device node can contain video interface port
>> +			    node for HDMI encoder according to [1].
>> +
>> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>> +
>> +Example:
>> +	sii9234@39 {
>> +		compatible = "sil,sii9234";
>> +		reg = <0x39>;
>> +		avcc33-supply = <&vcc33mhl>;
>> +		iovcc18-supply = <&vcc18mhl>;
>> +		avcc12-supply = <&vsil12>;
>> +		cvcc12-supply = <&vsil12>;
>> +		reset-gpios = <&gpf3 4 GPIO_ACTIVE_LOW>;
>> +		interrupt-parent = <&gpf3>;
>> +		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +		port {
>> +			mhl_to_hdmi: endpoint {
>> +				remote-endpoint = <&hdmi_to_mhl>;
>> +			};
>> +		};
>> +	};
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index adf9ae0..9dba16fd 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -84,6 +84,14 @@ config DRM_SII902X
>>   	---help---
>>   	  Silicon Image sii902x bridge chip driver.
>>   
>> +config DRM_SII9234
>> +	tristate "Silicon Image SII9234 HDMI/MHL bridge"
>> +	depends on OF
>> +	---help---
>> +	  Say Y here if you want support for the MHL interface.
>> +	  It is an I2C driver, that detects connection of MHL bridge
>> +	  and starts encapsulation of HDMI signal.
>> +
>>   config DRM_TOSHIBA_TC358767
>>   	tristate "Toshiba TC358767 eDP bridge"
>>   	depends on OF
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index defcf1e..e3d5eb0 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>>   obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>>   obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>>   obj-$(CONFIG_DRM_SII902X) += sii902x.o
>> +obj-$(CONFIG_DRM_SII9234) += sii9234.o
>>   obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>   obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>>   obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>> diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
>> new file mode 100644
>> index 0000000..b70d69a
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/sii9234.c
>> @@ -0,0 +1,993 @@
>> +/*
>> + * Copyright (C) 2017 Samsung Electronics
>> + *
>> + * Authors:
>> + *    Tomasz Stanislawski <t.stanislaws@samsung.com>
>> + *    Maciej Purski <m.purski@samsung.com>
>> + *
>> + * Based on sii9234 driver created by:
>> + *    Adam Hampson <ahampson@sta.samsung.com>
>> + *    Erik Gilling <konkers@android.com>
>> + *    Shankar Bandal <shankar.b@samsung.com>
>> + *    Dharam Kumar <dharam.kr@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program
>> + *
>> + */
>> +#include <drm/bridge/mhl.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_edid.h>
>> +
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/slab.h>
>> +
>> +#define CBUS_DEVCAP_OFFSET		0x80
>> +
>> +#define SII9234_MHL_VERSION		0x11
>> +#define SII9234_SCRATCHPAD_SIZE		0x10
>> +#define SII9234_INT_STAT_SIZE		0x33
>> +
>> +#define BIT_TMDS_CCTRL_TMDS_OE		BIT(4)
>> +#define MHL_HPD_OUT_OVR_EN		BIT(4)
>> +#define MHL_HPD_OUT_OVR_VAL		BIT(5)
>> +#define MHL_INIT_TIMEOUT		0x0C
>> +
>> +/* MHL Tx registers and bits */
>> +#define MHL_TX_SRST                     0x05
>> +#define MHL_TX_SYSSTAT_REG              0x09
>> +#define MHL_TX_INTR1_REG                0x71
>> +#define MHL_TX_INTR4_REG                0x74
>> +#define MHL_TX_INTR1_ENABLE_REG         0x75
>> +#define MHL_TX_INTR4_ENABLE_REG         0x78
>> +#define MHL_TX_INT_CTRL_REG             0x79
>> +#define MHL_TX_TMDS_CCTRL               0x80
>> +#define MHL_TX_DISC_CTRL1_REG           0x90
>> +#define MHL_TX_DISC_CTRL2_REG           0x91
>> +#define MHL_TX_DISC_CTRL3_REG           0x92
>> +#define MHL_TX_DISC_CTRL4_REG		0x93
>> +#define MHL_TX_DISC_CTRL5_REG           0x94
>> +#define MHL_TX_DISC_CTRL6_REG           0x95
>> +#define MHL_TX_DISC_CTRL7_REG           0x96
>> +#define MHL_TX_DISC_CTRL8_REG		0x97
>> +#define MHL_TX_STAT2_REG                0x99
>                                   ^^^^^^^^
> Sometimes tabs, sometimes spaces. Please make it consistent: tabs.

I'll fix it.

>
>
>> +#define MHL_TX_MHLTX_CTL1_REG           0xA0
>> +#define MHL_TX_MHLTX_CTL2_REG           0xA1
>> +#define MHL_TX_MHLTX_CTL4_REG           0xA3
>> +#define MHL_TX_MHLTX_CTL6_REG           0xA5
>> +#define MHL_TX_MHLTX_CTL7_REG           0xA6
>> +
>> +
>> +#define RSEN_STATUS			BIT(2)
>> +#define HPD_CHANGE_INT			BIT(6)
>> +#define RSEN_CHANGE_INT			BIT(5)
>> +#define RGND_READY_INT			BIT(6)
>> +#define VBUS_LOW_INT			BIT(5)
>> +#define CBUS_LKOUT_INT			BIT(4)
>> +#define MHL_DISC_FAIL_INT		BIT(3)
>> +#define MHL_EST_INT			BIT(2)
>> +#define HPD_CHANGE_INT_MASK		BIT(6)
>> +#define RSEN_CHANGE_INT_MASK		BIT(5)
>> +
>> +#define RGND_READY_MASK			BIT(6)
>> +#define CBUS_LKOUT_MASK			BIT(4)
>> +#define MHL_DISC_FAIL_MASK		BIT(3)
>> +#define MHL_EST_MASK			BIT(2)
>> +
>> +#define SKIP_GND			BIT(6)
>> +
>> +#define ATT_THRESH_SHIFT		0x04
>> +#define ATT_THRESH_MASK			(0x03 << ATT_THRESH_SHIFT)
>> +#define USB_D_OEN			BIT(3)
>> +#define DEGLITCH_TIME_MASK		0x07
>> +#define DEGLITCH_TIME_2MS		0
>> +#define DEGLITCH_TIME_4MS		1
>> +#define DEGLITCH_TIME_8MS		2
>> +#define DEGLITCH_TIME_16MS		3
>> +#define DEGLITCH_TIME_40MS		4
>> +#define DEGLITCH_TIME_50MS		5
>> +#define DEGLITCH_TIME_60MS		6
>> +#define DEGLITCH_TIME_128MS		7
>> +
>> +#define USB_D_OVR			BIT(7)
>> +#define USB_ID_OVR			BIT(6)
>> +#define DVRFLT_SEL			BIT(5)
>> +#define BLOCK_RGND_INT			BIT(4)
>> +#define SKIP_DEG			BIT(3)
>> +#define CI2CA_POL			BIT(2)
>> +#define CI2CA_WKUP			BIT(1)
>> +#define SINGLE_ATT			BIT(0)
>> +
>> +#define USB_D_ODN			BIT(5)
>> +#define VBUS_CHECK			BIT(2)
>> +#define RGND_INTP_MASK			0x03
>> +#define RGND_INTP_OPEN			0
>> +#define RGND_INTP_2K			1
>> +#define RGND_INTP_1K			2
>> +#define RGND_INTP_SHORT			3
>> +
>> +/* HDMI registers */
>> +#define HDMI_RX_TMDS0_CCTRL1_REG	0x10
>> +#define HDMI_RX_TMDS_CLK_EN_REG		0x11
>> +#define HDMI_RX_TMDS_CH_EN_REG		0x12
>> +#define HDMI_RX_PLL_CALREFSEL_REG	0x17
>> +#define HDMI_RX_PLL_VCOCAL_REG		0x1A
>> +#define HDMI_RX_EQ_DATA0_REG		0x22
>> +#define HDMI_RX_EQ_DATA1_REG		0x23
>> +#define HDMI_RX_EQ_DATA2_REG		0x24
>> +#define HDMI_RX_EQ_DATA3_REG		0x25
>> +#define HDMI_RX_EQ_DATA4_REG		0x26
>> +#define HDMI_RX_TMDS_ZONE_CTRL_REG	0x4C
>> +#define HDMI_RX_TMDS_MODE_CTRL_REG	0x4D
>> +
>> +/* CBUS registers */
>> +#define CBUS_INT_STATUS_1_REG		0x08
>> +#define CBUS_INTR1_ENABLE_REG		0x09
>> +#define CBUS_MSC_REQ_ABORT_REASON_REG	0x0D
>> +#define CBUS_INT_STATUS_2_REG		0x1E
>> +#define CBUS_INTR2_ENABLE_REG		0x1F
>> +#define CBUS_LINK_CONTROL_2_REG		0x31
>> +#define CBUS_MHL_STATUS_REG_0		0xB0
>> +#define CBUS_MHL_STATUS_REG_1		0xB1
>> +
>> +#define BIT_CBUS_RESET			BIT(3)
>> +#define SET_HPD_DOWNSTREAM		BIT(6)
>> +
>> +/* TPI registers */
>> +#define TPI_DPD_REG			0x3D
>> +
>> +/* timeouts */
> Timeout in what kind of units?

I will unify them to MSEC.

>
>> +#define T_SRC_VBUS_CBUS_TO_STABLE	200
>> +#define T_SRC_CBUS_FLOAT		100
>> +#define T_SRC_CBUS_DEGLITCH		2
>> +#define T_SRC_RXSENSE_DEGLITCH		110
>> +#define MHL1_MAX_CLK			75000
>> +
>> +/* I2C addresses */
> Rather useless comment.
>
>> +#define I2C_TPI_ADDR			0x3D
>> +#define I2C_HDMI_ADDR			0x49
>> +#define I2C_CBUS_ADDR			0x64
>> +
>> +enum sii9234_state {
>> +	ST_OFF,
>> +	ST_D3,
>> +	ST_RGND_INIT,
>> +	ST_RGND_1K,
>> +	ST_RSEN_HIGH,
>> +	ST_MHL_ESTABLISHED,
>> +	ST_FAILURE_DISCOVERY,
>> +	ST_FAILURE,
>> +};
>> +
>> +struct sii9234 {
>> +	struct i2c_client *client[4];
>> +	struct drm_bridge bridge;
>> +	struct device *dev;
>> +	struct gpio_desc *gpio_reset;
>> +	int i2c_error;
>> +	struct regulator_bulk_data supplies[4];
>> +
>> +	enum sii9234_state state;
>> +	struct mutex lock;
>> +};
>> +
>> +enum sii9234_client_id {
>> +	I2C_MHL,
>> +	I2C_TPI,
>> +	I2C_HDMI,
>> +	I2C_CBUS,
>> +};
>> +
>> +static char *sii9234_client_name[] = {
> static const char *
>
>> +	[I2C_MHL] = "MHL",
>> +	[I2C_TPI] = "TPI",
>> +	[I2C_HDMI] = "HDMI",
>> +	[I2C_CBUS] = "CBUS",
>> +};
>> +
>> +static int sii9234_writeb(struct sii9234 *ctx, int id, int offset,
>> +			    int value)
>> +{
>> +	int ret;
>> +	struct i2c_client *client = ctx->client[id];
>> +
>> +	if (ctx->i2c_error)
>> +		return ctx->i2c_error;
>> +
>> +	ret = i2c_smbus_write_byte_data(client, offset, value);
>> +	if (ret < 0)
>> +		dev_err(ctx->dev, "writeb: %4s[0x%02x] <- 0x%02x\n",
>> +			sii9234_client_name[id], offset, value);
>> +	ctx->i2c_error = ret;
>> +
>> +	return ret;
>> +}
>> +
>> +static int sii9234_writebm(struct sii9234 *ctx, int id, int offset,
>> +			    int value, int mask)
>> +{
>> +	int ret;
>> +	struct i2c_client *client = ctx->client[id];
>> +
>> +	if (ctx->i2c_error)
>> +		return ctx->i2c_error;
>> +
>> +	ret = i2c_smbus_write_byte(client, offset);
>> +	if (ret < 0) {
>> +		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
>> +			sii9234_client_name[id], offset, value);
>> +		ctx->i2c_error = ret;
>> +		return ret;
>> +	}
>> +
>> +	ret = i2c_smbus_read_byte(client);
>> +	if (ret < 0) {
>> +		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
>> +			sii9234_client_name[id], offset, value);
>> +		ctx->i2c_error = ret;
>> +		return ret;
>> +	}
>> +
>> +	value = (value & mask) | (ret & ~mask);
>> +
>> +	ret = i2c_smbus_write_byte_data(client, offset, value);
>> +	if (ret < 0) {
>> +		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
>> +			sii9234_client_name[id], offset, value);
>> +		ctx->i2c_error = ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int sii9234_readb(struct sii9234 *ctx, int id, int offset)
>> +{
>> +	int ret;
>> +	struct i2c_client *client = ctx->client[id];
>> +
>> +	if (ctx->i2c_error)
>> +		return ctx->i2c_error;
>> +
>> +	ret = i2c_smbus_write_byte(client, offset);
>> +	if (ret < 0) {
>> +		dev_err(ctx->dev, "readb: %4s[0x%02x]\n",
>> +			sii9234_client_name[id], offset);
>> +		ctx->i2c_error = ret;
>> +		return ret;
>> +	}
>> +
>> +	ret = i2c_smbus_read_byte(client);
>> +	if (ret < 0) {
>> +		dev_err(ctx->dev, "readb: %4s[0x%02x]\n",
>> +			sii9234_client_name[id], offset);
>> +		ctx->i2c_error = ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int sii9234_clear_error(struct sii9234 *ctx)
>> +{
>> +	int ret = ctx->i2c_error;
>> +
>> +	ctx->i2c_error = 0;
>> +
>> +	return ret;
>> +}
>> +
>> +#define mhl_tx_writeb(sii9234, offset, value) \
>> +	sii9234_writeb(sii9234, I2C_MHL, offset, value)
>> +#define mhl_tx_writebm(sii9234, offset, value, mask) \
>> +	sii9234_writebm(sii9234, I2C_MHL, offset, value, mask)
>> +#define mhl_tx_readb(sii9234, offset) \
>> +	sii9234_readb(sii9234, I2C_MHL, offset)
>> +#define cbus_writeb(sii9234, offset, value) \
>> +	sii9234_writeb(sii9234, I2C_CBUS, offset, value)
>> +#define cbus_writebm(sii9234, offset, value, mask) \
>> +	sii9234_writebm(sii9234, I2C_CBUS, offset, value, mask)
>> +#define cbus_readb(sii9234, offset) \
>> +	sii9234_readb(sii9234, I2C_CBUS, offset)
>> +#define hdmi_writeb(sii9234, offset, value) \
>> +	sii9234_writeb(sii9234, I2C_HDMI, offset, value)
>> +#define hdmi_writebm(sii9234, offset, value, mask) \
>> +	sii9234_writebm(sii9234, I2C_HDMI, offset, value, mask)
>> +#define hdmi_readb(sii9234, offset) \
>> +	sii9234_readb(sii9234, I2C_HDMI, offset)
>> +#define tpi_writeb(sii9234, offset, value) \
>> +	sii9234_writeb(sii9234, I2C_TPI, offset, value)
>> +#define tpi_writebm(sii9234, offset, value, mask) \
>> +	sii9234_writebm(sii9234, I2C_TPI, offset, value, mask)
>> +#define tpi_readb(sii9234, offset) \
>> +	sii9234_readb(sii9234, I2C_TPI, offset)
>> +
>> +static u8 sii9234_tmds_control(struct sii9234 *ctx, bool enable)
>> +{
>> +	mhl_tx_writebm(ctx, MHL_TX_TMDS_CCTRL, enable ? ~0 : 0,
>> +						BIT_TMDS_CCTRL_TMDS_OE);
>> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, enable ? ~0 : 0,
>> +				MHL_HPD_OUT_OVR_EN | MHL_HPD_OUT_OVR_VAL);
>> +	return sii9234_clear_error(ctx);
>> +}
>> +
>> +static int sii9234_cbus_reset(struct sii9234 *ctx)
>> +{
>> +	int i;
>> +
>> +	mhl_tx_writebm(ctx, MHL_TX_SRST, ~0, BIT_CBUS_RESET);
>> +	msleep(T_SRC_CBUS_DEGLITCH);
>> +	mhl_tx_writebm(ctx, MHL_TX_SRST, 0, BIT_CBUS_RESET);
>> +
>> +	for (i = 0; i < 4; i++) {
>> +		/* Enable WRITE_STAT interrupt for writes to all
>> +		 * 4 MSC Status registers.
>> +		 */
>> +		cbus_writeb(ctx, 0xE0 + i, 0xF2);
>> +		/* Enable SET_INT interrupt for writes to all
>> +		 * 4 MSC Interrupt registers.
>> +		 */
> Comments with leading /*:
> /*
>   * foo bar
>   * bar foo
>   */
>
> Please run checkpatch and fix the warnings everywhere.

I'll fix it.

>
>> +		cbus_writeb(ctx, 0xF0 + i, 0xF2);
>> +	}
>> +
>> +	return sii9234_clear_error(ctx);
>> +}
>> +
>> +/* Require to chek mhl imformation of samsung in cbus_init_register */
>> +static int sii9234_cbus_init(struct sii9234 *ctx)
>> +{
>> +	cbus_writeb(ctx, 0x07, 0xF2);
>> +	cbus_writeb(ctx, 0x40, 0x03);
>> +	cbus_writeb(ctx, 0x42, 0x06);
>> +	cbus_writeb(ctx, 0x36, 0x0C);
>> +	cbus_writeb(ctx, 0x3D, 0xFD);
>> +	cbus_writeb(ctx, 0x1C, 0x01);
>> +	cbus_writeb(ctx, 0x1D, 0x0F);
>> +	cbus_writeb(ctx, 0x44, 0x02);
>> +	/* Setup our devcap */
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEV_STATE, 0x00);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_MHL_VERSION,
>> +							SII9234_MHL_VERSION);
> Please align indentation with parenthesis.
>
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_CAT,
>> +							MHL_DCAP_CAT_SOURCE);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_ADOPTER_ID_H, 0x01);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_ADOPTER_ID_L, 0x41);
>> +
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_VID_LINK_MODE,
>> +			MHL_DCAP_VID_LINK_RGB444 | MHL_DCAP_VID_LINK_YCBCR444);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_VIDEO_TYPE,
>> +							MHL_DCAP_VT_GRAPHICS);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_LOG_DEV_MAP,
>> +							MHL_DCAP_LD_GUI);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_BANDWIDTH, 0x0F);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_FEATURE_FLAG,
>> +		    MHL_DCAP_FEATURE_RCP_SUPPORT | MHL_DCAP_FEATURE_RAP_SUPPORT
>> +		   | MHL_DCAP_FEATURE_SP_SUPPORT);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEVICE_ID_H, 0x0);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEVICE_ID_L, 0x0);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_SCRATCHPAD_SIZE,
>> +						SII9234_SCRATCHPAD_SIZE);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_INT_STAT_SIZE,
>> +						SII9234_INT_STAT_SIZE);
>> +	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_RESERVED, 0);
>> +	cbus_writebm(ctx, 0x31, 0x0C, 0x0C);
>> +	cbus_writeb(ctx, 0x30, 0x01);
>> +	cbus_writebm(ctx, 0x3C, 0x30, 0x38);
>> +	cbus_writebm(ctx, 0x22, 0x0D, 0x0F);
>> +	cbus_writebm(ctx, 0x2E, 0x15, 0x15);
>> +	cbus_writeb(ctx, CBUS_INTR1_ENABLE_REG, 0);
>> +	cbus_writeb(ctx, CBUS_INTR2_ENABLE_REG, 0);
>> +
>> +	return sii9234_clear_error(ctx);
>> +}
>> +
>> +static void force_usb_id_switch_open(struct sii9234 *ctx)
>> +{
>> +	/* Disable CBUS discovery */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, 0, 0x01);
>> +	/* Force USB ID switch to open */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, USB_ID_OVR);
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL3_REG, ~0, 0x86);
>> +	/* Force upstream HPD to 0 when not in MHL mode. */
>> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 0x30);
>> +}
>> +
>> +static void release_usb_id_switch_open(struct sii9234 *ctx)
>> +{
>> +	msleep(T_SRC_CBUS_FLOAT);
>> +	/* Clear USB ID switch to open */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, 0, USB_ID_OVR);
>> +	/* Enable CBUS discovery */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, ~0, 0x01);
>> +}
>> +
>> +static int sii9234_power_init(struct sii9234 *ctx)
>> +{
>> +	/* Force the SiI9234 into the D0 state. */
>> +	tpi_writeb(ctx, TPI_DPD_REG, 0x3F);
>> +	/* Enable TxPLL Clock */
>> +	hdmi_writeb(ctx, HDMI_RX_TMDS_CLK_EN_REG, 0x01);
>> +	/* Enable Tx Clock Path & Equalizer */
>> +	hdmi_writeb(ctx, HDMI_RX_TMDS_CH_EN_REG, 0x15);
>> +	/* Power Up TMDS */
>> +	mhl_tx_writeb(ctx, 0x08, 0x35);
>> +	return sii9234_clear_error(ctx);
>> +}
>> +
>> +static int sii9234_hdmi_init(struct sii9234 *ctx)
>> +{
>> +	hdmi_writeb(ctx, HDMI_RX_TMDS0_CCTRL1_REG, 0xC1);
>> +	hdmi_writeb(ctx, HDMI_RX_PLL_CALREFSEL_REG, 0x03);
>> +	hdmi_writeb(ctx, HDMI_RX_PLL_VCOCAL_REG, 0x20);
>> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA0_REG, 0x8A);
>> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA1_REG, 0x6A);
>> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA2_REG, 0xAA);
>> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA3_REG, 0xCA);
>> +	hdmi_writeb(ctx, HDMI_RX_EQ_DATA4_REG, 0xEA);
>> +	hdmi_writeb(ctx, HDMI_RX_TMDS_ZONE_CTRL_REG, 0xA0);
>> +	hdmi_writeb(ctx, HDMI_RX_TMDS_MODE_CTRL_REG, 0x00);
>> +	mhl_tx_writeb(ctx, MHL_TX_TMDS_CCTRL, 0x34);
>> +	hdmi_writeb(ctx, 0x45, 0x44);
>> +	hdmi_writeb(ctx, 0x31, 0x0A);
>> +	hdmi_writeb(ctx, HDMI_RX_TMDS0_CCTRL1_REG, 0xC1);
>> +
>> +	return sii9234_clear_error(ctx);
>> +}
>> +
>> +static int sii9234_mhl_tx_ctl_int(struct sii9234 *ctx)
>> +{
>> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL1_REG, 0xD0);
>> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL2_REG, 0xFC);
>> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL4_REG, 0xEB);
>> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL7_REG, 0x0C);
>> +
>> +	return sii9234_clear_error(ctx);
>> +}
>> +
>> +static int sii9234_reset(struct sii9234 *ctx)
>> +{
>> +	int ret;
>> +
>> +	sii9234_clear_error(ctx);
>> +
>> +	ret = sii9234_power_init(ctx);
>> +	if (ret < 0)
>> +		return ret;
>> +	ret = sii9234_cbus_reset(ctx);
>> +	if (ret < 0)
>> +		return ret;
>> +	ret = sii9234_hdmi_init(ctx);
>> +	if (ret < 0)
>> +		return ret;
>> +	ret = sii9234_mhl_tx_ctl_int(ctx);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* Enable HDCP Compliance safety */
>> +	mhl_tx_writeb(ctx, 0x2B, 0x01);
>> +	/* CBUS discovery cycle time for each drive and float = 150us */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, 0x04, 0x06);
>> +	/* Clear bit 6 (reg_skip_rgnd) */
>> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL2_REG, (1 << 7) /* Reserved */
>> +		| 2 << ATT_THRESH_SHIFT | DEGLITCH_TIME_50MS);
>> +	/* Changed from 66 to 65 for 94[1:0] = 01 = 5k reg_cbusmhl_pup_sel
>> +	 * 1.8V CBUS VTH & GND threshold
>> +	 * to meet CTS 3.3.7.2 spec
>> +	 */
>> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL5_REG, 0x77);
>> +	cbus_writebm(ctx, CBUS_LINK_CONTROL_2_REG, ~0, MHL_INIT_TIMEOUT);
>> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL6_REG, 0xA0);
>> +	/* RGND & single discovery attempt (RGND blocking) */
>> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL6_REG, BLOCK_RGND_INT |
>> +			       DVRFLT_SEL | SINGLE_ATT);
>> +	/* Use VBUS path of discovery state machine */
>> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL8_REG, 0);
>> +	/* 0x92[3] sets the CBUS / ID switch */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, USB_ID_OVR);
>> +	/* To allow RGND engine to operate correctly.
>> +	 * When moving the chip from D2 to D0 (power up, init regs)
>> +	 * the values should be
>> +	 * 94[1:0] = 01  reg_cbusmhl_pup_sel[1:0] should be set for 5k
>> +	 * 93[7:6] = 10  reg_cbusdisc_pup_sel[1:0] should be
>> +	 * set for 10k (default)
>> +	 * 93[5:4] = 00  reg_cbusidle_pup_sel[1:0] = open (default)
>> +	 */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL3_REG, ~0, 0x86);
>> +	/* Change from CC to 8C to match 5K
>> +	 * to meet CTS 3.3.72 spec
>> +	 */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, ~0, 0x8C);
>> +	/* Configure the interrupt as active high */
>> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 0x06);
>> +
>> +	msleep(25);
>> +
>> +	/* Release usb_id switch */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, 0,  USB_ID_OVR);
>> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL1_REG, 0x27);
>> +
>> +	ret = sii9234_clear_error(ctx);
>> +	if (ret < 0)
>> +		return ret;
>> +	ret = sii9234_cbus_init(ctx);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* Enable Auto soft reset on SCDT = 0 */
>> +	mhl_tx_writeb(ctx, 0x05, 0x04);
>> +	/* HDMI Transcode mode enable */
>> +	mhl_tx_writeb(ctx, 0x0D, 0x1C);
>> +	mhl_tx_writeb(ctx, MHL_TX_INTR4_ENABLE_REG,
>> +			       RGND_READY_MASK | CBUS_LKOUT_MASK |
>> +			       MHL_DISC_FAIL_MASK | MHL_EST_MASK);
>> +	mhl_tx_writeb(ctx, MHL_TX_INTR1_ENABLE_REG, 0x60);
>> +
>> +	/* This point is very important before measure RGND impedance */
>> +	force_usb_id_switch_open(ctx);
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, 0, 0xF0);
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL5_REG, 0, 0x03);
>> +	release_usb_id_switch_open(ctx);
>> +
>> +	/* Force upstream HPD to 0 when not in MHL mode */
>> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 1 << 5);
>> +	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, ~0, 1 << 4);
>> +
>> +	return sii9234_clear_error(ctx);
>> +}
>> +
>> +static int sii9234_goto_d3(struct sii9234 *ctx)
>> +{
>> +	int ret;
>> +
>> +	dev_dbg(ctx->dev, "sii9234: detection started d3\n");
>> +
>> +	ret = sii9234_reset(ctx);
>> +	if (ret < 0)
>> +		goto exit;
>> +
>> +	hdmi_writeb(ctx, 0x01, 0x03);
>> +	tpi_writebm(ctx, TPI_DPD_REG, 0, 1);
>> +	/* I2C above is expected to fail because power goes down */
>> +	sii9234_clear_error(ctx);
>> +
>> +	ctx->state = ST_D3;
>> +
>> +	return 0;
>> + exit:
>> +	dev_err(ctx->dev, "%s failed\n", __func__);
>> +	return -1;
>> +}
>> +
>> +static int sii9234_hw_on(struct sii9234 *ctx)
>> +{
>> +	return regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>> +}
>> +
>> +static void sii9234_hw_off(struct sii9234 *ctx)
>> +{
>> +	gpiod_set_value(ctx->gpio_reset, 1);
>> +	usleep_range(10000, 20000);
> These are big enough for msleep, unless precise wakeup within 10-20 ms
> is required?

Ok, I can change it to msleep.

>
>> +	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>> +}
>> +
>> +static void sii9234_hw_reset(struct sii9234 *ctx)
>> +{
>> +	gpiod_set_value(ctx->gpio_reset, 1);
>> +	usleep_range(10000, 20000);
> Same.
>
>> +	gpiod_set_value(ctx->gpio_reset, 0);
>> +}
>> +
>> +static void sii9234_cable_in(struct sii9234 *ctx)
>> +{
>> +	int ret;
>> +
>> +	mutex_lock(&ctx->lock);
>> +	if (ctx->state != ST_OFF)
>> +		goto unlock;
>> +	ret = sii9234_hw_on(ctx);
>> +	if (ret < 0)
>> +		goto unlock;
>> +
>> +	sii9234_hw_reset(ctx);
>> +	sii9234_goto_d3(ctx);
>> +	/* To avoid irq storm, when hw is in meta state */
>> +	enable_irq(to_i2c_client(ctx->dev)->irq);
>> +
>> +unlock:
>> +	mutex_unlock(&ctx->lock);
>> +}
>> +
>> +static void sii9234_cable_out(struct sii9234 *ctx)
>> +{
>> +	mutex_lock(&ctx->lock);
>> +
>> +	if (ctx->state == ST_OFF)
>> +		goto unlock;
>> +
>> +	disable_irq(to_i2c_client(ctx->dev)->irq);
>> +	tpi_writeb(ctx, TPI_DPD_REG, 0);
>> +	/* Turn on&off hpd festure for only QCT HDMI */
>> +	sii9234_hw_off(ctx);
>> +
>> +	ctx->state = ST_OFF;
>> +
>> +unlock:
>> +	mutex_unlock(&ctx->lock);
>> +}
>> +
>> +static enum sii9234_state sii9234_rgnd_ready_irq(struct sii9234 *ctx)
>> +{
>> +	int value;
>> +
>> +	if (ctx->state == ST_D3) {
>> +		int ret;
>> +
>> +		dev_dbg(ctx->dev, "RGND_READY_INT\n");
>> +		sii9234_hw_reset(ctx);
>> +
>> +		ret = sii9234_reset(ctx);
>> +		if (ret < 0) {
>> +			dev_err(ctx->dev, "sii9234_reset() failed\n");
>> +			return ST_FAILURE;
>> +		}
>> +
>> +		return ST_RGND_INIT;
>> +	}
>> +
>> +	/* Got interrupt in inappropriate state */
>> +	if (ctx->state != ST_RGND_INIT)
>> +		return ST_FAILURE;
>> +
>> +	value = mhl_tx_readb(ctx, MHL_TX_STAT2_REG);
>> +	if (sii9234_clear_error(ctx))
>> +		return ST_FAILURE;
>> +
>> +	if ((value & RGND_INTP_MASK) != RGND_INTP_1K) {
>> +		dev_warn(ctx->dev, "RGND is not 1k\n");
>> +		return ST_RGND_INIT;
>> +	}
>> +	dev_dbg(ctx->dev, "RGND 1K!!\n");
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, ~0, 0x8C);
>> +	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL5_REG, 0x77);
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, 0x05);
>> +	if (sii9234_clear_error(ctx))
>> +		return ST_FAILURE;
>> +
>> +	usleep_range(T_SRC_VBUS_CBUS_TO_STABLE * USEC_PER_MSEC,
>> +		     T_SRC_VBUS_CBUS_TO_STABLE * USEC_PER_MSEC);
> usleep for 200ms?
>
> Pretty long... maybe msleep()?

There should be msleep. I agree.

>
>> +
>> +	return ST_RGND_1K;
>> +}
>> +
>> +static enum sii9234_state sii9234_mhl_established(struct sii9234 *ctx)
>> +{
>> +	dev_dbg(ctx->dev, "mhl est interrupt\n");
>> +
>> +	/* Discovery override */
>> +	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL1_REG, 0x10);
>> +	/* Increase DDC translation layer timer (byte mode) */
>> +	cbus_writeb(ctx, 0x07, 0x32);
>> +	cbus_writebm(ctx, 0x44, ~0, 1 << 1);
>> +	/* Keep the discovery enabled. Need RGND interrupt */
>> +	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, ~0, 1);
>> +	mhl_tx_writeb(ctx, MHL_TX_INTR1_ENABLE_REG,
>> +	       RSEN_CHANGE_INT_MASK | HPD_CHANGE_INT_MASK);
>> +
>> +	if (sii9234_clear_error(ctx))
>> +		return ST_FAILURE;
>> +
>> +	return ST_MHL_ESTABLISHED;
>> +}
>> +
>> +static enum sii9234_state sii9234_hpd_change(struct sii9234 *ctx)
>> +{
>> +	int value;
>> +
>> +	value = cbus_readb(ctx, CBUS_MSC_REQ_ABORT_REASON_REG);
>> +	if (sii9234_clear_error(ctx))
>> +		return ST_FAILURE;
>> +
>> +	if (value & SET_HPD_DOWNSTREAM) {
>> +		/* Downstream HPD High, Enable TMDS */
>> +		sii9234_tmds_control(ctx, true);
>> +	} else {
>> +		/* Downstream HPD Low, Disable TMDS */
>> +		sii9234_tmds_control(ctx, false);
>> +	}
>> +
>> +	return ctx->state;
>> +}
>> +
>> +static enum sii9234_state sii9234_rsen_change(struct sii9234 *ctx)
>> +{
>> +	int value;
>> +
>> +	/* Work_around code to handle wrong interrupt */
>> +	if (ctx->state != ST_RGND_1K) {
>> +		dev_err(ctx->dev, "RSEN_HIGH without RGND_1K\n");
>> +		return ST_FAILURE;
>> +	}
>> +	value = mhl_tx_readb(ctx, MHL_TX_SYSSTAT_REG);
>> +	if (value < 0)
>> +		return ST_FAILURE;
>> +
>> +	if (value & RSEN_STATUS) {
>> +		dev_dbg(ctx->dev, "MHL cable connected.. RSEN High\n");
>> +		return ST_RSEN_HIGH;
>> +	}
>> +	dev_dbg(ctx->dev, "RSEN lost\n");
>> +	/* Once RSEN loss is confirmed,we need to check
>> +	 * based on cable status and chip power status,whether
>> +	 * it is SINK Loss(HDMI cable not connected, TV Off)
>> +	 * or MHL cable disconnection
>> +	 * TODO: Define the below mhl_disconnection()
>> +	 */
>> +	msleep(T_SRC_RXSENSE_DEGLITCH);
>> +	value = mhl_tx_readb(ctx, MHL_TX_SYSSTAT_REG);
>> +	if (value < 0)
>> +		return ST_FAILURE;
>> +	dev_dbg(ctx->dev, "sys_stat: %x\n", value);
>> +
>> +	if (value & RSEN_STATUS) {
>> +		dev_dbg(ctx->dev, "RSEN recovery\n");
>> +		return ST_RSEN_HIGH;
>> +	}
>> +	dev_dbg(ctx->dev, "RSEN Really LOW\n");
>> +	/* To meet CTS 3.3.22.2 spec */
>> +	sii9234_tmds_control(ctx, false);
>> +	force_usb_id_switch_open(ctx);
>> +	release_usb_id_switch_open(ctx);
>> +
>> +	return ST_FAILURE;
>> +}
>> +
>> +static irqreturn_t sii9234_irq_thread(int irq, void *data)
>> +{
>> +	struct sii9234 *ctx = data;
>> +	int intr1, intr4;
>> +	int intr1_en, intr4_en;
>> +	int cbus_intr1, cbus_intr2;
>> +
>> +	dev_dbg(ctx->dev, "%s\n", __func__);
>> +
>> +	mutex_lock(&ctx->lock);
>> +
>> +	intr1 = mhl_tx_readb(ctx, MHL_TX_INTR1_REG);
>> +	intr4 = mhl_tx_readb(ctx, MHL_TX_INTR4_REG);
>> +	intr1_en = mhl_tx_readb(ctx, MHL_TX_INTR1_ENABLE_REG);
>> +	intr4_en = mhl_tx_readb(ctx, MHL_TX_INTR4_ENABLE_REG);
>> +	cbus_intr1 = cbus_readb(ctx, CBUS_INT_STATUS_1_REG);
>> +	cbus_intr2 = cbus_readb(ctx, CBUS_INT_STATUS_2_REG);
>> +
>> +	if (sii9234_clear_error(ctx))
>> +		goto done;
>> +
>> +	dev_dbg(ctx->dev, "irq %02x/%02x %02x/%02x %02x/%02x\n",
>> +		 intr1, intr1_en, intr4, intr4_en, cbus_intr1, cbus_intr2);
>> +
>> +	if (intr4 & RGND_READY_INT)
>> +		ctx->state = sii9234_rgnd_ready_irq(ctx);
>> +	if (intr1 & RSEN_CHANGE_INT)
>> +		ctx->state = sii9234_rsen_change(ctx);
>> +	if (intr4 & MHL_EST_INT)
>> +		ctx->state = sii9234_mhl_established(ctx);
>> +	if (intr1 & HPD_CHANGE_INT)
>> +		ctx->state = sii9234_hpd_change(ctx);
>> +	if (intr4 & CBUS_LKOUT_INT)
>> +		ctx->state = ST_FAILURE;
>> +	if (intr4 & MHL_DISC_FAIL_INT)
>> +		ctx->state = ST_FAILURE_DISCOVERY;
>> +
>> + done:
>> +	/* Clean interrupt status and pending flags */
>> +	mhl_tx_writeb(ctx, MHL_TX_INTR1_REG, intr1);
>> +	mhl_tx_writeb(ctx, MHL_TX_INTR4_REG, intr4);
>> +	cbus_writeb(ctx, CBUS_MHL_STATUS_REG_0, 0xFF);
>> +	cbus_writeb(ctx, CBUS_MHL_STATUS_REG_1, 0xFF);
>> +	cbus_writeb(ctx, CBUS_INT_STATUS_1_REG, cbus_intr1);
>> +	cbus_writeb(ctx, CBUS_INT_STATUS_2_REG, cbus_intr2);
>> +
>> +	sii9234_clear_error(ctx);
>> +
>> +	if (ctx->state == ST_FAILURE) {
>> +		dev_dbg(ctx->dev, "try to reset after failure\n");
>> +		sii9234_hw_reset(ctx);
>> +		sii9234_goto_d3(ctx);
>> +	}
>> +
>> +	if (ctx->state == ST_FAILURE_DISCOVERY) {
>> +		dev_err(ctx->dev, "discovery failed, no power for MHL?\n");
>> +		tpi_writebm(ctx, TPI_DPD_REG, 0, 1);
>> +		ctx->state = ST_D3;
>> +	}
>> +
>> +	mutex_unlock(&ctx->lock);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +
>> +static int sii9234_init_resources(struct sii9234 *ctx,
>> +	struct i2c_client *client)
>> +{
>> +	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
>> +	int ret;
>> +
>> +	if (!ctx->dev->of_node) {
>> +		dev_err(ctx->dev, "not DT device\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	ctx->gpio_reset = devm_gpiod_get(ctx->dev, "reset", GPIOD_OUT_LOW);
>> +	if (IS_ERR(ctx->gpio_reset)) {
>> +		dev_err(ctx->dev, "failed to get reset gpio from DT\n");
>> +		return PTR_ERR(ctx->gpio_reset);
>> +	}
>> +
>> +	ctx->supplies[0].supply = "cvcc12";
>> +	ctx->supplies[1].supply = "avcc33";
>> +	ctx->supplies[2].supply = "iovcc18";
>> +	ctx->supplies[3].supply = "avcc12";
>> +	ret = devm_regulator_bulk_get(ctx->dev, 4, ctx->supplies);
>> +	if (ret) {
>> +		dev_err(ctx->dev, "regulator_bulk failed\n");
>> +		return ret;
>> +	}
>> +
>> +	ctx->client[I2C_MHL] = client;
>> +
>> +	ctx->client[I2C_TPI] = i2c_new_dummy(adapter, I2C_TPI_ADDR);
>> +	if (!ctx->client[I2C_TPI]) {
>> +		dev_err(ctx->dev, "failed to create TPI client\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	ctx->client[I2C_HDMI] = i2c_new_dummy(adapter, I2C_HDMI_ADDR);
>> +	if (!ctx->client[I2C_HDMI]) {
>> +		dev_err(ctx->dev, "failed to create HDMI RX client\n");
>> +		goto fail_tpi;
>> +	}
>> +
>> +	ctx->client[I2C_CBUS] = i2c_new_dummy(adapter, I2C_CBUS_ADDR);
>> +	if (!ctx->client[I2C_CBUS]) {
>> +		dev_err(ctx->dev, "failed to create CBUS client\n");
>> +		goto fail_hdmi;
>> +	}
> You are using i2c_smbus_* calls. Why not converting to regmap_i2c? It is
> preferred interface.

According to my search, there are only 5 drivers, which use regmap_i2c and
none of them in drm. If you think that it is really needed, could you please explain
why?

>
>> +
>> +	return 0;
>> +
>> +fail_hdmi:
>> +	i2c_unregister_device(ctx->client[I2C_HDMI]);
>> +fail_tpi:
>> +	i2c_unregister_device(ctx->client[I2C_TPI]);
>> +
>> +	return -ENODEV;
>> +}
>> +
>> +static void sii9234_deinit_resources(struct sii9234 *ctx)
>> +{
>> +	i2c_unregister_device(ctx->client[I2C_CBUS]);
>> +	i2c_unregister_device(ctx->client[I2C_HDMI]);
>> +	i2c_unregister_device(ctx->client[I2C_TPI]);
>> +}
>> +
>> +static inline struct sii9234 *bridge_to_sii9234(struct drm_bridge *bridge)
>> +{
>> +	return container_of(bridge, struct sii9234, bridge);
>> +}
>> +
>> +static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
>> +					const struct drm_display_mode *mode)
>> +{
>> +	if (mode->clock > MHL1_MAX_CLK)
>> +		return MODE_CLOCK_HIGH;
>> +
>> +	return MODE_OK;
>> +}
>> +
>> +static const struct drm_bridge_funcs sii9234_bridge_funcs = {
>> +	.mode_valid = sii9234_mode_valid,
>> +};
>> +
>> +static int sii9234_probe(struct i2c_client *client,
>> +					      const struct i2c_device_id *id)
>> +{
>> +	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
>> +	struct sii9234 *ctx;
>> +	struct device *dev = &client->dev;
>> +	int ret;
>> +
>> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> +	if (!ctx)
>> +		return -ENOMEM;
>> +
>> +	ctx->dev = dev;
>> +	mutex_init(&ctx->lock);
>> +
>> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
>> +		dev_err(dev, "I2C adapter lacks SMBUS feature\n");
>> +		return -EIO;
>> +	}
>> +
>> +	if (!client->irq) {
>> +		dev_err(dev, "no irq provided\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
>> +	ret = devm_request_threaded_irq(dev, client->irq, NULL,
>> +					sii9234_irq_thread,
>> +					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>> +					"sii9234", ctx);
>> +	if (ret < 0) {
>> +		dev_err(dev, "failed to install IRQ handler\n");
>> +		return ret;
>> +	}
> You are setting up interrupts before initialization of I2C. Can the
> interrupt happen in this short window?

It can't happen, because in the previous line there is a status flag set to
IRQ_NOAUTOEN. IRQs are enabled in sii9234_cable_in() function which is
called after initialization of I2C.

>
>> +
>> +	ret = sii9234_init_resources(ctx, client);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	i2c_set_clientdata(client, ctx);
>> +
>> +	ctx->bridge.funcs = &sii9234_bridge_funcs;
>> +	ctx->bridge.of_node = dev->of_node;
>> +	drm_bridge_add(&ctx->bridge);
>> +
>> +	sii9234_cable_in(ctx);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sii9234_remove(struct i2c_client *client)
>> +{
>> +	struct sii9234 *ctx = i2c_get_clientdata(client);
>> +
>> +	sii9234_cable_out(ctx);
>> +	drm_bridge_remove(&ctx->bridge);
>> +	sii9234_deinit_resources(ctx);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id sii9234_dt_match[] = {
>> +	{ .compatible = "sil,sii9234" },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, sii9234_dt_match);
>> +
>> +static const struct i2c_device_id sii9234_id[] = {
>> +	{ "SII9234", 0 },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(i2c, sii9234_id);
>> +
>> +static struct i2c_driver sii9234_driver = {
>> +	.driver = {
>> +		.name	= "sii9234",
>> +		.of_match_table = sii9234_dt_match,
>> +	},
>> +	.probe		= sii9234_probe,
>> +	.remove		= sii9234_remove,
>> +	.id_table = sii9234_id,
> Some assignments above are indented, some not...
>
> Best regards,
> Krzysztof
>
>> +};
>> +
>> +module_i2c_driver(sii9234_driver);
>> +MODULE_LICENSE("GPL");
>> -- 
>> 2.7.4
>>
>
>

Best regards,
Maciej

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Sept. 11, 2017, 1:18 p.m. UTC | #3
On Mon, Sep 11, 2017 at 1:42 PM, Maciej Purski <m.purski@samsung.com> wrote:
>>> +       ctx->client[I2C_MHL] = client;
>>> +
>>> +       ctx->client[I2C_TPI] = i2c_new_dummy(adapter, I2C_TPI_ADDR);
>>> +       if (!ctx->client[I2C_TPI]) {
>>> +               dev_err(ctx->dev, "failed to create TPI client\n");
>>> +               return -ENODEV;
>>> +       }
>>> +
>>> +       ctx->client[I2C_HDMI] = i2c_new_dummy(adapter, I2C_HDMI_ADDR);
>>> +       if (!ctx->client[I2C_HDMI]) {
>>> +               dev_err(ctx->dev, "failed to create HDMI RX client\n");
>>> +               goto fail_tpi;
>>> +       }
>>> +
>>> +       ctx->client[I2C_CBUS] = i2c_new_dummy(adapter, I2C_CBUS_ADDR);
>>> +       if (!ctx->client[I2C_CBUS]) {
>>> +               dev_err(ctx->dev, "failed to create CBUS client\n");
>>> +               goto fail_hdmi;
>>> +       }
>>
>> You are using i2c_smbus_* calls. Why not converting to regmap_i2c? It is
>> preferred interface.
>
>
> According to my search, there are only 5 drivers, which use regmap_i2c and
> none of them in drm. If you think that it is really needed, could you please
> explain
> why?

There are slightly more than five drivers:

$ git grep regmap_init_i2c | wc -l
352

... and even more using regmap interface in generic (not i2c).

I think this is really wanted because for free you get:
1. locking,
2. proper locking - with lockdep and nested mutexes :),
3. caching of accesses,
4. handling of endianness,
5. optionally a trivial way of handling interrupts (regmap_irq_chip),

Also this brings unified interface for most of the drivers regardless
of protocol used beneath (I2C, SPI and even MMIO but for simple cases
MMIO might not have much sense). This last argument actually brings
the most benefit for me because it abstracts driver from trivial I2C
implementation and it could allow even easy code-reuse for e.g. SPI
version.


>>
>>> +
>>> +       return 0;
>>> +
>>> +fail_hdmi:
>>> +       i2c_unregister_device(ctx->client[I2C_HDMI]);
>>> +fail_tpi:
>>> +       i2c_unregister_device(ctx->client[I2C_TPI]);
>>> +
>>> +       return -ENODEV;
>>> +}
>>> +
>>> +static void sii9234_deinit_resources(struct sii9234 *ctx)
>>> +{
>>> +       i2c_unregister_device(ctx->client[I2C_CBUS]);
>>> +       i2c_unregister_device(ctx->client[I2C_HDMI]);
>>> +       i2c_unregister_device(ctx->client[I2C_TPI]);
>>> +}
>>> +
>>> +static inline struct sii9234 *bridge_to_sii9234(struct drm_bridge
>>> *bridge)
>>> +{
>>> +       return container_of(bridge, struct sii9234, bridge);
>>> +}
>>> +
>>> +static enum drm_mode_status sii9234_mode_valid(struct drm_bridge
>>> *bridge,
>>> +                                       const struct drm_display_mode
>>> *mode)
>>> +{
>>> +       if (mode->clock > MHL1_MAX_CLK)
>>> +               return MODE_CLOCK_HIGH;
>>> +
>>> +       return MODE_OK;
>>> +}
>>> +
>>> +static const struct drm_bridge_funcs sii9234_bridge_funcs = {
>>> +       .mode_valid = sii9234_mode_valid,
>>> +};
>>> +
>>> +static int sii9234_probe(struct i2c_client *client,
>>> +                                             const struct i2c_device_id
>>> *id)
>>> +{
>>> +       struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
>>> +       struct sii9234 *ctx;
>>> +       struct device *dev = &client->dev;
>>> +       int ret;
>>> +
>>> +       ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>> +       if (!ctx)
>>> +               return -ENOMEM;
>>> +
>>> +       ctx->dev = dev;
>>> +       mutex_init(&ctx->lock);
>>> +
>>> +       if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>>> {
>>> +               dev_err(dev, "I2C adapter lacks SMBUS feature\n");
>>> +               return -EIO;
>>> +       }
>>> +
>>> +       if (!client->irq) {
>>> +               dev_err(dev, "no irq provided\n");
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
>>> +       ret = devm_request_threaded_irq(dev, client->irq, NULL,
>>> +                                       sii9234_irq_thread,
>>> +                                       IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>>> +                                       "sii9234", ctx);
>>> +       if (ret < 0) {
>>> +               dev_err(dev, "failed to install IRQ handler\n");
>>> +               return ret;
>>> +       }
>>
>> You are setting up interrupts before initialization of I2C. Can the
>> interrupt happen in this short window?
>
>
> It can't happen, because in the previous line there is a status flag set to
> IRQ_NOAUTOEN. IRQs are enabled in sii9234_cable_in() function which is
> called after initialization of I2C.

Ah, okay, I missed that. Thanks!

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Szyprowski Sept. 11, 2017, 1:39 p.m. UTC | #4
Hi Krzysztof,

On 2017-09-11 15:18, Krzysztof Kozlowski wrote:
> On Mon, Sep 11, 2017 at 1:42 PM, Maciej Purski <m.purski@samsung.com> wrote:
>>>> +       ctx->client[I2C_MHL] = client;
>>>> +
>>>> +       ctx->client[I2C_TPI] = i2c_new_dummy(adapter, I2C_TPI_ADDR);
>>>> +       if (!ctx->client[I2C_TPI]) {
>>>> +               dev_err(ctx->dev, "failed to create TPI client\n");
>>>> +               return -ENODEV;
>>>> +       }
>>>> +
>>>> +       ctx->client[I2C_HDMI] = i2c_new_dummy(adapter, I2C_HDMI_ADDR);
>>>> +       if (!ctx->client[I2C_HDMI]) {
>>>> +               dev_err(ctx->dev, "failed to create HDMI RX client\n");
>>>> +               goto fail_tpi;
>>>> +       }
>>>> +
>>>> +       ctx->client[I2C_CBUS] = i2c_new_dummy(adapter, I2C_CBUS_ADDR);
>>>> +       if (!ctx->client[I2C_CBUS]) {
>>>> +               dev_err(ctx->dev, "failed to create CBUS client\n");
>>>> +               goto fail_hdmi;
>>>> +       }
>>> You are using i2c_smbus_* calls. Why not converting to regmap_i2c? It is
>>> preferred interface.
>>
>> According to my search, there are only 5 drivers, which use regmap_i2c and
>> none of them in drm. If you think that it is really needed, could you please
>> explain
>> why?
> There are slightly more than five drivers:
>
> $ git grep regmap_init_i2c | wc -l
> 352
>
> ... and even more using regmap interface in generic (not i2c).
>
> I think this is really wanted because for free you get:
> 1. locking,
> 2. proper locking - with lockdep and nested mutexes :),
> 3. caching of accesses,
> 4. handling of endianness,
> 5. optionally a trivial way of handling interrupts (regmap_irq_chip),
>
> Also this brings unified interface for most of the drivers regardless
> of protocol used beneath (I2C, SPI and even MMIO but for simple cases
> MMIO might not have much sense). This last argument actually brings
> the most benefit for me because it abstracts driver from trivial I2C
> implementation and it could allow even easy code-reuse for e.g. SPI
> version.

Regmap make sense for multi-function chips, which require more than one
client driver (so proper locking is important). It also makes sense if
the chip is produced in more than one flavor (like i2c, spi, MMIO, etc).

None of the above takes place in this case... So in case of this driver
using regmap is IMHO an over-engineering.

 > ...

Best regards
Krzysztof Kozlowski Sept. 11, 2017, 3:19 p.m. UTC | #5
On Mon, Sep 11, 2017 at 3:39 PM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hi Krzysztof,
>
>
> On 2017-09-11 15:18, Krzysztof Kozlowski wrote:
>>
>> On Mon, Sep 11, 2017 at 1:42 PM, Maciej Purski <m.purski@samsung.com>
>> wrote:
>>>>>
>>>>> +       ctx->client[I2C_MHL] = client;
>>>>> +
>>>>> +       ctx->client[I2C_TPI] = i2c_new_dummy(adapter, I2C_TPI_ADDR);
>>>>> +       if (!ctx->client[I2C_TPI]) {
>>>>> +               dev_err(ctx->dev, "failed to create TPI client\n");
>>>>> +               return -ENODEV;
>>>>> +       }
>>>>> +
>>>>> +       ctx->client[I2C_HDMI] = i2c_new_dummy(adapter, I2C_HDMI_ADDR);
>>>>> +       if (!ctx->client[I2C_HDMI]) {
>>>>> +               dev_err(ctx->dev, "failed to create HDMI RX client\n");
>>>>> +               goto fail_tpi;
>>>>> +       }
>>>>> +
>>>>> +       ctx->client[I2C_CBUS] = i2c_new_dummy(adapter, I2C_CBUS_ADDR);
>>>>> +       if (!ctx->client[I2C_CBUS]) {
>>>>> +               dev_err(ctx->dev, "failed to create CBUS client\n");
>>>>> +               goto fail_hdmi;
>>>>> +       }
>>>>
>>>> You are using i2c_smbus_* calls. Why not converting to regmap_i2c? It is
>>>> preferred interface.
>>>
>>>
>>> According to my search, there are only 5 drivers, which use regmap_i2c
>>> and
>>> none of them in drm. If you think that it is really needed, could you
>>> please
>>> explain
>>> why?
>>
>> There are slightly more than five drivers:
>>
>> $ git grep regmap_init_i2c | wc -l
>> 352
>>
>> ... and even more using regmap interface in generic (not i2c).
>>
>> I think this is really wanted because for free you get:
>> 1. locking,
>> 2. proper locking - with lockdep and nested mutexes :),
>> 3. caching of accesses,
>> 4. handling of endianness,
>> 5. optionally a trivial way of handling interrupts (regmap_irq_chip),
>>
>> Also this brings unified interface for most of the drivers regardless
>> of protocol used beneath (I2C, SPI and even MMIO but for simple cases
>> MMIO might not have much sense). This last argument actually brings
>> the most benefit for me because it abstracts driver from trivial I2C
>> implementation and it could allow even easy code-reuse for e.g. SPI
>> version.
>
>
> Regmap make sense for multi-function chips, which require more than one
> client driver (so proper locking is important). It also makes sense if
> the chip is produced in more than one flavor (like i2c, spi, MMIO, etc).
>
> None of the above takes place in this case... So in case of this driver
> using regmap is IMHO an over-engineering.

Good points and true that setting up regmap requires some more code
around probe. However it will remove half or even more of your
interrupt handler. Less driver specific code, more generic code used.
Also current code around I2C (sii9234_writeb() etc) is not thread
safe... which for now is not a problem as the only entrance point is
the interrupt. Maybe it's fine then to use existing implementation
especially if you do not see any future enhancements to the driver.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
new file mode 100644
index 0000000..3ce7413
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
@@ -0,0 +1,34 @@ 
+Silicon Image SiI9234 HDMI/MHL bridge bindings
+
+Required properties:
+	- compatible : "sil,sii9234".
+	- reg : I2C address for TPI interface, use 0x39
+	- avcc33-supply : MHL/USB Switch Supply Voltage (3.3V)
+	- iovcc18-supply : I/O Supply Voltage (1.8V)
+	- avcc12-supply : TMDS Analog Supply Voltage (1.2V)
+	- cvcc12-supply : Digital Core Supply Voltage (1.2V)
+	- interrupts, interrupt-parent: interrupt specifier of INT pin
+	- reset-gpios: gpio specifier of RESET pin (active low)
+	- video interfaces: Device node can contain video interface port
+			    node for HDMI encoder according to [1].
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+	sii9234@39 {
+		compatible = "sil,sii9234";
+		reg = <0x39>;
+		avcc33-supply = <&vcc33mhl>;
+		iovcc18-supply = <&vcc18mhl>;
+		avcc12-supply = <&vsil12>;
+		cvcc12-supply = <&vsil12>;
+		reset-gpios = <&gpf3 4 GPIO_ACTIVE_LOW>;
+		interrupt-parent = <&gpf3>;
+		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+		port {
+			mhl_to_hdmi: endpoint {
+				remote-endpoint = <&hdmi_to_mhl>;
+			};
+		};
+	};
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index adf9ae0..9dba16fd 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -84,6 +84,14 @@  config DRM_SII902X
 	---help---
 	  Silicon Image sii902x bridge chip driver.
 
+config DRM_SII9234
+	tristate "Silicon Image SII9234 HDMI/MHL bridge"
+	depends on OF
+	---help---
+	  Say Y here if you want support for the MHL interface.
+	  It is an I2C driver, that detects connection of MHL bridge
+	  and starts encapsulation of HDMI signal.
+
 config DRM_TOSHIBA_TC358767
 	tristate "Toshiba TC358767 eDP bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index defcf1e..e3d5eb0 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -6,6 +6,7 @@  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
 obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
+obj-$(CONFIG_DRM_SII9234) += sii9234.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
new file mode 100644
index 0000000..b70d69a
--- /dev/null
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -0,0 +1,993 @@ 
+/*
+ * Copyright (C) 2017 Samsung Electronics
+ *
+ * Authors:
+ *    Tomasz Stanislawski <t.stanislaws@samsung.com>
+ *    Maciej Purski <m.purski@samsung.com>
+ *
+ * Based on sii9234 driver created by:
+ *    Adam Hampson <ahampson@sta.samsung.com>
+ *    Erik Gilling <konkers@android.com>
+ *    Shankar Bandal <shankar.b@samsung.com>
+ *    Dharam Kumar <dharam.kr@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program
+ *
+ */
+#include <drm/bridge/mhl.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_edid.h>
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#define CBUS_DEVCAP_OFFSET		0x80
+
+#define SII9234_MHL_VERSION		0x11
+#define SII9234_SCRATCHPAD_SIZE		0x10
+#define SII9234_INT_STAT_SIZE		0x33
+
+#define BIT_TMDS_CCTRL_TMDS_OE		BIT(4)
+#define MHL_HPD_OUT_OVR_EN		BIT(4)
+#define MHL_HPD_OUT_OVR_VAL		BIT(5)
+#define MHL_INIT_TIMEOUT		0x0C
+
+/* MHL Tx registers and bits */
+#define MHL_TX_SRST                     0x05
+#define MHL_TX_SYSSTAT_REG              0x09
+#define MHL_TX_INTR1_REG                0x71
+#define MHL_TX_INTR4_REG                0x74
+#define MHL_TX_INTR1_ENABLE_REG         0x75
+#define MHL_TX_INTR4_ENABLE_REG         0x78
+#define MHL_TX_INT_CTRL_REG             0x79
+#define MHL_TX_TMDS_CCTRL               0x80
+#define MHL_TX_DISC_CTRL1_REG           0x90
+#define MHL_TX_DISC_CTRL2_REG           0x91
+#define MHL_TX_DISC_CTRL3_REG           0x92
+#define MHL_TX_DISC_CTRL4_REG		0x93
+#define MHL_TX_DISC_CTRL5_REG           0x94
+#define MHL_TX_DISC_CTRL6_REG           0x95
+#define MHL_TX_DISC_CTRL7_REG           0x96
+#define MHL_TX_DISC_CTRL8_REG		0x97
+#define MHL_TX_STAT2_REG                0x99
+#define MHL_TX_MHLTX_CTL1_REG           0xA0
+#define MHL_TX_MHLTX_CTL2_REG           0xA1
+#define MHL_TX_MHLTX_CTL4_REG           0xA3
+#define MHL_TX_MHLTX_CTL6_REG           0xA5
+#define MHL_TX_MHLTX_CTL7_REG           0xA6
+
+
+#define RSEN_STATUS			BIT(2)
+#define HPD_CHANGE_INT			BIT(6)
+#define RSEN_CHANGE_INT			BIT(5)
+#define RGND_READY_INT			BIT(6)
+#define VBUS_LOW_INT			BIT(5)
+#define CBUS_LKOUT_INT			BIT(4)
+#define MHL_DISC_FAIL_INT		BIT(3)
+#define MHL_EST_INT			BIT(2)
+#define HPD_CHANGE_INT_MASK		BIT(6)
+#define RSEN_CHANGE_INT_MASK		BIT(5)
+
+#define RGND_READY_MASK			BIT(6)
+#define CBUS_LKOUT_MASK			BIT(4)
+#define MHL_DISC_FAIL_MASK		BIT(3)
+#define MHL_EST_MASK			BIT(2)
+
+#define SKIP_GND			BIT(6)
+
+#define ATT_THRESH_SHIFT		0x04
+#define ATT_THRESH_MASK			(0x03 << ATT_THRESH_SHIFT)
+#define USB_D_OEN			BIT(3)
+#define DEGLITCH_TIME_MASK		0x07
+#define DEGLITCH_TIME_2MS		0
+#define DEGLITCH_TIME_4MS		1
+#define DEGLITCH_TIME_8MS		2
+#define DEGLITCH_TIME_16MS		3
+#define DEGLITCH_TIME_40MS		4
+#define DEGLITCH_TIME_50MS		5
+#define DEGLITCH_TIME_60MS		6
+#define DEGLITCH_TIME_128MS		7
+
+#define USB_D_OVR			BIT(7)
+#define USB_ID_OVR			BIT(6)
+#define DVRFLT_SEL			BIT(5)
+#define BLOCK_RGND_INT			BIT(4)
+#define SKIP_DEG			BIT(3)
+#define CI2CA_POL			BIT(2)
+#define CI2CA_WKUP			BIT(1)
+#define SINGLE_ATT			BIT(0)
+
+#define USB_D_ODN			BIT(5)
+#define VBUS_CHECK			BIT(2)
+#define RGND_INTP_MASK			0x03
+#define RGND_INTP_OPEN			0
+#define RGND_INTP_2K			1
+#define RGND_INTP_1K			2
+#define RGND_INTP_SHORT			3
+
+/* HDMI registers */
+#define HDMI_RX_TMDS0_CCTRL1_REG	0x10
+#define HDMI_RX_TMDS_CLK_EN_REG		0x11
+#define HDMI_RX_TMDS_CH_EN_REG		0x12
+#define HDMI_RX_PLL_CALREFSEL_REG	0x17
+#define HDMI_RX_PLL_VCOCAL_REG		0x1A
+#define HDMI_RX_EQ_DATA0_REG		0x22
+#define HDMI_RX_EQ_DATA1_REG		0x23
+#define HDMI_RX_EQ_DATA2_REG		0x24
+#define HDMI_RX_EQ_DATA3_REG		0x25
+#define HDMI_RX_EQ_DATA4_REG		0x26
+#define HDMI_RX_TMDS_ZONE_CTRL_REG	0x4C
+#define HDMI_RX_TMDS_MODE_CTRL_REG	0x4D
+
+/* CBUS registers */
+#define CBUS_INT_STATUS_1_REG		0x08
+#define CBUS_INTR1_ENABLE_REG		0x09
+#define CBUS_MSC_REQ_ABORT_REASON_REG	0x0D
+#define CBUS_INT_STATUS_2_REG		0x1E
+#define CBUS_INTR2_ENABLE_REG		0x1F
+#define CBUS_LINK_CONTROL_2_REG		0x31
+#define CBUS_MHL_STATUS_REG_0		0xB0
+#define CBUS_MHL_STATUS_REG_1		0xB1
+
+#define BIT_CBUS_RESET			BIT(3)
+#define SET_HPD_DOWNSTREAM		BIT(6)
+
+/* TPI registers */
+#define TPI_DPD_REG			0x3D
+
+/* timeouts */
+#define T_SRC_VBUS_CBUS_TO_STABLE	200
+#define T_SRC_CBUS_FLOAT		100
+#define T_SRC_CBUS_DEGLITCH		2
+#define T_SRC_RXSENSE_DEGLITCH		110
+#define MHL1_MAX_CLK			75000
+
+/* I2C addresses */
+#define I2C_TPI_ADDR			0x3D
+#define I2C_HDMI_ADDR			0x49
+#define I2C_CBUS_ADDR			0x64
+
+enum sii9234_state {
+	ST_OFF,
+	ST_D3,
+	ST_RGND_INIT,
+	ST_RGND_1K,
+	ST_RSEN_HIGH,
+	ST_MHL_ESTABLISHED,
+	ST_FAILURE_DISCOVERY,
+	ST_FAILURE,
+};
+
+struct sii9234 {
+	struct i2c_client *client[4];
+	struct drm_bridge bridge;
+	struct device *dev;
+	struct gpio_desc *gpio_reset;
+	int i2c_error;
+	struct regulator_bulk_data supplies[4];
+
+	enum sii9234_state state;
+	struct mutex lock;
+};
+
+enum sii9234_client_id {
+	I2C_MHL,
+	I2C_TPI,
+	I2C_HDMI,
+	I2C_CBUS,
+};
+
+static char *sii9234_client_name[] = {
+	[I2C_MHL] = "MHL",
+	[I2C_TPI] = "TPI",
+	[I2C_HDMI] = "HDMI",
+	[I2C_CBUS] = "CBUS",
+};
+
+static int sii9234_writeb(struct sii9234 *ctx, int id, int offset,
+			    int value)
+{
+	int ret;
+	struct i2c_client *client = ctx->client[id];
+
+	if (ctx->i2c_error)
+		return ctx->i2c_error;
+
+	ret = i2c_smbus_write_byte_data(client, offset, value);
+	if (ret < 0)
+		dev_err(ctx->dev, "writeb: %4s[0x%02x] <- 0x%02x\n",
+			sii9234_client_name[id], offset, value);
+	ctx->i2c_error = ret;
+
+	return ret;
+}
+
+static int sii9234_writebm(struct sii9234 *ctx, int id, int offset,
+			    int value, int mask)
+{
+	int ret;
+	struct i2c_client *client = ctx->client[id];
+
+	if (ctx->i2c_error)
+		return ctx->i2c_error;
+
+	ret = i2c_smbus_write_byte(client, offset);
+	if (ret < 0) {
+		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
+			sii9234_client_name[id], offset, value);
+		ctx->i2c_error = ret;
+		return ret;
+	}
+
+	ret = i2c_smbus_read_byte(client);
+	if (ret < 0) {
+		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
+			sii9234_client_name[id], offset, value);
+		ctx->i2c_error = ret;
+		return ret;
+	}
+
+	value = (value & mask) | (ret & ~mask);
+
+	ret = i2c_smbus_write_byte_data(client, offset, value);
+	if (ret < 0) {
+		dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
+			sii9234_client_name[id], offset, value);
+		ctx->i2c_error = ret;
+	}
+
+	return ret;
+}
+
+static int sii9234_readb(struct sii9234 *ctx, int id, int offset)
+{
+	int ret;
+	struct i2c_client *client = ctx->client[id];
+
+	if (ctx->i2c_error)
+		return ctx->i2c_error;
+
+	ret = i2c_smbus_write_byte(client, offset);
+	if (ret < 0) {
+		dev_err(ctx->dev, "readb: %4s[0x%02x]\n",
+			sii9234_client_name[id], offset);
+		ctx->i2c_error = ret;
+		return ret;
+	}
+
+	ret = i2c_smbus_read_byte(client);
+	if (ret < 0) {
+		dev_err(ctx->dev, "readb: %4s[0x%02x]\n",
+			sii9234_client_name[id], offset);
+		ctx->i2c_error = ret;
+	}
+
+	return ret;
+}
+
+static int sii9234_clear_error(struct sii9234 *ctx)
+{
+	int ret = ctx->i2c_error;
+
+	ctx->i2c_error = 0;
+
+	return ret;
+}
+
+#define mhl_tx_writeb(sii9234, offset, value) \
+	sii9234_writeb(sii9234, I2C_MHL, offset, value)
+#define mhl_tx_writebm(sii9234, offset, value, mask) \
+	sii9234_writebm(sii9234, I2C_MHL, offset, value, mask)
+#define mhl_tx_readb(sii9234, offset) \
+	sii9234_readb(sii9234, I2C_MHL, offset)
+#define cbus_writeb(sii9234, offset, value) \
+	sii9234_writeb(sii9234, I2C_CBUS, offset, value)
+#define cbus_writebm(sii9234, offset, value, mask) \
+	sii9234_writebm(sii9234, I2C_CBUS, offset, value, mask)
+#define cbus_readb(sii9234, offset) \
+	sii9234_readb(sii9234, I2C_CBUS, offset)
+#define hdmi_writeb(sii9234, offset, value) \
+	sii9234_writeb(sii9234, I2C_HDMI, offset, value)
+#define hdmi_writebm(sii9234, offset, value, mask) \
+	sii9234_writebm(sii9234, I2C_HDMI, offset, value, mask)
+#define hdmi_readb(sii9234, offset) \
+	sii9234_readb(sii9234, I2C_HDMI, offset)
+#define tpi_writeb(sii9234, offset, value) \
+	sii9234_writeb(sii9234, I2C_TPI, offset, value)
+#define tpi_writebm(sii9234, offset, value, mask) \
+	sii9234_writebm(sii9234, I2C_TPI, offset, value, mask)
+#define tpi_readb(sii9234, offset) \
+	sii9234_readb(sii9234, I2C_TPI, offset)
+
+static u8 sii9234_tmds_control(struct sii9234 *ctx, bool enable)
+{
+	mhl_tx_writebm(ctx, MHL_TX_TMDS_CCTRL, enable ? ~0 : 0,
+						BIT_TMDS_CCTRL_TMDS_OE);
+	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, enable ? ~0 : 0,
+				MHL_HPD_OUT_OVR_EN | MHL_HPD_OUT_OVR_VAL);
+	return sii9234_clear_error(ctx);
+}
+
+static int sii9234_cbus_reset(struct sii9234 *ctx)
+{
+	int i;
+
+	mhl_tx_writebm(ctx, MHL_TX_SRST, ~0, BIT_CBUS_RESET);
+	msleep(T_SRC_CBUS_DEGLITCH);
+	mhl_tx_writebm(ctx, MHL_TX_SRST, 0, BIT_CBUS_RESET);
+
+	for (i = 0; i < 4; i++) {
+		/* Enable WRITE_STAT interrupt for writes to all
+		 * 4 MSC Status registers.
+		 */
+		cbus_writeb(ctx, 0xE0 + i, 0xF2);
+		/* Enable SET_INT interrupt for writes to all
+		 * 4 MSC Interrupt registers.
+		 */
+		cbus_writeb(ctx, 0xF0 + i, 0xF2);
+	}
+
+	return sii9234_clear_error(ctx);
+}
+
+/* Require to chek mhl imformation of samsung in cbus_init_register */
+static int sii9234_cbus_init(struct sii9234 *ctx)
+{
+	cbus_writeb(ctx, 0x07, 0xF2);
+	cbus_writeb(ctx, 0x40, 0x03);
+	cbus_writeb(ctx, 0x42, 0x06);
+	cbus_writeb(ctx, 0x36, 0x0C);
+	cbus_writeb(ctx, 0x3D, 0xFD);
+	cbus_writeb(ctx, 0x1C, 0x01);
+	cbus_writeb(ctx, 0x1D, 0x0F);
+	cbus_writeb(ctx, 0x44, 0x02);
+	/* Setup our devcap */
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEV_STATE, 0x00);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_MHL_VERSION,
+							SII9234_MHL_VERSION);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_CAT,
+							MHL_DCAP_CAT_SOURCE);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_ADOPTER_ID_H, 0x01);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_ADOPTER_ID_L, 0x41);
+
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_VID_LINK_MODE,
+			MHL_DCAP_VID_LINK_RGB444 | MHL_DCAP_VID_LINK_YCBCR444);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_VIDEO_TYPE,
+							MHL_DCAP_VT_GRAPHICS);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_LOG_DEV_MAP,
+							MHL_DCAP_LD_GUI);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_BANDWIDTH, 0x0F);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_FEATURE_FLAG,
+		    MHL_DCAP_FEATURE_RCP_SUPPORT | MHL_DCAP_FEATURE_RAP_SUPPORT
+		   | MHL_DCAP_FEATURE_SP_SUPPORT);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEVICE_ID_H, 0x0);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_DEVICE_ID_L, 0x0);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_SCRATCHPAD_SIZE,
+						SII9234_SCRATCHPAD_SIZE);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_INT_STAT_SIZE,
+						SII9234_INT_STAT_SIZE);
+	cbus_writeb(ctx, CBUS_DEVCAP_OFFSET + MHL_DCAP_RESERVED, 0);
+	cbus_writebm(ctx, 0x31, 0x0C, 0x0C);
+	cbus_writeb(ctx, 0x30, 0x01);
+	cbus_writebm(ctx, 0x3C, 0x30, 0x38);
+	cbus_writebm(ctx, 0x22, 0x0D, 0x0F);
+	cbus_writebm(ctx, 0x2E, 0x15, 0x15);
+	cbus_writeb(ctx, CBUS_INTR1_ENABLE_REG, 0);
+	cbus_writeb(ctx, CBUS_INTR2_ENABLE_REG, 0);
+
+	return sii9234_clear_error(ctx);
+}
+
+static void force_usb_id_switch_open(struct sii9234 *ctx)
+{
+	/* Disable CBUS discovery */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, 0, 0x01);
+	/* Force USB ID switch to open */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, USB_ID_OVR);
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL3_REG, ~0, 0x86);
+	/* Force upstream HPD to 0 when not in MHL mode. */
+	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 0x30);
+}
+
+static void release_usb_id_switch_open(struct sii9234 *ctx)
+{
+	msleep(T_SRC_CBUS_FLOAT);
+	/* Clear USB ID switch to open */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, 0, USB_ID_OVR);
+	/* Enable CBUS discovery */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, ~0, 0x01);
+}
+
+static int sii9234_power_init(struct sii9234 *ctx)
+{
+	/* Force the SiI9234 into the D0 state. */
+	tpi_writeb(ctx, TPI_DPD_REG, 0x3F);
+	/* Enable TxPLL Clock */
+	hdmi_writeb(ctx, HDMI_RX_TMDS_CLK_EN_REG, 0x01);
+	/* Enable Tx Clock Path & Equalizer */
+	hdmi_writeb(ctx, HDMI_RX_TMDS_CH_EN_REG, 0x15);
+	/* Power Up TMDS */
+	mhl_tx_writeb(ctx, 0x08, 0x35);
+	return sii9234_clear_error(ctx);
+}
+
+static int sii9234_hdmi_init(struct sii9234 *ctx)
+{
+	hdmi_writeb(ctx, HDMI_RX_TMDS0_CCTRL1_REG, 0xC1);
+	hdmi_writeb(ctx, HDMI_RX_PLL_CALREFSEL_REG, 0x03);
+	hdmi_writeb(ctx, HDMI_RX_PLL_VCOCAL_REG, 0x20);
+	hdmi_writeb(ctx, HDMI_RX_EQ_DATA0_REG, 0x8A);
+	hdmi_writeb(ctx, HDMI_RX_EQ_DATA1_REG, 0x6A);
+	hdmi_writeb(ctx, HDMI_RX_EQ_DATA2_REG, 0xAA);
+	hdmi_writeb(ctx, HDMI_RX_EQ_DATA3_REG, 0xCA);
+	hdmi_writeb(ctx, HDMI_RX_EQ_DATA4_REG, 0xEA);
+	hdmi_writeb(ctx, HDMI_RX_TMDS_ZONE_CTRL_REG, 0xA0);
+	hdmi_writeb(ctx, HDMI_RX_TMDS_MODE_CTRL_REG, 0x00);
+	mhl_tx_writeb(ctx, MHL_TX_TMDS_CCTRL, 0x34);
+	hdmi_writeb(ctx, 0x45, 0x44);
+	hdmi_writeb(ctx, 0x31, 0x0A);
+	hdmi_writeb(ctx, HDMI_RX_TMDS0_CCTRL1_REG, 0xC1);
+
+	return sii9234_clear_error(ctx);
+}
+
+static int sii9234_mhl_tx_ctl_int(struct sii9234 *ctx)
+{
+	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL1_REG, 0xD0);
+	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL2_REG, 0xFC);
+	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL4_REG, 0xEB);
+	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL7_REG, 0x0C);
+
+	return sii9234_clear_error(ctx);
+}
+
+static int sii9234_reset(struct sii9234 *ctx)
+{
+	int ret;
+
+	sii9234_clear_error(ctx);
+
+	ret = sii9234_power_init(ctx);
+	if (ret < 0)
+		return ret;
+	ret = sii9234_cbus_reset(ctx);
+	if (ret < 0)
+		return ret;
+	ret = sii9234_hdmi_init(ctx);
+	if (ret < 0)
+		return ret;
+	ret = sii9234_mhl_tx_ctl_int(ctx);
+	if (ret < 0)
+		return ret;
+
+	/* Enable HDCP Compliance safety */
+	mhl_tx_writeb(ctx, 0x2B, 0x01);
+	/* CBUS discovery cycle time for each drive and float = 150us */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, 0x04, 0x06);
+	/* Clear bit 6 (reg_skip_rgnd) */
+	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL2_REG, (1 << 7) /* Reserved */
+		| 2 << ATT_THRESH_SHIFT | DEGLITCH_TIME_50MS);
+	/* Changed from 66 to 65 for 94[1:0] = 01 = 5k reg_cbusmhl_pup_sel
+	 * 1.8V CBUS VTH & GND threshold
+	 * to meet CTS 3.3.7.2 spec
+	 */
+	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL5_REG, 0x77);
+	cbus_writebm(ctx, CBUS_LINK_CONTROL_2_REG, ~0, MHL_INIT_TIMEOUT);
+	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL6_REG, 0xA0);
+	/* RGND & single discovery attempt (RGND blocking) */
+	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL6_REG, BLOCK_RGND_INT |
+			       DVRFLT_SEL | SINGLE_ATT);
+	/* Use VBUS path of discovery state machine */
+	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL8_REG, 0);
+	/* 0x92[3] sets the CBUS / ID switch */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, USB_ID_OVR);
+	/* To allow RGND engine to operate correctly.
+	 * When moving the chip from D2 to D0 (power up, init regs)
+	 * the values should be
+	 * 94[1:0] = 01  reg_cbusmhl_pup_sel[1:0] should be set for 5k
+	 * 93[7:6] = 10  reg_cbusdisc_pup_sel[1:0] should be
+	 * set for 10k (default)
+	 * 93[5:4] = 00  reg_cbusidle_pup_sel[1:0] = open (default)
+	 */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL3_REG, ~0, 0x86);
+	/* Change from CC to 8C to match 5K
+	 * to meet CTS 3.3.72 spec
+	 */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, ~0, 0x8C);
+	/* Configure the interrupt as active high */
+	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 0x06);
+
+	msleep(25);
+
+	/* Release usb_id switch */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, 0,  USB_ID_OVR);
+	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL1_REG, 0x27);
+
+	ret = sii9234_clear_error(ctx);
+	if (ret < 0)
+		return ret;
+	ret = sii9234_cbus_init(ctx);
+	if (ret < 0)
+		return ret;
+
+	/* Enable Auto soft reset on SCDT = 0 */
+	mhl_tx_writeb(ctx, 0x05, 0x04);
+	/* HDMI Transcode mode enable */
+	mhl_tx_writeb(ctx, 0x0D, 0x1C);
+	mhl_tx_writeb(ctx, MHL_TX_INTR4_ENABLE_REG,
+			       RGND_READY_MASK | CBUS_LKOUT_MASK |
+			       MHL_DISC_FAIL_MASK | MHL_EST_MASK);
+	mhl_tx_writeb(ctx, MHL_TX_INTR1_ENABLE_REG, 0x60);
+
+	/* This point is very important before measure RGND impedance */
+	force_usb_id_switch_open(ctx);
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, 0, 0xF0);
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL5_REG, 0, 0x03);
+	release_usb_id_switch_open(ctx);
+
+	/* Force upstream HPD to 0 when not in MHL mode */
+	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, 0, 1 << 5);
+	mhl_tx_writebm(ctx, MHL_TX_INT_CTRL_REG, ~0, 1 << 4);
+
+	return sii9234_clear_error(ctx);
+}
+
+static int sii9234_goto_d3(struct sii9234 *ctx)
+{
+	int ret;
+
+	dev_dbg(ctx->dev, "sii9234: detection started d3\n");
+
+	ret = sii9234_reset(ctx);
+	if (ret < 0)
+		goto exit;
+
+	hdmi_writeb(ctx, 0x01, 0x03);
+	tpi_writebm(ctx, TPI_DPD_REG, 0, 1);
+	/* I2C above is expected to fail because power goes down */
+	sii9234_clear_error(ctx);
+
+	ctx->state = ST_D3;
+
+	return 0;
+ exit:
+	dev_err(ctx->dev, "%s failed\n", __func__);
+	return -1;
+}
+
+static int sii9234_hw_on(struct sii9234 *ctx)
+{
+	return regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+}
+
+static void sii9234_hw_off(struct sii9234 *ctx)
+{
+	gpiod_set_value(ctx->gpio_reset, 1);
+	usleep_range(10000, 20000);
+	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+}
+
+static void sii9234_hw_reset(struct sii9234 *ctx)
+{
+	gpiod_set_value(ctx->gpio_reset, 1);
+	usleep_range(10000, 20000);
+	gpiod_set_value(ctx->gpio_reset, 0);
+}
+
+static void sii9234_cable_in(struct sii9234 *ctx)
+{
+	int ret;
+
+	mutex_lock(&ctx->lock);
+	if (ctx->state != ST_OFF)
+		goto unlock;
+	ret = sii9234_hw_on(ctx);
+	if (ret < 0)
+		goto unlock;
+
+	sii9234_hw_reset(ctx);
+	sii9234_goto_d3(ctx);
+	/* To avoid irq storm, when hw is in meta state */
+	enable_irq(to_i2c_client(ctx->dev)->irq);
+
+unlock:
+	mutex_unlock(&ctx->lock);
+}
+
+static void sii9234_cable_out(struct sii9234 *ctx)
+{
+	mutex_lock(&ctx->lock);
+
+	if (ctx->state == ST_OFF)
+		goto unlock;
+
+	disable_irq(to_i2c_client(ctx->dev)->irq);
+	tpi_writeb(ctx, TPI_DPD_REG, 0);
+	/* Turn on&off hpd festure for only QCT HDMI */
+	sii9234_hw_off(ctx);
+
+	ctx->state = ST_OFF;
+
+unlock:
+	mutex_unlock(&ctx->lock);
+}
+
+static enum sii9234_state sii9234_rgnd_ready_irq(struct sii9234 *ctx)
+{
+	int value;
+
+	if (ctx->state == ST_D3) {
+		int ret;
+
+		dev_dbg(ctx->dev, "RGND_READY_INT\n");
+		sii9234_hw_reset(ctx);
+
+		ret = sii9234_reset(ctx);
+		if (ret < 0) {
+			dev_err(ctx->dev, "sii9234_reset() failed\n");
+			return ST_FAILURE;
+		}
+
+		return ST_RGND_INIT;
+	}
+
+	/* Got interrupt in inappropriate state */
+	if (ctx->state != ST_RGND_INIT)
+		return ST_FAILURE;
+
+	value = mhl_tx_readb(ctx, MHL_TX_STAT2_REG);
+	if (sii9234_clear_error(ctx))
+		return ST_FAILURE;
+
+	if ((value & RGND_INTP_MASK) != RGND_INTP_1K) {
+		dev_warn(ctx->dev, "RGND is not 1k\n");
+		return ST_RGND_INIT;
+	}
+	dev_dbg(ctx->dev, "RGND 1K!!\n");
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL4_REG, ~0, 0x8C);
+	mhl_tx_writeb(ctx, MHL_TX_DISC_CTRL5_REG, 0x77);
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL6_REG, ~0, 0x05);
+	if (sii9234_clear_error(ctx))
+		return ST_FAILURE;
+
+	usleep_range(T_SRC_VBUS_CBUS_TO_STABLE * USEC_PER_MSEC,
+		     T_SRC_VBUS_CBUS_TO_STABLE * USEC_PER_MSEC);
+
+	return ST_RGND_1K;
+}
+
+static enum sii9234_state sii9234_mhl_established(struct sii9234 *ctx)
+{
+	dev_dbg(ctx->dev, "mhl est interrupt\n");
+
+	/* Discovery override */
+	mhl_tx_writeb(ctx, MHL_TX_MHLTX_CTL1_REG, 0x10);
+	/* Increase DDC translation layer timer (byte mode) */
+	cbus_writeb(ctx, 0x07, 0x32);
+	cbus_writebm(ctx, 0x44, ~0, 1 << 1);
+	/* Keep the discovery enabled. Need RGND interrupt */
+	mhl_tx_writebm(ctx, MHL_TX_DISC_CTRL1_REG, ~0, 1);
+	mhl_tx_writeb(ctx, MHL_TX_INTR1_ENABLE_REG,
+	       RSEN_CHANGE_INT_MASK | HPD_CHANGE_INT_MASK);
+
+	if (sii9234_clear_error(ctx))
+		return ST_FAILURE;
+
+	return ST_MHL_ESTABLISHED;
+}
+
+static enum sii9234_state sii9234_hpd_change(struct sii9234 *ctx)
+{
+	int value;
+
+	value = cbus_readb(ctx, CBUS_MSC_REQ_ABORT_REASON_REG);
+	if (sii9234_clear_error(ctx))
+		return ST_FAILURE;
+
+	if (value & SET_HPD_DOWNSTREAM) {
+		/* Downstream HPD High, Enable TMDS */
+		sii9234_tmds_control(ctx, true);
+	} else {
+		/* Downstream HPD Low, Disable TMDS */
+		sii9234_tmds_control(ctx, false);
+	}
+
+	return ctx->state;
+}
+
+static enum sii9234_state sii9234_rsen_change(struct sii9234 *ctx)
+{
+	int value;
+
+	/* Work_around code to handle wrong interrupt */
+	if (ctx->state != ST_RGND_1K) {
+		dev_err(ctx->dev, "RSEN_HIGH without RGND_1K\n");
+		return ST_FAILURE;
+	}
+	value = mhl_tx_readb(ctx, MHL_TX_SYSSTAT_REG);
+	if (value < 0)
+		return ST_FAILURE;
+
+	if (value & RSEN_STATUS) {
+		dev_dbg(ctx->dev, "MHL cable connected.. RSEN High\n");
+		return ST_RSEN_HIGH;
+	}
+	dev_dbg(ctx->dev, "RSEN lost\n");
+	/* Once RSEN loss is confirmed,we need to check
+	 * based on cable status and chip power status,whether
+	 * it is SINK Loss(HDMI cable not connected, TV Off)
+	 * or MHL cable disconnection
+	 * TODO: Define the below mhl_disconnection()
+	 */
+	msleep(T_SRC_RXSENSE_DEGLITCH);
+	value = mhl_tx_readb(ctx, MHL_TX_SYSSTAT_REG);
+	if (value < 0)
+		return ST_FAILURE;
+	dev_dbg(ctx->dev, "sys_stat: %x\n", value);
+
+	if (value & RSEN_STATUS) {
+		dev_dbg(ctx->dev, "RSEN recovery\n");
+		return ST_RSEN_HIGH;
+	}
+	dev_dbg(ctx->dev, "RSEN Really LOW\n");
+	/* To meet CTS 3.3.22.2 spec */
+	sii9234_tmds_control(ctx, false);
+	force_usb_id_switch_open(ctx);
+	release_usb_id_switch_open(ctx);
+
+	return ST_FAILURE;
+}
+
+static irqreturn_t sii9234_irq_thread(int irq, void *data)
+{
+	struct sii9234 *ctx = data;
+	int intr1, intr4;
+	int intr1_en, intr4_en;
+	int cbus_intr1, cbus_intr2;
+
+	dev_dbg(ctx->dev, "%s\n", __func__);
+
+	mutex_lock(&ctx->lock);
+
+	intr1 = mhl_tx_readb(ctx, MHL_TX_INTR1_REG);
+	intr4 = mhl_tx_readb(ctx, MHL_TX_INTR4_REG);
+	intr1_en = mhl_tx_readb(ctx, MHL_TX_INTR1_ENABLE_REG);
+	intr4_en = mhl_tx_readb(ctx, MHL_TX_INTR4_ENABLE_REG);
+	cbus_intr1 = cbus_readb(ctx, CBUS_INT_STATUS_1_REG);
+	cbus_intr2 = cbus_readb(ctx, CBUS_INT_STATUS_2_REG);
+
+	if (sii9234_clear_error(ctx))
+		goto done;
+
+	dev_dbg(ctx->dev, "irq %02x/%02x %02x/%02x %02x/%02x\n",
+		 intr1, intr1_en, intr4, intr4_en, cbus_intr1, cbus_intr2);
+
+	if (intr4 & RGND_READY_INT)
+		ctx->state = sii9234_rgnd_ready_irq(ctx);
+	if (intr1 & RSEN_CHANGE_INT)
+		ctx->state = sii9234_rsen_change(ctx);
+	if (intr4 & MHL_EST_INT)
+		ctx->state = sii9234_mhl_established(ctx);
+	if (intr1 & HPD_CHANGE_INT)
+		ctx->state = sii9234_hpd_change(ctx);
+	if (intr4 & CBUS_LKOUT_INT)
+		ctx->state = ST_FAILURE;
+	if (intr4 & MHL_DISC_FAIL_INT)
+		ctx->state = ST_FAILURE_DISCOVERY;
+
+ done:
+	/* Clean interrupt status and pending flags */
+	mhl_tx_writeb(ctx, MHL_TX_INTR1_REG, intr1);
+	mhl_tx_writeb(ctx, MHL_TX_INTR4_REG, intr4);
+	cbus_writeb(ctx, CBUS_MHL_STATUS_REG_0, 0xFF);
+	cbus_writeb(ctx, CBUS_MHL_STATUS_REG_1, 0xFF);
+	cbus_writeb(ctx, CBUS_INT_STATUS_1_REG, cbus_intr1);
+	cbus_writeb(ctx, CBUS_INT_STATUS_2_REG, cbus_intr2);
+
+	sii9234_clear_error(ctx);
+
+	if (ctx->state == ST_FAILURE) {
+		dev_dbg(ctx->dev, "try to reset after failure\n");
+		sii9234_hw_reset(ctx);
+		sii9234_goto_d3(ctx);
+	}
+
+	if (ctx->state == ST_FAILURE_DISCOVERY) {
+		dev_err(ctx->dev, "discovery failed, no power for MHL?\n");
+		tpi_writebm(ctx, TPI_DPD_REG, 0, 1);
+		ctx->state = ST_D3;
+	}
+
+	mutex_unlock(&ctx->lock);
+
+	return IRQ_HANDLED;
+}
+
+
+static int sii9234_init_resources(struct sii9234 *ctx,
+	struct i2c_client *client)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	int ret;
+
+	if (!ctx->dev->of_node) {
+		dev_err(ctx->dev, "not DT device\n");
+		return -ENODEV;
+	}
+
+	ctx->gpio_reset = devm_gpiod_get(ctx->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpio_reset)) {
+		dev_err(ctx->dev, "failed to get reset gpio from DT\n");
+		return PTR_ERR(ctx->gpio_reset);
+	}
+
+	ctx->supplies[0].supply = "cvcc12";
+	ctx->supplies[1].supply = "avcc33";
+	ctx->supplies[2].supply = "iovcc18";
+	ctx->supplies[3].supply = "avcc12";
+	ret = devm_regulator_bulk_get(ctx->dev, 4, ctx->supplies);
+	if (ret) {
+		dev_err(ctx->dev, "regulator_bulk failed\n");
+		return ret;
+	}
+
+	ctx->client[I2C_MHL] = client;
+
+	ctx->client[I2C_TPI] = i2c_new_dummy(adapter, I2C_TPI_ADDR);
+	if (!ctx->client[I2C_TPI]) {
+		dev_err(ctx->dev, "failed to create TPI client\n");
+		return -ENODEV;
+	}
+
+	ctx->client[I2C_HDMI] = i2c_new_dummy(adapter, I2C_HDMI_ADDR);
+	if (!ctx->client[I2C_HDMI]) {
+		dev_err(ctx->dev, "failed to create HDMI RX client\n");
+		goto fail_tpi;
+	}
+
+	ctx->client[I2C_CBUS] = i2c_new_dummy(adapter, I2C_CBUS_ADDR);
+	if (!ctx->client[I2C_CBUS]) {
+		dev_err(ctx->dev, "failed to create CBUS client\n");
+		goto fail_hdmi;
+	}
+
+	return 0;
+
+fail_hdmi:
+	i2c_unregister_device(ctx->client[I2C_HDMI]);
+fail_tpi:
+	i2c_unregister_device(ctx->client[I2C_TPI]);
+
+	return -ENODEV;
+}
+
+static void sii9234_deinit_resources(struct sii9234 *ctx)
+{
+	i2c_unregister_device(ctx->client[I2C_CBUS]);
+	i2c_unregister_device(ctx->client[I2C_HDMI]);
+	i2c_unregister_device(ctx->client[I2C_TPI]);
+}
+
+static inline struct sii9234 *bridge_to_sii9234(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct sii9234, bridge);
+}
+
+static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
+					const struct drm_display_mode *mode)
+{
+	if (mode->clock > MHL1_MAX_CLK)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
+static const struct drm_bridge_funcs sii9234_bridge_funcs = {
+	.mode_valid = sii9234_mode_valid,
+};
+
+static int sii9234_probe(struct i2c_client *client,
+					      const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct sii9234 *ctx;
+	struct device *dev = &client->dev;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->dev = dev;
+	mutex_init(&ctx->lock);
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(dev, "I2C adapter lacks SMBUS feature\n");
+		return -EIO;
+	}
+
+	if (!client->irq) {
+		dev_err(dev, "no irq provided\n");
+		return -EINVAL;
+	}
+
+	irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
+	ret = devm_request_threaded_irq(dev, client->irq, NULL,
+					sii9234_irq_thread,
+					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+					"sii9234", ctx);
+	if (ret < 0) {
+		dev_err(dev, "failed to install IRQ handler\n");
+		return ret;
+	}
+
+	ret = sii9234_init_resources(ctx, client);
+	if (ret < 0)
+		return ret;
+
+	i2c_set_clientdata(client, ctx);
+
+	ctx->bridge.funcs = &sii9234_bridge_funcs;
+	ctx->bridge.of_node = dev->of_node;
+	drm_bridge_add(&ctx->bridge);
+
+	sii9234_cable_in(ctx);
+
+	return 0;
+}
+
+static int sii9234_remove(struct i2c_client *client)
+{
+	struct sii9234 *ctx = i2c_get_clientdata(client);
+
+	sii9234_cable_out(ctx);
+	drm_bridge_remove(&ctx->bridge);
+	sii9234_deinit_resources(ctx);
+
+	return 0;
+}
+
+static const struct of_device_id sii9234_dt_match[] = {
+	{ .compatible = "sil,sii9234" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sii9234_dt_match);
+
+static const struct i2c_device_id sii9234_id[] = {
+	{ "SII9234", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, sii9234_id);
+
+static struct i2c_driver sii9234_driver = {
+	.driver = {
+		.name	= "sii9234",
+		.of_match_table = sii9234_dt_match,
+	},
+	.probe		= sii9234_probe,
+	.remove		= sii9234_remove,
+	.id_table = sii9234_id,
+};
+
+module_i2c_driver(sii9234_driver);
+MODULE_LICENSE("GPL");