From patchwork Sun Sep 17 09:39:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 112825 Delivered-To: patch@linaro.org Received: by 10.140.106.117 with SMTP id d108csp2503463qgf; Sun, 17 Sep 2017 02:39:30 -0700 (PDT) X-Received: by 10.159.198.7 with SMTP id f7mr11094218plo.38.1505641170335; Sun, 17 Sep 2017 02:39:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1505641170; cv=none; d=google.com; s=arc-20160816; b=CheJpB9MDbACAyCc85Uu20UcsqsRd60OG6cSJV+E9JPp2fu94BGu7udP3HSVixQYqK NIzLv6t8Vaubpqrs1n5HODS8bw7HLMI564v1xy5Ch92mmBSm2SrryP0NOaRgICsMxPl/ /pN5x9pLJnWDeARsat4sUajc3f6JD2FMlaCb2AlWh3KvKWP2Rmh2dqVjFQWo2UxalqeT QaVBPfA/gp8WB32AMoUcasOxwIKEUsv1qh4e9E03u5TDL1O4uhbYD8voOJMqBSVC654c /ndhmN5+PlTrHkzrsrF9I5yS9NVI8m/Qhd+3MtW42ZYj/AtdzhxKHkVo85eomu+yNW0E Btfg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:dkim-signature:arc-authentication-results; bh=sMEO26rO/HVCUOd9MrbeOMiq+/TQB6bwmfsePMgFblE=; b=ZGqdzZftOckXleNf0fV+DrFdaWQUEfJRUqnxNi3VVsU1hdXTH8NP6JwvMB/Pe5JrvH wbObE/GvlzbDtySFcNTd1wiprpMcLdZZ+vr9F/oYJ5F59lEtQ/lSEhyOVU0mGTKldfyn FNH1cLOj06KMBLcXehKVyT7OfV3tDCi/rHLsz64XzbDUK3rj9SFT0MGT33YsSfcnCZXw vkGXwVbE7/W3p6A/NPubjCUVMp+3TROc9JDM2PZdgnV3d2AWns1E1zozTunBwgriUQ2E kjPF7/FThTW/VU1qIpvESLIt06kyCr0Vc+y7eMn2r33fhy3hdupjXD18iHGF9ycPxpQW 6TYA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=SRGZkUh+; spf=pass (google.com: best guess record for domain of linux-i2c-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-i2c-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t8si3374838plz.730.2017.09.17.02.39.30; Sun, 17 Sep 2017 02:39:30 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-i2c-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=SRGZkUh+; spf=pass (google.com: best guess record for domain of linux-i2c-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-i2c-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750861AbdIQJj3 (ORCPT + 3 others); Sun, 17 Sep 2017 05:39:29 -0400 Received: from mail-lf0-f50.google.com ([209.85.215.50]:50500 "EHLO mail-lf0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751263AbdIQJj2 (ORCPT ); Sun, 17 Sep 2017 05:39:28 -0400 Received: by mail-lf0-f50.google.com with SMTP id d4so5626130lfj.7 for ; Sun, 17 Sep 2017 02:39:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=sMEO26rO/HVCUOd9MrbeOMiq+/TQB6bwmfsePMgFblE=; b=SRGZkUh+jlmM1XcDRuKpPEAYVkJ+SmUKXMGlF9mXtDjVD+KI5JI3D8s7uWgDrahZTz paTo5ppD9lotyHZB0YR299RAMoSHrWc9v75d8nJZoFPypKMRS99g8GsDPqbZ+XlrYTfN lXixBBk3vfSEeSwNuv90/MnjIcYBFvm0xoFwo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=sMEO26rO/HVCUOd9MrbeOMiq+/TQB6bwmfsePMgFblE=; b=aldcPtHfxP72edN12W68n58/4utZoP4VsM4LQUiru357EAHk3mFdEJxvx2qThhjBNS O1spl2H39aqkW2fWzmCqQd/HGmlu0Bxqznd05vU/4BdkYboY3L/3LzpzhbnA+Y04ZJIa Xn+DfoiYzZTvgGnyB1o+wowseJBgMxoc9OKVxjXnqmNcqlUWscz9npHoA86DrUgm+kL0 HoS5P8bqPmVNmapifb3mbJfAI1afn+ko8PQraCN8tMouuLkOxgsHFXY8gS0tsj6jKKhO uzzJjHC/3LkSS4yO+ayMPBwZwELeMRLUByVfO0ZY8zaUYQ8aYrbLCsTtgNIb6BnhIBIu aciQ== X-Gm-Message-State: AHPjjUhfZ8tfkzOVZ4o9NVGVsQfU31Byh+0mA+qqgu2RqVHrq4eJMTZW xish4YTJ3Nog7rhG X-Google-Smtp-Source: AOwi7QBrPfO+kzp6ycm0cRuVHanAWqPQ/X0J1Kr/TXlL5xuEoNy6Yt6LzJRRUA0sUwp2c/fQTssX3A== X-Received: by 10.46.80.17 with SMTP id e17mr4685246ljb.78.1505641166623; Sun, 17 Sep 2017 02:39:26 -0700 (PDT) Received: from fabina.bredbandsbolaget.se (c-2209e055.014-348-6c756e10.cust.bredbandsbolaget.se. [85.224.9.34]) by smtp.gmail.com with ESMTPSA id t84sm974559lfi.21.2017.09.17.02.39.25 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 17 Sep 2017 02:39:25 -0700 (PDT) From: Linus Walleij To: Wolfram Sang , linux-i2c@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org, linux-mips@linux-mips.org, adi-buildroot-devel@lists.sourceforge.net, Geert Uytterhoeven , Linus Walleij Subject: [PATCH 3/7] i2c: gpio: Enforce open drain through gpiolib Date: Sun, 17 Sep 2017 11:39:02 +0200 Message-Id: <20170917093906.16325-4-linus.walleij@linaro.org> X-Mailer: git-send-email 2.13.5 In-Reply-To: <20170917093906.16325-1-linus.walleij@linaro.org> References: <20170917093906.16325-1-linus.walleij@linaro.org> Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org The I2C GPIO bitbang driver currently emulates open drain behaviour by implementing what the gpiolib already does: not actively driving the line high, instead setting it to input. This makes no sense. Use the new facility in gpiolib to request the lines enforced into open drain mode, and let the open drain emulation already present in the gpiolib kick in and handle this. As a bonus: if the GPIO driver in the back-end actually supports open drain in hardware using the .set_config() callback, it will be utilized. That's correct: we never used that hardware feature before, instead relying on emulating open drain even if the GPIO controller could actually handle this for us. Users will sometimes get messages like this: gpio-485 (?): enforced open drain please flag it properly in DT/ACPI DSDT/board file gpio-486 (?): enforced open drain please flag it properly in DT/ACPI DSDT/board file i2c-gpio gpio-i2c: using lines 485 (SDA) and 486 (SCL) Which is completely proper: since the line is used as open drain, it should actually be flagged properly with e.g. gpios = <&gpio0 5 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>, <&gpio0 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>; Or similar facilities in board file descriptor tables or ACPI DSDT. Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Fix a minor typo in error path (copy-paste scl was sda) --- drivers/i2c/busses/i2c-gpio.c | 102 ++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 63 deletions(-) -- 2.13.5 diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c index b4664037eded..97b9c29e9429 100644 --- a/drivers/i2c/busses/i2c-gpio.c +++ b/drivers/i2c/busses/i2c-gpio.c @@ -25,23 +25,6 @@ struct i2c_gpio_private_data { struct i2c_gpio_platform_data pdata; }; -/* Toggle SDA by changing the direction of the pin */ -static void i2c_gpio_setsda_dir(void *data, int state) -{ - struct i2c_gpio_private_data *priv = data; - - /* - * This is a way of saying "do not drive - * me actively high" which means emulating open drain. - * The right way to do this is for gpiolib to - * handle this, by the function below. - */ - if (state) - gpiod_direction_input(priv->sda); - else - gpiod_direction_output(priv->sda, 0); -} - /* * Toggle SDA by changing the output value of the pin. This is only * valid for pins configured as open drain (i.e. setting the value @@ -54,17 +37,6 @@ static void i2c_gpio_setsda_val(void *data, int state) gpiod_set_value(priv->sda, state); } -/* Toggle SCL by changing the direction of the pin. */ -static void i2c_gpio_setscl_dir(void *data, int state) -{ - struct i2c_gpio_private_data *priv = data; - - if (state) - gpiod_direction_input(priv->scl); - else - gpiod_direction_output(priv->scl, 0); -} - /* * Toggle SCL by changing the output value of the pin. This is used * for pins that are configured as open drain and for output-only @@ -116,30 +88,13 @@ static int i2c_gpio_probe(struct platform_device *pdev) struct i2c_gpio_platform_data *pdata; struct i2c_algo_bit_data *bit_data; struct i2c_adapter *adap; + enum gpiod_flags gflags; int ret; priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; - /* First get the GPIO pins; if it fails, we'll defer the probe. */ - priv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, GPIOD_OUT_HIGH); - if (IS_ERR(priv->sda)) { - ret = PTR_ERR(priv->sda); - /* FIXME: hack in the old code, is this really necessary? */ - if (ret == -EINVAL) - ret = -EPROBE_DEFER; - return ret; - } - priv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, GPIOD_OUT_LOW); - if (IS_ERR(priv->scl)) { - ret = PTR_ERR(priv->scl); - /* FIXME: hack in the old code, is this really necessary? */ - if (ret == -EINVAL) - ret = -EPROBE_DEFER; - return ret; - } - adap = &priv->adap; bit_data = &priv->bit_data; pdata = &priv->pdata; @@ -157,27 +112,48 @@ static int i2c_gpio_probe(struct platform_device *pdev) } /* - * FIXME: this is a hack emulating the open drain emulation - * that gpiolib can already do for us. Make all clients properly - * flag their lines as open drain and get rid of this property - * and the special callback. + * First get the GPIO pins; if it fails, we'll defer the probe. + * If the SDA line is marked from platform data or device tree as + * "open drain" it means something outside of our control is making + * this line being handled as open drain, and we should just handle + * it as any other output. Else we enforce open drain as this is + * required for an I2C bus. */ - if (pdata->sda_is_open_drain) { - gpiod_direction_output(priv->sda, 1); - bit_data->setsda = i2c_gpio_setsda_val; - } else { - gpiod_direction_input(priv->sda); - bit_data->setsda = i2c_gpio_setsda_dir; + if (pdata->sda_is_open_drain) + gflags = GPIOD_OUT_HIGH; + else + gflags = GPIOD_OUT_HIGH_OPEN_DRAIN; + priv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, gflags); + if (IS_ERR(priv->sda)) { + ret = PTR_ERR(priv->sda); + /* FIXME: hack in the old code, is this really necessary? */ + if (ret == -EINVAL) + ret = -EPROBE_DEFER; + return ret; } - - if (pdata->scl_is_open_drain || pdata->scl_is_output_only) { - gpiod_direction_output(priv->scl, 1); - bit_data->setscl = i2c_gpio_setscl_val; - } else { - gpiod_direction_input(priv->scl); - bit_data->setscl = i2c_gpio_setscl_dir; + /* + * If the SCL line is marked from platform data or device tree as + * "open drain" it means something outside of our control is making + * this line being handled as open drain, and we should just handle + * it as any other output. Else we enforce open drain as this is + * required for an I2C bus. + */ + if (pdata->scl_is_open_drain) + gflags = GPIOD_OUT_LOW; + else + gflags = GPIOD_OUT_LOW_OPEN_DRAIN; + priv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, gflags); + if (IS_ERR(priv->scl)) { + ret = PTR_ERR(priv->scl); + /* FIXME: hack in the old code, is this really necessary? */ + if (ret == -EINVAL) + ret = -EPROBE_DEFER; + return ret; } + bit_data->setsda = i2c_gpio_setsda_val; + bit_data->setscl = i2c_gpio_setscl_val; + if (!pdata->scl_is_output_only) bit_data->getscl = i2c_gpio_getscl; bit_data->getsda = i2c_gpio_getsda;