Received: by 2002:a25:7ec1:0:0:0:0:0 with SMTP id z184csp4318097ybc; Fri, 22 Nov 2019 00:31:51 -0800 (PST) X-Google-Smtp-Source: APXvYqwquhXv1MVLtqvfQEQlnvCyxvs3/FSxuj+3eZvgiv9zp/e9VY/EDISO1BXwBGYOo4USgwJW X-Received: by 2002:a17:906:69cb:: with SMTP id g11mr20533386ejs.328.1574411511677; Fri, 22 Nov 2019 00:31:51 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1574411511; cv=none; d=google.com; s=arc-20160816; b=uNOJukbZD+0rdV9Z+TTOuA1Ik5yD59M8F0ZoauMXjlTu4gpzGZJE1lWI6BJ0Y9+YUH eLs5h+Uu0TqL3r5ElX/GvxfOiVyN17B1quq5K3in3g2CrLwPryl9isCYxwGuZRqBNLJ6 wMHUqdsiYICFxWfJADHhDL3J6olJDg4ov3+1JV9rEKjAAz4YDmE80k2rXP5LjSuCZsNS a8rommDWameKEXy2DjeFRR0OcjKorJGBBWVPnA2KqjBmBBUt0/PuGydyQ9mOKr7rdyh5 DOqdk9lTpTbh9j20fbR35zvChSegsV2fKgx04V7429yVG18NXNdtu89evRNhCy7Z8dAc rd2w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:ironport-sdr:ironport-sdr; bh=yTS4/95FajDooWIcFcs0xFI91yGHW8Aiy1UIaDH4yZY=; b=Eif+gN/vxR9hL6du68L6cirDX+369yK4m7FMXcTOrcraxS4jBPgRHZIkfNHHkWGr/J 63324kUP6+/Vn/4C55e+SX1Gu4FCVbUSrxhzylGCgdQShiUjo/tV/WjFGQxn+2c2joe+ omsfjuuxrMBWLw6UWN/9CWpEdHRNaunYFoaguCnwADwvzG5SDnhnV5llBXG5+fF+GjAg ZKRq6CB6P5Y5U5u+KgywHgZRqtQLY11jUScvcVAEna8pIKsjH+G20dPr36gUiFvd8lri 1OFygCXLKsH6P7461KYuWWvvUIK2b6cafKU/IvpHTkJEVLZGPWjLxO+YIcSZ2EZZW6/m wtDQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id h53si4281536edh.200.2019.11.22.00.31.27; Fri, 22 Nov 2019 00:31:51 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727781AbfKVI1D (ORCPT + 99 others); Fri, 22 Nov 2019 03:27:03 -0500 Received: from esa1.mentor.iphmx.com ([68.232.129.153]:23094 "EHLO esa1.mentor.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727116AbfKVI1B (ORCPT ); Fri, 22 Nov 2019 03:27:01 -0500 IronPort-SDR: DSBZQ6mN7a7QbSzH56ulacYF8e9s4LFNZs+86CabF76uCR/6WvIcJaAb+l2enzJ/8Xi2mfmvLB ZZy0dysZDTWanCUZLTSOSPmsg2DOspWKI6ht81fs5MT/P71w2dZ1JABmiKLnxjq86vkr+zzNZK hVhnzEejymB1fcst41KREOObjInAf9FyLIxTBBdUf2OE+LHsmAyGZbe73D576ASkX8ddtwlO2U 5ICHs8Gh5RpXsqUR6JbKQJnaREl+hiAefMIj1EiJ0uM6VusYekGPVHGo0/R7UYKowXNQFkr0F3 2g8= X-IronPort-AV: E=Sophos;i="5.69,229,1571731200"; d="scan'208";a="45291197" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 22 Nov 2019 00:26:59 -0800 IronPort-SDR: ZJ/12K09rbuRyQ/2BW7+8aKfj+cIoNCdaEa0bnyrf/RE4qRdNdOewF5QwVdtayBcKxbHgdEHgc eV0ee4gPtVsObp05sbuc9mH6ZE13QSZWfnkSupodDWlNSM1BgTRRtxawTSwpPjqa96jFza/JOn k7VzLoUDe/dQpEPwJTe/Kg753yOFtCrm/mEhjjitZ8jCTGcTtVX1pkVWpCZ89O03VRSm7u4+Sm UrXaLdewrsfGrRUu4mwcDLKAurMdRUIl6/H/1RqBWZiSIB+fy9QeaxRhLjEsnEHFj2WedPzY6n SHY= From: Jiada Wang To: , , , , CC: , , , , Subject: [PATCH v6 47/48] Input: atmel_mxt_ts: Implement synchronization during various operation Date: Fri, 22 Nov 2019 17:24:01 +0900 Message-ID: <20191122082402.18173-48-jiada_wang@mentor.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20191122082402.18173-1-jiada_wang@mentor.com> References: <20191122082402.18173-1-jiada_wang@mentor.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Sanjeev Chugh There could be scope of race conditions when sysfs is being handled and at the same time, device removal is occurring. For example, we don't want the device removal to begin if the Atmel device cfg update is going on or firmware update is going on. In such cases, wait for device update to be completed before the removal continues. Thread Thread 2: ========================= ========================= mxt_update_fw_store() mxt_remove() mutex_lock(&data->lock) ... mxt_initialize() //Tries to acquire lock request_firmware_nowait() mutex_lock(&data->lock) ... ==>waits for lock() ... . ... . mutex_unlock(&data->lock) . //Gets lock and proceeds mxt_free_input_device(); ... mutex_unlock(&data->lock) //Frees atmel driver data kfree(data) If the request_firmware_nowait() completes after the driver removal, and callback is triggered. But kernel crashes since the module is already removed. This commit adds state machine to serialize such scenarios. Signed-off-by: Sanjeev Chugh Signed-off-by: Bhuvanesh Surachari Signed-off-by: Jiada Wang --- drivers/input/touchscreen/atmel_mxt_ts.c | 222 ++++++++++++++++++++--- 1 file changed, 196 insertions(+), 26 deletions(-) diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index a867960dc59b..243820b102b8 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -224,6 +224,7 @@ enum t100_type { #define MXT_POWERON_DELAY 150 /* msec */ #define MXT_BOOTLOADER_WAIT 36E5 /* 1 minute */ #define MXT_WATCHDOG_TIMEOUT 1000 /* msec */ +#define MXT_CONFIG_TIMEOUT 1000 /* msec */ /* Command to unlock bootloader */ #define MXT_UNLOCK_CMD_MSB 0xaa @@ -247,6 +248,20 @@ enum t100_type { #define DEBUG_MSG_MAX 200 +enum device_state { + MXT_STATE_READY, + MXT_STATE_UPDATING_CONFIG, + MXT_STATE_UPDATING_CONFIG_ASYNC, + MXT_STATE_START, + MXT_STATE_STOP, + MXT_STATE_GOING_AWAY +}; + +enum mxt_cmd { + UPDATE_CFG, + UPDATE_FW +}; + struct mxt_info { u8 family_id; u8 variant_id; @@ -426,11 +441,15 @@ struct mxt_data { /* Indicates whether device is in suspend */ bool suspended; - /* Indicates whether device is updating configuration */ - bool updating_config; + struct mutex lock; unsigned int mtu; bool t25_status; + + /* State handling for probe/remove, open/close and config update */ + enum device_state e_state; + + struct completion update_cfg_completion; }; struct mxt_vb2_buffer { @@ -1657,6 +1676,7 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id) struct mxt_data *data = dev_id; int ret; + mutex_lock(&data->lock); data->mxt_status.intp_triggered = true; if (data->in_bootloader) { @@ -1684,6 +1704,8 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id) exit: data->mxt_status.intp_triggered = false; + mutex_unlock(&data->lock); + return ret; } @@ -2264,6 +2286,8 @@ static void mxt_free_object_table(struct mxt_data *data) video_unregister_device(&data->dbg.vdev); v4l2_device_unregister(&data->dbg.v4l2); #endif + mutex_lock(&data->lock); + data->object_table = NULL; kfree(data->info); data->info = NULL; @@ -2293,6 +2317,8 @@ static void mxt_free_object_table(struct mxt_data *data) data->T100_reportid_min = 0; data->T100_reportid_max = 0; data->max_reportid = 0; + + mutex_unlock(&data->lock); } static int mxt_parse_object_table(struct mxt_data *data, @@ -2974,8 +3000,15 @@ static int mxt_configure_objects(struct mxt_data *data, static void mxt_config_cb(const struct firmware *cfg, void *ctx) { + struct mxt_data *data = ctx; + mxt_configure_objects(ctx, cfg); release_firmware(cfg); + complete(&data->update_cfg_completion); + mutex_lock(&data->lock); + if (data->e_state != MXT_STATE_GOING_AWAY) + data->e_state = MXT_STATE_READY; + mutex_unlock(&data->lock); } static int mxt_bootloader_status(struct mxt_data *data) @@ -3088,6 +3121,15 @@ static int mxt_initialize(struct mxt_data *data) goto err_free_sysfs; if (data->cfg_name) { + mutex_lock(&data->lock); + if (data->e_state != MXT_STATE_GOING_AWAY) { + data->e_state = MXT_STATE_UPDATING_CONFIG_ASYNC; + } else { + mutex_unlock(&data->lock); + return -EBUSY; + } + reinit_completion(&data->update_cfg_completion); + mutex_unlock(&data->lock); error = request_firmware_nowait(THIS_MODULE, true, data->cfg_name, &client->dev, @@ -3867,30 +3909,58 @@ static int mxt_update_file_name(struct device *dev, char **file_name, return 0; } +static int mxt_process_operation(struct mxt_data *data, + enum mxt_cmd cmd, + void *cmd_data); + static ssize_t mxt_update_fw_store(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t count) + struct device_attribute *attr, + const char *buf, size_t count) { struct mxt_data *data = dev_get_drvdata(dev); + char *filename = NULL; + int ret; + + ret = mxt_update_file_name(dev, &filename, buf, count); + if (ret) + goto out; + + ret = mxt_process_operation(data, UPDATE_FW, filename); + kfree(filename); + + if (ret) + goto out; + + return count; +out: + return ret; +} + +static int mxt_fw_update(struct mxt_data *data, + const char *filename) +{ + struct device *dev = &data->client->dev; + unsigned int len = 0; int error; - error = mxt_update_file_name(dev, &data->fw_name, buf, count); + len = strlen(filename); + error = mxt_update_file_name(dev, &data->fw_name, filename, len); if (error) return error; error = mxt_load_fw(dev); if (error) { dev_err(dev, "The firmware update failed(%d)\n", error); - count = error; - } else { - dev_info(dev, "The firmware update succeeded\n"); - - error = mxt_initialize(data); - if (error) - return error; + return error; } - return count; + error = mxt_initialize(data); + if (error) + return error; + + dev_info(dev, "The firmware update succeeded\n"); + + return error; } static ssize_t mxt_update_cfg_store(struct device *dev, @@ -3898,14 +3968,38 @@ static ssize_t mxt_update_cfg_store(struct device *dev, const char *buf, size_t count) { struct mxt_data *data = dev_get_drvdata(dev); + char *filename = NULL; + int ret; + + ret = mxt_update_file_name(dev, &filename, buf, count); + if (ret) + goto out; + + ret = mxt_process_operation(data, UPDATE_CFG, filename); + kfree(filename); + + if (ret) + goto out; + + return count; +out: + return ret; +} + +static int mxt_cfg_update(struct mxt_data *data, + char *filename) +{ + struct device *dev = &data->client->dev; const struct firmware *cfg; + unsigned int len = 0; int ret; - ret = mxt_update_file_name(dev, &data->cfg_name, buf, count); + len = strlen(filename); + ret = mxt_update_file_name(dev, &data->cfg_name, filename, len); if (ret) return ret; - ret = request_firmware(&cfg, data->cfg_name, dev); + ret = request_firmware(&cfg, data->cfg_name, &data->client->dev); if (ret < 0) { dev_err(dev, "Failure to request config file %s\n", data->cfg_name); @@ -3913,8 +4007,6 @@ static ssize_t mxt_update_cfg_store(struct device *dev, goto out; } - data->updating_config = true; - mxt_free_input_device(data); if (data->suspended) { @@ -3930,15 +4022,8 @@ static ssize_t mxt_update_cfg_store(struct device *dev, } ret = mxt_configure_objects(data, cfg); - if (ret) - goto release; - - ret = count; - -release: release_firmware(cfg); out: - data->updating_config = false; return ret; } @@ -4202,8 +4287,17 @@ static int mxt_start(struct mxt_data *data) { int ret = 0; - if (!data->suspended || data->in_bootloader) + mutex_lock(&data->lock); + if (!data->suspended) { + mutex_unlock(&data->lock); return 0; + } + if (data->in_bootloader || data->e_state != MXT_STATE_READY) { + mutex_unlock(&data->lock); + return -EBUSY; + } + data->e_state = MXT_STATE_START; + mutex_unlock(&data->lock); switch (data->suspend_mode) { case MXT_SUSPEND_T9_CTRL: @@ -4247,8 +4341,12 @@ static int mxt_start(struct mxt_data *data) ret = mxt_acquire_irq(data); } + mutex_lock(&data->lock); if (!ret) data->suspended = false; + if (data->e_state != MXT_STATE_GOING_AWAY) + data->e_state = MXT_STATE_READY; + mutex_unlock(&data->lock); return ret; } @@ -4257,8 +4355,19 @@ static int mxt_stop(struct mxt_data *data) { int ret; - if (data->suspended || data->in_bootloader || data->updating_config) + mutex_lock(&data->lock); + if (data->suspended) { + mutex_unlock(&data->lock); return 0; + } + if (data->in_bootloader || (data->e_state != MXT_STATE_READY && + data->e_state != MXT_STATE_GOING_AWAY)) { + mutex_unlock(&data->lock); + return -EBUSY; + } + if (data->e_state != MXT_STATE_GOING_AWAY) + data->e_state = MXT_STATE_STOP; + mutex_unlock(&data->lock); switch (data->suspend_mode) { case MXT_SUSPEND_T9_CTRL: @@ -4288,8 +4397,15 @@ static int mxt_stop(struct mxt_data *data) break; } + mutex_lock(&data->lock); data->suspended = true; + + if (data->e_state != MXT_STATE_GOING_AWAY) + data->e_state = MXT_STATE_READY; + mutex_unlock(&data->lock); + return 0; + } static int mxt_input_open(struct input_dev *dev) @@ -4443,12 +4559,15 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0", client->adapter->nr, client->addr); + mutex_init(&data->lock); + data->client = client; i2c_set_clientdata(client, data); init_completion(&data->chg_completion); init_completion(&data->reset_completion); init_completion(&data->crc_completion); + init_completion(&data->update_cfg_completion); mutex_init(&data->debug_msg_lock); data->suspend_mode = dmi_check_system(chromebook_T9_suspend_dmi) ? @@ -4542,6 +4661,18 @@ static int mxt_remove(struct i2c_client *client) { struct mxt_data *data = i2c_get_clientdata(client); + mutex_lock(&data->lock); + if (data->e_state == MXT_STATE_UPDATING_CONFIG_ASYNC || + data->e_state == MXT_STATE_UPDATING_CONFIG) { + data->e_state = MXT_STATE_GOING_AWAY; + mutex_unlock(&data->lock); + mxt_wait_for_completion(data, &data->update_cfg_completion, + MXT_CONFIG_TIMEOUT); + } else { + data->e_state = MXT_STATE_GOING_AWAY; + mutex_unlock(&data->lock); + } + disable_irq(data->irq); sysfs_remove_group(&client->dev.kobj, &mxt_fw_attr_group); if (data->reset_gpio) { @@ -4598,6 +4729,45 @@ static int __maybe_unused mxt_resume(struct device *dev) return ret; } +static int mxt_process_operation(struct mxt_data *data, + enum mxt_cmd cmd, + void *cmd_data) +{ + int ret = 0; + + mutex_lock(&data->lock); + if (data->e_state != MXT_STATE_READY) { + mutex_unlock(&data->lock); + dev_err(&data->client->dev, "Atmel touch device is shutting down\n"); + return -EBUSY; + } + data->e_state = MXT_STATE_UPDATING_CONFIG; + reinit_completion(&data->update_cfg_completion); + mutex_unlock(&data->lock); + + switch (cmd) { + case UPDATE_CFG: + case UPDATE_FW: + if (cmd == UPDATE_CFG) + ret = mxt_cfg_update(data, (char *)cmd_data); + else + ret = mxt_fw_update(data, (char *)cmd_data); + break; + + default: + break; + } + mutex_lock(&data->lock); + if (data->e_state != MXT_STATE_UPDATING_CONFIG_ASYNC) { + complete(&data->update_cfg_completion); + if (data->e_state != MXT_STATE_GOING_AWAY) + data->e_state = MXT_STATE_READY; + } + mutex_unlock(&data->lock); + + return ret; +} + static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume); static const struct of_device_id mxt_of_match[] = { -- 2.17.1