Patch Detail

HTTP 200 OK
Allow: GET, PUT, PATCH, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

{
    "id": 812184,
    "url": "http://patchwork.ozlabs.org/api/patches/812184/?format=api",
    "web_url": "http://patchwork.ozlabs.org/project/linux-i2c/patch/20170910214424.14945-4-linus.walleij@linaro.org/",
    "project": {
        "id": 35,
        "url": "http://patchwork.ozlabs.org/api/projects/35/?format=api",
        "name": "Linux I2C development",
        "link_name": "linux-i2c",
        "list_id": "linux-i2c.vger.kernel.org",
        "list_email": "linux-i2c@vger.kernel.org",
        "web_url": "",
        "scm_url": "",
        "webscm_url": "",
        "list_archive_url": "",
        "list_archive_url_format": "",
        "commit_url_format": ""
    },
    "msgid": "<20170910214424.14945-4-linus.walleij@linaro.org>",
    "list_archive_url": null,
    "date": "2017-09-10T21:44:22",
    "name": "[3/5] i2c: gpio: Enforce open drain through gpiolib",
    "commit_ref": null,
    "pull_url": null,
    "state": "superseded",
    "archived": false,
    "hash": "01a041afd4f8b18ff677b8ab7ea252de639d9f3f",
    "submitter": {
        "id": 7055,
        "url": "http://patchwork.ozlabs.org/api/people/7055/?format=api",
        "name": "Linus Walleij",
        "email": "linus.walleij@linaro.org"
    },
    "delegate": null,
    "mbox": "http://patchwork.ozlabs.org/project/linux-i2c/patch/20170910214424.14945-4-linus.walleij@linaro.org/mbox/",
    "series": [
        {
            "id": 2418,
            "url": "http://patchwork.ozlabs.org/api/series/2418/?format=api",
            "web_url": "http://patchwork.ozlabs.org/project/linux-i2c/list/?series=2418",
            "date": "2017-09-10T21:44:19",
            "name": "I2C GPIO to use gpiolibs open drain",
            "version": 1,
            "mbox": "http://patchwork.ozlabs.org/series/2418/mbox/"
        }
    ],
    "comments": "http://patchwork.ozlabs.org/api/patches/812184/comments/",
    "check": "pending",
    "checks": "http://patchwork.ozlabs.org/api/patches/812184/checks/",
    "tags": {},
    "related": [],
    "headers": {
        "Return-Path": "<linux-i2c-owner@vger.kernel.org>",
        "X-Original-To": "incoming@patchwork.ozlabs.org",
        "Delivered-To": "patchwork-incoming@bilbo.ozlabs.org",
        "Authentication-Results": [
            "ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)",
            "ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"K9wnJTVx\"; dkim-atps=neutral"
        ],
        "Received": [
            "from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xr4N173bfz9t2V\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 11 Sep 2017 07:44:49 +1000 (AEST)",
            "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751901AbdIJVos (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tSun, 10 Sep 2017 17:44:48 -0400",
            "from mail-lf0-f44.google.com ([209.85.215.44]:35104 \"EHLO\n\tmail-lf0-f44.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751827AbdIJVos (ORCPT\n\t<rfc822; linux-i2c@vger.kernel.org>); Sun, 10 Sep 2017 17:44:48 -0400",
            "by mail-lf0-f44.google.com with SMTP id d17so14433161lfe.2\n\tfor <linux-i2c@vger.kernel.org>; Sun, 10 Sep 2017 14:44:47 -0700 (PDT)",
            "from fabina.bredbandsbolaget.se\n\t(c-2209e055.014-348-6c756e10.cust.bredbandsbolaget.se.\n\t[85.224.9.34]) by smtp.gmail.com with ESMTPSA id\n\tc69sm1461546ljd.42.2017.09.10.14.44.45\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 10 Sep 2017 14:44:45 -0700 (PDT)"
        ],
        "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references;\n\tbh=qamg7dzUxgSbxr03vx7pNM+c8xh7ROyHnpPPQZG2SdY=;\n\tb=K9wnJTVxh3aqlGk5rMXhFJ7HDpxxJjICrNJEnSzuZSl6Lkt3A0TZ532jflbhaUOyPj\n\tXZoF0GOJJZOLE3T028lJFYtpPDkOKGh+s1IaM1RiAGcw+6KJ9GuRc8h5xGmWt6rZr8wI\n\toxUlXRBYh7k0EWhEdAkiaTPRGpTugN6PN/c6w=",
        "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references;\n\tbh=qamg7dzUxgSbxr03vx7pNM+c8xh7ROyHnpPPQZG2SdY=;\n\tb=I5xrA4Gh62DKi0SWYDX1ndURlS4yr5ndmXhNQgJyAPPIuqrpM7w2+RdSHlzVv3HFQT\n\ts3ACsbo1ef5tNZgu4RlwOwMHRcms9PEX+F2KW5ZIVQj2FxNQy+Sxewhj7qLhEmTra377\n\tXD4VUpgROGVGEt5oy04L83T9Wp0NHHwY39bgRtcpZ+kxi87eLfSOr192S3XL7WMMvkff\n\tWvVcLFQUfUOyTV/ikiyU4dJghbS3uyArOog8QDMwqT4j06i2kk0PVvIbe6Q9dFqqtTPB\n\tj68O7FUJ9QNV2uYXNQjUsh2rqAvI9XNaSDTTVAhsfdlbSzhFE3SOWr+mg5SJNKJr/leb\n\tJ8YQ==",
        "X-Gm-Message-State": "AHPjjUjPL6lnQ/XB4wmeNprEgTg3XeR6LX1nnx5OyhY0rlJ5tUrEAIgy\n\t8KKhts36tBJh9ljv",
        "X-Google-Smtp-Source": "AOwi7QBsTEddbKcMVr9lo+LaMZfjoo6hYJNH57JHWlX93mwIvqoJWaPKSy/3QAxs6VZJOd6Hktb3/g==",
        "X-Received": "by 10.46.4.220 with SMTP id a89mr3530476ljf.40.1505079886476;\n\tSun, 10 Sep 2017 14:44:46 -0700 (PDT)",
        "From": "Linus Walleij <linus.walleij@linaro.org>",
        "To": "Wolfram Sang <wsa@the-dreams.de>, linux-i2c@vger.kernel.org",
        "Cc": "linux-arm-kernel@lists.infradead.org, linux-mips@linux-mips.org,\n\tadi-buildroot-devel@lists.sourceforge.net,\n\tLinus Walleij <linus.walleij@linaro.org>",
        "Subject": "[PATCH 3/5] i2c: gpio: Enforce open drain through gpiolib",
        "Date": "Sun, 10 Sep 2017 23:44:22 +0200",
        "Message-Id": "<20170910214424.14945-4-linus.walleij@linaro.org>",
        "X-Mailer": "git-send-email 2.13.5",
        "In-Reply-To": "<20170910214424.14945-1-linus.walleij@linaro.org>",
        "References": "<20170910214424.14945-1-linus.walleij@linaro.org>",
        "Sender": "linux-i2c-owner@vger.kernel.org",
        "Precedence": "bulk",
        "List-ID": "<linux-i2c.vger.kernel.org>",
        "X-Mailing-List": "linux-i2c@vger.kernel.org"
    },
    "content": "The I2C GPIO bitbang driver currently emulates open drain\nbehaviour by implementing what the gpiolib already does:\nnot actively driving the line high, instead setting it to\ninput.\n\nThis makes no sense. Use the new facility in gpiolib to\nrequest the lines enforced into open drain mode, and let\nthe open drain emulation already present in the gpiolib\nkick in and handle this.\n\nAs a bonus: if the GPIO driver in the back-end actually\nsupports open drain in hardware using the .set_config()\ncallback, it will be utilized. That's correct: we never\nused that hardware feature before, instead relying on\nemulating open drain even if the GPIO controller could\nactually handle this for us.\n\nUsers will sometimes get messages like this:\ngpio-485 (?): enforced open drain please flag it properly\n  in DT/ACPI DSDT/board file\ngpio-486 (?): enforced open drain please flag it properly\n  in DT/ACPI DSDT/board file\ni2c-gpio gpio-i2c: using lines 485 (SDA) and 486 (SCL)\n\nWhich is completely proper: since the line is used as\nopen drain, it should actually be flagged properly with\ne.g.\n\ngpios = <&gpio0 5 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>,\n        <&gpio0 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;\n\nOr similar facilities in board file descriptor tables\nor ACPI DSDT.\n\nSigned-off-by: Linus Walleij <linus.walleij@linaro.org>\n---\n drivers/i2c/busses/i2c-gpio.c | 102 ++++++++++++++++--------------------------\n 1 file changed, 39 insertions(+), 63 deletions(-)",
    "diff": "diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c\nindex 49e8b3ab30be..18ac4cf2ee75 100644\n--- a/drivers/i2c/busses/i2c-gpio.c\n+++ b/drivers/i2c/busses/i2c-gpio.c\n@@ -25,23 +25,6 @@ struct i2c_gpio_private_data {\n \tstruct i2c_gpio_platform_data pdata;\n };\n \n-/* Toggle SDA by changing the direction of the pin */\n-static void i2c_gpio_setsda_dir(void *data, int state)\n-{\n-\tstruct i2c_gpio_private_data *priv = data;\n-\n-\t/*\n-\t * This is a way of saying \"do not drive\n-\t * me actively high\" which means emulating open drain.\n-\t * The right way to do this is for gpiolib to\n-\t * handle this, by the function below.\n-\t */\n-\tif (state)\n-\t\tgpiod_direction_input(priv->sda);\n-\telse\n-\t\tgpiod_direction_output(priv->sda, 0);\n-}\n-\n /*\n  * Toggle SDA by changing the output value of the pin. This is only\n  * valid for pins configured as open drain (i.e. setting the value\n@@ -54,17 +37,6 @@ static void i2c_gpio_setsda_val(void *data, int state)\n \tgpiod_set_value(priv->sda, state);\n }\n \n-/* Toggle SCL by changing the direction of the pin. */\n-static void i2c_gpio_setscl_dir(void *data, int state)\n-{\n-\tstruct i2c_gpio_private_data *priv = data;\n-\n-\tif (state)\n-\t\tgpiod_direction_input(priv->scl);\n-\telse\n-\t\tgpiod_direction_output(priv->scl, 0);\n-}\n-\n /*\n  * Toggle SCL by changing the output value of the pin. This is used\n  * for pins that are configured as open drain and for output-only\n@@ -116,30 +88,13 @@ static int i2c_gpio_probe(struct platform_device *pdev)\n \tstruct i2c_gpio_platform_data *pdata;\n \tstruct i2c_algo_bit_data *bit_data;\n \tstruct i2c_adapter *adap;\n+\tenum gpiod_flags gflags;\n \tint ret;\n \n \tpriv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);\n \tif (!priv)\n \t\treturn -ENOMEM;\n \n-\t/* First get the GPIO pins; if it fails, we'll defer the probe. */\n-\tpriv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, GPIOD_OUT_HIGH);\n-\tif (IS_ERR(priv->sda)) {\n-\t\tret = PTR_ERR(priv->sda);\n-\t\t/* FIXME: hack in the old code, is this really necessary? */\n-\t\tif (ret == -EINVAL)\n-\t\t\tret = -EPROBE_DEFER;\n-\t\treturn ret;\n-\t}\n-\tpriv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, GPIOD_OUT_LOW);\n-\tif (IS_ERR(priv->scl)) {\n-\t\tret = PTR_ERR(priv->sda);\n-\t\t/* FIXME: hack in the old code, is this really necessary? */\n-\t\tif (ret == -EINVAL)\n-\t\t\tret = -EPROBE_DEFER;\n-\t\treturn ret;\n-\t}\n-\n \tadap = &priv->adap;\n \tbit_data = &priv->bit_data;\n \tpdata = &priv->pdata;\n@@ -157,27 +112,48 @@ static int i2c_gpio_probe(struct platform_device *pdev)\n \t}\n \n \t/*\n-\t * FIXME: this is a hack emulating the open drain emulation\n-\t * that gpiolib can already do for us. Make all clients properly\n-\t * flag their lines as open drain and get rid of this property\n-\t * and the special callback.\n+\t * First get the GPIO pins; if it fails, we'll defer the probe.\n+\t * If the SDA line is marked from platform data or device tree as\n+\t * \"open drain\" it means something outside of our control is making\n+\t * this line being handled as open drain, and we should just handle\n+\t * it as any other output. Else we enforce open drain as this is\n+\t * required for an I2C bus.\n \t */\n-\tif (pdata->sda_is_open_drain) {\n-\t\tgpiod_direction_output(priv->sda, 1);\n-\t\tbit_data->setsda = i2c_gpio_setsda_val;\n-\t} else {\n-\t\tgpiod_direction_input(priv->sda);\n-\t\tbit_data->setsda = i2c_gpio_setsda_dir;\n+\tif (pdata->sda_is_open_drain)\n+\t\tgflags = GPIOD_OUT_HIGH;\n+\telse\n+\t\tgflags = GPIOD_OUT_HIGH_OPEN_DRAIN;\n+\tpriv->sda = devm_gpiod_get_index(&pdev->dev, NULL, 0, gflags);\n+\tif (IS_ERR(priv->sda)) {\n+\t\tret = PTR_ERR(priv->sda);\n+\t\t/* FIXME: hack in the old code, is this really necessary? */\n+\t\tif (ret == -EINVAL)\n+\t\t\tret = -EPROBE_DEFER;\n+\t\treturn ret;\n \t}\n-\n-\tif (pdata->scl_is_open_drain || pdata->scl_is_output_only) {\n-\t\tgpiod_direction_output(priv->scl, 1);\n-\t\tbit_data->setscl = i2c_gpio_setscl_val;\n-\t} else {\n-\t\tgpiod_direction_input(priv->scl);\n-\t\tbit_data->setscl = i2c_gpio_setscl_dir;\n+\t/*\n+\t * If the SCL line is marked from platform data or device tree as\n+\t * \"open drain\" it means something outside of our control is making\n+\t * this line being handled as open drain, and we should just handle\n+\t * it as any other output. Else we enforce open drain as this is\n+\t * required for an I2C bus.\n+\t */\n+\tif (pdata->scl_is_open_drain)\n+\t\tgflags = GPIOD_OUT_LOW;\n+\telse\n+\t\tgflags = GPIOD_OUT_LOW_OPEN_DRAIN;\n+\tpriv->scl = devm_gpiod_get_index(&pdev->dev, NULL, 1, gflags);\n+\tif (IS_ERR(priv->scl)) {\n+\t\tret = PTR_ERR(priv->sda);\n+\t\t/* FIXME: hack in the old code, is this really necessary? */\n+\t\tif (ret == -EINVAL)\n+\t\t\tret = -EPROBE_DEFER;\n+\t\treturn ret;\n \t}\n \n+\tbit_data->setsda = i2c_gpio_setsda_val;\n+\tbit_data->setscl = i2c_gpio_setscl_val;\n+\n \tif (!pdata->scl_is_output_only)\n \t\tbit_data->getscl = i2c_gpio_getscl;\n \tbit_data->getsda = i2c_gpio_getsda;\n",
    "prefixes": [
        "3/5"
    ]
}