Re: [U-Boot] [PATCH 5/7] EXYNOS: support EXYNOS MIPI DSI interface driver.

Hi,

Please see minor comments below.

On Thu, 05 Apr 2012 15:29:29 +0900 Donghwa Lee dh09.lee@samsung.com wrote:

EXYNOS SoC platform has MIPI-DSI controller and MIPI-DSI based LCD Panel could be used with it. This patch supports MIPI-DSI driver based Samsung SoC chip.

LCD panel driver based MIPI-DSI should be registered to MIPI-DSI driver at board file and LCD panel driver specific function registered to mipi_dsim_ddi structure at lcd panel init function called system init. In the MIPI-DSI driver, find lcd panel driver by using registered lcd panel name, and then initialize lcd panel driver.

Signed-off-by: Donghwa Lee dh09.lee@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com Signed-off-by: Inki Dae inki.dae@samsung.com

---

arch/arm/include/asm/arch-exynos/dsim.h | 181 +++++++ arch/arm/include/asm/arch-exynos/mipi_dsim.h | 400 +++++++++++++++ drivers/video/exynos_mipi_dsi.c | 257 ++++++++++ drivers/video/exynos_mipi_dsi_common.c | 645 +++++++++++++++++++++++++ drivers/video/exynos_mipi_dsi_common.h | 48 ++ drivers/video/exynos_mipi_dsi_lowlevel.c | 669 ++++++++++++++++++++++++++ drivers/video/exynos_mipi_dsi_lowlevel.h | 111 +++++ 7 files changed, 2311 insertions(+), 0 deletions(-) create mode 100644 arch/arm/include/asm/arch-exynos/dsim.h create mode 100644 arch/arm/include/asm/arch-exynos/mipi_dsim.h create mode 100644 drivers/video/exynos_mipi_dsi.c create mode 100644 drivers/video/exynos_mipi_dsi_common.c create mode 100644 drivers/video/exynos_mipi_dsi_common.h create mode 100644 drivers/video/exynos_mipi_dsi_lowlevel.c create mode 100644 drivers/video/exynos_mipi_dsi_lowlevel.h

...

diff --git a/arch/arm/include/asm/arch-exynos/mipi_dsim.h b/arch/arm/include/asm/arch-exynos/mipi_dsim.h new file mode 100644 index 0000000..4ae6830 --- /dev/null +++ b/arch/arm/include/asm/arch-exynos/mipi_dsim.h

...

+/*

• • struct mipi_dsim_device - global interface for mipi-dsi driver.
• • @dev: driver model representation of the device.
• • @id: unique device id.
• • @clock: pointer to MIPI-DSI clock of clock framework.
• • @irq: interrupt number to MIPI-DSI controller.
• • @reg_base: base address to memory mapped SRF of MIPI-DSI controller.
• • (virtual address)
• • @lock: the mutex protecting this data structure.

These above documented structure members are not in the below struct for U-Boot, so please remove their description.

• • @dsim_info: infomation for configuring mipi-dsi controller.

The member in the struct is named dsim_config, please update this description accordingly.

• • @master_ops: callbacks to mipi-dsi operations.
• • @dsim_lcd_dev: pointer to activated ddi device.
• • (it would be registered by mipi-dsi driver.)
• • @dsim_lcd_drv: pointer to activated_ddi driver.
• • (it would be registered by mipi-dsi driver.)
• • @lcd_info: pointer to mipi_lcd_info structure.

Drop @lcd_info description.

• • @state: specifies status of MIPI-DSI controller.
• • the status could be RESET, INIT, STOP, HSCLKEN and ULPS.
• • @resume_complete: indicates whether resume operation is completed or not.

Drop @resume_complete.

• • @data_lane: specifiec enabled data lane number.
• • this variable would be set by driver according to e_no_data_lane
• • automatically.
• • @e_clk_src: select byte clock source.
• • @pd: pointer to MIPI-DSI driver platform data.
• */

+struct mipi_dsim_device {

• struct mipi_dsim_config *dsim_config;
• struct mipi_dsim_master_ops *master_ops;
• struct mipi_dsim_lcd_device *dsim_lcd_dev;
• struct mipi_dsim_lcd_driver *dsim_lcd_drv;
• unsigned int state;
• unsigned int data_lane;
• enum mipi_dsim_byte_clk_src e_clk_src;
• struct exynos_platform_mipi_dsim *pd;

+};

