Message ID | 20230913062950.4968-2-wsa+renesas@sang-engineering.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9ecd:0:b0:3f2:4152:657d with SMTP id t13csp1271438vqx; Wed, 13 Sep 2023 11:14:03 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH/LGxUMge2iXfo1Q4eMTkka3HFQZ8wFk4MVW7xjydJw2ciGi0sJdUua/Qd/+645zRYarzi X-Received: by 2002:a05:6358:5918:b0:137:d0bb:6036 with SMTP id g24-20020a056358591800b00137d0bb6036mr3232031rwf.1.1694628843132; Wed, 13 Sep 2023 11:14:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694628843; cv=none; d=google.com; s=arc-20160816; b=FZvL29na5QObp+e/z5zhVhXP+dgPy5ZR5/DAsCjr3x8y0tLJ0j09+sJEv2CX8iimAN jPcujJGyezjXIOM8nfyNFdFjM5sHugrhJ2D9Cc2VA8puIpHTOPOgYEmSLveQto3C0G5r 7zqA/JAE8QyxQ8NGy7Ym3N5eRcaB/2i1IwfQ0ytA4fb8dML0yyP83lpwJ41ZxliWAfjH Q6GDnJbBJpB2Uf8gfVnGwyPqPYY2dJPgR2avToTDDaUD/oX+5LJzPkeH58Y/ZXwWHP4w SV3lHantUP+Yd5vWIO71Ucg8To5NWkO5XFiI2DDftGMICHVGfGRq2dMSoZ8MVuZOz7/Z BolQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=wLrtGigwnWYRorUxMB9W5jEAxAni9AJCBP3owuD+mI8=; fh=lrLzEfEu8D9iPdLNPO82+2Qlh0eBBJoplZ21AHWP9Yo=; b=Cv1ysOa8CElnOkZIyvT66bPaFCo7/IIjfvnmQPtpM2KtJVw8+hCQWPO6/xKEjYpOz0 3mxKOs0GsFzLka8dMk5NS51aO5UxAkNdFnD1CvN5hr0a41Bj9D1lmBKSSxdI4hWmvBsz V8BSNZEJUjGhUCYlL1FT/LjIJHNerQWKrIkozwjJl1EQXVMbv2OS2/eryk+7xy8GffZI fH8HiVVlWZ2/fqFesng+lgL14T9iXSL4FoemLvhpIv/2cDwTJnQKbhQErqR7GdICgRQI aXjQMOimin9v50GXchXJceTJhXVDA0grOM2LXqNJ2ew2NiDwVHHurjKFFFKxqCRCrYym c1vA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sang-engineering.com header.s=k1 header.b=FqnMbi3o; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from howler.vger.email (howler.vger.email. [23.128.96.34]) by mx.google.com with ESMTPS id b9-20020a633409000000b00566016fc08csi10833632pga.83.2023.09.13.11.14.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 11:14:03 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34; Authentication-Results: mx.google.com; dkim=pass header.i=@sang-engineering.com header.s=k1 header.b=FqnMbi3o; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by howler.vger.email (Postfix) with ESMTP id BC7B1834101B; Tue, 12 Sep 2023 23:30:14 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238355AbjIMGaH (ORCPT <rfc822;pwkd43@gmail.com> + 36 others); Wed, 13 Sep 2023 02:30:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33326 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238342AbjIMGaC (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 13 Sep 2023 02:30:02 -0400 Received: from mail.zeus03.de (www.zeus03.de [194.117.254.33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9BC131735 for <linux-kernel@vger.kernel.org>; Tue, 12 Sep 2023 23:29:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= sang-engineering.com; h=from:to:cc:subject:date:message-id :in-reply-to:references:mime-version:content-transfer-encoding; s=k1; bh=wLrtGigwnWYRorUxMB9W5jEAxAni9AJCBP3owuD+mI8=; b=FqnMbi 3o/3bQO3nMGNDFOzsNPfdQy3mdrilI4AdD9QDc8y4CWe4CoRqplUlksd7M6Hi3eT ixunPkA6N1AJ+cEYrqmDKkUw0mv/Tn8rytRDk719NHdgu9b3Ub3hhVMxJn9vB1Jw HWD9TaopfSiGAbDWDZKheU+kspzJbHIYkUP/3lmIuVi7uD+KNGmPmOqULTN/WV9F btjakU6hIe7P/FvVwg14OB/r+/WwzGF4inxxuivG+OR1E9SRvRHTtDK5ogfPBGVD CX/pqcgox8Stwqf2f+jkWoOHsx3G/CUwdfzMKW+XzB0u5YqkjhXt7+/UFawRx0pw xoJgDQH7rLtdRG4Q== Received: (qmail 489546 invoked from network); 13 Sep 2023 08:29:56 +0200 Received: by mail.zeus03.de with ESMTPSA (TLS_AES_256_GCM_SHA384 encrypted, authenticated); 13 Sep 2023 08:29:56 +0200 X-UD-Smtp-Session: l3s3148p1@eQccsDcFiqMujnuS From: Wolfram Sang <wsa+renesas@sang-engineering.com> To: linux-renesas-soc@vger.kernel.org Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>, Andi Shyti <andi.shyti@kernel.org>, Philipp Zabel <p.zabel@pengutronix.de>, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH RFT 1/2] i2c: rcar: reset controller is mandatory for Gen3+ Date: Wed, 13 Sep 2023 08:29:48 +0200 Message-Id: <20230913062950.4968-2-wsa+renesas@sang-engineering.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20230913062950.4968-1-wsa+renesas@sang-engineering.com> References: <20230913062950.4968-1-wsa+renesas@sang-engineering.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Tue, 12 Sep 2023 23:30:14 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1776947133906033534 X-GMAIL-MSGID: 1776947133906033534 |
Series |
i2c: rcar: improve Gen3 support
|
|
Commit Message
Wolfram Sang
Sept. 13, 2023, 6:29 a.m. UTC
Initially, we only needed a reset controller to make sure RXDMA works at
least once per transfer. Meanwhile, documentation has been updated. It
now says that a reset has to be performed prior every transaction, also
if it is non-DMA. So, make the reset controller a requirement instead of
being optional.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-rcar.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
Comments
Hi Wolfram, Thanks for your patch! On Wed, Sep 13, 2023 at 8:41 AM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > Initially, we only needed a reset controller to make sure RXDMA works at > least once per transfer. Meanwhile, documentation has been updated. It > now says that a reset has to be performed prior every transaction, also > if it is non-DMA. So, make the reset controller a requirement instead of > being optional. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/drivers/i2c/busses/i2c-rcar.c > +++ b/drivers/i2c/busses/i2c-rcar.c > @@ -838,12 +838,10 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap, > > /* Gen3 needs a reset before allowing RXDMA once */ > if (priv->devtype == I2C_RCAR_GEN3) { > - priv->flags |= ID_P_NO_RXDMA; > - if (!IS_ERR(priv->rstc)) { > - ret = rcar_i2c_do_reset(priv); > - if (ret == 0) > - priv->flags &= ~ID_P_NO_RXDMA; > - } > + priv->flags &= ~ID_P_NO_RXDMA; > + ret = rcar_i2c_do_reset(priv); > + if (ret) > + priv->flags |= ID_P_NO_RXDMA; This is pre-existing, but if rcar_i2c_do_reset() returns an error, that means the I2C block couldn't get out of reset. Are we sure we can still do PIO transfers in that case, or should this be considered a fatal error? > } > > rcar_i2c_init(priv); > @@ -1096,11 +1094,13 @@ static int rcar_i2c_probe(struct platform_device *pdev) > > if (priv->devtype == I2C_RCAR_GEN3) { > priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > - if (!IS_ERR(priv->rstc)) { > - ret = reset_control_status(priv->rstc); > - if (ret < 0) > - priv->rstc = ERR_PTR(-ENOTSUPP); > - } > + if (IS_ERR(priv->rstc)) > + return dev_err_probe(&pdev->dev, PTR_ERR(priv->rstc), > + "couldn't get reset"); > + > + ret = reset_control_status(priv->rstc); > + if (ret < 0) > + return ret; This is a pre-existing check, but do you really need it? This condition will be true if the reset is still asserted, which could happen due to some glitch, or force-booting into a new kernel using kexec. And AFAIUI, that should be resolved by the call to rcar_i2c_do_reset() above. > } > > /* Stay always active when multi-master to keep arbitration working */ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks! > > + priv->flags &= ~ID_P_NO_RXDMA; > > + ret = rcar_i2c_do_reset(priv); > > + if (ret) > > + priv->flags |= ID_P_NO_RXDMA; > > This is pre-existing, but if rcar_i2c_do_reset() returns an error, > that means the I2C block couldn't get out of reset. Are we sure we > can still do PIO transfers in that case, or should this be considered > a fatal error? Makes sense. I will double check what to do here. > > + ret = reset_control_status(priv->rstc); > > + if (ret < 0) > > + return ret; > > This is a pre-existing check, but do you really need it? > This condition will be true if the reset is still asserted, which > could happen due to some glitch, or force-booting into a new kernel > using kexec. And AFAIUI, that should be resolved by the call to > rcar_i2c_do_reset() above. This check is needed to ensure reset_control_status() really works because we need it in rcar_i2c_do_reset(). From the docs: "reset_control_status - returns a negative errno if not supported,..." The code only checks for that, not for the status of the reset line. All the best, Wolfram
Hi Wolfram, On Tue, Sep 19, 2023 at 11:19 AM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > > + ret = reset_control_status(priv->rstc); > > > + if (ret < 0) > > > + return ret; > > > > This is a pre-existing check, but do you really need it? > > This condition will be true if the reset is still asserted, which > > could happen due to some glitch, or force-booting into a new kernel > > using kexec. And AFAIUI, that should be resolved by the call to > > rcar_i2c_do_reset() above. > > This check is needed to ensure reset_control_status() really works > because we need it in rcar_i2c_do_reset(). From the docs: > > "reset_control_status - returns a negative errno if not supported,..." > > The code only checks for that, not for the status of the reset line. Right, I missed the negative. I don't think this can fail on R-Car Gen2+ (using the CPG/MSSR driver) if devm_reset_control_get_exclusive() succeeded, but it's prudent, in case the block is every reused on a different SoC family. Gr{oetje,eeting}s, Geert
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c index 5e97635faf78..342c3747f415 100644 --- a/drivers/i2c/busses/i2c-rcar.c +++ b/drivers/i2c/busses/i2c-rcar.c @@ -838,12 +838,10 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap, /* Gen3 needs a reset before allowing RXDMA once */ if (priv->devtype == I2C_RCAR_GEN3) { - priv->flags |= ID_P_NO_RXDMA; - if (!IS_ERR(priv->rstc)) { - ret = rcar_i2c_do_reset(priv); - if (ret == 0) - priv->flags &= ~ID_P_NO_RXDMA; - } + priv->flags &= ~ID_P_NO_RXDMA; + ret = rcar_i2c_do_reset(priv); + if (ret) + priv->flags |= ID_P_NO_RXDMA; } rcar_i2c_init(priv); @@ -1096,11 +1094,13 @@ static int rcar_i2c_probe(struct platform_device *pdev) if (priv->devtype == I2C_RCAR_GEN3) { priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); - if (!IS_ERR(priv->rstc)) { - ret = reset_control_status(priv->rstc); - if (ret < 0) - priv->rstc = ERR_PTR(-ENOTSUPP); - } + if (IS_ERR(priv->rstc)) + return dev_err_probe(&pdev->dev, PTR_ERR(priv->rstc), + "couldn't get reset"); + + ret = reset_control_status(priv->rstc); + if (ret < 0) + return ret; } /* Stay always active when multi-master to keep arbitration working */