[{"id":1799500,"web_url":"http://patchwork.ozlabs.org/comment/1799500/","msgid":"","list_archive_url":null,"date":"2017-11-06T08:35:53","subject":"Re: [PATCH RFT 3/5] i2c: sh_mobile: manually \"inline\" two short\n\tfunctions","submitter":{"id":703,"url":"http://patchwork.ozlabs.org/api/people/703/","name":"Geert Uytterhoeven","email":"geert@linux-m68k.org"},"content":"Hi Wolfram,\n\nOn Thu, Nov 2, 2017 at 1:47 PM, Wolfram Sang\n wrote:\n> Those two functions are very short and only called once. The code\n> becomes easier to understand if the code is directly put into the main\n> xfer function.\n>\n> Signed-off-by: Wolfram Sang \n> ---\n> drivers/i2c/busses/i2c-sh_mobile.c | 28 +++++++++-------------------\n> 1 file changed, 9 insertions(+), 19 deletions(-)\n>\n> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c\n> index a1253df9574594..cdd7a746b9d1ed 100644\n> --- a/drivers/i2c/busses/i2c-sh_mobile.c\n> +++ b/drivers/i2c/busses/i2c-sh_mobile.c\n> @@ -298,23 +298,6 @@ static int sh_mobile_i2c_init(struct sh_mobile_i2c_data *pd)\n> return 0;\n> }\n>\n> -static void activate_ch(struct sh_mobile_i2c_data *pd)\n> -{\n> - /* Wake up device and enable clock */\n> - pm_runtime_get_sync(pd->dev);\n> - clk_prepare_enable(pd->clk);\n> -}\n> -\n> -static void deactivate_ch(struct sh_mobile_i2c_data *pd)\n> -{\n> - /* Disable channel */\n> - iic_set_clr(pd, ICCR, 0, ICCR_ICE);\n> -\n> - /* Disable clock and mark device as idle */\n> - clk_disable_unprepare(pd->clk);\n> - pm_runtime_put_sync(pd->dev);\n> -}\n> -\n> static unsigned char i2c_op(struct sh_mobile_i2c_data *pd,\n> enum sh_mobile_i2c_op op, unsigned char data)\n> {\n> @@ -717,7 +700,9 @@ static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,\n> int i;\n> long timeout;\n>\n> - activate_ch(pd);\n> + /* Wake up device and enable clock */\n> + pm_runtime_get_sync(pd->dev);\n> + clk_prepare_enable(pd->clk);\n\nSuggestion for further cleanup: the call to clk_prepare_enable() should\nnot be needed, due to Runtime PM taking care of that.\nThis is also the case on SH, which uses the legacy clock domain\n(drivers/sh/pm_runtime.c).\n\n>\n> /* Process all messages */\n> for (i = 0; i < num; i++) {\n> @@ -754,7 +739,12 @@ static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,\n> break;\n> }\n>\n> - deactivate_ch(pd);\n> + /* Disable channel */\n> + iic_set_clr(pd, ICCR, 0, ICCR_ICE);\n> +\n> + /* Disable clock and mark device as idle */\n> + clk_disable_unprepare(pd->clk);\n\nLikewise for clk_disable_unprepare().\n\n> + pm_runtime_put_sync(pd->dev);\n\nGr{oetje,eeting}s,\n\n Geert\n\n--\nGeert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org\n\nIn personal conversations with technical people, I call myself a hacker. But\nwhen I'm talking to journalists I just say \"programmer\" or something like that.\n -- Linus Torvalds","headers":{"Return-Path":"","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=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"fwcbjK42\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yVm9T1VsHz9s7h\n\tfor ;\n\tMon, 6 Nov 2017 19:35:57 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750760AbdKFIfz (ORCPT );\n\tMon, 6 Nov 2017 03:35:55 -0500","from mail-qk0-f193.google.com ([209.85.220.193]:56734 \"EHLO\n\tmail-qk0-f193.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750728AbdKFIfy (ORCPT\n\t); Mon, 6 Nov 2017 03:35:54 -0500","by mail-qk0-f193.google.com with SMTP id l194so9934762qke.13;\n\tMon, 06 Nov 2017 00:35:54 -0800 (PST)","by 10.200.45.66 with HTTP; Mon, 6 Nov 2017 00:35:53 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=7uyew+i6PGAB3mECv/tp+v6T9vfW2KdkMIApCWnHEJM=;\n\tb=fwcbjK42iFPB6OJcMLJYhC+TU89r/t3zxL86CkwHyeyJLNzif8OnQa7expXleTAq+p\n\tk6X8CXlK8lYxCWYHkOLtC/Z5JO61zDLqzX8oqj7TxZKIEgXDovlD/boMXTn1kPm6On8c\n\t9W+va58rkOPzlVApV/d067NlYnijZ8vKPDfMgYFae3XeFgN/f3vlV8ytpxANcc86Rrh2\n\tFHv6YiDR51xQm+sOE3zPlNM5+0AjJ0TGsNgbAiGa4jfAtDEUvRZKsDsBVxYSY0qdnJSa\n\t+cg1eOG9DAhxx39udB5Rvt4/a6P7/JemMGJMfgOOVot65+EuagtzhKSE8L+3XgjybnFP\n\tpL6A==","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:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=7uyew+i6PGAB3mECv/tp+v6T9vfW2KdkMIApCWnHEJM=;\n\tb=HZ6dcpiFrrQsbdCRDoCMmxwM0gnFqDs4ZAzoyRi4bkiZFFLRu5LjSTZWeP84Tclrot\n\tMrHR2ead2xw3Z0vFyRK6I0TFFj63gY5v7fv4QylPV70B51Z0avXhlgT6sWdBCixnzwxU\n\t9X2JmN2wqCKpTckyBA0rZybGt/ZIHw5cHArrcq8iitGmYDKmrY6+lJCHnmolsWsqJr5B\n\tS1q1EDhuvVMC9fM/GDtpbBUSsVNPFqLZ3UxCUOOUO1LIMxLctYKw29fLPzH7rLpte8/7\n\t88oPw5Mx3l+GcmF/vDhQoe/s12Elv+YIMvi0DIwjpzFzajK01lEBZ8K2GKNGZkC2Ewg+\n\tOsug==","X-Gm-Message-State":"AMCzsaXS1+fufm5wnQmqdjjRqN3kWD1v4Vlh9vGHiS6dawUPEEMQilLn\n\t8R0F/bee/mgl7c9BnQfJhkID9wJiDem8sJz6dQ0=","X-Google-Smtp-Source":"ABhQp+S7S1ZIGTbG34a4AFgz1axxbGxbfGblplCk5tDpjhYJiWsRHsi3/WOeADPeQGFqd95TLgE7OYTLX6AgvwM1DKA=","X-Received":"by 10.55.127.66 with SMTP id a63mr20008139qkd.90.1509957353968; \n\tMon, 06 Nov 2017 00:35:53 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<20171102124731.10484-4-wsa+renesas@sang-engineering.com>","References":"<20171102124731.10484-1-wsa+renesas@sang-engineering.com>\n\t<20171102124731.10484-4-wsa+renesas@sang-engineering.com>","From":"Geert Uytterhoeven ","Date":"Mon, 6 Nov 2017 09:35:53 +0100","X-Google-Sender-Auth":"E55eJpbhIfKut3QXy9d2_9QRBUU","Message-ID":"","Subject":"Re: [PATCH RFT 3/5] i2c: sh_mobile: manually \"inline\" two short\n\tfunctions","To":"Wolfram Sang ","Cc":"Linux I2C ,\n\tLinux-Renesas ,\n\tYoshihiro Shimoda ,\n\tJacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1799512,"web_url":"http://patchwork.ozlabs.org/comment/1799512/","msgid":"<20171106085421.zgwz4dn5pmpjpodu@ninjato>","list_archive_url":null,"date":"2017-11-06T08:54:21","subject":"Re: [PATCH RFT 3/5] i2c: sh_mobile: manually \"inline\" two short\n\tfunctions","submitter":{"id":22495,"url":"http://patchwork.ozlabs.org/api/people/22495/","name":"Wolfram Sang","email":"wsa@the-dreams.de"},"content":"> > - activate_ch(pd);\n> > + /* Wake up device and enable clock */\n> > + pm_runtime_get_sync(pd->dev);\n> > + clk_prepare_enable(pd->clk);\n> \n> Suggestion for further cleanup: the call to clk_prepare_enable() should\n> not be needed, due to Runtime PM taking care of that.\n> This is also the case on SH, which uses the legacy clock domain\n> (drivers/sh/pm_runtime.c).\n\nAh, I didn't know that about SH. Thanks for the heads up!","headers":{"Return-Path":"","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=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yVmZn0Dsgz9s4q\n\tfor ;\n\tMon, 6 Nov 2017 19:54:25 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751599AbdKFIyX (ORCPT );\n\tMon, 6 Nov 2017 03:54:23 -0500","from sauhun.de ([88.99.104.3]:50333 \"EHLO pokefinder.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751140AbdKFIyX (ORCPT );\n\tMon, 6 Nov 2017 03:54:23 -0500","from localhost (p54B33909.dip0.t-ipconnect.de [84.179.57.9])\n\tby pokefinder.org (Postfix) with ESMTPSA id B7A442C34A9;\n\tMon, 6 Nov 2017 09:54:21 +0100 (CET)"],"Date":"Mon, 6 Nov 2017 09:54:21 +0100","From":"Wolfram Sang ","To":"Geert Uytterhoeven ","Cc":"Wolfram Sang ,\n\tLinux I2C ,\n\tLinux-Renesas ,\n\tYoshihiro Shimoda ,\n\tJacopo Mondi ","Subject":"Re: [PATCH RFT 3/5] i2c: sh_mobile: manually \"inline\" two short\n\tfunctions","Message-ID":"<20171106085421.zgwz4dn5pmpjpodu@ninjato>","References":"<20171102124731.10484-1-wsa+renesas@sang-engineering.com>\n\t<20171102124731.10484-4-wsa+renesas@sang-engineering.com>\n\t","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"2pjkuecogt7npvat\"","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}}]