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 X-Spam-Level: X-Spam-Status: No, score=-13.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 707CAC433DF for ; Mon, 3 Aug 2020 13:29:15 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 343552076C for ; Mon, 3 Aug 2020 13:29:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="iV7/mCW5"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=microchip.com header.i=@microchip.com header.b="HsV/7S4J"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=microchiptechnology.onmicrosoft.com header.i=@microchiptechnology.onmicrosoft.com header.b="g60GRc6g" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 343552076C Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=microchip.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:Content-ID:In-Reply-To:References: Message-ID:Date:Subject:To:From:Reply-To:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=VFLcziu96ZAA8iAv1rrIm+bGegNeh6+K5L6MVWeCcGA=; b=iV7/mCW5kSylqWgOV3lzofSjT nWmbWNtlyKsF7RBwtmePU1ROcQX8mB1lstQHxgMeyegRpJwcaA8To7upudGWjJzZWRuK9NquMvRh8 v9zomODH6fBiJhhSqBCXVc3Ge+PEcVzkK5PDo1Vbf6d3MfIHK1Xp4FdbNTiS8DIkVASTu3V/VgEyI uE2pSFb6BV3uCvT5jxdrtVsaNFyzV11+4pTqCn+RaJCsz2sI/v78TZdZ6aPFmgEJxvCWc2hS/y8aW cJh0AV4VSNQAvqu0EXhjiYhe7Tufhkoys5ofT1c/hEOH/a0thi1E8Oh4u5D+2aZ9ggBf2TqKl+FcL Rqq59C8yg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k2aV6-00043T-MC; Mon, 03 Aug 2020 13:27:36 +0000 Received: from esa5.microchip.iphmx.com ([216.71.150.166]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k2aV3-00042T-BQ for linux-arm-kernel@lists.infradead.org; Mon, 03 Aug 2020 13:27:34 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1596461254; x=1627997254; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=Lbn4IUBl1j8lk/uLWk/eFKgVKfFng2Pf8iCkGPiDDXc=; b=HsV/7S4J4sAaLafc7pH2mJrzj0k8kyD0/qINbVRmfiJlAtERYrbtQTT+ xRXt502PvAfE5FxLp8TjV/n0Q9up7/rB//7AZCLsiPj7+lL/0WGJwbFjV dUC0/Inc92gtRyYMvJ5qeUfOVd51COnpS1QGdgKceiheBY8yuSJSaq+MO q7F7W8Nodv90KESyoea3j2pWb3c737WMV8Te2IGTUdZ4cWk+QpLnrPL8o /uH0p0/QyJJmNFfRcKk7udjAI+YSMiD2mhjXMipKvUPbrzRXrvwFzTfuk RlgqsFiYaLrzbc5a6Hx1O7mr0OJnPclWVJdTfVAcIsQ2PT+BZgxK1i8td Q==; IronPort-SDR: SoLNEFkO/0DWxRxyNpV/ISkoyfIM6R9qTy4Velxb+XMdL9otahBJdEJOUTFSCSqXnaTwosKqZp WPfy06VCG1TQ5GdJkO3QYWkdQ8SGgEpZH1Y5nN4LFsbZrTwo9cO3kKPH96sdnRQTyrAznx4C8F DWIR994v5ib4lCcvxpla0LKXWXfxBQyUQe9vySGzHXD5ExrOtwh1CqIIxx4SzKWojJzflvTBv0 1sxXAaXyYBVoNzcocguqK7TyYjhwQlD2fT3H/zFGcnAY4ffL/uoyb0fmYMMLtw4bGGFmUmtb44 LOk= X-IronPort-AV: E=Sophos;i="5.75,430,1589266800"; d="scan'208";a="85621073" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa5.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 03 Aug 2020 06:27:31 -0700 Received: from chn-vm-ex02.mchp-main.com (10.10.85.144) by chn-vm-ex01.mchp-main.com (10.10.85.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1979.3; Mon, 3 Aug 2020 06:27:26 -0700 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (10.10.215.89) by email.microchip.com (10.10.87.72) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1979.3 via Frontend Transport; Mon, 3 Aug 2020 06:27:26 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hLAgVZFg2neR6568bKjzL1afd5n5XP7e3wIOweryw2XXJQWP5Y/5dvRrJyOGEiegdGT9MP96hZ84EOT2PC/dC3O/5glTpnoHE5dPfTcJkavqGsQVDOiABR+RELMd0jp+9tNdTXl8ZiXdOQxlY+nx8SiL6Y6209eU98h2if8jpVwcQqCx/AxGg/YN2EDyzMwMpqu3JjitkLOj//RPxMj2Oez+jbA4LZF7yAxTIMLw2RgJw3+IekCPfM1Qp8I9TN1WpGuqL5Ju8Y3fCBYviq8w08VahvEnAXwjEztGASf2JKeBdsmmYuY9wF0obnd7uCzuYCFu1sd4fR+ilCGarCybpg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6/EjGCf/hhsRxlEp2oescU/rD7/Pfq9Tm4NeuyN4X88=; b=nGzinxobfHUpwg6XhSDaiaPl9Y9p8k3Uix0PkgNIJP1avZUnPMnPM4Ws8oB/3D4tMYbdH52EwlAw4P2Aip0cmynMWdIe0C8R155C9By9AVQJPBEpjjV8QdfI0NdtoFqVy1b1bcmE0daapXO8Iqru7EE7zgQY8h/xvLjc9Tm+N6uWU0Qt28HUZSLgWZDOI8ytXwrBFj00QXhQFAXS4sV0B5xPjiH6kJDVbZOezW99BwY/E79SGK9/K80Oetf0x27sJ7HHqTLMHVd1ooWTJQ+O0sOMk86EKUl0bGOL+gqa2EBJA8eN+lRyffI7ZXD3vaqOgfnAcf8iDNHvuHB1WzcoXw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microchip.com; dmarc=pass action=none header.from=microchip.com; dkim=pass header.d=microchip.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microchiptechnology.onmicrosoft.com; s=selector2-microchiptechnology-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6/EjGCf/hhsRxlEp2oescU/rD7/Pfq9Tm4NeuyN4X88=; b=g60GRc6gAz/UO/mvXfPZ7wH3GiDxRGLEdYfxaMx2pXr1hF04+gxD1Dasfp9s1g0nbpXvEmXmMKfOHudBuOaZo8jWbKFxVjzsGIB+1x53s61JhD6xjcZS+Ye6b/w1o/4E/zj0hJjy4sN+/hN0UeWK3ozepwyW8f26zIEx7dQXku4= Received: from SN6PR11MB3504.namprd11.prod.outlook.com (2603:10b6:805:d0::17) by SN6PR11MB3039.namprd11.prod.outlook.com (2603:10b6:805:d3::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3239.16; Mon, 3 Aug 2020 13:27:29 +0000 Received: from SN6PR11MB3504.namprd11.prod.outlook.com ([fe80::c83b:2062:4e59:8ebf]) by SN6PR11MB3504.namprd11.prod.outlook.com ([fe80::c83b:2062:4e59:8ebf%7]) with mapi id 15.20.3239.021; Mon, 3 Aug 2020 13:27:28 +0000 From: To: Subject: Re: Re: [RFC PATCH 2/4] i2c: core: add generic I2C GPIO recovery Thread-Topic: Re: [RFC PATCH 2/4] i2c: core: add generic I2C GPIO recovery Thread-Index: AQHWRkTSAu6ZxU19YEGUx3XaLWZruaklTrYAgAFYYwA= Date: Mon, 3 Aug 2020 13:27:28 +0000 Message-ID: <084249c3-dd2b-a975-6c8d-8045def0293e@microchip.com> References: <20200619141904.910889-1-codrin.ciubotariu@microchip.com> <20200619141904.910889-3-codrin.ciubotariu@microchip.com> <20200802165446.GA10193@kunai> In-Reply-To: <20200802165446.GA10193@kunai> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=microchip.com; x-originating-ip: [84.232.220.208] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 848bb43d-6c22-474f-0b55-08d837b0f822 x-ms-traffictypediagnostic: SN6PR11MB3039: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-bypassexternaltag: True x-ms-oob-tlc-oobclassifiers: OLM:5516; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 7iUI55TaRdqLIh16+2rb+VUilYj/vUrmIo8IfaubZZXE8PFKksYtT5bbm/C3LY+MmhcvwT4WCxzIC9g7HSHnL13ZzziyyKnRQsAuoezVVwPg8fktGSDTvZGtjiqyrEHWijDpH3Ci7H6RDmm3powHYD9Yd0niL2hLl8o9BdczZ6xTqjmKct1Q12VRJPUGFIX0YGxrPSVPRM6liLq4T16VEhoF9U8JGdtf2Q+w+ekoTT6Rl3AXQ2mbutDW6qmJTj6Dg1BphO2xulx8PyP4y5I+KdiDVIloHHqWY1CX5NBmotLGBOBRQDGAN4rsJhkR9FkvrXOCJvkkW06p3oiykK88o6HuNaa3aNOiC3S9t5y4nnxR5BwO9e2991asXyc+g89/O5kssneMWVKa+EsTL/Z+YQ== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:SN6PR11MB3504.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(366004)(346002)(39860400002)(396003)(376002)(136003)(2616005)(83380400001)(91956017)(71200400001)(66476007)(66556008)(64756008)(66446008)(8936002)(8676002)(2906002)(66946007)(76116006)(6916009)(316002)(186003)(6512007)(36756003)(31686004)(54906003)(26005)(6506007)(6486002)(53546011)(31696002)(478600001)(5660300002)(4326008)(86362001)(41533002)(43740500002); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: RAKTLn5Rz3tRqCLQ2F+91P7dJyGOrlbJoIt2ZnqxuLHPBBkeWtZWeiPfadqO83/TnSM6wx2pAPGt3PDA77ZqhLyYkELQqZN5hnJMnS2MZ6X2O031baq0JI4fr4zt+r2yECVzih/95aNhU+avda5eNO5D58b7kcSj9mtXTIDieyW56bHtR19F4P+L/rzRIBCmn0eLgOmpBLUwlmLWm4VNjgG9mBcBFmxKqfYkusJfjrashl9eXzXXg/3EZBMziRTQwHbRo2qgEVkq3ac4Q/SiDNnm0fj3NIdoIR4DZQynQ7Fl90Q3IL2LuF1ITCh0UpcAhhtcyBhAi8yD0PmVyRlcxnNQUDSrhkvf1LsHVtNzZq+xGhUZB/qAleiKJBhedqPKE+5RYDP/Jmzm6Rg63obnLKR859RulDsjc5RMVKxiJe2k/OQvGcsfTPu6PiXEeKasylz5LDK4ZIh9SgO95Zw0kXYUMyxq2rIcYUFLsZZellKXeGzjcL5AnBpVuLM+Exnwj1FktygnwW9sABTAR0yWqYn9CxwU1tyvQvJa6Vw8rvDh72u4DUIXXiNQphiIg8t7P1yk1+f3zjSc/OSJOf4gptonW5DkTxzWCVY6NhbFNZFHfgRVRdMoc8CcPIW8ri+gC729aRbeTftwIunMzKpfjw== Content-ID: <191FBE1E6110D846965BC0996AEFA8C6@namprd11.prod.outlook.com> MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: SN6PR11MB3504.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 848bb43d-6c22-474f-0b55-08d837b0f822 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Aug 2020 13:27:28.7880 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Lv9FItNzWSPbLXMF2Or04YvMMLdk3xOWE73aFWNfjs+LTf9DcAiWkiC1gjfpj1qs7xLFH6ynM3XTDSB7y/4hA1QdLaY9d76mKFAzn7hcskU= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR11MB3039 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200803_092733_685249_A841AC64 X-CRM114-Status: GOOD ( 30.48 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, alexandre.belloni@bootlin.com, kamel.bouhara@bootlin.com, linux-kernel@vger.kernel.org, Ludovic.Desroches@microchip.com, robh+dt@kernel.org, linux-i2c@vger.kernel.org, linux@armlinux.org.uk, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 02.08.2020 19:54, Wolfram Sang wrote: > On Fri, Jun 19, 2020 at 05:19:02PM +0300, Codrin Ciubotariu wrote: >> Multiple I2C bus drivers use similar bindings to obtain information needed >> for I2C recovery. For example, for platforms using device-tree, the >> properties look something like this: >> >> &i2c { >> ... >> pinctrl-names = "default", "gpio"; >> // or pinctrl-names = "default", "recovery"; >> pinctrl-0 = <&pinctrl_i2c_default>; >> pinctrl-1 = <&pinctrl_i2c_gpio>; >> sda-gpios = <&pio 0 GPIO_ACTIVE_HIGH>; >> scl-gpios = <&pio 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; >> ... >> } >> >> For this reason, we can add this common initialization in the core. This >> way, other I2C bus drivers will be able to support GPIO recovery just by >> providing a pointer to platform's pinctrl and calling i2c_recover_bus() >> when SDA is stuck low. >> >> Signed-off-by: Codrin Ciubotariu > > Thanks, it looks a lot like what I had in mind! > >> --- >> drivers/i2c/i2c-core-base.c | 119 ++++++++++++++++++++++++++++++++++++ >> include/linux/i2c.h | 11 ++++ >> 2 files changed, 130 insertions(+) >> >> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c >> index d1f278f73011..4ee29fec4e93 100644 >> --- a/drivers/i2c/i2c-core-base.c >> +++ b/drivers/i2c/i2c-core-base.c >> @@ -32,6 +32,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -179,6 +180,8 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap) >> struct i2c_bus_recovery_info *bri = adap->bus_recovery_info; >> int i = 0, scl = 1, ret = 0; >> >> + if (bri->pinctrl) >> + pinctrl_select_state(bri->pinctrl, bri->pins_gpio); > > I think this should come after 'prepare_recovery'. It may be that > 'prepare_recovery' already needs to select the pinctrl state to avoid a > glitch. In this version, there would be a glitch then. If we move it > down, the doubled 'pinctrl_select_state' would be a noop then. Agree > >> if (bri->prepare_recovery) >> bri->prepare_recovery(adap); >> >> @@ -236,6 +239,8 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap) >> >> if (bri->unprepare_recovery) >> bri->unprepare_recovery(adap); >> + if (bri->pinctrl) >> + pinctrl_select_state(bri->pinctrl, bri->pins_default); > > Here it is OK and will still work with the PXA version which needs to > select the state on its own. > >> >> return ret; >> } >> @@ -251,6 +256,118 @@ int i2c_recover_bus(struct i2c_adapter *adap) >> } >> EXPORT_SYMBOL_GPL(i2c_recover_bus); >> >> +static void i2c_gpio_init_pinctrl_recovery(struct i2c_adapter *adap) >> +{ >> + struct i2c_bus_recovery_info *bri = adap->bus_recovery_info; >> + struct device *dev = &adap->dev; >> + struct pinctrl *p = bri->pinctrl; >> + >> + /* >> + * we can't change states without pinctrl, so remove the states if >> + * available > > s/available/populated/ ? OK > >> + */ >> + if (!p) { >> + bri->pins_default = NULL; >> + bri->pins_gpio = NULL; >> + return; >> + } >> + >> + if (!bri->pins_default) { >> + bri->pins_default = pinctrl_lookup_state(p, >> + PINCTRL_STATE_DEFAULT); >> + if (IS_ERR(bri->pins_default)) { >> + dev_dbg(dev, PINCTRL_STATE_DEFAULT " state not found for GPIO recovery\n"); >> + bri->pins_default = NULL; >> + >> + goto cleanup_pinctrl; > > I'd leave out the goto here. It is OK to check both parameters, I think. since default state is missing, the next check can be skipped, but I agree that removing the label makes things easier to read. > >> + } >> + } >> + if (!bri->pins_gpio) { >> + bri->pins_gpio = pinctrl_lookup_state(p, "gpio"); >> + if (IS_ERR(bri->pins_gpio)) >> + bri->pins_gpio = pinctrl_lookup_state(p, "recovery"); >> + >> + if (IS_ERR(bri->pins_gpio)) { >> + dev_dbg(dev, "no gpio or recovery state found for GPIO recovery\n"); >> + bri->pins_gpio = NULL; >> + >> + goto cleanup_pinctrl; > > This goto is not needed... right > >> + } >> + } >> + >> +cleanup_pinctrl: > > ... and this label can go then. Also nicer to read, I'd say. I will remove it. > >> + /* for pinctrl state changes, we need all the information */ >> + if (!bri->pins_default || !bri->pins_gpio) { >> + bri->pinctrl = NULL; >> + bri->pins_default = NULL; >> + bri->pins_gpio = NULL; >> + } else { >> + dev_info(dev, "using pinctrl states for GPIO recovery"); >> + } >> +} >> + >> +static int i2c_gpio_init_generic_recovery(struct i2c_adapter *adap) >> +{ >> + struct i2c_bus_recovery_info *bri = adap->bus_recovery_info; >> + struct device *dev = &adap->dev; >> + struct gpio_desc *gpiod; >> + int ret = 0; >> + >> + /* don't touch the recovery information if the driver is not using >> + * generic SCL recovery >> + */ > > Not kernel comment style. Right, sorry about this. Will fix. > >> + if (bri->recover_bus && bri->recover_bus != i2c_generic_scl_recovery) >> + return 0; > > No need for the first condition. 'i2c_generic_scl_recovery' is > definately not NULL :) It is not the same thing. Without the first check, it will return 0 if bri->recover_bus is NULL, which is not what we want. If bri->recover_bus is NULL (and the pintrl states and gpios are in place) we can set recover_bus to i2c_generic_scl_recovery and use the generic recovery. > >> + >> + /* >> + * pins might be taken as GPIO, so we might as well inform pinctrl about > > s/might as well/should/ OK > >> + * this and move the state to GPIO >> + */ >> + if (bri->pinctrl) >> + pinctrl_select_state(bri->pinctrl, bri->pins_gpio); >> + >> + /* >> + * if there is incomplete or no recovery information, see if generic >> + * GPIO recovery is available >> + */ >> + if (!bri->scl_gpiod) { >> + gpiod = devm_gpiod_get(dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN); >> + if (PTR_ERR(gpiod) == -EPROBE_DEFER) { >> + ret = -EPROBE_DEFER; >> + goto cleanup_pinctrl_state; >> + } >> + if (!IS_ERR(gpiod)) { >> + bri->scl_gpiod = gpiod; >> + bri->recover_bus = i2c_generic_scl_recovery; >> + dev_info(dev, "using generic GPIOs for recovery\n"); >> + } >> + } > > I think this extra code from the PXA driver makes sense in case SDA was > released while we were executing this code: > > 1383 /* > 1384 * We have SCL. Pull SCL low and wait a bit so that SDA glitches > 1385 * have no effect. > 1386 */ > 1387 gpiod_direction_output(bri->scl_gpiod, 0); > 1388 udelay(10); > 1389 bri->sda_gpiod = devm_gpiod_get(dev, "sda", GPIOD_OUT_HIGH_OPEN_DRAIN); > 1390 > 1391 /* Wait a bit in case of a SDA glitch, and then release SCL. */ > 1392 udelay(10); > 1393 gpiod_direction_output(bri->scl_gpiod, 1); I agree. I will add it. > >> + >> + /* SDA GPIOD line is optional, so we care about DEFER only */ >> + if (!bri->sda_gpiod) { >> + gpiod = devm_gpiod_get(dev, "sda", GPIOD_IN); >> + if (PTR_ERR(gpiod) == -EPROBE_DEFER) { >> + ret = -EPROBE_DEFER; >> + goto cleanup_pinctrl_state; >> + } >> + if (!IS_ERR(gpiod)) >> + bri->sda_gpiod = gpiod; >> + } >> + >> +cleanup_pinctrl_state: >> + /* change the state of the pins back to their default state */ >> + if (bri->pinctrl) >> + pinctrl_select_state(bri->pinctrl, bri->pins_default); >> + >> + return ret; >> +} >> + > > Rest looks good! If you have some time for this now, I will make sure to > get it into 5.9. With these minor things fixed, this is good to go, me > thinks. > I agree will all your suggestions, except with the removal of if (bri->recover_bus) . If you agree with this, I can send the next version and remove the RFC. Thanks and best regards, Codrin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel