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=-10.0 required=3.0 tests=BAYES_00,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 0F7FFC4742C for ; Wed, 11 Nov 2020 08:53:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B5F80206E3 for ; Wed, 11 Nov 2020 08:53:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726274AbgKKIw6 (ORCPT ); Wed, 11 Nov 2020 03:52:58 -0500 Received: from mail-ed1-f66.google.com ([209.85.208.66]:41068 "EHLO mail-ed1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726263AbgKKIw5 (ORCPT ); Wed, 11 Nov 2020 03:52:57 -0500 Received: by mail-ed1-f66.google.com with SMTP id t9so1434312edq.8; Wed, 11 Nov 2020 00:52:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=FR2fk7Xv4lSmlOc9Dd3jcBxGV7rUmCYq9p4pf48MgkE=; b=tox7I0j/B7PNhimf9a/kzxON4iMlFC7xWD/Lpw7EATJfO1VEDVXvUe87/rdgBcu0Vf FhXEuczvHuXub5YMuen2Racx++0oncxhhvxyNV2+VTnJDfqGHfHlIEv6XWcdKN6rihnx lIsGN1wZUycIcajeVsMjILEUgJxgZT//WNK2KnQC9sjCfclJ5nXRot2B5a5tL4FK7PU5 sZ0xAmNQySb9QnAlBvOHlqa/flu2cg2v70QllNuNfVO25A+HCB5ALuRYvk0yO857M2FH 4vb94Y20ejL0AVFN9WcaZCgkexUFtGTXME2JNlKdo7SSTeB5Nut616dE0D2T3bmj6hQm 7D/w== X-Gm-Message-State: AOAM530Q0zgLIQibVOLYD1cJ+s8L/Ul/PGS4bS+I67xMiL/EXRI7L9A1 1uMcSSPf4qoPMyK2OF3L6Cg= X-Google-Smtp-Source: ABdhPJwji6ikeI/Mly+Q6njYPrhT9ggbKIXuArTDm7S8d2rj8FwDw4fd/HOZiSUe1NQbAdVvsnmJTA== X-Received: by 2002:a05:6402:54c:: with SMTP id i12mr26442371edx.9.1605084774010; Wed, 11 Nov 2020 00:52:54 -0800 (PST) Received: from kozik-lap (adsl-84-226-167-205.adslplus.ch. [84.226.167.205]) by smtp.googlemail.com with ESMTPSA id f19sm596038edm.70.2020.11.11.00.52.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Nov 2020 00:52:52 -0800 (PST) Date: Wed, 11 Nov 2020 09:52:50 +0100 From: Krzysztof Kozlowski To: Dmitry Osipenko Cc: Thierry Reding , Jonathan Hunter , Georgi Djakov , Rob Herring , Michael Turquette , Stephen Boyd , Peter De Schrijver , MyungJoo Ham , Kyungmin Park , Chanwoo Choi , Mikko Perttunen , Viresh Kumar , Peter Geis , Nicolas Chauvet , linux-tegra@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v8 10/26] memory: tegra30-emc: Factor out clk initialization Message-ID: <20201111085250.GA11589@kozik-lap> References: <20201111011456.7875-1-digetx@gmail.com> <20201111011456.7875-11-digetx@gmail.com> <20201111085115.GA4050@kozik-lap> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20201111085115.GA4050@kozik-lap> Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org On Wed, Nov 11, 2020 at 09:51:15AM +0100, Krzysztof Kozlowski wrote: > On Wed, Nov 11, 2020 at 04:14:40AM +0300, Dmitry Osipenko wrote: > > Factor out clk initialization and make it resource-managed. This makes > > easier to follow code and will help to make further changes cleaner. > > > > Signed-off-by: Dmitry Osipenko > > --- > > drivers/memory/tegra/tegra30-emc.c | 70 ++++++++++++++++++++---------- > > 1 file changed, 47 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c > > index d27df842a667..1df42e212d73 100644 > > --- a/drivers/memory/tegra/tegra30-emc.c > > +++ b/drivers/memory/tegra/tegra30-emc.c > > @@ -1550,6 +1550,49 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc) > > return err; > > } > > > > +static void devm_tegra_emc_unset_callback(void *data) > > +{ > > + tegra20_clk_set_emc_round_callback(NULL, NULL); > > +} > > + > > +static void devm_tegra_emc_unreg_clk_notifier(void *data) > > +{ > > + struct tegra_emc *emc = data; > > + > > + clk_notifier_unregister(emc->clk, &emc->clk_nb); > > +} > > + > > +static int tegra_emc_init_clk(struct tegra_emc *emc) > > +{ > > + int err; > > + > > + tegra20_clk_set_emc_round_callback(emc_round_rate, emc); > > + > > + err = devm_add_action_or_reset(emc->dev, devm_tegra_emc_unset_callback, > > + NULL); > > + if (err) > > + return err; > > + > > + emc->clk = devm_clk_get(emc->dev, NULL); > > + if (IS_ERR(emc->clk)) { > > + dev_err(emc->dev, "failed to get EMC clock: %pe\n", emc->clk); > > + return PTR_ERR(emc->clk); > > + } > > + > > + err = clk_notifier_register(emc->clk, &emc->clk_nb); > > + if (err) { > > + dev_err(emc->dev, "failed to register clk notifier: %d\n", err); > > + return err; > > + } > > + > > + err = devm_add_action_or_reset(emc->dev, > > + devm_tegra_emc_unreg_clk_notifier, emc); > > + if (err) > > + return err; > > + > > + return 0; > > +} > > + > > static int tegra_emc_probe(struct platform_device *pdev) > > { > > struct device_node *np; > > @@ -1599,25 +1642,13 @@ static int tegra_emc_probe(struct platform_device *pdev) > > return err; > > } > > > > - tegra20_clk_set_emc_round_callback(emc_round_rate, emc); > > - > > - emc->clk = devm_clk_get(&pdev->dev, "emc"); > > - if (IS_ERR(emc->clk)) { > > - err = PTR_ERR(emc->clk); > > - dev_err(&pdev->dev, "failed to get emc clock: %d\n", err); > > - goto unset_cb; > > - } > > - > > - err = clk_notifier_register(emc->clk, &emc->clk_nb); > > - if (err) { > > - dev_err(&pdev->dev, "failed to register clk notifier: %d\n", > > - err); > > - goto unset_cb; > > - } > > + err = tegra_emc_init_clk(emc); > > + if (err) > > + return err; > > > > err = tegra_emc_opp_table_init(emc); > > if (err) > > - goto unreg_notifier; > > + return err; > > > > platform_set_drvdata(pdev, emc); > > tegra_emc_rate_requests_init(emc); > > @@ -1632,13 +1663,6 @@ static int tegra_emc_probe(struct platform_device *pdev) > > try_module_get(THIS_MODULE); > > > > return 0; > > - > > -unreg_notifier: > > - clk_notifier_unregister(emc->clk, &emc->clk_nb); > > You added this code in patch #8, so adding-and-removing a piece of code Correction: you added this in patch #9. Best regards, Krzysztof > is a nice hint that this patch should be before. Don't add new code > which later you simplify. Move all bugfixes and all simplifications to > beginning of patchset. > > That's quite similar case to v6 where you put bugfixes in the middle > of patchset. > 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=-10.0 required=3.0 tests=BAYES_00,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 BE3BAC56202 for ; Wed, 11 Nov 2020 08:52:58 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6157C206FB for ; Wed, 11 Nov 2020 08:52:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6157C206FB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 756F689FEA; Wed, 11 Nov 2020 08:52:57 +0000 (UTC) Received: from mail-ed1-f67.google.com (mail-ed1-f67.google.com [209.85.208.67]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7CC0E89FEA for ; Wed, 11 Nov 2020 08:52:55 +0000 (UTC) Received: by mail-ed1-f67.google.com with SMTP id b9so1420513edu.10 for ; Wed, 11 Nov 2020 00:52:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=FR2fk7Xv4lSmlOc9Dd3jcBxGV7rUmCYq9p4pf48MgkE=; b=FSfVaENOMCZ+w2Il6IWljJFcW3vbOudehR/hnkQHEgR+JzEekFxCHPnfwqe5bYKRzV BQevNoGwYgHwPDR23nMN9sQIt0tIS9hZ7jVlORpmlzaypHtaf5yy2YwrGd20Xcms2F00 Uvw5hRI64p9ETqIy4UMM7pjbXfVlTCY/+dDZXnvdGO8YlukMjrU3xV6TA1Mzgxw3NIB7 wphEVWlmPyZef5sW3emu1zUNN6c65tggls9lTNFTCTxFjwp4wA9KiYFJsTkI1GouQiL2 Wtoiwf98ClZL1WnaCHJgByu+wxP/txUzBz8AObaqUut69kDGTGg+qwX8nKgZ8e/UQj4p IXdg== X-Gm-Message-State: AOAM530MevLczot4viASnGmEO3smpYubQ8U0GaMgfZB5sZuXv3B1JFE3 +1FGzM0JEmvnGsWzUlavy4g= X-Google-Smtp-Source: ABdhPJwji6ikeI/Mly+Q6njYPrhT9ggbKIXuArTDm7S8d2rj8FwDw4fd/HOZiSUe1NQbAdVvsnmJTA== X-Received: by 2002:a05:6402:54c:: with SMTP id i12mr26442371edx.9.1605084774010; Wed, 11 Nov 2020 00:52:54 -0800 (PST) Received: from kozik-lap (adsl-84-226-167-205.adslplus.ch. [84.226.167.205]) by smtp.googlemail.com with ESMTPSA id f19sm596038edm.70.2020.11.11.00.52.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Nov 2020 00:52:52 -0800 (PST) Date: Wed, 11 Nov 2020 09:52:50 +0100 From: Krzysztof Kozlowski To: Dmitry Osipenko Subject: Re: [PATCH v8 10/26] memory: tegra30-emc: Factor out clk initialization Message-ID: <20201111085250.GA11589@kozik-lap> References: <20201111011456.7875-1-digetx@gmail.com> <20201111011456.7875-11-digetx@gmail.com> <20201111085115.GA4050@kozik-lap> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201111085115.GA4050@kozik-lap> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter De Schrijver , Mikko Perttunen , dri-devel@lists.freedesktop.org, Nicolas Chauvet , Stephen Boyd , Viresh Kumar , Michael Turquette , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Rob Herring , Jonathan Hunter , Chanwoo Choi , Kyungmin Park , Thierry Reding , MyungJoo Ham , Peter Geis , linux-tegra@vger.kernel.org, Georgi Djakov Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, Nov 11, 2020 at 09:51:15AM +0100, Krzysztof Kozlowski wrote: > On Wed, Nov 11, 2020 at 04:14:40AM +0300, Dmitry Osipenko wrote: > > Factor out clk initialization and make it resource-managed. This makes > > easier to follow code and will help to make further changes cleaner. > > > > Signed-off-by: Dmitry Osipenko > > --- > > drivers/memory/tegra/tegra30-emc.c | 70 ++++++++++++++++++++---------- > > 1 file changed, 47 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c > > index d27df842a667..1df42e212d73 100644 > > --- a/drivers/memory/tegra/tegra30-emc.c > > +++ b/drivers/memory/tegra/tegra30-emc.c > > @@ -1550,6 +1550,49 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc) > > return err; > > } > > > > +static void devm_tegra_emc_unset_callback(void *data) > > +{ > > + tegra20_clk_set_emc_round_callback(NULL, NULL); > > +} > > + > > +static void devm_tegra_emc_unreg_clk_notifier(void *data) > > +{ > > + struct tegra_emc *emc = data; > > + > > + clk_notifier_unregister(emc->clk, &emc->clk_nb); > > +} > > + > > +static int tegra_emc_init_clk(struct tegra_emc *emc) > > +{ > > + int err; > > + > > + tegra20_clk_set_emc_round_callback(emc_round_rate, emc); > > + > > + err = devm_add_action_or_reset(emc->dev, devm_tegra_emc_unset_callback, > > + NULL); > > + if (err) > > + return err; > > + > > + emc->clk = devm_clk_get(emc->dev, NULL); > > + if (IS_ERR(emc->clk)) { > > + dev_err(emc->dev, "failed to get EMC clock: %pe\n", emc->clk); > > + return PTR_ERR(emc->clk); > > + } > > + > > + err = clk_notifier_register(emc->clk, &emc->clk_nb); > > + if (err) { > > + dev_err(emc->dev, "failed to register clk notifier: %d\n", err); > > + return err; > > + } > > + > > + err = devm_add_action_or_reset(emc->dev, > > + devm_tegra_emc_unreg_clk_notifier, emc); > > + if (err) > > + return err; > > + > > + return 0; > > +} > > + > > static int tegra_emc_probe(struct platform_device *pdev) > > { > > struct device_node *np; > > @@ -1599,25 +1642,13 @@ static int tegra_emc_probe(struct platform_device *pdev) > > return err; > > } > > > > - tegra20_clk_set_emc_round_callback(emc_round_rate, emc); > > - > > - emc->clk = devm_clk_get(&pdev->dev, "emc"); > > - if (IS_ERR(emc->clk)) { > > - err = PTR_ERR(emc->clk); > > - dev_err(&pdev->dev, "failed to get emc clock: %d\n", err); > > - goto unset_cb; > > - } > > - > > - err = clk_notifier_register(emc->clk, &emc->clk_nb); > > - if (err) { > > - dev_err(&pdev->dev, "failed to register clk notifier: %d\n", > > - err); > > - goto unset_cb; > > - } > > + err = tegra_emc_init_clk(emc); > > + if (err) > > + return err; > > > > err = tegra_emc_opp_table_init(emc); > > if (err) > > - goto unreg_notifier; > > + return err; > > > > platform_set_drvdata(pdev, emc); > > tegra_emc_rate_requests_init(emc); > > @@ -1632,13 +1663,6 @@ static int tegra_emc_probe(struct platform_device *pdev) > > try_module_get(THIS_MODULE); > > > > return 0; > > - > > -unreg_notifier: > > - clk_notifier_unregister(emc->clk, &emc->clk_nb); > > You added this code in patch #8, so adding-and-removing a piece of code Correction: you added this in patch #9. Best regards, Krzysztof > is a nice hint that this patch should be before. Don't add new code > which later you simplify. Move all bugfixes and all simplifications to > beginning of patchset. > > That's quite similar case to v6 where you put bugfixes in the middle > of patchset. > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel