From patchwork Wed Jan 10 13:17:06 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 10155151 X-Patchwork-Delegate: geert@linux-m68k.org Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8DA4C60231 for ; Wed, 10 Jan 2018 13:18:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7DB4928497 for ; Wed, 10 Jan 2018 13:18:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6DD1C284F9; Wed, 10 Jan 2018 13:18:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E9B0D28497 for ; Wed, 10 Jan 2018 13:18:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754910AbeAJNSU (ORCPT ); Wed, 10 Jan 2018 08:18:20 -0500 Received: from mail-io0-f196.google.com ([209.85.223.196]:42825 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754385AbeAJNRH (ORCPT ); Wed, 10 Jan 2018 08:17:07 -0500 Received: by mail-io0-f196.google.com with SMTP id 25so1615374ioj.9 for ; Wed, 10 Jan 2018 05:17:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=QV+Mb2NMsxzypB87d7IWRm1JbdJJcJmbJpSv2OuEWWE=; b=Ms8Xrx9OHuWaovpJ0V1tLBgjvkk+PuTBNix1QaFHM7MZauUAvIYDCLEPqy+3XvXRdh X3ZdmEWFFAYe26/SH8LkxEqoYOJOGn9txeBCrJnECrgH7Tie+qCgQvaeRpsEgb/Mqi9J giJpKBjt0+WmKKfe8OLHSwq18khhXGASAhdPo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=QV+Mb2NMsxzypB87d7IWRm1JbdJJcJmbJpSv2OuEWWE=; b=WHNdZ8j7T8QWIHs4Ie7e6Ip0cl8IKjJXh2Bws+t1uPqHw4gf6iYgeGBGxo2OuAJVIA G421EFXGkHhs/mynTmXinMVg9X75Pni8/FQLkQrHgTRUWpvlrxJGIJk1fGCnDWbczLHL H7SkaM1ccLr/+pQwQ0s5648SrLXvbOH4LwkjYLCoz8mC9uACjKnpnDK/fGgZYPtSA0H7 X9J61XJyXlVx/BcpP0GjA8zHhTKXYRUZOZH3hHK7ZEAV3fvdILVANQ73Ieg+wq3ZUN9e aSB1ifDn2QvUVw1L9gpsPY+CKT25iIwOZxGPk7Z+dtPwWQGoxw4v19KYLuSKrkdbM95s GOUw== X-Gm-Message-State: AKwxytd18LY8JCjWgo1F3U7V146G0CfIUgUluly0kT5NxTDtmBmxAQ8j JdgQ7gzuLqxEdauEKxu8dnl13Jq3MWC2tdYv0blH3Q== X-Google-Smtp-Source: ACJfBou6OBpmiCiXQnZi76IvkJnxsKuL0KkI9hoxgGt/wydNGnizqltihjCWhWYequfIv0hBQKvi+IDio1+Whn8OTaE= X-Received: by 10.107.195.68 with SMTP id t65mr785415iof.175.1515590226599; Wed, 10 Jan 2018 05:17:06 -0800 (PST) MIME-Version: 1.0 Received: by 10.79.198.7 with HTTP; Wed, 10 Jan 2018 05:17:06 -0800 (PST) In-Reply-To: <1515521301-4375-1-git-send-email-geert+renesas@glider.be> References: <1515521301-4375-1-git-send-email-geert+renesas@glider.be> From: Linus Walleij Date: Wed, 10 Jan 2018 14:17:06 +0100 Message-ID: Subject: Re: [PATCH] gpio: Add missing open drain/source handling to gpiod_set_value_cansleep() To: Geert Uytterhoeven Cc: Wolfram Sang , =?UTF-8?B?SmFuIEt1bmRyw6F0?= , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , linux-gpio@vger.kernel.org, linux-i2c@vger.kernel.org, Linux-Renesas , "linux-kernel@vger.kernel.org" Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Jan 9, 2018 at 7:08 PM, Geert Uytterhoeven wrote: > Since commit f11a04464ae57e8d ("i2c: gpio: Enable working over slow > can_sleep GPIOs"), probing the i2c RTC connected to an i2c-gpio bus on > r8a7740/armadillo fails with: > > rtc-s35390a 0-0030: error resetting chip > rtc-s35390a: probe of 0-0030 failed with error -5 > > More debug code reveals: > > i2c i2c-0: master_xfer[0] R, addr=0x30, len=1 > i2c i2c-0: NAK from device addr 0x30 msg #0 > s35390a_get_reg: ret = -6 > > Commit 02e479808b5d62f8 ("gpio: Alter semantics of *raw* operations to > actually be raw") moved open drain/source handling from > gpiod_set_raw_value_commit() to gpiod_set_value(), but forgot to take > into account that gpiod_set_value_cansleep() also needs this handling. > The i2c protocol mandates that i2c signals are open drain, hence i2c > communication fails. > > Fix this by adding the missing handling to gpiod_set_value_cansleep(), > using a new common helper __gpiod_set_value_nocheck(). > > Fixes: 02e479808b5d62f8 ("gpio: Alter semantics of *raw* operations to actually be raw") > Signed-off-by: Geert Uytterhoeven > --- > The bad gpio commit is in v4.15-rc1 and later. > The i2c commit exposing this is not yet upstream, but in i2c/for-next. Whoops! It appears Meltdown and Spectre saves my ass because now I have time to get this upstream before v4.15... Applied for fixes with the following tweaks: I removed the __prefix (I don't like them) and added kerneldoc. value = !value; @@ -2919,7 +2928,7 @@ void gpiod_set_value(struct gpio_desc *desc, int value) { VALIDATE_DESC_VOID(desc); WARN_ON(desc->gdev->chip->can_sleep); - __gpiod_set_value_nocheck(desc, value); + gpiod_set_value_nocheck(desc, value); } EXPORT_SYMBOL_GPL(gpiod_set_value); @@ -3247,7 +3256,7 @@ void gpiod_set_value_cansleep(struct gpio_desc *desc, int value) { might_sleep_if(extra_checks); VALIDATE_DESC_VOID(desc); - __gpiod_set_value_nocheck(desc, value); + gpiod_set_value_nocheck(desc, value); } EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep); Yours, Linus Walleij diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 1693bb9ab72c..14532d9576e4 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -2892,7 +2892,16 @@ void gpiod_set_raw_value(struct gpio_desc *desc, int value) } EXPORT_SYMBOL_GPL(gpiod_set_raw_value); -static void __gpiod_set_value_nocheck(struct gpio_desc *desc, int value) +/** + * gpiod_set_value_nocheck() - set a GPIO line value without checking + * @desc: the descriptor to set the value on + * @value: value to set + * + * This sets the value of a GPIO line backing a descriptor, applying + * different semantic quirks like active low and open drain/source + * handling. + */ +static void gpiod_set_value_nocheck(struct gpio_desc *desc, int value) { if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))