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?