+/*

• • struct exynos_platform_mipi_dsim - interface to platform data
• • for mipi-dsi driver.
• • @lcd_panel_name: specifies lcd panel name registered to mipi-dsi driver.
• • lcd panel driver searched would be actived.
• • @dsim_config: pointer of structure for configuring mipi-dsi controller.
• • @lcd_panel_info: pointer for lcd panel specific structure.
• • this structure specifies width, height, timing and polarity and so on.
• • @mipi_power: callback pointer for enabling or disabling mipi power.
• • @phy_enable: pointer to a callback controlling D-PHY enable/reset

lcd_power from below struct is not in the description above.

• */

+struct exynos_platform_mipi_dsim {

• char lcd_panel_name[PANEL_NAME_SIZE];
• struct mipi_dsim_config *dsim_config;
• void *lcd_panel_info;
• int (*lcd_power)(void);
• int (*mipi_power)(void);
• void (*phy_enable)(unsigned int enable, unsigned int dev_index);

+};

Please add an empty line here.

+/*

• • struct mipi_dsim_master_ops - callbacks to mipi-dsi operations.
• • @cmd_write: transfer command to lcd panel at LP mode.
• • @cmd_read: read command from rx register.
• • @get_dsim_frame_done: get the status that all screen data have been
• • transferred to mipi-dsi.
• • @clear_dsim_frame_done: clear frame done status.
• • @get_fb_frame_done: get frame done status of display controller.
• • @trigger: trigger display controller.
• • • this one would be used only in case of CPU mode.
• */

and drop this empty line here.

+struct mipi_dsim_master_ops {

• int (*cmd_write)(struct mipi_dsim_device *dsim, unsigned int data_id,

• unsigned int data0, unsigned int data1);
  

• int (*cmd_read)(struct mipi_dsim_device *dsim, unsigned int data_id,

• unsigned int data0, unsigned int data1);
  

• int (*get_dsim_frame_done)(struct mipi_dsim_device *dsim);
• int (*clear_dsim_frame_done)(struct mipi_dsim_device *dsim);
• int (*get_fb_frame_done)(void);
• void (*trigger)(struct fb_info *info);

+};

+/*

• • device structure for mipi-dsi based lcd panel.
• • @name: name of the device to use with this device, or an
• • alias for that name.
• • @dev: driver model representation of the device.

drop @dev description.

• • @id: id of device to be registered.
• • @bus_id: bus id for identifing connected bus
• • and this bus id should be same as id of mipi_dsim_device.
• • @irq: irq number for signaling when framebuffer transfer of
• • lcd panel module is completed.
• • this irq would be used only for MIPI-DSI based CPU mode lcd panel.

drop @irq description.

• • @master: pointer to mipi-dsi master device object.
• • @platform_data: lcd panel specific platform data.
• */

+struct mipi_dsim_lcd_device {

• char *name;
• char *panel_id;

This panel_id seems to be not used anywhere? Drop it?

• int panel_type;
• int id;
• int bus_id;
• int reverse;
• struct mipi_dsim_device *master;
• void *platform_data;

+};

+/*

• • driver structure for mipi-dsi based lcd panel.
• • this structure should be registered by lcd panel driver.
• • mipi-dsi driver seeks lcd panel registered through name field
• • and calls these callback functions in appropriate time.
• • @name: name of the driver to use with this device, or an
• • alias for that name.
• • @id: id of driver to be registered.
• • this id would be used for finding device object registered.
• */

