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=-15.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 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 40595C433ED for ; Mon, 26 Apr 2021 08:49:12 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 8D5B260233 for ; Mon, 26 Apr 2021 08:49:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8D5B260233 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=canonical.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=xNu1FAGwYFWSpuP3Y8ewPHwpM7FDhXb1iRZTEivhO9A=; b=hbd7m3SqmTm+5GSHp/x/jichJ YdGBmlU/Lbyg7QUAuRdDWXizQVRCcgKPSCmpbKVqLdN+1dA/I4dWU4JpAYPG5gNp44f5pWhufg8cS d1MQRvZxbjO4Kw6dVYYOrCT6+bNTwzZtzCJUNq6Ays1SvtZKMoS2oqIhv18r9KP/++NLQp0UoKPM1 VKLnbEQZE1cUh3RvILnpu9Jtb1dstvvPpap92Q36TXZmNZti60fi5vyJlwjFaAAxgUg5Vmx1GQvdT UmlHvj+ZW1hydZ9CAmp+BBl9QiT/9wtqJWCXvNcRh3tqB9vIr1KhNP1Yg70csfYNGSivHHfoDQCKv GRndqjPTw==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lawtl-007ESc-E0; Mon, 26 Apr 2021 08:47:21 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lawti-007ESK-Ut for linux-arm-kernel@desiato.infradead.org; Mon, 26 Apr 2021 08:47:19 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:References:Cc:To: Subject:Sender:Reply-To:Content-ID:Content-Description; bh=WNiJ9j/ReCCELnSWT4R1Xa4Eh6A3I3MqyvJFgayyiu0=; b=N06+rgFHaPA7ajK/tOK/g/T6CU T7MQiUGWcgCKqMpq6TnPTd9K6BsOok3fsJGxJpF3K3cD1bbTEnz8SNdQVQ/weTDWVyzUGHI7uV5WA znBz0HjC1J9FQVQhDdF2UG5MXGnlS51XnFQaPnt/dye7CxPiCLBwr8HZHPLfNUQrq9F1O7HUxF7pL ibSDbICarIc7W/IK5p3O+7Sgupn80DHY/DYUNJiCKi1ZYdO0nb3Tht/sRJDP4b03SFCyzRDnFHA4M Jr3DJiRWxcdI8FLwkdOyuGetE7tj705X0R73vU/aUZU5Gm0KEiuJp2CT320TDTlQoDv3cvtQ6cnN7 NwKSLygA==; Received: from youngberry.canonical.com ([91.189.89.112]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lawte-00FpyW-Vz for linux-arm-kernel@lists.infradead.org; Mon, 26 Apr 2021 08:47:16 +0000 Received: from mail-wm1-f71.google.com ([209.85.128.71]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lawtd-0004ph-I2 for linux-arm-kernel@lists.infradead.org; Mon, 26 Apr 2021 08:47:13 +0000 Received: by mail-wm1-f71.google.com with SMTP id c76-20020a1c9a4f0000b02901429c95d1c8so159715wme.9 for ; Mon, 26 Apr 2021 01:47:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=WNiJ9j/ReCCELnSWT4R1Xa4Eh6A3I3MqyvJFgayyiu0=; b=GsYWjeDrxGbk3FbzyP8o1/pudsAWkqd25Y1brI0+PY+Ws8HT8RIxX5TsHdbgy+Yn9M f0g8MEj+pMvT4VjyWtecVupPtIg1vQoc5PmEy78sOh8ADDFbPJgmIXsyXToCBJNZgNga KvEgoZ0zuxhso4CxRa3HnsM/yQdTwb6Oj7WFshWt3GtequbdljPte7tzJQtBEQbS5rHI ellgyquvJ/rhRTLPxJAQRbW0NrhQXt8GXLjL8TKoP8Uw2iXxUH4C2VODKhP9neao659H ebsOUrbBg90oUrnNAy7J2HKjI64IARx1Crie2BT3vCRkxF7BmfmgFk/iheb7PvMtXGoG lC8w== X-Gm-Message-State: AOAM5326qUDvgT6lzmu4PgfDJW2ak8oZfnSIVaq2IGQiClAeSkq5zTRc sm9Q84VvMr3mDAwp08Dp3m+Al4K3EtAc0ENIP1cxMeRc8YIg1hypxhiunqIBTIk60PB180q1DBY aeL0szVJKsHH2nmFV4Y0/If0BS0MCFhDR+hS8k6lPE3llSnfvIjsJ X-Received: by 2002:a1c:7e82:: with SMTP id z124mr20054568wmc.51.1619426833031; Mon, 26 Apr 2021 01:47:13 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyrUFfbGUozT8GEmNRuPdPF+EWL2jPVTerTlxDnmXt9wwCFYRqudvrmg9R48/z7Z4pCqLkpZQ== X-Received: by 2002:a1c:7e82:: with SMTP id z124mr20054556wmc.51.1619426832804; Mon, 26 Apr 2021 01:47:12 -0700 (PDT) Received: from [192.168.1.115] (xdsl-188-155-180-75.adslplus.ch. [188.155.180.75]) by smtp.gmail.com with ESMTPSA id l9sm19848389wrz.7.2021.04.26.01.47.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 26 Apr 2021 01:47:12 -0700 (PDT) Subject: Re: [PATCH 03/10] memory: tegra: Push suspend/resume into SoC drivers To: Thierry Reding Cc: Jon Hunter , Dmitry Osipenko , linux-tegra@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20210420165237.3523732-1-thierry.reding@gmail.com> <20210420165237.3523732-4-thierry.reding@gmail.com> From: Krzysztof Kozlowski Message-ID: Date: Mon, 26 Apr 2021 10:47:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210420165237.3523732-4-thierry.reding@gmail.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210426_014715_202604_201DF296 X-CRM114-Status: GOOD ( 27.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 20/04/2021 18:52, Thierry Reding wrote: > From: Thierry Reding > > Continuing the scheme of unification, push suspend/resume callbacks into > per-SoC driver so that they can be properly parameterized. > > While at it, also move the ->init() callback into the new tegra_mc_ops > structure to keep things clean. Please split this part. This is just moving pointer from one structure to another, quite small change. The rest of the patchset is quite different - you now call tegra_mc_suspend() from a per-SoC driver and move the code around. > > Signed-off-by: Thierry Reding > --- > drivers/memory/tegra/mc.c | 24 ++++++++--------------- > drivers/memory/tegra/tegra186.c | 27 ++++++++++++++++++++++---- > drivers/memory/tegra/tegra20.c | 34 ++++++++++++++++++++++++++++++++- > include/soc/tegra/mc.h | 9 +++++++-- > 4 files changed, 71 insertions(+), 23 deletions(-) > > diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c > index b7e104bf6614..2b21131d779c 100644 > --- a/drivers/memory/tegra/mc.c > +++ b/drivers/memory/tegra/mc.c > @@ -829,8 +829,8 @@ static int tegra_mc_probe(struct platform_device *pdev) > > mc->debugfs.root = debugfs_create_dir("mc", NULL); > > - if (mc->soc->init) { > - err = mc->soc->init(mc); > + if (mc->soc->ops && mc->soc->ops->init) { > + err = mc->soc->ops->init(mc); > if (err < 0) > dev_err(&pdev->dev, "failed to initialize SoC driver: %d\n", > err); > @@ -867,30 +867,22 @@ static int tegra_mc_probe(struct platform_device *pdev) > return 0; > } > > -static int tegra_mc_suspend(struct device *dev) > +static int __maybe_unused tegra_mc_suspend(struct device *dev) > { > struct tegra_mc *mc = dev_get_drvdata(dev); > - int err; > > - if (IS_ENABLED(CONFIG_TEGRA_IOMMU_GART) && mc->gart) { > - err = tegra_gart_suspend(mc->gart); > - if (err) > - return err; > - } > + if (mc->soc->ops && mc->soc->ops->suspend) > + return mc->soc->ops->suspend(mc); > > return 0; > } > > -static int tegra_mc_resume(struct device *dev) > +static int __maybe_unused tegra_mc_resume(struct device *dev) > { > struct tegra_mc *mc = dev_get_drvdata(dev); > - int err; > > - if (IS_ENABLED(CONFIG_TEGRA_IOMMU_GART) && mc->gart) { > - err = tegra_gart_resume(mc->gart); > - if (err) > - return err; > - } > + if (mc->soc->ops && mc->soc->ops->resume) > + return mc->soc->ops->resume(mc); > > return 0; > } > diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c > index 8e77567d1378..9d3fdb609d55 100644 > --- a/drivers/memory/tegra/tegra186.c > +++ b/drivers/memory/tegra/tegra186.c > @@ -45,6 +45,17 @@ static void tegra186_mc_program_sid(struct tegra_mc *mc) > } > } > > +static int tegra186_mc_resume(struct tegra_mc *mc) > +{ > + tegra186_mc_program_sid(mc); > + > + return 0; > +} > + > +static const struct tegra_mc_ops tegra186_mc_ops = { > + .resume = tegra186_mc_resume, > +}; > + > #if defined(CONFIG_ARCH_TEGRA_186_SOC) > static const struct tegra_mc_client tegra186_mc_clients[] = { > { > @@ -701,6 +712,7 @@ static const struct tegra_mc_client tegra186_mc_clients[] = { > static const struct tegra_mc_soc tegra186_mc_soc = { > .num_clients = ARRAY_SIZE(tegra186_mc_clients), > .clients = tegra186_mc_clients, > + .ops = &tegra186_mc_ops, > }; > #endif > > @@ -1909,6 +1921,7 @@ static const struct tegra_mc_client tegra194_mc_clients[] = { > static const struct tegra_mc_soc tegra194_mc_soc = { > .num_clients = ARRAY_SIZE(tegra194_mc_clients), > .clients = tegra194_mc_clients, > + .ops = &tegra186_mc_ops, > }; > #endif > > @@ -1961,22 +1974,28 @@ static const struct of_device_id tegra186_mc_of_match[] = { > }; > MODULE_DEVICE_TABLE(of, tegra186_mc_of_match); > > -static int __maybe_unused tegra186_mc_suspend(struct device *dev) > +static int __maybe_unused tegra_mc_suspend(struct device *dev) > { > + struct tegra_mc *mc = dev_get_drvdata(dev); > + > + if (mc->soc->ops && mc->soc->ops->suspend) > + return mc->soc->ops->suspend(mc); > + > return 0; > } > > -static int __maybe_unused tegra186_mc_resume(struct device *dev) > +static int __maybe_unused tegra_mc_resume(struct device *dev) > { > struct tegra_mc *mc = dev_get_drvdata(dev); > > - tegra186_mc_program_sid(mc); > + if (mc->soc->ops && mc->soc->ops->resume) > + return mc->soc->ops->resume(mc); > > return 0; > } > > static const struct dev_pm_ops tegra186_mc_pm_ops = { > - SET_SYSTEM_SLEEP_PM_OPS(tegra186_mc_suspend, tegra186_mc_resume) > + SET_SYSTEM_SLEEP_PM_OPS(tegra_mc_suspend, tegra_mc_resume) What's the benefit here? You basically define your own suspend-resume ops, on top of PM suspend-resume ops... Before it was quite obvious code - the Tegra186 MC driver had very simple suspend/resume which did simple job. Now it feels like trickier code to follow - Tegra186 driver calls it's resume (with the same name as others - another confusion) which is a simple wrapper calling somewhere else (need to jump to assinment of resume()). Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel