On Mon, Jun 28, 2010 at 08:38:11PM +0900, Joonyoung Shim wrote:
> +
> +static int qt602240_load_fw(struct device *dev, const char *fn)
> +{
> + struct qt602240_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> + const struct firmware *fw = NULL;
> + unsigned int frame_size;
> + unsigned int pos = 0;
> + int ret;
> +
> + if (!data) {
> + dev_err(dev, "The data is NULL\n");
> + return -EFAULT;
> + }
> +
> + ret = request_firmware(&fw, fn, dev);
> + if (ret < 0) {
> + dev_err(dev, "Unable to open firmware %s\n", fn);
> + return -ENOMEM;
> + }
> +
> + /* Change to the bootloader mode */
> + qt602240_write_object(data, QT602240_GEN_COMMAND,
> + QT602240_COMMAND_RESET, QT602240_BOOT_VALUE);
> + msleep(QT602240_RESET_TIME);
> +
> + /* Change to slave address of bootloader */
> + if (client->addr == QT602240_APP_LOW)
> + client->addr = QT602240_BOOT_LOW;
> + else
> + client->addr = QT602240_BOOT_HIGH;
> +
> + ret = qt602240_check_bootloader(client, QT602240_WAITING_BOOTLOAD_CMD);
> + if (ret < 0)
> + goto err_fw;
> +
> + /* Unlock bootloader */
> + qt602240_unlock_bootloader(client);
> +
> + while (pos < fw->size) {
> + ret = qt602240_check_bootloader(client,
> + QT602240_WAITING_FRAME_DATA);
> + if (ret < 0)
> + goto err_fw;
> +
> + frame_size = ((*(fw->data + pos) << 8) | *(fw->data + pos + 1));
> +
> + /* We should add 2 at frame size as the the firmware data is not
> + * included the CRC bytes.
> + */
> + frame_size += 2;
> +
> + /* Write one frame to device */
> + qt602240_fw_write(client, fw->data + pos, frame_size);
> +
> + ret = qt602240_check_bootloader(client,
> + QT602240_FRAME_CRC_PASS);
> + if (ret < 0)
> + goto err_fw;
> +
> + pos += frame_size;
> +
> + dev_dbg(dev, "Updated %zd bytes / %zd bytes\n", pos, fw->size);
> + }
> +
> +err_fw:
> + /* Change to slave address of application */
> + if (client->addr == QT602240_BOOT_LOW)
> + client->addr = QT602240_APP_LOW;
> + else
> + client->addr = QT602240_APP_HIGH;
You are missing call to release_firmware() and thus leaking memory.
> +
> + return ret;
> +}
> +
> +static ssize_t qt602240_update_fw_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct qt602240_data *data = dev_get_drvdata(dev);
> + unsigned int version;
> + int ret;
> +
> + if (sscanf(buf, "%u", &version) != 1) {
> + dev_err(dev, "Invalid values\n");
> + return -EINVAL;
> + }
> +
> + /* Firmware update supports from version 21 */
> + if ((data->info.version < QT602240_VER_21) ||
> + (version < QT602240_VER_21)) {
> + dev_err(dev, "Firmware update supports from version 21\n");
"Firmware updates suppored starting with version 21"
> + return -EINVAL;
> + }
> +
> + /* Interrupt disable */
> + disable_irq(data->irq);
> +
> + ret = qt602240_load_fw(dev, QT602240_FW_NAME);
> + if (ret < 0)
> + dev_err(dev, "The firmware update failed(%d)\n", ret);
> + else {
> + dev_dbg(dev, "The firmware update successed\n");
"succeeded"
> +
> + /* Wait for reset */
> + msleep(QT602240_FWRESET_TIME);
> +
> + kfree(data->object_table);
> +
> + qt602240_initialize(data);
> + }
> +
> + /* Interrupt enable */
> + enable_irq(data->irq);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(object, 0444, qt602240_object_show, NULL);
> +static DEVICE_ATTR(update_fw, 0664, NULL, qt602240_update_fw_store);
> +
> +static struct attribute *qt602240_attrs[] = {
> + &dev_attr_object.attr,
> + &dev_attr_update_fw.attr,
> + NULL
> +};
> +
> +static const struct attribute_group qt602240_attr_group = {
> + .attrs = qt602240_attrs,
> +};
> +
> +static int qt602240_input_open(struct input_dev *dev)
> +{
> + struct qt602240_data *data = input_get_drvdata(dev);
> +
> + /* Touch enable */
> + qt602240_write_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_CTRL,
> + 0x83);
Do you want to do this if device is suspended?
> +
> + return 0;
> +}
> +
> +static void qt602240_input_close(struct input_dev *dev)
> +{
> + struct qt602240_data *data = input_get_drvdata(dev);
> +
> + /* Touch disable */
> + qt602240_write_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_CTRL,
> + 0);
Same here.
> +}
> +
> +static int __devinit qt602240_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct qt602240_data *data;
> + struct input_dev *input_dev;
> + int ret;
> +
> + if (!client->dev.platform_data)
> + return -EINVAL;
> +
> + data = kzalloc(sizeof(struct qt602240_data), GFP_KERNEL);
> +
> + input_dev = input_allocate_device();
> + if (!data || !input_dev) {
> + dev_err(&client->dev, "Failed to allocate memory\n");
> + ret = -ENOMEM;
> + goto err_free_mem;
> + }
> +
> + input_dev->name = "AT42QT602240/ATMXT224 Touchscreen";
> + input_dev->id.bustype = BUS_I2C;
> + input_dev->dev.parent = &client->dev;
> + input_dev->open = qt602240_input_open;
> + input_dev->close = qt602240_input_close;
> +
> + __set_bit(EV_ABS, input_dev->evbit);
> + __set_bit(EV_KEY, input_dev->evbit);
> + __set_bit(BTN_TOUCH, input_dev->keybit);
> +
> + /* For single touch */
> + input_set_abs_params(input_dev, ABS_X, 0, QT602240_MAX_XC, 0,
> + 0);
> + input_set_abs_params(input_dev, ABS_Y, 0, QT602240_MAX_YC, 0,
> + 0);
> +
> + /* For multi touch */
> + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
> + QT602240_MAX_AREA, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0,
> + QT602240_MAX_XC, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
> + QT602240_MAX_YC, 0, 0);
> +
> + input_set_drvdata(input_dev, data);
> +
> + INIT_WORK(&data->ts_resume_work, qt602240_resume_worker);
> + data->client = client;
> + data->input_dev = input_dev;
> + data->pdata = client->dev.platform_data;
> + data->irq = client->irq;
> +
> + i2c_set_clientdata(client, data);
> +
> + ret = qt602240_initialize(data);
> + if (ret < 0)
> + goto err_free_object;
> +
> + ret = request_threaded_irq(client->irq, NULL, qt602240_interrupt,
> + IRQF_TRIGGER_FALLING, client->dev.driver->name, data);
> +
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to register interrupt\n");
> + goto err_free_object;
> + }
> +
> + ret = input_register_device(input_dev);
> + if (ret < 0)
> + goto err_free_irq;
> +
> + ret = sysfs_create_group(&client->dev.kobj, &qt602240_attr_group);
> + if (ret)
> + goto err_unregister_device;
> +
> + return 0;
> +
> +err_unregister_device:
> + input_unregister_device(input_dev);
> +err_free_irq:
> + free_irq(client->irq, data);
> +err_free_object:
> + kfree(data->object_table);
> +err_free_mem:
> + input_free_device(input_dev);
> + kfree(data);
> + return ret;
> +}
> +
> +static int __devexit qt602240_remove(struct i2c_client *client)
> +{
> + struct qt602240_data *data = i2c_get_clientdata(client);
> +
> + free_irq(data->irq, data);
> + cancel_work_sync(&data->ts_resume_work);
> + input_unregister_device(data->input_dev);
> + kfree(data->object_table);
> + kfree(data);
> +
> + sysfs_remove_group(&client->dev.kobj, &qt602240_attr_group);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int qt602240_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> + struct qt602240_data *data = i2c_get_clientdata(client);
> + struct input_dev *input_dev = data->input_dev;
> +
> + mutex_lock(&input_dev->mutex);
> +
> + if (input_dev->users)
> + /* Touch disable */
> + qt602240_write_object(data, QT602240_TOUCH_MULTI,
> + QT602240_TOUCH_CTRL, 0);
> +
> + mutex_unlock(&input_dev->mutex);
> +
> + return 0;
> +}
> +
> +static int qt602240_resume(struct i2c_client *client)
> +{
> + struct qt602240_data *data = i2c_get_clientdata(client);
> +
> + schedule_work(&data->ts_resume_work);
Like I said, I am concerned with pulling resume work out of line. The
rest of the system might start accessing device that is not ready yet
but is already marked as resumed. I'd rather did that short sleep right
here in resume function.
Also, please CC Jean Delvare to make sure I2C bits look good.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