+struct mipi_dsim_lcd_driver {

• char *name;
• int id;
• int (*mipi_panel_init)(struct mipi_dsim_device *dsim_dev);
• void (*mipi_display_on)(struct mipi_dsim_device *dsim_dev);

Only 'name' and 'id' is documented above. Document these callbacks, too.

...

+/*

• • exynos_dsim_phy_enable - global MIPI-DSI receiver D-PHY control
• • @pdev: MIPI-DSIM platform device

Drop @pdev description.

• • @on: true to enable D-PHY and deassert its reset
• • false to disable D-PHY
• */

+int exynos_dsim_phy_enable(int on);

...

diff --git a/drivers/video/exynos_mipi_dsi.c b/drivers/video/exynos_mipi_dsi.c new file mode 100644 index 0000000..704f6a7 --- /dev/null +++ b/drivers/video/exynos_mipi_dsi.c @@ -0,0 +1,257 @@ +/*

• • Copyright (C) 2012 Samsung Electronics
• • Author: InKi Dae inki.dae@samsung.com
• • Author: Donghwa Lee dh09.lee@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; if not, write to the Free Software
• • Foundation, Inc., 59 Temple Place, Suite 330, Boston,
• • MA 02111-1307 USA
• */

+#include <ubi_uboot.h> +#include <common.h> +#include <lcd.h> +#include <asm/errno.h>

Please drop ubi_uboot.h, lcd.h and errno.h here and better use

#include <common.h> #include <malloc.h> #include <linux/err.h>

+#include <asm/arch/dsim.h> +#include <asm/arch/mipi_dsim.h> +#include <asm/arch/power.h> +#include <asm/arch/cpu.h> +#include <asm/arch/clk.h> +#include <linux/types.h>

types.h can be dropped too, I think.

+#include <linux/list.h>

+#include "exynos_mipi_dsi_lowlevel.h" +#include "exynos_mipi_dsi_common.h"

+#define master_to_driver(a) (a->dsim_lcd_drv) +#define master_to_device(a) (a->dsim_lcd_dev)

+static struct exynos_platform_mipi_dsim *dsim_pd;

+struct mipi_dsim_ddi {

• int bus_id;
• struct list_head list;
• struct mipi_dsim_lcd_device *dsim_lcd_dev;
• struct mipi_dsim_lcd_driver *dsim_lcd_drv;

+};

+static LIST_HEAD(dsim_ddi_list); +static LIST_HEAD(dsim_lcd_dev_list);

+int exynos_mipi_dsi_register_lcd_device(struct mipi_dsim_lcd_device *lcd_dev) +{

• struct mipi_dsim_ddi *dsim_ddi;
• if (!lcd_dev) {

• debug("mipi_dsim_lcd_device is NULL.\n");
  

• return -EFAULT;
  

• }
• if (!lcd_dev->name) {

• debug("dsim_lcd_device name is NULL.\n");
  

• return -EFAULT;
  

• }
• dsim_ddi = kzalloc(sizeof(struct mipi_dsim_ddi), GFP_KERNEL);
• if (!dsim_ddi) {

• debug("failed to allocate dsim_ddi object.\n");
  

• return -EFAULT;
  

• }
• dsim_ddi->dsim_lcd_dev = lcd_dev;
• list_add_tail(&dsim_ddi->list, &dsim_ddi_list);
• return 0;

+}

+struct mipi_dsim_ddi

• *exynos_mipi_dsi_find_lcd_device(struct mipi_dsim_lcd_driver *lcd_drv)

+{

• struct mipi_dsim_ddi *dsim_ddi;
• struct mipi_dsim_lcd_device *lcd_dev;
• list_for_each_entry(dsim_ddi, &dsim_ddi_list, list) {

• lcd_dev = dsim_ddi->dsim_lcd_dev;
  

• if (!lcd_dev)
  

• 	continue;
  

• if (lcd_drv->id >= 0) {
  

• 	if ((strcmp(lcd_drv->name, lcd_dev->name)) == 0 &&
  

• 			lcd_drv->id == lcd_dev->id) {
  

• 		/**
  

• 		 * bus_id would be used to identify
  

• 		 * connected bus.
  

• 		 */
  

• 		dsim_ddi->bus_id = lcd_dev->bus_id;
  

• 		return dsim_ddi;
  

• 	}
  

• } else {
  

• 	if ((strcmp(lcd_drv->name, lcd_dev->name)) == 0) {
  

• 		/**
  

• 		 * bus_id would be used to identify
  

• 		 * connected bus.
  

• 		 */
  

• 		dsim_ddi->bus_id = lcd_dev->bus_id;
  

• 		return dsim_ddi;
  

• 	}
  

• }
  

• kfree(dsim_ddi);
  

• list_del(&dsim_ddi_list);
  

• }
• return NULL;

+}

+int exynos_mipi_dsi_register_lcd_driver(struct mipi_dsim_lcd_driver *lcd_drv) +{

• struct mipi_dsim_ddi *dsim_ddi;
• if (!lcd_drv) {

• debug("mipi_dsim_lcd_driver is NULL.\n");
  

• return -EFAULT;
  

• }
• if (!lcd_drv->name) {

• debug("dsim_lcd_driver name is NULL.\n");
  

• return -EFAULT;
  

• }
• dsim_ddi = exynos_mipi_dsi_find_lcd_device(lcd_drv);
• if (!dsim_ddi) {

• debug("mipi_dsim_ddi object not found.\n");
  

• return -EFAULT;
  

• }
• dsim_ddi->dsim_lcd_drv = lcd_drv;
• debug("registered panel driver(%s) to mipi-dsi driver.\n",

• lcd_drv->name);
  

• return 0;

+}

+struct mipi_dsim_ddi

• *exynos_mipi_dsi_bind_lcd_ddi(struct mipi_dsim_device *dsim,

• 	const char *name)
  

+{

• struct mipi_dsim_ddi *dsim_ddi;
• struct mipi_dsim_lcd_driver *lcd_drv;
• struct mipi_dsim_lcd_device *lcd_dev;
• list_for_each_entry(dsim_ddi, &dsim_ddi_list, list) {

• lcd_drv = dsim_ddi->dsim_lcd_drv;
  

• lcd_dev = dsim_ddi->dsim_lcd_dev;
  

• if (!lcd_drv || !lcd_dev)
  

• 		continue;
  

Please remove one tab before 'contunue'.

• debug("lcd_drv->id = %d, lcd_dev->id = %d\n",
  

• 			lcd_drv->id, lcd_dev->id);
  

• if ((strcmp(lcd_drv->name, name) == 0)) {
  

• 	lcd_dev->master = dsim;
  

• 	dsim->dsim_lcd_dev = lcd_dev;
  

• 	dsim->dsim_lcd_drv = lcd_drv;
  

• 	return dsim_ddi;
  

• }
  

• }
• return NULL;

+}

+/* define MIPI-DSI Master operations. */ +static struct mipi_dsim_master_ops master_ops = {

• .cmd_write = exynos_mipi_dsi_wr_data,
• .get_dsim_frame_done = exynos_mipi_dsi_get_frame_done_status,
• .clear_dsim_frame_done = exynos_mipi_dsi_clear_frame_done,

+};

+int exynos_mipi_dsi_init(void) +{

• struct mipi_dsim_device *dsim;
• struct mipi_dsim_config *dsim_config;
• struct mipi_dsim_ddi *dsim_ddi;
• dsim = kzalloc(sizeof(struct mipi_dsim_device), GFP_KERNEL);
• if (!dsim) {

• debug("failed to allocate dsim object.\n");
  

• return -EFAULT;
  

• }
• /* get mipi_dsim_config. */
• dsim_config = dsim_pd->dsim_config;
• if (dsim_config == NULL) {

• debug("failed to get dsim config data.\n");
  

• return -EFAULT;
  

• }
• dsim->pd = dsim_pd;
• dsim->dsim_config = dsim_config;
• dsim->master_ops = &master_ops;

Please remove one empty line.

• /* bind lcd ddi matched with panel name. */
• dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim, dsim_pd->lcd_panel_name);
• if (!dsim_ddi) {

• debug("mipi_dsim_ddi object not found.\n");
  

• return -ENOSYS;
  

• }

...

diff --git a/drivers/video/exynos_mipi_dsi_common.c b/drivers/video/exynos_mipi_dsi_common.c new file mode 100644 index 0000000..08611b6 --- /dev/null +++ b/drivers/video/exynos_mipi_dsi_common.c

...

+static unsigned int dpll_table[15] = {

• 100, 120, 170, 220, 270,
• 320, 390, 450, 510, 560,
• 640, 690, 770, 870, 950 };

Please put '};' to the next line.

+static void exynos_mipi_dsi_long_data_wr(struct mipi_dsim_device *dsim,

• unsigned int data0, unsigned int data1)
  

+{

• unsigned int data_cnt = 0, payload = 0;
• /* in case that data count is more then 4 */
• for (data_cnt = 0; data_cnt < data1; data_cnt += 4) {

• /*
  

•  * after sending 4bytes per one time,
  

•  * send remainder data less then 4.
  

•  */
  

• if ((data1 - data_cnt) < 4) {
  

• 	if ((data1 - data_cnt) == 3) {
  

• 		payload = *(u8 *)(data0 + data_cnt) |
  

• 		    (*(u8 *)(data0 + (data_cnt + 1))) << 8 |
  

Please indent by tab in the above line, it will fit into 80 chars limit.

• 			(*(u8 *)(data0 + (data_cnt + 2))) << 16;
  

• 	debug("count = 3 payload = %x, %x %x %x\n",
  

• 		payload, *(u8 *)(data0 + data_cnt),
  

• 		*(u8 *)(data0 + (data_cnt + 1)),
  

• 		*(u8 *)(data0 + (data_cnt + 2)));
  

• 	} else if ((data1 - data_cnt) == 2) {
  

• 		payload = *(u8 *)(data0 + data_cnt) |
  

• 			(*(u8 *)(data0 + (data_cnt + 1))) << 8;
  

• 	debug("count = 2 payload = %x, %x %x\n", payload,
  

• 		*(u8 *)(data0 + data_cnt),
  

• 		*(u8 *)(data0 + (data_cnt + 1)));
  

• 	} else if ((data1 - data_cnt) == 1) {
  

• 		payload = *(u8 *)(data0 + data_cnt);
  

• 	}
  

• 	exynos_mipi_dsi_wr_tx_data(dsim, payload);
  

• /* send 4bytes per one time. */
  

• } else {
  

• 	payload = *(u8 *)(data0 + data_cnt) |
  

• 		(*(u8 *)(data0 + (data_cnt + 1))) << 8 |
  

• 		(*(u8 *)(data0 + (data_cnt + 2))) << 16 |
  

• 		(*(u8 *)(data0 + (data_cnt + 3))) << 24;
  

• 	debug("count = 4 payload = %x, %x %x %x %x\n",
  

• 		payload, *(u8 *)(data0 + data_cnt),
  

• 		*(u8 *)(data0 + (data_cnt + 1)),
  

• 		*(u8 *)(data0 + (data_cnt + 2)),
  

• 		*(u8 *)(data0 + (data_cnt + 3)));
  

• 	exynos_mipi_dsi_wr_tx_data(dsim, payload);
  

• }
  

exynos_mipi_dsi_wr_tx_data() is called in both above cases, so you can simplify to call exynos_mipi_dsi_wr_tx_data() only here.

• }

+}

+int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id,

• unsigned int data0, unsigned int data1)

