From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 00AD2C7618B for ; Fri, 26 Jul 2019 10:42:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BDBF2218EA for ; Fri, 26 Jul 2019 10:42:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1564137740; bh=X7zomnphSoatQ2ExsZCymGrIKUnr+7mD1sl02mTPfh4=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=Ewmxic21mYR/aUOBPKze9tZBB4X/H1yd6VgYlB9NffPuUIld7NqzdrwAwxdh7fKVJ bWcE9fXZx01679oNEjLBwcpEkaEobckysfJX5Unr8SZt0vD6IIwunTxUQlScfNOsDq nome7ev1ePo/4UF1y76rWprTQpxoVx9THvxNp9Ig= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726655AbfGZKmT (ORCPT ); Fri, 26 Jul 2019 06:42:19 -0400 Received: from mail.kernel.org ([198.145.29.99]:52634 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726203AbfGZKmT (ORCPT ); Fri, 26 Jul 2019 06:42:19 -0400 Received: from mail-lj1-f181.google.com (mail-lj1-f181.google.com [209.85.208.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 2142122CB9; Fri, 26 Jul 2019 10:42:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1564137737; bh=X7zomnphSoatQ2ExsZCymGrIKUnr+7mD1sl02mTPfh4=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=xOIT7hQdaE/9saCJ016SORI9yKJR4VdvxCbG1H173OLs1s6mHJXKyd7rc897GqvTd gKGbYk2M0FXGKg+ThrcP47kYqY52jrdZIUTa1y2tkWazRdgbUglzRXmdiVxFD9JWf4 /rjQ/hqg3pM0VOSD8zR3w9k9kqGBqh95ky+LeizI= Received: by mail-lj1-f181.google.com with SMTP id r9so51018570ljg.5; Fri, 26 Jul 2019 03:42:17 -0700 (PDT) X-Gm-Message-State: APjAAAVDXb0Q5x4XbONwUkcsxIi2/ifvFlxLaIxo3aYUjeskf/uI71Ho 8v35IMSOwEap4g5K8BGskqPXEgimI6tnZ65E+RI= X-Google-Smtp-Source: APXvYqwjjPIUcxkurCk45AI+olnU8bXwOyRqJMGB5KvMZHbMvG1vzBey1kV3fpe6+DzCzU8EHqwv22xT+qk5FHRVVEY= X-Received: by 2002:a2e:b4d4:: with SMTP id r20mr23905543ljm.5.1564137735265; Fri, 26 Jul 2019 03:42:15 -0700 (PDT) MIME-Version: 1.0 References: <20190723122016.30279-1-a.swigon@partner.samsung.com> <20190723122016.30279-2-a.swigon@partner.samsung.com> In-Reply-To: From: Krzysztof Kozlowski Date: Fri, 26 Jul 2019 12:42:03 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init() To: cwchoi00@gmail.com Cc: =?UTF-8?B?QXJ0dXIgxZp3aWdvxYQ=?= , devicetree , linux-arm-kernel , linux-samsung-soc , linux-kernel , Linux PM list , dri-devel , Chanwoo Choi , MyungJoo Ham , Inki Dae , Seung-Woo Kim , georgi.djakov@linaro.org, Marek Szyprowski Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 25 Jul 2019 at 14:44, Chanwoo Choi wrote: > > 2019=EB=85=84 7=EC=9B=94 24=EC=9D=BC (=EC=88=98) =EC=98=A4=EC=A0=84 8:09,= Artur =C5=9Awigo=C5=84 =EB=8B=98=EC=9D=B4 = =EC=9E=91=EC=84=B1: > > > > This patch adds a new static function, exynos_bus_profile_init(), extra= cted > > from exynos_bus_probe(). > > > > Signed-off-by: Artur =C5=9Awigo=C5=84 > > --- > > drivers/devfreq/exynos-bus.c | 106 ++++++++++++++++++++--------------- > > 1 file changed, 60 insertions(+), 46 deletions(-) > > > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.= c > > index d9f377912c10..d8f1efaf2d49 100644 > > --- a/drivers/devfreq/exynos-bus.c > > +++ b/drivers/devfreq/exynos-bus.c > > @@ -372,12 +372,69 @@ static int exynos_bus_parse_of(struct device_node= *np, > > return ret; > > } > > > > +static int exynos_bus_profile_init(struct exynos_bus *bus, > > + struct devfreq_dev_profile *profile) > > +{ > > + struct device *dev =3D bus->dev; > > + struct devfreq_simple_ondemand_data *ondemand_data; > > + int ret; > > + > > + /* Initialize the struct profile and governor data for parent d= evice */ > > + profile->polling_ms =3D 50; > > + profile->target =3D exynos_bus_target; > > + profile->get_dev_status =3D exynos_bus_get_dev_status; > > + profile->exit =3D exynos_bus_exit; > > + > > + ondemand_data =3D devm_kzalloc(dev, sizeof(*ondemand_data), GFP= _KERNEL); > > + if (!ondemand_data) { > > + ret =3D -ENOMEM; > > + goto err; > > + } > > + ondemand_data->upthreshold =3D 40; > > + ondemand_data->downdifferential =3D 5; > > + > > + /* Add devfreq device to monitor and handle the exynos bus */ > > + bus->devfreq =3D devm_devfreq_add_device(dev, profile, > > + DEVFREQ_GOV_SIMPLE_ONDE= MAND, > > + ondemand_data); > > + if (IS_ERR(bus->devfreq)) { > > + dev_err(dev, "failed to add devfreq device\n"); > > + ret =3D PTR_ERR(bus->devfreq); > > + goto err; > > + } > > + > > + /* Register opp_notifier to catch the change of OPP */ > > + ret =3D devm_devfreq_register_opp_notifier(dev, bus->devfreq); > > + if (ret < 0) { > > + dev_err(dev, "failed to register opp notifier\n"); > > + goto err; > > + } > > + > > + /* > > + * Enable devfreq-event to get raw data which is used to determ= ine > > + * current bus load. > > + */ > > + ret =3D exynos_bus_enable_edev(bus); > > + if (ret < 0) { > > + dev_err(dev, "failed to enable devfreq-event devices\n"= ); > > + goto err; > > + } > > + > > + ret =3D exynos_bus_set_event(bus); > > + if (ret < 0) { > > + dev_err(dev, "failed to set event to devfreq-event devi= ces\n"); > > + goto err; > > + } > > + > > +err: > > + return ret; > > +} > > + > > static int exynos_bus_probe(struct platform_device *pdev) > > { > > struct device *dev =3D &pdev->dev; > > struct device_node *np =3D dev->of_node, *node; > > struct devfreq_dev_profile *profile; > > - struct devfreq_simple_ondemand_data *ondemand_data; > > struct devfreq_passive_data *passive_data; > > struct devfreq *parent_devfreq; > > struct exynos_bus *bus; > > @@ -418,52 +475,9 @@ static int exynos_bus_probe(struct platform_device= *pdev) > > if (ret < 0) > > goto err; > > > > - /* Initialize the struct profile and governor data for parent d= evice */ > > - profile->polling_ms =3D 50; > > - profile->target =3D exynos_bus_target; > > - profile->get_dev_status =3D exynos_bus_get_dev_status; > > - profile->exit =3D exynos_bus_exit; > > - > > - ondemand_data =3D devm_kzalloc(dev, sizeof(*ondemand_data), GFP= _KERNEL); > > - if (!ondemand_data) { > > - ret =3D -ENOMEM; > > + ret =3D exynos_bus_profile_init(bus, profile); > > + if (ret < 0) > > goto err; > > - } > > - ondemand_data->upthreshold =3D 40; > > - ondemand_data->downdifferential =3D 5; > > - > > - /* Add devfreq device to monitor and handle the exynos bus */ > > - bus->devfreq =3D devm_devfreq_add_device(dev, profile, > > - DEVFREQ_GOV_SIMPLE_ONDE= MAND, > > - ondemand_data); > > - if (IS_ERR(bus->devfreq)) { > > - dev_err(dev, "failed to add devfreq device\n"); > > - ret =3D PTR_ERR(bus->devfreq); > > - goto err; > > - } > > - > > - /* Register opp_notifier to catch the change of OPP */ > > - ret =3D devm_devfreq_register_opp_notifier(dev, bus->devfreq); > > - if (ret < 0) { > > - dev_err(dev, "failed to register opp notifier\n"); > > - goto err; > > - } > > - > > - /* > > - * Enable devfreq-event to get raw data which is used to determ= ine > > - * current bus load. > > - */ > > - ret =3D exynos_bus_enable_edev(bus); > > - if (ret < 0) { > > - dev_err(dev, "failed to enable devfreq-event devices\n"= ); > > - goto err; > > - } > > - > > - ret =3D exynos_bus_set_event(bus); > > - if (ret < 0) { > > - dev_err(dev, "failed to set event to devfreq-event devi= ces\n"); > > - goto err; > > - } > > > > goto out; > > passive: > > -- > > 2.17.1 > > > > NACK. > > It has not any benefit and I don't understand reason why it is necessary. > I don't agree. Please drop it. The probe has 12 local variables and around 140 lines of code (so much more than coding style recommendations). Therefore splitting some logical part out of probe to make code better organized and more readable is pretty obvious benefit. Best regards, Krzysztof