[{"id":1766290,"web_url":"http://patchwork.ozlabs.org/comment/1766290/","msgid":"<CA+M3ks4y0u=ozKiq=xHFvcKJcBjDZDQysU6Mac+i1nEGTW6xOg@mail.gmail.com>","list_archive_url":null,"date":"2017-09-11T12:55:19","subject":"Re: [RESEND PATCH v3 0/5] Add support for the STM32F7 I2C","submitter":{"id":66874,"url":"http://patchwork.ozlabs.org/api/people/66874/","name":"Benjamin Gaignard","email":"benjamin.gaignard@linaro.org"},"content":"2017-09-01 9:34 GMT+02:00 Pierre-Yves MORDRET <pierre-yves.mordret@st.com>:\n> This patchset adds support for the I2C controller embedded in STM32F7xx SoC.\n> It enables I2C transfer in interrupt mode with Standard-mode, Fast-mode and\n> Fast-mode+ bus speed.\n\nHi Wolfram,\n\nI notice that those patches aren't in pull request for 4.14 and not in\ni2c-next branch.\nWhat can we do to progress in this topic? Does issues/remarks still\nneed to be fixed in this code ?\n\nThanks for your advices,\nBenjamin\n\n> ---\n>  Version history:\n>     v3:\n>         * Move stm32f7_i2c_match above stm32f7_i2c_driver\n>         * of_device_get_match_data instead of of_match_device\n>         * Improve I2C Speed DT gathering\n>         * dev_err into dev_dbg for Arbitration loss\n>         * Remove useless space aligned\n>\n>     v2:\n>         * Implement an I2C timings computation algorithm instead of static\n>           values(bindings). Algorithm uses generic I2C SCL Falling/Rising\n>           bindings and System clock to compute its timings.\n>         * I2C Device Tree Update\n> ---\n> Pierre-Yves MORDRET (5):\n>   dt-bindings: i2c-stm32: Document the STM32F7 I2C bindings\n>   i2c: i2c-stm32f4: use generic definition of speed enum\n>   i2c: i2c-stm32f7: add driver\n>   ARM: dts: stm32: Add I2C1 support for STM32F746 SoC\n>   ARM: dts: stm32: Add I2C1 support for STM32F746 eval board\n>\n>  .../devicetree/bindings/i2c/i2c-stm32.txt          |  29 +-\n>  arch/arm/boot/dts/stm32746g-eval.dts               |   8 +\n>  arch/arm/boot/dts/stm32f746.dtsi                   |  22 +\n>  drivers/i2c/busses/Kconfig                         |  10 +\n>  drivers/i2c/busses/Makefile                        |   1 +\n>  drivers/i2c/busses/i2c-stm32.h                     |  20 +\n>  drivers/i2c/busses/i2c-stm32f4.c                   |  18 +-\n>  drivers/i2c/busses/i2c-stm32f7.c                   | 974 +++++++++++++++++++++\n>  8 files changed, 1068 insertions(+), 14 deletions(-)\n>  create mode 100644 drivers/i2c/busses/i2c-stm32.h\n>  create mode 100644 drivers/i2c/busses/i2c-stm32f7.c\n>\n> --\n> 2.7.4\n>\n>\n> _______________________________________________\n> linux-arm-kernel mailing list\n> linux-arm-kernel@lists.infradead.org\n> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel","headers":{"Return-Path":"<linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org>","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org header.b=\"FTSKOD3q\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"fTsHno41\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrSbD4l5pz9s7B\n\tfor <incoming-imx@patchwork.ozlabs.org>;\n\tMon, 11 Sep 2017 22:55:52 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1drOFh-0001sY-E3; Mon, 11 Sep 2017 12:55:49 +0000","from mail-io0-x22c.google.com ([2607:f8b0:4001:c06::22c])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1drOFd-0001nb-5z for linux-arm-kernel@lists.infradead.org;\n\tMon, 11 Sep 2017 12:55:47 +0000","by mail-io0-x22c.google.com with SMTP id j141so25333152ioj.4\n\tfor <linux-arm-kernel@lists.infradead.org>;\n\tMon, 11 Sep 2017 05:55:21 -0700 (PDT)","by 10.157.80.140 with HTTP; Mon, 11 Sep 2017 05:55:19 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:\n\tReferences:In-Reply-To:MIME-Version:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=+nAIXb1t2LdVnhZr0ooz6BYLHdo3xQFHSrl7MChLfng=;\n\tb=FTSKOD3q383hm7\n\tviFbYD3vQlPcApk2KDZpYye7qg1M8EjAK7bRM9oRINl2jGLnz7UClZ1OvkuTQDG4Ag3YpodyQ3psR\n\tbLML9CIJ6hoR0UTuhQ4HBrbFm1GPEDTh39bJAI6q7UilRleww6UF+2me0sSVutnoEWBFHDQ+ekfWw\n\t43g42IjduT1zOWmZekxVLGrog6Re5T8vKZ3cIjKMEPcjTpK03dds1GaV1DZoq9G/7ug1qkOdRM0+0\n\t13NGv4o1z+/NmDoEaX84wiAbkYTwADRTqTK4n9XY9XYSvgbIsO8wZND9+qPxS99KMPpLYaz3N9Co9\n\t+cfO4LFvn7aVGc8fDtyg==;","v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=01X+qECgzdXFN/eTFfePsmQl1LROrdjfg9/mGYNips8=;\n\tb=fTsHno41rdVICapwjnZB0nvwRqjxNChuRaxt80+uX0z45rL2cYbbeHDjyqcfLEkk6V\n\tZAOvwhXjnW5xcRn3JBfTs3MjNSqRVHf3XQijhgE1yNDFUsnGK9J6VI46rClLJo+6bpsw\n\tm/+cQVm4kbt8am2ZqhFgVNsAyx2ClWKX4uWss="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=01X+qECgzdXFN/eTFfePsmQl1LROrdjfg9/mGYNips8=;\n\tb=OJuVmkJxv7JLuvUTpR1+Wojo85CHRiAFGmQ9EsfBUGC2wDLdBy46irgMk2P80hFGyZ\n\t0Rp5mg0D5a6z2iwIw+udONRJR42re7aEZ1aEhlE3VMfM8MfAgBQw5bkDD5ugha3JD7lX\n\tIu99V9PsYLkyhN3RRPlHhc7yMymLSMzUx5ABqs0prlvb++g1wz0XJqb4GZ9fHtC6BwQH\n\tnSOOFj/v5b9/51Rgm9LOChOVo44xIvGMgoCprFxgUlnBjd29jNfCNmirOy+V9nOVXtxV\n\tty5PH4i2KFkrHyr5gUSKujholK0/UXmCFcx418a8d5wbgibApoilHh6iheN8hjDxDSuX\n\t8RtA==","X-Gm-Message-State":"AHPjjUiAUw/iDUgIhnyHTN7bwMKOm0ZuB/7kE4fqyawQuDqfBY1qpU+N\n\tKMI28YhqaJ1mrhdFq5pmPtebLoGpQnuS","X-Google-Smtp-Source":"AOwi7QAqU3sL97FS9xB4eBwWzCwo0s6a+Vl7BC5Y300QikQY0CJgMyjy+oQb6ZiTjCjUMB7Ew2DxfTefSyok8rW1uiA=","X-Received":"by 10.202.3.213 with SMTP id 204mr12016015oid.205.1505134520423; \n\tMon, 11 Sep 2017 05:55:20 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1504251255-20469-1-git-send-email-pierre-yves.mordret@st.com>","References":"<1504251255-20469-1-git-send-email-pierre-yves.mordret@st.com>","From":"Benjamin Gaignard <benjamin.gaignard@linaro.org>","Date":"Mon, 11 Sep 2017 14:55:19 +0200","Message-ID":"<CA+M3ks4y0u=ozKiq=xHFvcKJcBjDZDQysU6Mac+i1nEGTW6xOg@mail.gmail.com>","Subject":"Re: [RESEND PATCH v3 0/5] Add support for the STM32F7 I2C","To":"Pierre-Yves MORDRET <pierre-yves.mordret@st.com>","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170911_055545_268841_279AE759 ","X-CRM114-Status":"GOOD (  16.02  )","X-Spam-Score":"-2.7 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details:   (-2.7 points)\n\tpts rule name              description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow\n\ttrust [2607:f8b0:4001:c06:0:0:0:22c listed in] [list.dnswl.org]\n\t-0.0 SPF_PASS               SPF: sender matches SPF record\n\t-1.9 BAYES_00               BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]\n\t-0.1 DKIM_VALID Message has at least one valid DKIM or DK signature\n\t0.1 DKIM_SIGNED            Message has a DKIM or DK signature,\n\tnot necessarily valid\n\t-0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from\n\tauthor's domain","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":"<http://lists.infradead.org/mailman/options/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>","List-Archive":"<http://lists.infradead.org/pipermail/linux-arm-kernel/>","List-Post":"<mailto:linux-arm-kernel@lists.infradead.org>","List-Help":"<mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>","List-Subscribe":"<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>","Cc":"Mark Rutland <mark.rutland@arm.com>, devicetree@vger.kernel.org,\n\tAlexandre Torgue <alexandre.torgue@st.com>,\n\tWolfram Sang <wsa@the-dreams.de>, Russell King <linux@armlinux.org.uk>,\n\tLinux Kernel Mailing List <linux-kernel@vger.kernel.org>,\n\tRob Herring <robh+dt@kernel.org>, linux-i2c@vger.kernel.org,\n\tMaxime Coquelin <mcoquelin.stm32@gmail.com>,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"\"linux-arm-kernel\" <linux-arm-kernel-bounces@lists.infradead.org>","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1768196,"web_url":"http://patchwork.ozlabs.org/comment/1768196/","msgid":"<20170913212639.2zyvg24364ovwfxh@ninjato>","list_archive_url":null,"date":"2017-09-13T21:26:39","subject":"Re: [RESEND PATCH v3 3/5] i2c: i2c-stm32f7: add driver","submitter":{"id":22495,"url":"http://patchwork.ozlabs.org/api/people/22495/","name":"Wolfram Sang","email":"wsa@the-dreams.de"},"content":"Hi,\n\nthanks for this driver!\n\n> +/**\n> + * struct stm32f7_i2c_spec - private i2c specification timing\n> + * @rate: I2C bus speed (Hz)\n> + * @rate_min: 80% of I2C bus speed (Hz)\n> + * @rate_max: 120% of I2C bus speed (Hz)\n\nYou would generate a clock which is higher than the requested one?\nThis is highly unusual. Any special reason?\n\n> + * @fall_max: Max fall time of both SDA and SCL signals (ns)\n> + * @rise_max: Max rise time of both SDA and SCL signals (ns)\n> + * @hddat_min: Min data hold time (ns)\n> + * @vddat_max: Max data valid time (ns)\n> + * @sudat_min: Min data setup time (ns)\n> + * @l_min: Min low period of the SCL clock (ns)\n> + * @h_min: Min high period of the SCL clock (ns)\n> + */\n> +static struct stm32f7_i2c_spec i2c_specs[] = {\n> +\t[STM32_I2C_SPEED_STANDARD] = {\n> +\t\t.rate = 100000,\n> +\t\t.rate_min = 8000,\n\nThis is not 80%. Typo?\n\n> +\t\t.rate_max = 120000,\n> +\t\t.fall_max = 300,\n> +\t\t.rise_max = 1000,\n> +\t\t.hddat_min = 0,\n> +\t\t.vddat_max = 3450,\n> +\t\t.sudat_min = 250,\n> +\t\t.l_min = 4700,\n> +\t\t.h_min = 4000,\n> +\t},\n\n...\n\n> +\t/*\n> +\t * Among Prescaler possibilities discovered above figures out SCL Low\n> +\t * and High Period. Provided:\n> +\t * - SCL Low Period has to be higher than Low Period of tehs SCL Clock\n\ntehs?\n\n> +\t *   defined by I2C Specification. I2C Clock has to be lower than\n> +\t *   (SCL Low Period - Analog/Digital filters) / 4.\n> +\t * - SCL High Period has to be lower than High Period of the SCL Clock\n> +\t *   defined by I2C Specification\n> +\t * - I2C Clock has to be lower than SCL High Period\n> +\t */\n\n...\n\n> +\t/* NACK received */\n> +\tif (status & STM32F7_I2C_ISR_NACKF) {\n> +\t\tdev_dbg(i2c_dev->dev, \"<%s>: Receive NACK\\n\", __func__);\n> +\t\twritel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);\n> +\t\tf7_msg->result = -EBADE;\n\n-ENXIO (see Documentation/i2c/fault-codes)\n\n...\n\n> +\ttimeout = wait_for_completion_timeout(&i2c_dev->complete,\n> +\t\t\t\t\t      i2c_dev->adap.timeout);\n> +\tret = f7_msg->result;\n> +\n> +\tif (!timeout) {\n> +\t\tdev_dbg(i2c_dev->dev, \"Access to slave 0x%x timed out\\n\",\n> +\t\t\ti2c_dev->msg->addr);\n> +\t\tret = -ETIMEDOUT;\n> +\t}\n\nCould you rename the variable to time_left? It looks strange, basically:\n\n\tif (!timeout)\n\t\treturn -ETIMEDOUT\n\n...\n\n> +\tadap->retries = 0;\n\nWhy no retries when you check for arbitration lost?\n\n> +\tadap->algo = &stm32f7_i2c_algo;\n> +\tadap->dev.parent = &pdev->dev;\n> +\tadap->dev.of_node = pdev->dev.of_node;\n> +\n> +\tinit_completion(&i2c_dev->complete);\n> +\n> +\tret = i2c_add_adapter(adap);\n> +\tif (ret) {\n> +\t\tdev_err(&pdev->dev, \"Failed to add adapter\\n\");\n\nPlease remove, the core will print info when adding fails.\n\n\nRest looks good!\n\nThanks,\n\n   Wolfram","headers":{"Return-Path":"<linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org>","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"lUB/Daa4\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsvrN0qy4z9s81\n\tfor <incoming-imx@patchwork.ozlabs.org>;\n\tThu, 14 Sep 2017 07:27:16 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsFBf-0003ru-LC; Wed, 13 Sep 2017 21:27:11 +0000","from sauhun.de ([88.99.104.3] helo=pokefinder.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsFBW-0003nr-Sc for linux-arm-kernel@lists.infradead.org;\n\tWed, 13 Sep 2017 21:27:08 +0000","from localhost (p54B333DB.dip0.t-ipconnect.de [84.179.51.219])\n\tby pokefinder.org (Postfix) with ESMTPSA id 21A9B2C3256;\n\tWed, 13 Sep 2017 23:26:40 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type:Cc:\n\tList-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:\n\tIn-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To:\n\tContent-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:\n\tResent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner;\n\tbh=l7gZADmfVBCFy5xaaHGyh9fGaverWetGGUEnmDpKecQ=;\n\tb=lUB/Daa4X7xyIizGiBd+IY37+\n\tyeWxM2mcYhhxr+lrO+3gDiRXh5iC32K5eUqRgb0xPU3aqZ574Mww3JA0CTeOx+ooH+zflJUJ+fo4x\n\trCqODBBjfgEALlPbEIe7J71BqDE6isgE4XEeLQs+VBxGRDoXeuOJyGQ5vhP8yJmD1ZvMoDWjBTqCm\n\tDXWLyM9C0g6OTAefKkH18Dt79PKX8CRv6EmRZzB/GUpsovEgqAD1X4/mYc5dQ5gwZdZrjUNuLGEHd\n\tuUoHe6+cQwFIPK73iziE4XmuNgZLJtAWNfNh4cgrTpIgE+jGLwJ4lkOjq7jsXMfRazJ2PYlm/TeFp\n\tHAZMj27Ww==;","Date":"Wed, 13 Sep 2017 23:26:39 +0200","From":"Wolfram Sang <wsa@the-dreams.de>","To":"Pierre-Yves MORDRET <pierre-yves.mordret@st.com>","Subject":"Re: [RESEND PATCH v3 3/5] i2c: i2c-stm32f7: add driver","Message-ID":"<20170913212639.2zyvg24364ovwfxh@ninjato>","References":"<1504251255-20469-1-git-send-email-pierre-yves.mordret@st.com>\n\t<1504251255-20469-4-git-send-email-pierre-yves.mordret@st.com>","MIME-Version":"1.0","In-Reply-To":"<1504251255-20469-4-git-send-email-pierre-yves.mordret@st.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170913_142703_185347_29A26E30 ","X-CRM114-Status":"GOOD (  12.95  )","X-Spam-Score":"-1.9 (-)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details:   (-1.9 points)\n\tpts rule name              description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.0 SPF_PASS               SPF: sender matches SPF record\n\t-0.0 SPF_HELO_PASS          SPF: HELO matches SPF record\n\t-1.9 BAYES_00               BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":"<http://lists.infradead.org/mailman/options/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>","List-Archive":"<http://lists.infradead.org/pipermail/linux-arm-kernel/>","List-Post":"<mailto:linux-arm-kernel@lists.infradead.org>","List-Help":"<mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>","List-Subscribe":"<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>","Cc":"Mark Rutland <mark.rutland@arm.com>, devicetree@vger.kernel.org,\n\tAlexandre Torgue <alexandre.torgue@st.com>,\n\tRussell King <linux@armlinux.org.uk>, linux-kernel@vger.kernel.org,\n\tRob Herring <robh+dt@kernel.org>, linux-i2c@vger.kernel.org,\n\tMaxime Coquelin <mcoquelin.stm32@gmail.com>,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============1165088522616285393==\"","Sender":"\"linux-arm-kernel\" <linux-arm-kernel-bounces@lists.infradead.org>","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1768406,"web_url":"http://patchwork.ozlabs.org/comment/1768406/","msgid":"<63ecfac9-990b-e6c5-d9bf-7708bacc10d2@st.com>","list_archive_url":null,"date":"2017-09-14T08:11:11","subject":"Re: [RESEND PATCH v3 3/5] i2c: i2c-stm32f7: add driver","submitter":{"id":71499,"url":"http://patchwork.ozlabs.org/api/people/71499/","name":"Pierre Yves MORDRET","email":"pierre-yves.mordret@st.com"},"content":"On 09/13/2017 11:26 PM, Wolfram Sang wrote:\n> Hi,\n> \n> thanks for this driver!\n> \n>> +/**\n>> + * struct stm32f7_i2c_spec - private i2c specification timing\n>> + * @rate: I2C bus speed (Hz)\n>> + * @rate_min: 80% of I2C bus speed (Hz)\n>> + * @rate_max: 120% of I2C bus speed (Hz)\n> \n> You would generate a clock which is higher than the requested one?\n> This is highly unusual. Any special reason?\n\nWell. I allow the clock to be higher than expected.\nLooking at I2C spec again it turns out the mode specifies the max: no overshoot\nof the clock. I will lock max to 100% then.\nWill be fixed\n\n> \n>> + * @fall_max: Max fall time of both SDA and SCL signals (ns)\n>> + * @rise_max: Max rise time of both SDA and SCL signals (ns)\n>> + * @hddat_min: Min data hold time (ns)\n>> + * @vddat_max: Max data valid time (ns)\n>> + * @sudat_min: Min data setup time (ns)\n>> + * @l_min: Min low period of the SCL clock (ns)\n>> + * @h_min: Min high period of the SCL clock (ns)\n>> + */\n>> +static struct stm32f7_i2c_spec i2c_specs[] = {\n>> +\t[STM32_I2C_SPEED_STANDARD] = {\n>> +\t\t.rate = 100000,\n>> +\t\t.rate_min = 8000,\n> \n> This is not 80%. Typo?\n\nYep. This is a typo\n\n> \n>> +\t\t.rate_max = 120000,\n>> +\t\t.fall_max = 300,\n>> +\t\t.rise_max = 1000,\n>> +\t\t.hddat_min = 0,\n>> +\t\t.vddat_max = 3450,\n>> +\t\t.sudat_min = 250,\n>> +\t\t.l_min = 4700,\n>> +\t\t.h_min = 4000,\n>> +\t},\n> \n> ...\n> \n>> +\t/*\n>> +\t * Among Prescaler possibilities discovered above figures out SCL Low\n>> +\t * and High Period. Provided:\n>> +\t * - SCL Low Period has to be higher than Low Period of tehs SCL Clock\n> \n> tehs?\n\nOops.\n\n> \n>> +\t *   defined by I2C Specification. I2C Clock has to be lower than\n>> +\t *   (SCL Low Period - Analog/Digital filters) / 4.\n>> +\t * - SCL High Period has to be lower than High Period of the SCL Clock\n>> +\t *   defined by I2C Specification\n>> +\t * - I2C Clock has to be lower than SCL High Period\n>> +\t */\n> \n> ...\n> \n>> +\t/* NACK received */\n>> +\tif (status & STM32F7_I2C_ISR_NACKF) {\n>> +\t\tdev_dbg(i2c_dev->dev, \"<%s>: Receive NACK\\n\", __func__);\n>> +\t\twritel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);\n>> +\t\tf7_msg->result = -EBADE;\n> \n> -ENXIO (see Documentation/i2c/fault-codes)\n\nOK\n\n> \n> ...\n> \n>> +\ttimeout = wait_for_completion_timeout(&i2c_dev->complete,\n>> +\t\t\t\t\t      i2c_dev->adap.timeout);\n>> +\tret = f7_msg->result;\n>> +\n>> +\tif (!timeout) {\n>> +\t\tdev_dbg(i2c_dev->dev, \"Access to slave 0x%x timed out\\n\",\n>> +\t\t\ti2c_dev->msg->addr);\n>> +\t\tret = -ETIMEDOUT;\n>> +\t}\n> \n> Could you rename the variable to time_left? It looks strange, basically:\n> \n> \tif (!timeout)\n> \t\treturn -ETIMEDOUT\n> \n\nokay\n\n> ...\n> \n>> +\tadap->retries = 0;\n> \n> Why no retries when you check for arbitration lost?\n> \n>> +\tadap->algo = &stm32f7_i2c_algo;\n>> +\tadap->dev.parent = &pdev->dev;\n>> +\tadap->dev.of_node = pdev->dev.of_node;\n>> +\n>> +\tinit_completion(&i2c_dev->complete);\n>> +\n>> +\tret = i2c_add_adapter(adap);\n>> +\tif (ret) {\n>> +\t\tdev_err(&pdev->dev, \"Failed to add adapter\\n\");\n> \n> Please remove, the core will print info when adding fails.\n> \n\nI will\n\n> \n> Rest looks good!\n\nGreat !\n\n> \n> Thanks,\n> \n>    Wolfram\n> \n\nThanks","headers":{"Return-Path":"<linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org>","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"beDiWAPB\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtB8Z1Zbbz9sP1\n\tfor <incoming-imx@patchwork.ozlabs.org>;\n\tThu, 14 Sep 2017 18:12:14 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsPFp-0005wa-Qm; Thu, 14 Sep 2017 08:12:09 +0000","from mx08-00178001.pphosted.com ([91.207.212.93]\n\thelo=mx07-00178001.pphosted.com)\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsPFh-0005ow-IA for linux-arm-kernel@lists.infradead.org;\n\tThu, 14 Sep 2017 08:12:07 +0000","from pps.filterd (m0046660.ppops.net [127.0.0.1])\n\tby mx08-.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv8E898vS026290; Thu, 14 Sep 2017 10:11:24 +0200","from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35])\n\tby mx08-00178001.pphosted.com with ESMTP id 2cv5e55xef-1\n\t(version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT);\n\tThu, 14 Sep 2017 10:11:24 +0200","from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9])\n\tby beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 8DA8534;\n\tThu, 14 Sep 2017 08:11:22 +0000 (GMT)","from Webmail-eu.st.com (sfhdag5node2.st.com [10.75.127.14])\n\tby zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 5F12A15BA;\n\tThu, 14 Sep 2017 08:11:22 +0000 (GMT)","from [10.201.23.236] (10.75.127.45) by SFHDAG5NODE2.st.com\n\t(10.75.127.14) with Microsoft SMTP Server (TLS) id 15.0.1178.4;\n\tThu, 14 Sep 2017 10:11:21 +0200"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:\n\tMessage-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description\n\t:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=UCfTfNbmL/lgFRPbP8uLGK7MKqdTjpCrvwEvUV88IJ0=;\n\tb=beDiWAPBm++dsD\n\tE/oSK7G0g8DxLVfZvRN8n5PmbPbLWu0R5hNqX9CVBDcqfl2Aibn2RJvswo7PepTdwde+DGgrq30jN\n\t1bBDChZPGqU1102x9SNH5e0Rl9/5pK6VGfph8jSgLJmkm+bT2CuJQC682qmovaHas5vCQF9NzC/Y+\n\tA0DH/9FI1aMHM03XLc2BpGA3fCxCER9X+iRO7JrH4O0EQotOD46mlz4aobDVlkUyOpfEYGsw/q4G5\n\takC3eTu0ToH+tQ5AOFaGjY+DVjv3FF6iuD3GRo0/b63oRMQbmO0fjIvp1JgDmg1NQC6lJkwK6LnNy\n\tDhbU7HZ//8QFeIqOHH0g==;","Subject":"Re: [RESEND PATCH v3 3/5] i2c: i2c-stm32f7: add driver","To":"Wolfram Sang <wsa@the-dreams.de>","References":"<1504251255-20469-1-git-send-email-pierre-yves.mordret@st.com>\n\t<1504251255-20469-4-git-send-email-pierre-yves.mordret@st.com>\n\t<20170913212639.2zyvg24364ovwfxh@ninjato>","From":"Pierre Yves MORDRET <pierre-yves.mordret@st.com>","Message-ID":"<63ecfac9-990b-e6c5-d9bf-7708bacc10d2@st.com>","Date":"Thu, 14 Sep 2017 10:11:11 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170913212639.2zyvg24364ovwfxh@ninjato>","Content-Language":"en-US","X-Originating-IP":"[10.75.127.45]","X-ClientProxiedBy":"SFHDAG1NODE3.st.com (10.75.127.3) To SFHDAG5NODE2.st.com\n\t(10.75.127.14)","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-14_02:, , signatures=0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170914_011201_920676_42C20652 ","X-CRM114-Status":"GOOD (  14.94  )","X-Spam-Score":"-2.6 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details:   (-2.6 points)\n\tpts rule name              description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow trust [91.207.212.93 listed in list.dnswl.org]\n\t-0.0 SPF_PASS               SPF: sender matches SPF record\n\t-1.9 BAYES_00               BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":"<http://lists.infradead.org/mailman/options/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>","List-Archive":"<http://lists.infradead.org/pipermail/linux-arm-kernel/>","List-Post":"<mailto:linux-arm-kernel@lists.infradead.org>","List-Help":"<mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>","List-Subscribe":"<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>","Cc":"Mark Rutland <mark.rutland@arm.com>, devicetree@vger.kernel.org,\n\tAlexandre Torgue <alexandre.torgue@st.com>,\n\tRussell King <linux@armlinux.org.uk>, linux-kernel@vger.kernel.org,\n\tRob Herring <robh+dt@kernel.org>, linux-i2c@vger.kernel.org,\n\tMaxime Coquelin <mcoquelin.stm32@gmail.com>,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" <linux-arm-kernel-bounces@lists.infradead.org>","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}}]