+{

• unsigned int timeout = TRY_GET_FIFO_TIMEOUT;
• unsigned long delay_val, delay;
• unsigned int check_rx_ack = 0;
• if (dsim->state == DSIM_STATE_ULPS) {

• debug("state is ULPS.\n");
  

• return -EINVAL;
  

• }
• delay_val = MHZ / dsim->dsim_config->esc_clk;
• delay = 10 * delay_val;
• udelay(delay * 1000);

we have a common mdelay(), so please use

mdelay(delay);

• /* only if transfer mode is LPDT, wait SFR becomes empty. */
• if (dsim->state == DSIM_STATE_STOP) {

• while (!(exynos_mipi_dsi_get_fifo_state(dsim) &
  

• 		SFR_HEADER_EMPTY)) {
  

• 	if ((timeout--) > 0)
  

• 		udelay(1000);
  

• 	else {
  

• 		debug("SRF header fifo is not empty.\n");
  

• 		return -EINVAL;
  

• 	}
  

• }
  

• }
• switch (data_id) {
• /* short packet types of packet types for command. */
• case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM:
• case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM:
• case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
• case MIPI_DSI_DCS_SHORT_WRITE:
• case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
• case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE:

• debug("data0 = %x data1 = %x\n",
  

• 		data0, data1);
  

• exynos_mipi_dsi_wr_tx_header(dsim, data_id, data0, data1);
  

• if (check_rx_ack)
  

• 	/* process response func should be implemented */
  

• 	return 0;
  

• else
  

• 	return -EINVAL;
  

Please use following style for multiline if statements

if (check_rx_ack) { /* process response func should be implemented */ return 0; } else { return -EINVAL; }

• /* general command */
• case MIPI_DSI_COLOR_MODE_OFF:
• case MIPI_DSI_COLOR_MODE_ON:
• case MIPI_DSI_SHUTDOWN_PERIPHERAL:
• case MIPI_DSI_TURN_ON_PERIPHERAL:

• exynos_mipi_dsi_wr_tx_header(dsim, data_id, data0, data1);
  

• if (check_rx_ack)
  

• 	/* process response func should be implemented. */
  

• 	return 0;
  

• else
  

• 	return -EINVAL;
  

same applies here. There are other similar statements in this file. Please fix them too.

• /* packet types for video data */
• case MIPI_DSI_V_SYNC_START:
• case MIPI_DSI_V_SYNC_END:
• case MIPI_DSI_H_SYNC_START:
• case MIPI_DSI_H_SYNC_END:
• case MIPI_DSI_END_OF_TRANSMISSION:

• return 0;
  

• /* short and response packet types for command */
• case MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM:
• case MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM:
• case MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM:
• case MIPI_DSI_DCS_READ:

• exynos_mipi_dsi_clear_all_interrupt(dsim);
  

• exynos_mipi_dsi_wr_tx_header(dsim, data_id, data0, data1);
  

• /* process response func should be implemented. */
  

• return 0;
  

• /* long packet type and null packet */
• case MIPI_DSI_NULL_PACKET:
• case MIPI_DSI_BLANKING_PACKET:

• return 0;
  

• case MIPI_DSI_GENERIC_LONG_WRITE:
• case MIPI_DSI_DCS_LONG_WRITE:
• {

• unsigned int size, data_cnt = 0, payload = 0;
  

• size = data1 * 4;
  

The local variable 'size' is not really used here, I get a compiler warning: exynos_mipi_dsi_common.c: In function 'exynos_mipi_dsi_wr_data': exynos_mipi_dsi_common.c:200:16: warning: variable 'size' set but not used [-Wunused-but-set-variable]

So please drop it.

• /* if data count is less then 4, then send 3bytes data.  */
  

• if (data1 < 4) {
  

• 	payload = *(u8 *)(data0) |
  

• 		*(u8 *)(data0 + 1) << 8 |
  

• 		*(u8 *)(data0 + 2) << 16;
  

• 	exynos_mipi_dsi_wr_tx_data(dsim, payload);
  

• 	debug("count = %d payload = %x,%x %x %x\n",
  

• 		data1, payload,
  

• 		*(u8 *)(data0 + data_cnt),
  

• 		*(u8 *)(data0 + (data_cnt + 1)),
  

• 		*(u8 *)(data0 + (data_cnt + 2)));
  

• /* in case that data count is more then 4 */
  

• } else
  

• 	exynos_mipi_dsi_long_data_wr(dsim, data0, data1);
  

} else { /* in case that data count is more then 4 */ exynos_mipi_dsi_long_data_wr(dsim, data0, data1); }

• /* put data into header fifo */
  

• exynos_mipi_dsi_wr_tx_header(dsim, data_id, data1 & 0xff,
  

• 	(data1 & 0xff00) >> 8);
  

• }
• if (check_rx_ack)

• /* process response func should be implemented. */
  

• return 0;
  

• else

• return -EINVAL;
  

• /* packet typo for video data */
• case MIPI_DSI_PACKED_PIXEL_STREAM_16:
• case MIPI_DSI_PACKED_PIXEL_STREAM_18:
• case MIPI_DSI_PIXEL_STREAM_3BYTE_18:
• case MIPI_DSI_PACKED_PIXEL_STREAM_24:

• if (check_rx_ack)
  

• 	/* process response func should be implemented. */
  

• 	return 0;
  

• else
  

• 	return -EINVAL;
  

• default:

• debug("data id %x is not supported current DSI spec.\n",
  

• 	data_id);
  

• return -EINVAL;
  

• }
• return 0;

