Re: [PATCH 2/2] drm/bridge: Add tc358768 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21.01.2020 16:11, Peter Ujfalusi wrote:
> Hi Andrzej,
>
> On 16/01/2020 16.35, Andrzej Hajda wrote:
>> On 17.12.2019 11:15, Peter Ujfalusi wrote:
>>> Add basic support for the Toshiba TC358768 RGB to DSI bridge.
>>> Not all the features of the TC358768 is implemented by the initial driver:
>>> MIPI_DSI_MODE_VIDEO and MIPI_DSI_FMT_RGB888 is only supported and tested.
>>>
>>> Only write is implemented for mipi_dsi_host_ops.transfer.
>>>
>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
>>> ---
>>>  drivers/gpu/drm/bridge/Kconfig    |  10 +
>>>  drivers/gpu/drm/bridge/Makefile   |   1 +
>>>  drivers/gpu/drm/bridge/tc358768.c | 963 ++++++++++++++++++++++++++++++
>>>  3 files changed, 974 insertions(+)
>>>  create mode 100644 drivers/gpu/drm/bridge/tc358768.c
>>>
>>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>>> index ccc698c44f58..fd65666702e1 100644
>>> --- a/drivers/gpu/drm/bridge/Kconfig
>>> +++ b/drivers/gpu/drm/bridge/Kconfig
>>> @@ -122,6 +122,16 @@ config DRM_TOSHIBA_TC358767
>>>  	---help---
>>>  	  Toshiba TC358767 eDP bridge chip driver.
>>>  
>>> +config DRM_TOSHIBA_TC358768
>>> +	tristate "Toshiba TC358768 MIPI DSI bridge"
>>> +	depends on OF
>>> +	select DRM_KMS_HELPER
>>> +	select REGMAP_I2C
>>> +	select DRM_PANEL
>>> +	select DRM_MIPI_DSI
>>> +	help
>>> +	  Toshiba TC358768AXBG/TC358778XBG DSI bridge chip driver.
>>> +
>>>  config DRM_TI_TFP410
>>>  	tristate "TI TFP410 DVI/HDMI bridge"
>>>  	depends on OF
>>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>>> index a6c7dd7727ea..204696e30572 100644
>>> --- a/drivers/gpu/drm/bridge/Makefile
>>> +++ b/drivers/gpu/drm/bridge/Makefile
>>> @@ -11,6 +11,7 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
>>>  obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
>>>  obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>>>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>> +obj-$(CONFIG_DRM_TOSHIBA_TC358768) += tc358768.o
>>>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>>>  obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>>>  obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
>>> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
>>> new file mode 100644
>>> index 000000000000..63571191b1c4
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/tc358768.c
>>> @@ -0,0 +1,963 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + *  Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com
>>
>> Just FYI, we have 2020 already, maybe update needed.
> Oh, how time flies ;)
>
>>
>>> + *  Author: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/device.h>
>>> +#include <linux/gpio/consumer.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/regulator/consumer.h>
>>
>> alphabetic order
> Ok.
>
>>> +
>>> +#include <drm/drm_atomic_helper.h>
>>> +#include <drm/drm_crtc_helper.h>
>>> +#include <drm/drm_drv.h>
>>> +#include <drm/drm_of.h>
>>> +#include <drm/drm_panel.h>
>>> +#include <drm/drm_mipi_dsi.h>
>>> +#include <video/mipi_display.h>
>>> +#include <video/videomode.h>
>>> +
>>> +/* Global (16-bit addressable) */
>>> +#define TC358768_CHIPID			0x0000
>>> +#define TC358768_SYSCTL			0x0002
>>> +#define TC358768_CONFCTL		0x0004
>>> +#define TC358768_VSDLY			0x0006
>>> +#define TC358768_DATAFMT		0x0008
>>> +#define TC358768_GPIOEN			0x000E
>>> +#define TC358768_GPIODIR		0x0010
>>> +#define TC358768_GPIOIN			0x0012
>>> +#define TC358768_GPIOOUT		0x0014
>>> +#define TC358768_PLLCTL0		0x0016
>>> +#define TC358768_PLLCTL1		0x0018
>>> +#define TC358768_CMDBYTE		0x0022
>>> +#define TC358768_PP_MISC		0x0032
>>> +#define TC358768_DSITX_DT		0x0050
>>> +#define TC358768_FIFOSTATUS		0x00F8
>>> +
>>> +/* Debug (16-bit addressable) */
>>> +#define TC358768_VBUFCTRL		0x00E0
>>> +#define TC358768_DBG_WIDTH		0x00E2
>>> +#define TC358768_DBG_VBLANK		0x00E4
>>> +#define TC358768_DBG_DATA		0x00E8
>>> +
>>> +/* TX PHY (32-bit addressable) */
>>> +#define TC358768_CLW_DPHYCONTTX		0x0100
>>> +#define TC358768_D0W_DPHYCONTTX		0x0104
>>> +#define TC358768_D1W_DPHYCONTTX		0x0108
>>> +#define TC358768_D2W_DPHYCONTTX		0x010C
>>> +#define TC358768_D3W_DPHYCONTTX		0x0110
>>> +#define TC358768_CLW_CNTRL		0x0140
>>> +#define TC358768_D0W_CNTRL		0x0144
>>> +#define TC358768_D1W_CNTRL		0x0148
>>> +#define TC358768_D2W_CNTRL		0x014C
>>> +#define TC358768_D3W_CNTRL		0x0150
>>> +
>>> +/* TX PPI (32-bit addressable) */
>>> +#define TC358768_STARTCNTRL		0x0204
>>> +#define TC358768_DSITXSTATUS		0x0208
>>> +#define TC358768_LINEINITCNT		0x0210
>>> +#define TC358768_LPTXTIMECNT		0x0214
>>> +#define TC358768_TCLK_HEADERCNT		0x0218
>>> +#define TC358768_TCLK_TRAILCNT		0x021C
>>> +#define TC358768_THS_HEADERCNT		0x0220
>>> +#define TC358768_TWAKEUP		0x0224
>>> +#define TC358768_TCLK_POSTCNT		0x0228
>>> +#define TC358768_THS_TRAILCNT		0x022C
>>> +#define TC358768_HSTXVREGCNT		0x0230
>>> +#define TC358768_HSTXVREGEN		0x0234
>>> +#define TC358768_TXOPTIONCNTRL		0x0238
>>> +#define TC358768_BTACNTRL1		0x023C
>>> +
>>> +/* TX CTRL (32-bit addressable) */
>>> +#define TC358768_DSI_CONTROL		0x040C
>>> +#define TC358768_DSI_STATUS		0x0410
>>> +#define TC358768_DSI_INT		0x0414
>>> +#define TC358768_DSI_INT_ENA		0x0418
>>> +#define TC358768_DSICMD_RDFIFO		0x0430
>>> +#define TC358768_DSI_ACKERR		0x0434
>>> +#define TC358768_DSI_ACKERR_INTENA	0x0438
>>> +#define TC358768_DSI_ACKERR_HALT	0x043c
>>> +#define TC358768_DSI_RXERR		0x0440
>>> +#define TC358768_DSI_RXERR_INTENA	0x0444
>>> +#define TC358768_DSI_RXERR_HALT		0x0448
>>> +#define TC358768_DSI_ERR		0x044C
>>> +#define TC358768_DSI_ERR_INTENA		0x0450
>>> +#define TC358768_DSI_ERR_HALT		0x0454
>>> +#define TC358768_DSI_CONFW		0x0500
>>> +#define TC358768_DSI_LPCMD		0x0500
>>> +#define TC358768_DSI_RESET		0x0504
>>> +#define TC358768_DSI_INT_CLR		0x050C
>>> +#define TC358768_DSI_START		0x0518
>>> +
>>> +/* DSITX CTRL (16-bit addressable) */
>>> +#define TC358768_DSICMD_TX		0x0600
>>> +#define TC358768_DSICMD_TYPE		0x0602
>>> +#define TC358768_DSICMD_WC		0x0604
>>> +#define TC358768_DSICMD_WD0		0x0610
>>> +#define TC358768_DSICMD_WD1		0x0612
>>> +#define TC358768_DSICMD_WD2		0x0614
>>> +#define TC358768_DSICMD_WD3		0x0616
>>> +#define TC358768_DSI_EVENT		0x0620
>>> +#define TC358768_DSI_VSW		0x0622
>>> +#define TC358768_DSI_VBPR		0x0624
>>> +#define TC358768_DSI_VACT		0x0626
>>> +#define TC358768_DSI_HSW		0x0628
>>> +#define TC358768_DSI_HBPR		0x062A
>>> +#define TC358768_DSI_HACT		0x062C
>>> +
>>> +static const char * const tc358768_supplies[] = {
>>> +	"vddc", "vddmipi", "vddio"
>>> +};
>>> +
>>> +struct tc358768_dsi_output {
>>> +	struct mipi_dsi_device *dev;
>>> +	struct drm_panel *panel;
>>> +	struct drm_bridge *bridge;
>>> +};
>>> +
>>> +struct tc358768_priv {
>>> +	struct device *dev;
>>> +	struct regmap *regmap;
>>> +	struct gpio_desc *reset_gpio;
>>> +	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358768_supplies)];
>>> +	struct clk *refclk;
>>> +
>>> +	struct mipi_dsi_host dsi_host;
>>> +	struct drm_bridge bridge;
>>> +	struct tc358768_dsi_output output;
>>
>> Since tc358768_dsi_output is used only here, you can define it here as
>> well, up to you.
> I think I have done it like this to avoid thinking about prefixes for
> what is under the tc358768_dsi_output.
> I'll take a look if it will look better unpacked here.

