Thread (5 messages) 5 messages, 2 authors, 2013-02-08

Re: [PATCH 1/1] OMAP4: DSS: Add panel for Blaze Tablet boards

From: Andi Shyti <hidden>
Date: 2013-02-07 15:08:47
Also in: linux-omap, lkml

Hi,
TC358765 is DSI-to-LVDS transmitter from Toshiba, used in
OMAP44XX Blaze Tablet and Blaze Tablet2 boards.
I had a really fast look and I have few comments
+static int tc358765_read_register(struct omap_dss_device *dssdev,
+					u16 reg, u32 *val)
+{
+	int ret = 0;
+	pm_runtime_get_sync(&dssdev->dev);
+	/* I2C is preferred way of reading, but fall back to DSI
+	 * if I2C didn't got initialized
+	*/
+	if (tc358765_i2c)
+		ret = tc358765_i2c_read(reg, val);
+	else
+		ret = tc358765_dsi_read(dssdev, reg, val);
+	pm_runtime_put_sync(&dssdev->dev);
+
+	return ret;
+}
+
+static int tc358765_write_register(struct omap_dss_device *dssdev, u16 reg,
+		u32 value)
+{
+	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
+	u8 buf[6];
+	int r;
+
+	buf[0] = (reg >> 0) & 0xff;
+	buf[1] = (reg >> 8) & 0xff;
+	buf[2] = (value >> 0) & 0xff;
+	buf[3] = (value >> 8) & 0xff;
+	buf[4] = (value >> 16) & 0xff;
+	buf[5] = (value >> 24) & 0xff;
+
+	r = dsi_vc_generic_write_nosync(dssdev, d2d->channel1, buf, 6);
+	if (r)
+		dev_err(&dssdev->dev, "reg write reg(%x) val(%x) failed: %d\n",
+			       reg, value, r);
+	return r;
+}
+
+/****************************
+********* DEBUG *************
+****************************/
+#ifdef CONFIG_TC358765_DEBUG
+static int tc358765_write_register_i2c(u16 reg, u32 val)
+{
+	int ret = -ENODEV;
+	unsigned char buf[6];
+	struct i2c_msg msg;
+
+	if (!tc358765_i2c) {
+		dev_err(tc358765_debug.dev, "%s: I2C not initilized\n",
+							__func__);
+		return ret;
+	}
+
+	buf[0] = (reg >> 8) & 0xff;
+	buf[1] = (reg >> 0) & 0xff;
+	buf[2] = (val >> 0) & 0xff;
+	buf[3] = (val >> 8) & 0xff;
+	buf[4] = (val >> 16) & 0xff;
+	buf[5] = (val >> 24) & 0xff;
+	msg.addr = tc358765_i2c->client->addr;
+	msg.len = sizeof(buf);
+	msg.flags = 0;
+	msg.buf = buf;
+
+	mutex_lock(&tc358765_i2c->xfer_lock);
+	ret = i2c_transfer(tc358765_i2c->client->adapter, &msg, 1);
+	mutex_unlock(&tc358765_i2c->xfer_lock);
+
+	if (ret != 1)
+		return ret;
+	return 0;
+}
What about using smbus?
+
+	if (copy_from_user(&buf, ubuf, size))
+		return -EFAULT;
+
+	buf[size-1] = '\0';
+	if (sscanf(buf, "%s %x", name, &value) != 2) {
+		dev_err(dev, "%s: unable to parse input\n", __func__);
+		return -1;
+	}
+
+	if (!tc358765_i2c) {
+		dev_warn(dev,
+			"failed to write register: I2C not initialized\n");
+		return -ENODEV;
+	}
+
+	reg_count = sizeof(tc358765_regs) / sizeof(tc358765_regs[0]);
+	for (i = 0; i < reg_count; i++) {
+		if (!strcmp(name, tc358765_regs[i].name)) {
+			if (!(tc358765_regs[i].perm & A_WO)) {
+				dev_err(dev, "%s is write-protected\n", name);
+				return -EACCES;
+			}
+
+			error = tc358765_write_register_i2c(
+				tc358765_regs[i].reg, value);
+			if (error) {
+				dev_err(dev, "%s: failed to write %s\n",
+					__func__, name);
+				return -1;
Could avoid returning -1 instead of returning a correct errno?
+	gpio_set_value(dssdev->reset_gpio, 1);
+	udelay(200);
+	/* reset the panel */
+	gpio_set_value(dssdev->reset_gpio, 0);
+	/* assert reset */
+	udelay(200);
+	gpio_set_value(dssdev->reset_gpio, 1);
+	/* wait after releasing reset */
+	msleep(200);
I invite you to have a look at

Documentation/timers/timers-howto.txt
+	/* reset LVDS-PHY */
+	tc358765_write_register(dssdev, LVPHY0, (1 << 22));
+	mdelay(2);
You should give me a really good reason for using mdelay.
+	if (r) {
+		dev_err(&dssdev->dev, "failed to configure DSI pins\n");
+		goto err_disp_enable;
+	};
        ^^^

???
+static int tc358765_i2c_probe(struct i2c_client *client,
+				   const struct i2c_device_id *id)
+{
+	tc358765_i2c = kzalloc(sizeof(*tc358765_i2c), GFP_KERNEL);
what about devm_kzalloc?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help