+}

...

+int exynos_mipi_dsi_set_clock(struct mipi_dsim_device *dsim,

• unsigned int byte_clk_sel, unsigned int enable)

+{

• unsigned int esc_div;
• unsigned long esc_clk_error_rate;
• unsigned long hs_clk = 0, byte_clk = 0, escape_clk = 0;
• if (enable) {

• dsim->e_clk_src = byte_clk_sel;
  

• /* Escape mode clock and byte clock source */
  

• exynos_mipi_dsi_set_byte_clock_src(dsim, byte_clk_sel);
  

• /* DPHY, DSIM Link : D-PHY clock out */
  

• if (byte_clk_sel == DSIM_PLL_OUT_DIV8) {
  

• 	hs_clk = exynos_mipi_dsi_change_pll(dsim,
  

• 		dsim->dsim_config->p, dsim->dsim_config->m,
  

• 		dsim->dsim_config->s);
  

• 	if (hs_clk == 0) {
  

• 		debug("failed to get hs clock.\n");
  

• 		return -EINVAL;
  

• 	}
  

• 	byte_clk = hs_clk / 8;
  

• 	exynos_mipi_dsi_enable_pll_bypass(dsim, 0);
  

• 	exynos_mipi_dsi_pll_on(dsim, 1);
  

• /* DPHY : D-PHY clock out, DSIM link : external clock out */
  

• } else if (byte_clk_sel == DSIM_EXT_CLK_DIV8)
  

• 	debug("not support EXT CLK source for MIPI DSIM\n");
  

• else if (byte_clk_sel == DSIM_EXT_CLK_BYPASS)
  

• 	debug("not support EXT CLK source for MIPI DSIM\n");
  

• /* escape clock divider */
  

• esc_div = byte_clk / (dsim->dsim_config->esc_clk);
  

• debug("esc_div = %d, byte_clk = %lu, esc_clk = %lu\n",
  

• 	esc_div, byte_clk, dsim->dsim_config->esc_clk);
  

• if ((byte_clk / esc_div) >= (20 * MHZ) ||
  

• 		(byte_clk / esc_div) >
  

• 			dsim->dsim_config->esc_clk)
  

• 	esc_div += 1;
  

if ((byte_clk / esc_div) >= (20 * MHZ) || (byte_clk / esc_div) > dsim->dsim_config->esc_clk) esc_div += 1;

...

+int exynos_mipi_dsi_init_link(struct mipi_dsim_device *dsim) +{

• unsigned int time_out = 100;
• switch (dsim->state) {
• case DSIM_STATE_INIT:

• exynos_mipi_dsi_init_fifo_pointer(dsim, 0x1f);
  

• /* dsi configuration */
  

• exynos_mipi_dsi_init_config(dsim);
  

• exynos_mipi_dsi_enable_lane(dsim, DSIM_LANE_CLOCK, 1);
  

• exynos_mipi_dsi_enable_lane(dsim, dsim->data_lane, 1);
  

• /* set clock configuration */
  

• exynos_mipi_dsi_set_clock(dsim,
  

• 			dsim->dsim_config->e_byte_clk, 1);
  

• /* check clock and data lane state are stop state */
  

• while (!(exynos_mipi_dsi_is_lane_state(dsim))) {
  

• 	time_out--;
  

• 	if (time_out == 0) {
  

• 		debug("DSI Master is not stop state.\n");
  

• 		debug("Check initialization process\n");
  

• 		return -EINVAL;
  

• 	}
  

• }
  

• if (time_out != 0) {
  

This check is not necessary here since this condition is always true. Please remove it.

• 	debug("DSI Master driver has been completed.\n");
  

• 	debug("DSI Master state is stop state\n");
  

• }
  

• dsim->state = DSIM_STATE_STOP;
  

• /* BTA sequence counters */
  

• exynos_mipi_dsi_set_stop_state_counter(dsim,
  

• 	dsim->dsim_config->stop_holding_cnt);
  

• exynos_mipi_dsi_set_bta_timeout(dsim,
  

• 	dsim->dsim_config->bta_timeout);
  

• exynos_mipi_dsi_set_lpdr_timeout(dsim,
  

• 	dsim->dsim_config->rx_timeout);
  

Please drop empty line.

• return 0;
  

• default:

• debug("DSI Master is already init.\n");
  

• return 0;
  

• }
• return 0;

+}

...

diff --git a/drivers/video/exynos_mipi_dsi_lowlevel.c b/drivers/video/exynos_mipi_dsi_lowlevel.c new file mode 100644 index 0000000..cc52ede --- /dev/null +++ b/drivers/video/exynos_mipi_dsi_lowlevel.c

...

+#include <common.h> +#include <lcd.h> +#include <asm/errno.h>

Please drop lcd.h and asm/errno.h

+#include <asm/arch/dsim.h> +#include <asm/arch/mipi_dsim.h> +#include <asm/arch/power.h> +#include <asm/arch/cpu.h> +#include <linux/types.h> +#include <linux/list.h>

Please drop linux/types.h and linux/list.h

...

+void exynos_mipi_dsi_sw_release(struct mipi_dsim_device *dsim) +{

• struct exynos_mipi_dsim *mipi_dsim =

• (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
  

• unsigned int reg = readl(&mipi_dsim->intsrc);
• reg |= INTSRC_SWRST_RELEASE;
• writel(reg, &mipi_dsim->intsrc);
• reg = 0;
• reg = readl(&mipi_dsim->intsrc);

Please replace the above two lines by

readl(&mipi_dsim->intsrc);

...

+void exynos_mipi_dsi_init_config(struct mipi_dsim_device *dsim) +{

• struct mipi_dsim_config *dsim_config = dsim->dsim_config;
• struct exynos_mipi_dsim *mipi_dsim =

• (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
  

• unsigned int cfg = (readl(&mipi_dsim->config)) &

• ~((1 << DSIM_EOT_PACKET_SHIFT) |
  

• (0x1f << DSIM_HSA_MODE_SHIFT) |
  

• (0x3 << DSIM_NUM_OF_DATALANE_SHIFT));
  

• cfg = (dsim_config->auto_flush << DSIM_AUTO_FLUSH_SHIFT) |

Shouldn't this be cfg |= (dsim_config->auto_flush << DSIM_AUTO_FLUSH_SHIFT) | ?

• (dsim_config->eot_disable << DSIM_EOT_PACKET_SHIFT) |
  

• (dsim_config->auto_vertical_cnt << DSIM_AUTO_MODE_SHIFT) |
  

• (dsim_config->hse << DSIM_HSE_MODE_SHIFT) |
  

• (dsim_config->hfp << DSIM_HFP_MODE_SHIFT) |
  

• (dsim_config->hbp << DSIM_HBP_MODE_SHIFT) |
  

• (dsim_config->hsa << DSIM_HSA_MODE_SHIFT) |
  

• (dsim_config->e_no_data_lane << DSIM_NUM_OF_DATALANE_SHIFT);
  

• writel(cfg, &mipi_dsim->config);

+}

...

+void exynos_mipi_dsi_enable_lane(struct mipi_dsim_device *dsim,

• 	unsigned int lane, unsigned int enable)
  

+{

• unsigned int reg;
• struct exynos_mipi_dsim *mipi_dsim =

• (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
  

• reg = readl(&mipi_dsim->config);
• if (enable)

• reg |= DSIM_LANE_ENx(lane);
  

• else

• reg &= ~DSIM_LANE_ENx(lane);
  

• writel(reg, &mipi_dsim->config);

+}

Please drop one empty line.

...

+void exynos_mipi_dsi_enable_esc_clk_on_lane(struct mipi_dsim_device *dsim,

• unsigned int lane_sel, unsigned int enable)
  

+{

• struct exynos_mipi_dsim *mipi_dsim =

• (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
  

• unsigned int reg = readl(&mipi_dsim->clkctrl);
• if (enable)

• reg |= DSIM_LANE_ESC_CLKEN(lane_sel);
  

• else

Please drop empty line.

• reg &= ~DSIM_LANE_ESC_CLKEN(lane_sel);
  

• writel(reg, &mipi_dsim->clkctrl);

+}

...

+void exynos_mipi_dsi_clear_all_interrupt(struct mipi_dsim_device *dsim) +{

• struct exynos_mipi_dsim *mipi_dsim =

• (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
  

• unsigned int reg = readl(&mipi_dsim->intsrc);
• reg |= 0xffffffff;
• writel(reg, &mipi_dsim->intsrc);

Is reading the intsrc register necessary here? Can't you just do writel(0xffffffff, &mipi_dsim->intsrc); ?

...

+unsigned int exynos_mipi_dsi_get_fifo_state(struct mipi_dsim_device *dsim) +{

• unsigned int ret;
• struct exynos_mipi_dsim *mipi_dsim =

• (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
  

• ret = readl(&mipi_dsim->fifoctrl) & ~(0x1f);
• return ret;

you can eliminate local variable 'ret' here and just do

return readl(&mipi_dsim->fifoctrl) & ~(0x1f);

+}

Thanks, Anatolij