I though rather about in-place anonymous struct definition:

+    struct tc358768_dsi_output {
+        struct mipi_dsi_device *dev;
+        struct drm_panel *panel;
+        struct drm_bridge *bridge;
+    } output;

But, as I said - up to you.

>
>>> +
>>> +	const struct drm_display_mode *mode;
>>> +	u32 pd_lines; /* number of Parallel Port Input Data Lines */
>>> +	u32 dsi_lanes; /* number of DSI Lanes */
>>> +
>>> +	u32 fbd;
>>> +	u32 prd;
>>> +	u32 frs;
>>
>> Looks ugly, please rename these 3 vars to sth more readable, or at least
>> document them (if the names comes from the specification).
> Sure, I agree.
>
>>> +
>>> +	u32 dsiclk; /* pll_clk / 2 */
>>> +};
>>> +
>>> +static inline struct tc358768_priv *dsi_host_to_tc358768(struct mipi_dsi_host
>>> +							 *host)
>>> +{
>>> +	return container_of(host, struct tc358768_priv, dsi_host);
>>> +}
>>> +
>>> +static inline struct tc358768_priv *bridge_to_tc358768(struct drm_bridge
>>> +						       *bridge)
>>> +{
>>> +	return container_of(bridge, struct tc358768_priv, bridge);
>>> +}
>>> +
>>> +static int tc358768_write(struct tc358768_priv *priv, u32 reg, u32 val)
>>> +{
>>> +	size_t count = 2;
>>> +
>>> +	/* 16-bit register? */
>>> +	if (reg < 0x100 || reg >= 0x600)
>>> +		count = 1;
>>> +
>>> +	return regmap_bulk_write(priv->regmap, reg, &val, count);
>>> +}
>>> +
>>> +static int tc358768_read(struct tc358768_priv *priv, u32 reg, u32 *val)
>>> +{
>>> +	size_t count = 2;
>>> +
>>> +	/* 16-bit register? */
>>> +	if (reg < 0x100 || reg >= 0x600) {
>>> +		*val = 0;
>>> +		count = 1;
>>> +	}
>>> +
>>> +	return regmap_bulk_read(priv->regmap, reg, val, count);
>>> +}
>>> +
>>> +static int tc358768_update_bits(struct tc358768_priv *priv, u32 reg, u32 mask,
>>> +				u32 val)
>>> +{
>>> +	int ret;
>>> +	u32 tmp, orig;
>>> +
>>> +	ret = tc358768_read(priv, reg, &orig);
>>> +	if (ret != 0)
>>> +		return ret;
>>> +
>>> +	tmp = orig & ~mask;
>>> +	tmp |= val & mask;
>>> +
>>> +	if (tmp != orig)
>>> +		ret = tc358768_write(priv, reg, tmp);
>>> +
>>> +	return ret;
>>> +}
>>
>> You have defined 3 hw access functions which can fail but you do not
>> check their return values in later usage. Does regmap/i2c logs sth in
>> case of error?
>>
>> If not, maybe error logging should be added at least in these accessors.
>>
>> The best would be to propagate errors to callers, either traditional way
>> (add check after each call), either you can use 'error state' pattern,
>> which I recommend (see for example to 'ctx->error' usage in sil-sii8620.c).
> OK, I'll clean this up
>
>>> +
>>> +static void tc358768_sw_reset(struct tc358768_priv *priv)
>>> +{
>>> +	/* Assert Reset */
>>> +	tc358768_write(priv, TC358768_SYSCTL, 1);
>>> +	/* Release Reset, Exit Sleep */
>>> +	tc358768_write(priv, TC358768_SYSCTL, 0);
>>> +}
>>> +
>>> +static void tc358768_hw_enable(struct tc358768_priv *priv, bool enable)
>>> +{
>>> +	int ret;
>>> +
>>> +	if (enable) {
>>> +		ret = regulator_bulk_enable(ARRAY_SIZE(priv->supplies),
>>> +					    priv->supplies);
>>> +		if (ret < 0)
>>> +			dev_err(priv->dev, "error enabling regulators (%d)\n",
>>> +				ret);
>>> +		usleep_range(200, 300);
>>> +	}
>>> +
>>> +	/*
>>> +	 * The RESX is active low (GPIO_ACTIVE_LOW).
>>> +	 * Invert the 'enable' value to get correct assertion for the gpio:
>>> +	 * enable == true: reset_gpio needs to be DEASSERTED (value = 0)
>>> +	 * enable == false: reset_gpio needs to be ASSERTED (value = 1)
>>> +	 */
>>> +	gpiod_set_value_cansleep(priv->reset_gpio, !enable);
>>> +
>>> +	regcache_cache_only(priv->regmap, !enable);
>>> +	if (enable) {
>>> +		/* wait for encoder clocks to stabilize */
>>> +		usleep_range(1000, 2000);
>>> +
>>> +		regcache_sync(priv->regmap);
>>
>> What do you need to sync here?
> Right, we do a full reconfiguration after this, you are correct, there
> is no need to sync and the regmap's REGCACHE_RBTREE can be REGCACHE_NONE
> safely.
>
>>> +	} else {
>>> +		ret = regulator_bulk_disable(ARRAY_SIZE(priv->supplies),
>>> +					     priv->supplies);
>>> +		if (ret < 0)
>>> +			dev_err(priv->dev, "error disabling regulators (%d)\n",
>>> +				ret);
>>> +	}
>>> +}
>>
>> Above function looks ugly - these 'if (enable)' and similar are hard to
>> read, maybe two separate hw_enable, hw_disable function would look
>> better, more readable, up to you.
> OK, I'll separate the enable and disable.
>
>>> +
>>> +static u32 tc358768_pll_to_pclk(struct tc358768_priv *priv, u32 pll_clk)
>>> +{
>>> +	return (u32)div_u64((u64)pll_clk * priv->dsi_lanes, priv->pd_lines);
>>> +}
>>> +
>>> +static u32 tc358768_pclk_to_pll(struct tc358768_priv *priv, u32 pclk)
>>> +{
>>> +	return (u32)div_u64((u64)pclk * priv->pd_lines, priv->dsi_lanes);
>>> +}
>>> +
>>> +static int tc358768_calc_pll(struct tc358768_priv *priv)
>>> +{
>>> +	const u32 frs_limits[] = {
>>> +		1000000000,
>>> +		500000000,
>>> +		250000000,
>>> +		125000000,
>>> +		62500000
>>> +	};
>>> +	unsigned long refclk;
>>> +	u32 prd, target_pll, i, max_pll, min_pll;
>>> +	u32 frs, best_diff, best_pll, best_prd, best_fbd;
>>> +
>>> +	target_pll = tc358768_pclk_to_pll(priv, priv->mode->clock * 1000);
>>> +
>>> +	/* pll_clk = RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)] */
>>> +
>> I guess below code:
>>> +	frs = UINT_MAX;
>>> +
>>> +	for (i = 0; i < ARRAY_SIZE(frs_limits) - 1; i++) {
>>> +		if (target_pll < frs_limits[i] &&
>>> +		    target_pll >= frs_limits[i + 1]) {
>>> +			frs = i;
>>> +			max_pll = frs_limits[i];
>>> +			min_pll = frs_limits[i + 1];
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	if (frs == UINT_MAX)
>>> +		return -EINVAL;
>>
>> can be simplified:
>>
>> for (i = 0; i < ARRAY_SIZE(frs_limits); i++)
>>
>>     if (target_pll >= frs_limits[i])
>>
>>        break;
>>
>> if (i == ARRAY_SIZE(frs_limits) || i == 0)
>>
>>     return -EINVAL;
>>
>> frs = i - 1;
>>
>> max_pll = frs_limits[i-1];
>>
>> min_pll = frs_limits[i];
> Yes, it enhances readability as well.
>
>>> +
>>> +	refclk = clk_get_rate(priv->refclk);
>>> +
>>> +	best_diff = UINT_MAX;
>>> +	best_pll = 0;
>>> +	best_prd = 0;
>>> +	best_fbd = 0;
>>> +
>>> +	for (prd = 0; prd < 16; ++prd) {
>>> +		u32 divisor = (prd + 1) * (1 << frs);
>>> +		u32 fbd;
>>> +
>>> +		for (fbd = 0; fbd < 512; ++fbd) {
>>> +			u32 pll, diff;
>>> +
>>> +			pll = (u32)div_u64((u64)refclk * (fbd + 1), divisor);
>>> +
>>> +			if (pll >= max_pll || pll < min_pll)
>>> +				continue;
>>> +
>>> +			diff = max(pll, target_pll) - min(pll, target_pll);
>>> +
>>> +			if (diff < best_diff) {
>>> +				best_diff = diff;
>>> +				best_pll = pll;
>>> +				best_prd = prd;
>>> +				best_fbd = fbd;
>>> +			}
>>> +
>>> +			if (best_diff == 0)
>>> +				break;
>>> +		}
>>> +
>>> +		if (best_diff == 0)
>>> +			break;
>> why another check here?
> To break out from the top for() loop also in case exact match has been
> found.


Ahh, OK. So maybe you should put "if (diff == 0) goto found" inside "if
(diff < best_diff)" block, in such case goto is not considered harmful
:), and is more readable.


Regards

Andrzej


>
>>> +	}
>>> +
>>> +	if (best_diff == UINT_MAX) {
>>> +		dev_err(priv->dev, "could not find suitable PLL setup\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	priv->fbd = best_fbd;
>>> +	priv->prd = best_prd;
>>> +	priv->frs = frs;
>>> +	priv->dsiclk = best_pll / 2;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int tc358768_dsi_host_attach(struct mipi_dsi_host *host,
>>> +				    struct mipi_dsi_device *dev)
>>> +{
>>> +	struct tc358768_priv *priv = dsi_host_to_tc358768(host);
>>> +	struct drm_bridge *bridge;
>>> +	struct drm_panel *panel;
>>> +	struct device_node *ep;
>>> +	int ret;
>>> +
>>> +	if (dev->lanes > 4) {
>>> +		dev_err(priv->dev, "unsupported number of data lanes(%u)\n",
>>> +			dev->lanes);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	/*
>>> +	 * tc358768 supports both Video and Pulse mode, but the driver only
>>> +	 * implements Video (event) mode currently
>>> +	 */
>>> +	if (!(dev->mode_flags & MIPI_DSI_MODE_VIDEO)) {
>>> +		dev_err(priv->dev, "Only MIPI_DSI_MODE_VIDEO is supported\n");
>>> +		return -ENOTSUPP;
>>> +	}
>>> +
>>> +	/*
>>> +	 * tc358768 supports RGB888, RGB666, RGB666_PACKED and RGB565, but only
>>> +	 * RGB888 is verified.
>>> +	 */
>>> +	if (dev->format != MIPI_DSI_FMT_RGB888) {
>>> +		dev_warn(priv->dev, "Only MIPI_DSI_FMT_RGB888 tested!\n");
>>> +		return -ENOTSUPP;
>>> +	}
>>> +
>>> +	ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0, &panel,
>>> +					  &bridge);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (panel) {
>>> +		bridge = drm_panel_bridge_add_typed(panel,
>>> +						    DRM_MODE_CONNECTOR_DSI);
>>> +		if (IS_ERR(bridge))
>>> +			return PTR_ERR(bridge);
>>> +	}
>>> +
>>> +	priv->output.dev = dev;
>>> +	priv->output.bridge = bridge;
>>> +	priv->output.panel = panel;
>>> +
>>> +	priv->dsi_lanes = dev->lanes;
>>> +
>>> +	/* get input ep (port0/endpoint0) */
>>> +	ret = -EINVAL;
>>> +	ep = of_graph_get_endpoint_by_regs(host->dev->of_node, 0, 0);
>>> +	if (ep) {
>>> +		ret = of_property_read_u32(ep, "data-lines", &priv->pd_lines);
>>> +
>>> +		of_node_put(ep);
>>> +	}
>>> +
>>> +	if (ret)
>>> +		priv->pd_lines = mipi_dsi_pixel_format_to_bpp(dev->format);
>>> +
>>> +	drm_bridge_add(&priv->bridge);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int tc358768_dsi_host_detach(struct mipi_dsi_host *host,
>>> +				    struct mipi_dsi_device *dev)
>>> +{
>>> +	struct tc358768_priv *priv = dsi_host_to_tc358768(host);
>>> +
>>> +	drm_bridge_remove(&priv->bridge);
>>> +	if (priv->output.panel)
>>> +		drm_panel_bridge_remove(priv->output.bridge);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static ssize_t tc358768_dsi_host_transfer(struct mipi_dsi_host *host,
>>> +					  const struct mipi_dsi_msg *msg)
>>> +{
>>> +	struct tc358768_priv *priv = dsi_host_to_tc358768(host);
>>> +	struct mipi_dsi_packet packet;
>>> +	int ret;
>>> +
>>> +	if (msg->rx_len) {
>>> +		dev_warn(priv->dev, "MIPI rx is not supported\n");
>>> +		return -ENOTSUPP;
>>> +	}
>>> +
>>> +	if (msg->tx_len > 8) {
>>> +		dev_warn(priv->dev, "Maximum 8 byte MIPI tx is supported\n");
>>> +		return -ENOTSUPP;
>>> +	}
>>> +
>>> +	ret = mipi_dsi_create_packet(&packet, msg);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	tc358768_hw_enable(priv, true);
>>> +
>>> +	if (mipi_dsi_packet_format_is_short(msg->type)) {
>>> +		tc358768_write(priv, TC358768_DSICMD_TYPE,
>>> +			       (0x10 << 8) | (packet.header[0] & 0x3f));
>>> +		tc358768_write(priv, TC358768_DSICMD_WC, 0);
>>> +		tc358768_write(priv, TC358768_DSICMD_WD0,
>>> +			       (packet.header[2] << 8) | packet.header[1]);
>>> +	} else {
>>> +		int i, j;
>>> +
>>> +		tc358768_write(priv, TC358768_DSICMD_TYPE,
>>> +			       (0x40 << 8) | (packet.header[0] & 0x3f));
>>> +		tc358768_write(priv, TC358768_DSICMD_WC, packet.payload_length);
>>> +		for (i = 0; i < packet.payload_length; i += 2) {
>>> +			u16 val = 0;
>>> +
>>> +			for (j = 0; j < 2 && i + j < packet.payload_length; j++)
>>> +				val |= packet.payload[i + j] << (8 * j);
>>> +
>>
>> Quite clever, but barely readable, maybe:
>>
>> u16 val = packet.payload[i];
>>
>> if (i + 1 < packet.payload_length)
>>
>>   val |= packet.payload[i + 1] << 8;
> yes, looks simpler.
>
>>
>>> +			tc358768_write(priv, TC358768_DSICMD_WD0 + i, val);
>>> +		}
>>> +	}
>>> +
>>> +	/* start transfer */
>>> +	tc358768_write(priv, TC358768_DSICMD_TX, 1);
>>> +
>>> +	tc358768_hw_enable(priv, false);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct mipi_dsi_host_ops tc358768_dsi_host_ops = {
>>> +	.attach = tc358768_dsi_host_attach,
>>> +	.detach = tc358768_dsi_host_detach,
>>> +	.transfer = tc358768_dsi_host_transfer,
>>> +};
>>> +
>>> +static int tc358768_bridge_attach(struct drm_bridge *bridge)
>>> +{
>>> +	struct tc358768_priv *priv = bridge_to_tc358768(bridge);
>>> +
>>> +	if (!drm_core_check_feature(bridge->dev, DRIVER_ATOMIC)) {
>>> +		dev_err(priv->dev, "needs atomic updates support\n");
>>> +		return -ENOTSUPP;
>>> +	}
>>> +
>>> +	return drm_bridge_attach(bridge->encoder, priv->output.bridge, bridge);
>>> +}
>>> +
>>> +static enum drm_mode_status
>>> +tc358768_bridge_mode_valid(struct drm_bridge *bridge,
>>> +			   const struct drm_display_mode *mode)
>>> +{
>>> +	struct tc358768_priv *priv = bridge_to_tc358768(bridge);
>>> +
>>> +	priv->mode = mode;
>>
>> This is dangerous, you should assume mode pointer provided in this
>> callback is valid only till end of function.
>>
>> If you need mode later you can use this ugly pointer:
>>
>> priv->bridge.encoder->crtc->state->adjusted_mode
>>
>> or alternatively copy mode to priv (also ugly, but at least no dangling
>> pointers).
> OK, I'll see how it can be reworked.
>
>>
>>
>>> +
>>> +	if (tc358768_calc_pll(priv))
>>> +		return MODE_CLOCK_RANGE;
>> tc358768_calc_pll modifies state, mode_valid callback shouldn't do it.
>>
>>
>> Regards
>>
>> Andrzej
> Thank you for the review,
> - Péter
>
>>> +
>>> +	return MODE_OK;
>>> +}
>>> +
>>> +static void tc358768_bridge_disable(struct drm_bridge *bridge)
>>> +{
>>> +	struct tc358768_priv *priv = bridge_to_tc358768(bridge);
>>> +
>>> +	/* set FrmStop */
>>> +	tc358768_update_bits(priv, TC358768_PP_MISC, BIT(15), BIT(15));
>>> +
>>> +	/* wait at least for one frame */
>>> +	msleep(50);
>>> +
>>> +	/* clear PP_en */
>>> +	tc358768_update_bits(priv, TC358768_CONFCTL, BIT(6), 0);
>>> +
>>> +	/* set RstPtr */
>>> +	tc358768_update_bits(priv, TC358768_PP_MISC, BIT(14), BIT(14));
>>> +
>>> +	tc358768_hw_enable(priv, false);
>>> +}
>>> +
>>> +static void tc358768_setup_pll(struct tc358768_priv *priv)
>>> +{
>>> +	u32 fbd, prd, frs;
>>> +
>>> +	fbd = priv->fbd;
>>> +	prd = priv->prd;
>>> +	frs = priv->frs;
>>> +
>>> +	dev_dbg(priv->dev, "PLL: refclk %lu, fbd %u, prd %u, frs %u\n",
>>> +		clk_get_rate(priv->refclk), fbd, prd, frs);
>>> +	dev_dbg(priv->dev, "PLL: pll_clk: %u, DSIClk %u, DSIByteClk %u\n",
>>> +		priv->dsiclk * 2, priv->dsiclk, priv->dsiclk / 4);
>>> +	dev_dbg(priv->dev, "PLL: pclk %u (panel: %u)\n",
>>> +		tc358768_pll_to_pclk(priv, priv->dsiclk * 2),
>>> +		priv->mode->clock * 1000);
>>> +
>>> +	/* PRD[15:12] FBD[8:0] */
>>> +	tc358768_write(priv, TC358768_PLLCTL0, (prd << 12) | fbd);
>>> +
>>> +	/* FRS[11:10] LBWS[9:8] CKEN[4] RESETB[1] EN[0] */
>>> +	tc358768_write(priv, TC358768_PLLCTL1,
>>> +		       (frs << 10) | (0x2 << 8) | BIT(1) | BIT(0));
>>> +
>>> +	/* wait for lock */
>>> +	usleep_range(1000, 2000);
>>> +
>>> +	/* FRS[11:10] LBWS[9:8] CKEN[4] PLL_CKEN[4] RESETB[1] EN[0] */
>>> +	tc358768_write(priv, TC358768_PLLCTL1,
>>> +		       (frs << 10) | (0x2 << 8) | BIT(4) | BIT(1) | BIT(0));
>>> +}
>>> +
>>> +#define TC358768_PRECISION	1000
>>> +static u32 tc358768_ns_to_cnt(u32 ns, u32 period_nsk)
>>> +{
>>> +	return (ns * TC358768_PRECISION + period_nsk) / period_nsk;
>>> +}
>>> +
>>> +static u32 tc358768_to_ns(u32 nsk)
>>> +{
>>> +	return (nsk / TC358768_PRECISION);
>>> +}
>>> +
>>> +static void tc358768_bridge_enable(struct drm_bridge *bridge)
>>> +{
>>> +	struct tc358768_priv *priv = bridge_to_tc358768(bridge);
>>> +	const struct drm_display_mode *mode = priv->mode;
>>> +	struct mipi_dsi_device *dsi_dev = priv->output.dev;
>>> +	u32 val, val2, lptxcnt, hact, data_type;
>>> +	u32 dsiclk = priv->dsiclk;
>>> +	u32 dsibclk = dsiclk / 4;
>>> +	u32 dsibclk_nsk, dsiclk_nsk, ui_nsk, phy_delay_nsk;
>>> +	int i;
>>> +
>>> +	tc358768_hw_enable(priv, true);
>>> +
>>> +	tc358768_sw_reset(priv);
>>> +
>>> +	tc358768_setup_pll(priv);
>>> +
>>> +	/* VSDly[9:0] */
>>> +	tc358768_write(priv, TC358768_VSDLY, 1);
>>> +
>>> +	/* Data Format Control Register */
>>> +	val = BIT(2) | BIT(1) | BIT(0); /* rdswap_en | dsitx_en | txdt_en */
>>> +	switch (dsi_dev->format) {
>>> +	case MIPI_DSI_FMT_RGB888:
>>> +		val |= (0x3 << 4);
>>> +		hact = mode->hdisplay * 3;
>>> +		data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
>>> +		break;
>>> +	case MIPI_DSI_FMT_RGB666:
>>> +		val |= (0x4 << 4);
>>> +		hact = mode->hdisplay * 3;
>>> +		data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
>>> +		break;
>>> +
>>> +	case MIPI_DSI_FMT_RGB666_PACKED:
>>> +		val |= (0x4 << 4) | BIT(3);
>>> +		hact = mode->hdisplay * 18 / 8;
>>> +		data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
>>> +		break;
>>> +
>>> +	case MIPI_DSI_FMT_RGB565:
>>> +		val |= (0x5 << 4);
>>> +		hact = mode->hdisplay * 2;
>>> +		data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
>>> +		break;
>>> +	default:
>>> +		dev_err(priv->dev, "Invalid data format (%u)\n",
>>> +			dsi_dev->format);
>>> +		return;
>>> +	}
>>> +
>>> +	tc358768_write(priv, TC358768_DATAFMT, val);
>>> +	tc358768_write(priv, TC358768_DSITX_DT, data_type);
>>> +
>>> +	/* Enable D-PHY (HiZ->LP11) */
>>> +	tc358768_write(priv, TC358768_CLW_CNTRL, 0x0000);
>>> +	/* Enable lanes */
>>> +	for (i = 0; i < dsi_dev->lanes; i++)
>>> +		tc358768_write(priv, TC358768_D0W_CNTRL + i * 4, 0x0000);
>>> +
>>> +	/* DSI Timings */
>>> +	dsibclk_nsk = (u32)div_u64((u64)1000000000 * TC358768_PRECISION,
>>> +				  dsibclk);
>>> +	dsiclk_nsk = (u32)div_u64((u64)1000000000 * TC358768_PRECISION, dsiclk);
>>> +	ui_nsk = dsiclk_nsk / 2;
>>> +	phy_delay_nsk = dsibclk_nsk + 2 * dsiclk_nsk;
>>> +	dev_dbg(priv->dev, "dsiclk_nsk: %u\n", dsiclk_nsk);
>>> +	dev_dbg(priv->dev, "ui_nsk: %u\n", ui_nsk);
>>> +	dev_dbg(priv->dev, "dsibclk_nsk: %u\n", dsibclk_nsk);
>>> +	dev_dbg(priv->dev, "phy_delay_nsk: %u\n", phy_delay_nsk);
>>> +
>>> +	/* LP11 > 100us for D-PHY Rx Init */
>>> +	val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1;
>>> +	dev_dbg(priv->dev, "LINEINITCNT: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_LINEINITCNT, val);
>>> +
>>> +	/* LPTimeCnt > 50ns */
>>> +	val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1;
>>> +	lptxcnt = val;
>>> +	dev_dbg(priv->dev, "LPTXTIMECNT: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_LPTXTIMECNT, val);
>>> +
>>> +	/* 38ns < TCLK_PREPARE < 95ns */
>>> +	val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1;
>>> +	/* TCLK_PREPARE > 300ns */
>>> +	val2 = tc358768_ns_to_cnt(300 + tc358768_to_ns(3 * ui_nsk),
>>> +				  dsibclk_nsk);
>>> +	val |= (val2 - tc358768_to_ns(phy_delay_nsk - dsibclk_nsk)) << 8;
>>> +	dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_TCLK_HEADERCNT, val);
>>> +
>>> +	/* TCLK_TRAIL > 60ns + 3*UI */
>>> +	val = 60 + tc358768_to_ns(3 * ui_nsk);
>>> +	val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 5;
>>> +	dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_TCLK_TRAILCNT, val);
>>> +
>>> +	/* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */
>>> +	val = 50 + tc358768_to_ns(4 * ui_nsk);
>>> +	val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
>>> +	/* THS_ZERO > 145ns + 10*UI */
>>> +	val2 = tc358768_ns_to_cnt(145 - tc358768_to_ns(ui_nsk), dsibclk_nsk);
>>> +	val |= (val2 - tc358768_to_ns(phy_delay_nsk)) << 8;
>>> +	dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_THS_HEADERCNT, val);
>>> +
>>> +	/* TWAKEUP > 1ms in lptxcnt steps */
>>> +	val = tc358768_ns_to_cnt(1020000, dsibclk_nsk);
>>> +	val = val / (lptxcnt + 1) - 1;
>>> +	dev_dbg(priv->dev, "TWAKEUP: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_TWAKEUP, val);
>>> +
>>> +	/* TCLK_POSTCNT > 60ns + 52*UI */
>>> +	val = tc358768_ns_to_cnt(60 + tc358768_to_ns(52 * ui_nsk),
>>> +				 dsibclk_nsk) - 3;
>>> +	dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_TCLK_POSTCNT, val);
>>> +
>>> +	/* 60ns + 4*UI < THS_PREPARE < 105ns + 12*UI */
>>> +	val = tc358768_ns_to_cnt(60 + tc358768_to_ns(15 * ui_nsk),
>>> +				 dsibclk_nsk) - 5;
>>> +	dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_THS_TRAILCNT, val);
>>> +
>>> +	val = BIT(0);
>>> +	for (i = 0; i < dsi_dev->lanes; i++)
>>> +		val |= BIT(i + 1);
>>> +	tc358768_write(priv, TC358768_HSTXVREGEN, val);
>>> +
>>> +	if (!(dsi_dev->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS))
>>> +		tc358768_write(priv, TC358768_TXOPTIONCNTRL, 0x1);
>>> +
>>> +	/* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
>>> +	val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
>>> +	val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
>>> +	val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk),
>>> +				  dsibclk_nsk) - 2;
>>> +	val |= val2 << 16;
>>> +	dev_dbg(priv->dev, "BTACNTRL1: 0x%x\n", val);
>>> +	tc358768_write(priv, TC358768_BTACNTRL1, val);
>>> +
>>> +	/* START[0] */
>>> +	tc358768_write(priv, TC358768_STARTCNTRL, 1);
>>> +
>>> +	/* Set event mode */
>>> +	tc358768_write(priv, TC358768_DSI_EVENT, 1);
>>> +
>>> +	/* vsw (+ vbp) */
>>> +	tc358768_write(priv, TC358768_DSI_VSW,
>>> +		       mode->vtotal - mode->vsync_start);
>>> +	/* vbp (not used in event mode) */
>>> +	tc358768_write(priv, TC358768_DSI_VBPR, 0);
>>> +	/* vact */
>>> +	tc358768_write(priv, TC358768_DSI_VACT, mode->vdisplay);
>>> +
>>> +	/* (hsw + hbp) * byteclk * ndl / pclk */
>>> +	val = (u32)div_u64((mode->htotal - mode->hsync_start) *
>>> +			   ((u64)priv->dsiclk / 4) * priv->dsi_lanes,
>>> +			   mode->clock * 1000);
>>> +	tc358768_write(priv, TC358768_DSI_HSW, val);
>>> +	/* hbp (not used in event mode) */
>>> +	tc358768_write(priv, TC358768_DSI_HBPR, 0);
>>> +	/* hact (bytes) */
>>> +	tc358768_write(priv, TC358768_DSI_HACT, hact);
>>> +
>>> +	/* VSYNC polarity */
>>> +	if (!(mode->flags & DRM_MODE_FLAG_NVSYNC))
>>> +		tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5), BIT(5));
>>> +	/* HSYNC polarity */
>>> +	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
>>> +		tc358768_update_bits(priv, TC358768_PP_MISC, BIT(0), BIT(0));
>>> +
>>> +	/* Start DSI Tx */
>>> +	tc358768_write(priv, TC358768_DSI_START, 0x1);
>>> +
>>> +	/* Configure DSI_Control register */
>>> +	val = (6 << 29) | (0x3 << 24); /* CLEAR, DSI_Control */
>>> +	val |= BIT(7) | BIT(5) | 0x3 << 1 | BIT(0);
>>> +	tc358768_write(priv, TC358768_DSI_CONFW, val);
>>> +
>>> +	val = (5 << 29) | (0x3 << 24); /* SET, DSI_Control */
>>> +	if (!(dsi_dev->mode_flags & MIPI_DSI_MODE_LPM))
>>> +		val |= BIT(7);
>>> +	if (!(dsi_dev->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS))
>>> +		val |= BIT(5);
>>> +	val |= (dsi_dev->lanes - 1) << 1;
>>> +	if (dsi_dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET)
>>> +		val |= BIT(0);
>>> +	tc358768_write(priv, TC358768_DSI_CONFW, val);
>>> +
>>> +	val = (6 << 29) | (0x3 << 24); /* CLEAR, DSI_Control */
>>> +	val |= 0x8000; /* DSI mode */
>>> +	tc358768_write(priv, TC358768_DSI_CONFW, val);
>>> +
>>> +	/* clear FrmStop and RstPtr */
>>> +	tc358768_update_bits(priv, TC358768_PP_MISC, 0x3 << 14, 0);
>>> +
>>> +	/* set PP_en */
>>> +	tc358768_update_bits(priv, TC358768_CONFCTL, BIT(6), BIT(6));
>>> +}
>>> +
>>> +static const struct drm_bridge_funcs tc358768_bridge_funcs = {
>>> +	.attach = tc358768_bridge_attach,
>>> +	.mode_valid = tc358768_bridge_mode_valid,
>>> +	.disable = tc358768_bridge_disable,
>>> +	.enable = tc358768_bridge_enable,
>>> +};
>>> +
>>> +static const struct drm_bridge_timings default_tc358768_timings = {
>>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE
>>> +		 | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE
>>> +		 | DRM_BUS_FLAG_DE_HIGH,
>>> +};
>>> +
>>> +static bool tc358768_is_reserved_reg(unsigned int reg)
>>> +{
>>> +	switch (reg) {
>>> +	case 0x114 ... 0x13f:
>>> +	case 0x200:
>>> +	case 0x20c:
>>> +	case 0x400 ... 0x408:
>>> +	case 0x41c ... 0x42f:
>>> +		return true;
>>> +	default:
>>> +		return false;
>>> +	}
>>> +}
>>> +
>>> +static bool tc358768_writeable_reg(struct device *dev, unsigned int reg)
>>> +{
>>> +	if (tc358768_is_reserved_reg(reg))
>>> +		return false;
>>> +
>>> +	switch (reg) {
>>> +	case TC358768_CHIPID:
>>> +	case TC358768_FIFOSTATUS:
>>> +	case TC358768_DSITXSTATUS ... (TC358768_DSITXSTATUS + 2):
>>> +	case TC358768_DSI_CONTROL ... (TC358768_DSI_INT_ENA + 2):
>>> +	case TC358768_DSICMD_RDFIFO ... (TC358768_DSI_ERR_HALT + 2):
>>> +		return false;
>>> +	default:
>>> +		return true;
>>> +	}
>>> +}
>>> +
>>> +static bool tc358768_readable_reg(struct device *dev, unsigned int reg)
>>> +{
>>> +	if (tc358768_is_reserved_reg(reg))
>>> +		return false;
>>> +
>>> +	switch (reg) {
>>> +	case TC358768_STARTCNTRL:
>>> +	case TC358768_DSI_CONFW ... (TC358768_DSI_CONFW + 2):
>>> +	case TC358768_DSI_INT_CLR ... (TC358768_DSI_INT_CLR + 2):
>>> +	case TC358768_DSI_START ... (TC358768_DSI_START + 2):
>>> +	case TC358768_DBG_DATA:
>>> +		return false;
>>> +	default:
>>> +		return true;
>>> +	}
>>> +}
>>> +
>>> +static bool tc358768_volatile_reg(struct device *dev, unsigned int reg)
>>> +{
>>> +	switch (reg) {
>>> +	case TC358768_FIFOSTATUS:
>>> +	case TC358768_STARTCNTRL ... (TC358768_DSITXSTATUS + 2):
>>> +	case TC358768_DSI_CONTROL ... (TC358768_DSI_INT_ENA + 2):
>>> +	case TC358768_DSICMD_RDFIFO ... (TC358768_DSI_ERR_HALT + 2):
>>> +	case TC358768_DSICMD_TX:
>>> +		return true;
>>> +	default:
>>> +		return false;
>>> +	}
>>> +}
>>> +
>>> +static const struct regmap_config tc358768_regmap_config = {
>>> +	.name = "tc358768",
>>> +	.reg_bits = 16,
>>> +	.val_bits = 16,
>>> +	.max_register = TC358768_DSI_HACT,
>>> +	.cache_type = REGCACHE_RBTREE,
>>> +	.writeable_reg = tc358768_writeable_reg,
>>> +	.readable_reg = tc358768_readable_reg,
>>> +	.volatile_reg = tc358768_volatile_reg,
>>> +	.reg_format_endian = REGMAP_ENDIAN_BIG,
>>> +	.val_format_endian = REGMAP_ENDIAN_BIG,
>>> +};
>>> +
>>> +static const struct i2c_device_id tc358768_i2c_ids[] = {
>>> +	{ "tc358768", 0 },
>>> +	{ "tc358778", 0 },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, tc358768_i2c_ids);
>>> +
>>> +static const struct of_device_id tc358768_of_ids[] = {
>>> +	{ .compatible = "toshiba,tc358768", },
>>> +	{ .compatible = "toshiba,tc358778", },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, tc358768_of_ids);
>>> +
>>> +static int tc358768_get_regulators(struct tc358768_priv *priv)
>>> +{
>>> +	int i, ret;
>>> +
>>> +	for (i = 0; i < ARRAY_SIZE(priv->supplies); ++i)
>>> +		priv->supplies[i].supply = tc358768_supplies[i];
>>> +
>>> +	ret = devm_regulator_bulk_get(priv->dev, ARRAY_SIZE(priv->supplies),
>>> +				      priv->supplies);
>>> +	if (ret < 0)
>>> +		dev_err(priv->dev, "failed to get regulators: %d\n", ret);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int tc358768_i2c_probe(struct i2c_client *client,
>>> +			      const struct i2c_device_id *id)
>>> +{
>>> +	struct tc358768_priv *priv;
>>> +	struct device *dev = &client->dev;
>>> +	struct device_node *np = dev->of_node;
>>> +	int ret;
>>> +
>>> +	if (!np)
>>> +		return -ENODEV;
>>> +
>>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>> +	if (!priv)
>>> +		return -ENOMEM;
>>> +
>>> +	dev_set_drvdata(dev, priv);
>>> +	priv->dev = dev;
>>> +
>>> +	ret = tc358768_get_regulators(priv);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	priv->refclk = devm_clk_get(dev, "refclk");
>>> +	if (IS_ERR(priv->refclk))
>>> +		return PTR_ERR(priv->refclk);
>>> +
>>> +	/*
>>> +	 * RESX is low active, to disable tc358768 initially (keep in reset)
>>> +	 * the gpio line must be LOW. This is the ASSERTED state of
>>> +	 * GPIO_ACTIVE_LOW (GPIOD_OUT_HIGH == ASSERTED).
>>> +	 */
>>> +	priv->reset_gpio  = devm_gpiod_get_optional(dev, "reset",
>>> +						    GPIOD_OUT_HIGH);
>>> +	if (IS_ERR(priv->reset_gpio))
>>> +		return PTR_ERR(priv->reset_gpio);
>>> +
>>> +	priv->regmap = devm_regmap_init_i2c(client, &tc358768_regmap_config);
>>> +	if (IS_ERR(priv->regmap)) {
>>> +		dev_err(dev, "Failed to init regmap\n");
>>> +		return PTR_ERR(priv->regmap);
>>> +	}
>>> +
>>> +	if (priv->reset_gpio)
>>> +		regcache_cache_only(priv->regmap, true);
>>> +
>>> +	priv->dsi_host.dev = dev;
>>> +	priv->dsi_host.ops = &tc358768_dsi_host_ops;
>>> +
>>> +	priv->bridge.funcs = &tc358768_bridge_funcs;
>>> +	priv->bridge.timings = &default_tc358768_timings;
>>> +	priv->bridge.of_node = np;
>>> +
>>> +	i2c_set_clientdata(client, priv);
>>> +
>>> +	ret = mipi_dsi_host_register(&priv->dsi_host);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int tc358768_i2c_remove(struct i2c_client *client)
>>> +{
>>> +	struct tc358768_priv *priv = i2c_get_clientdata(client);
>>> +
>>> +	mipi_dsi_host_unregister(&priv->dsi_host);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct i2c_driver tc358768_driver = {
>>> +	.driver = {
>>> +		.name = "tc358768",
>>> +		.of_match_table = tc358768_of_ids,
>>> +	},
>>> +	.id_table = tc358768_i2c_ids,
>>> +	.probe = tc358768_i2c_probe,
>>> +	.remove	= tc358768_i2c_remove,
>>> +};
>>> +module_i2c_driver(tc358768_driver);
>>> +
>>> +MODULE_AUTHOR("Peter Ujfalusi <peter.ujfalusi@xxxxxx>");
>>> +MODULE_DESCRIPTION("TC358768AXBG/TC358778XBG DSI bridge");
>>> +MODULE_LICENSE("GPL v2");
>>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux