From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C5F5DC27C4F for ; Fri, 21 Jun 2024 14:38:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Type:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To :Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=bmmWGPZLjK3G3SA6NooN/QLEoyW6TtuGBY6WdnfAAco=; b=Xme58ApAPhusayzRyGDrrKG/Gn QVqb+lq/d2sUK5syM0Q7L7BagNaeNvRr1fDh+bubO4iekKcQtMVXKjAyFVtyBEq09K6PZrcEgxpv4 m9N98vfWOG/6FQVsTRoHyb+ZVMx3WV3X6Pu7WuX0vPuyw93kch7KXjIn1HlybdTxIJXjWW7l33TJh kGKyTks3rzy/hbll1Imkd2d4nkyt0Ccojg6sQJVLBFWDLXEfsQaGnndd1jAsZAvT5X5UdLgDWM9Ks on/0GO/uubeRb6m1CEnbWBr11Em/1gwTV/vg5EL+VlY3mk6GjQTtnqLTvkRcHmbgCllWv5ItZDli1 X4rl3aFw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sKfPM-00000009XNT-3k0J; Fri, 21 Jun 2024 14:38:32 +0000 Received: from mail-wm1-x32d.google.com ([2a00:1450:4864:20::32d]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sKfPJ-00000009XMS-17Eb for linux-arm-kernel@lists.infradead.org; Fri, 21 Jun 2024 14:38:30 +0000 Received: by mail-wm1-x32d.google.com with SMTP id 5b1f17b1804b1-421a1b834acso17789005e9.3 for ; Fri, 21 Jun 2024 07:38:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718980707; x=1719585507; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=bmmWGPZLjK3G3SA6NooN/QLEoyW6TtuGBY6WdnfAAco=; b=QowaBefjxjq9hDe1xU3HAW4vXvle20nDacRUnV/aMv/AgwpCtxcgWuMrSWCujlVFVc MOHsjrd/vXGJdx/D8CV6frrADNkpwXcENaGECACjhiVPy8PhYoRVcXeaxeASBDIhVzBd NNrTTOuGbos98BtfMqMTJ1NyMjeEBdm7Sn4GxBsqkgiBvXuxTRYzOxPfmPrghlYAA/rC FquABjy8p2GmblWsTTXOAwikirtjkS+p7I5Fp04/pcPt0ZYc8dyPXd0C+MDid17/pDAO ptaCbazHl54oHVT9e15ktlCywz4yYKOefBByofQulT7I03j9qvdMoAoEcg5Vr7eC6lRC FzKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718980707; x=1719585507; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=bmmWGPZLjK3G3SA6NooN/QLEoyW6TtuGBY6WdnfAAco=; b=OtnSGvQT2kEah+7WbEeQ5UXhQursFf2aER/6U37sQKZKVl8zlYWTuAQ/UiZS+XJm32 T2p6JCsYT8Z9OQ1sRBqMLFa8XvhBk0ONcHHUQ8lqubbx7oNl24ZUiCmL37gpmvXh54jm uOMfTJh/Zsk0IdBy42cVpa0JIUExpHDFpe2W5CSgny4cSvy854oke6zQPOSCwwrbM76A JzOCKqAQEdtxmUVcHDiHjsme6P6wKR01/PjUcpo4/D/efM8bThubIY+owQmjHykCwTCX DO3lF3xxWQsWc87J4IHIctfAZBKLQd3sRdlsmdO4piQjDQOB6IZtyPK0wVo8cScTxv3g MN3Q== X-Forwarded-Encrypted: i=1; AJvYcCUdp8gWIXTn+8p1sXV037Hl/pa6UCaaxyXqIi/FPWJyJrac9HcPjb1QQwne9iz9lums+LjVuMbchbJ+gEiREtwc+SYM/s/bIy5UaCR8ctNvlJG/CVk= X-Gm-Message-State: AOJu0YxSh2+bwQTTGqHTS7bLy/MQK0Cu+CM7eexxijrzsEcTCcLrc5I9 kVlHErSgF6Xgr+ZAgvf2a7M4QZox77x1GbBSfkpLOodcyJ9wKA62 X-Google-Smtp-Source: AGHT+IGTFyJEJlx/pYBjW3mrPn4TfjUgSZJZrcUjTy19xTD34e1W/E9Pk33yXvWlmRrhHDj2wrVZ1A== X-Received: by 2002:a05:600c:35cf:b0:424:80f3:c001 with SMTP id 5b1f17b1804b1-42480f3c18dmr22951215e9.8.1718980707028; Fri, 21 Jun 2024 07:38:27 -0700 (PDT) Received: from eichest-laptop ([2a02:168:af72:0:b162:502a:9bd1:4c8b]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4247d208b27sm67482395e9.30.2024.06.21.07.38.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Jun 2024 07:38:26 -0700 (PDT) Date: Fri, 21 Jun 2024 16:38:25 +0200 From: Stefan Eichenberger To: Dmitry Torokhov Subject: Re: [PATCH v4 1/4] Input: atmel_mxt_ts - add power off and power on functions Message-ID: References: <20240417090527.15357-1-eichest@gmail.com> <20240417090527.15357-2-eichest@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240621_073829_352750_0F60821D X-CRM114-Status: GOOD ( 32.15 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: robh@kernel.org, conor+dt@kernel.org, nick@shmanahar.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linus.walleij@linaro.org, linux-input@vger.kernel.org, alexandre.belloni@bootlin.com, Stefan Eichenberger , krzysztof.kozlowski+dt@linaro.org, claudiu.beznea@tuxon.dev, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Dmitry On Thu, Jun 20, 2024 at 08:37:40AM -0700, Dmitry Torokhov wrote: > Hi Stefan, > > On Wed, Apr 17, 2024 at 11:05:24AM +0200, Stefan Eichenberger wrote: > > From: Stefan Eichenberger > > > > Add a separate function for power off and power on instead of calling > > regulator_bulk_enable and regulator_bulk_disable directly. > > > > Signed-off-by: Stefan Eichenberger > > --- > > drivers/input/touchscreen/atmel_mxt_ts.c | 59 +++++++++++++++--------- > > 1 file changed, 37 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > > index 542a31448c8f..52867ce3b9b6 100644 > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > > @@ -1307,6 +1307,38 @@ static int mxt_soft_reset(struct mxt_data *data) > > return 0; > > } > > > > +static int mxt_power_on(struct mxt_data *data) > > +{ > > + int error; > > + > > + error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), > > + data->regulators); > > + if (error) { > > + dev_err(&data->client->dev, "failed to enable regulators: %d\n", > > + error); > > + return error; > > + } > > + > > + msleep(MXT_BACKUP_TIME); > > + > > + if (data->reset_gpio) { > > + /* Wait a while and then de-assert the RESET GPIO line */ > > + msleep(MXT_RESET_GPIO_TIME); > > + gpiod_set_value(data->reset_gpio, 0); > > + msleep(MXT_RESET_INVALID_CHG); > > + } > > + > > + return 0; > > +} > > + > > +static void mxt_power_off(struct mxt_data *data) > > +{ > > + if (data->reset_gpio) > > + gpiod_set_value(data->reset_gpio, 1); > > + > > + regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators); > > +} > > + > > static void mxt_update_crc(struct mxt_data *data, u8 cmd, u8 value) > > { > > /* > > @@ -3305,25 +3337,9 @@ static int mxt_probe(struct i2c_client *client) > > return error; > > } > > > > - error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), > > - data->regulators); > > - if (error) { > > - dev_err(&client->dev, "failed to enable regulators: %d\n", > > - error); > > + error = mxt_power_on(data); > > + if (error) > > return error; > > - } > > - /* > > - * The device takes 40ms to come up after power-on according > > - * to the mXT224 datasheet, page 13. > > - */ > > - msleep(MXT_BACKUP_TIME); > > - > > - if (data->reset_gpio) { > > - /* Wait a while and then de-assert the RESET GPIO line */ > > - msleep(MXT_RESET_GPIO_TIME); > > - gpiod_set_value(data->reset_gpio, 0); > > - msleep(MXT_RESET_INVALID_CHG); > > - } > > > > /* > > * Controllers like mXT1386 have a dedicated WAKE line that could be > > @@ -3361,8 +3377,8 @@ static int mxt_probe(struct i2c_client *client) > > mxt_free_input_device(data); > > mxt_free_object_table(data); > > err_disable_regulators: > > - regulator_bulk_disable(ARRAY_SIZE(data->regulators), > > - data->regulators); > > + mxt_power_off(data); > > + > > return error; > > } > > > > @@ -3374,8 +3390,7 @@ static void mxt_remove(struct i2c_client *client) > > sysfs_remove_group(&client->dev.kobj, &mxt_attr_group); > > mxt_free_input_device(data); > > mxt_free_object_table(data); > > - regulator_bulk_disable(ARRAY_SIZE(data->regulators), > > - data->regulators); > > + mxt_power_off(data); > > This change means that on unbind we will leave with GPIO line asserted. > Won't this potentially cause some current leakage? Why do we need to > have reset asserted here? This is correct, but I checked the datasheet of three different maxTouch models and all of them have the reset line low active. This means we had a current leakage before this patch. Now it is fixed because we assert the reset line, which sets the pin to 0. I also think it makes sense if we look at the power on sequence. There we first power on the controller before we release the reset line. Without asserting it on unbind this would never trigger a reset after a power on. Regards, Stefan