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=-12.2 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 E5DD6C433ED for ; Mon, 26 Apr 2021 12:25:40 +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 3F46B60233 for ; Mon, 26 Apr 2021 12:25:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3F46B60233 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.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-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Reply-To:Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=HjGctXhtBaoNZCfGVOTedZ3qg7KO24CYfHwplduLV44=; b=LmbrTzzDLUo+8ALeMCt1s9IFh EofZi5BF3yQqzT9l0Ek9UVQxrUNEqwC/i/lLW02nHLbzrd6HGyAcw0Kx4PI8s1xCVapRI7a2JS+7c 0x6mp/mMgyY0z+L7hZZg6U2eScsuTLyy14xDNdkfZb6Tq48ZxwiXKmlX48sgPc3ySlb+zLcTH1r6O x2JQO3vYJMvY2LAW5UWwtToB1+xD4OSJJqPnG+AGp0cnwHG2WGf6GA4OEnGHUUYZfzDU6e4FdiqsS 8qUFonL6v3cRHmlqHyxwM3VcxO+uETT5q7lE3UU+cnA3OEi8efUK65DCukK9+QED6ppu32P7iv4QZ 2A3PmffuA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lb0HS-007bMh-Er; Mon, 26 Apr 2021 12:24:02 +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 1lb0HP-007bML-7z for linux-arm-kernel@desiato.infradead.org; Mon, 26 Apr 2021 12:23:59 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=8swn70jS2igG/qafgPHRcbPgOFVezO2bodI9Xxqjq4Y=; b=X8FXsODgcslEtEj33sniiX8/6a gW+iQNLymuanJ1Q43kyLRFuTVatsJNgN0BOUZuCYEz/nW52WlcKKMvxitkxuH57HJ6gASIha6Ablj oRn3hkLKQutZT9eTtrrosguPXED6hDdhdGEr/6yafEtANveUSM/0l5lNi230nJMGyfs1YCzjBzV43 zHuWNbFlwqI81RapEYjlb/a0oYjaHFfGlVW6bzIGMyKzq1YB6JzvvP+43KCbEdH3FaH9BK1h1ESmq rFge6YQZhTM+fgFthn/7m2COK4JQmlmdqtStu22D9Hi5fvtNNlv9U8V8OZZhADQWTa3rIsc11xsAi wiJTVbjg==; Received: from mail-wr1-x432.google.com ([2a00:1450:4864:20::432]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lb0HM-00FxCE-Bj for linux-arm-kernel@lists.infradead.org; Mon, 26 Apr 2021 12:23:58 +0000 Received: by mail-wr1-x432.google.com with SMTP id r7so43675488wrm.1 for ; Mon, 26 Apr 2021 05:23:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=8swn70jS2igG/qafgPHRcbPgOFVezO2bodI9Xxqjq4Y=; b=n2dCqeqBlAg0pMtEOTGB5EIo/TVLkmAy6hSltDzgtYM6xWf1D4PexXryrMSv7fWcAT 86W+f7EkH2ppY3XwKexeRAY41adthczkdab7VJYmWydSgeOdzDLwfZjIZWOcnH7adBds lvKQKwtMwwXEBYmnRvsfF+SMmN4Kw3x9wF7quesDz+678QntYP9hOy4cu+hXdXfEdHMc 9nmPmyfl/7IfSJtmEwkoNXu/M0kanjNxCYo6bci24m56Hre4MBQWdSbanNfDD9dLef9L lVFwXAhbDAkBEbDCM5iELotGUMq1Pjy7daQfqaEpYscALTJ1/Qt1bNTDt6zy8HWr8miA L5Pg== 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:user-agent; bh=8swn70jS2igG/qafgPHRcbPgOFVezO2bodI9Xxqjq4Y=; b=MBGWpo8916z+IgaufT/Wrx9Vm+rRtJYbTO25M7qqYoYgfFhFqQZcDdgOWXim2z6hSg amE+ATKjPobEYIjd6HSHGvX71JWO3a38URoBOxmNi2HMw6zrwU2gttf/hscQWFmy9mf5 auRlm3wHXvIBOIwHp+aoxhzVoOWBmCMX7FBG9gouRL2PcGxFQEf08uQIsbZviYrfW60r nDzsncIGMRjqkdyjMcbgF3F9XZ12XEmDT4m1uWYOetiaqBaJOPKConB4QHCOUbQVjcP8 CVMN8+7OaEzOcdOYDM9EbZJ5JZmQbsAU07j5zRYJtO3f9k52ibfpTi8TC3v8CXWDxgXM WWWg== X-Gm-Message-State: AOAM532sm8l6sVHKHO9KKzBIRCGgLChUoZcHvblLJMc4MeMvXyxFtcK1 xtix/ggRLBYcSesFxgP5Nxc= X-Google-Smtp-Source: ABdhPJyAAgxwNKwU3lStxhsmsEyrJyTSvAB0OKEg8PdzOwMzxJgVfK5YVC0+XfG86MqA7Z5ETQ0jmQ== X-Received: by 2002:a5d:6e06:: with SMTP id h6mr22552762wrz.201.1619439835087; Mon, 26 Apr 2021 05:23:55 -0700 (PDT) Received: from localhost ([62.96.65.119]) by smtp.gmail.com with ESMTPSA id d14sm10837551wrp.12.2021.04.26.05.23.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Apr 2021 05:23:53 -0700 (PDT) Date: Mon, 26 Apr 2021 14:24:46 +0200 From: Thierry Reding To: Krzysztof Kozlowski Cc: Jon Hunter , Dmitry Osipenko , linux-tegra@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 03/10] memory: tegra: Push suspend/resume into SoC drivers Message-ID: References: <20210420165237.3523732-1-thierry.reding@gmail.com> <20210420165237.3523732-4-thierry.reding@gmail.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/2.0.6 (98f8cb83) (2021-03-06) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210426_052356_449611_2D917FEA X-CRM114-Status: GOOD ( 35.18 ) 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: multipart/mixed; boundary="===============9153591086171595993==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============9153591086171595993== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="QyCVhmrfZDBfN07X" Content-Disposition: inline --QyCVhmrfZDBfN07X Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Apr 26, 2021 at 10:47:11AM +0200, Krzysztof Kozlowski wrote: > On 20/04/2021 18:52, Thierry Reding wrote: > > From: Thierry Reding > >=20 > > Continuing the scheme of unification, push suspend/resume callbacks into > > per-SoC driver so that they can be properly parameterized. > >=20 > > While at it, also move the ->init() callback into the new tegra_mc_ops > > structure to keep things clean. >=20 > Please split this part. This is just moving pointer from one structure > to another, quite small change. >=20 > The rest of the patchset is quite different - you now call > tegra_mc_suspend() from a per-SoC driver and move the code around. Okay, I think I can split that off into a separate patch. > > 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(-) > >=20 > > 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 *p= dev) > > =20 > > mc->debugfs.root =3D debugfs_create_dir("mc", NULL); > > =20 > > - if (mc->soc->init) { > > - err =3D mc->soc->init(mc); > > + if (mc->soc->ops && mc->soc->ops->init) { > > + err =3D 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; > > } > > =20 > > -static int tegra_mc_suspend(struct device *dev) > > +static int __maybe_unused tegra_mc_suspend(struct device *dev) > > { > > struct tegra_mc *mc =3D dev_get_drvdata(dev); > > - int err; > > =20 > > - if (IS_ENABLED(CONFIG_TEGRA_IOMMU_GART) && mc->gart) { > > - err =3D tegra_gart_suspend(mc->gart); > > - if (err) > > - return err; > > - } > > + if (mc->soc->ops && mc->soc->ops->suspend) > > + return mc->soc->ops->suspend(mc); > > =20 > > return 0; > > } > > =20 > > -static int tegra_mc_resume(struct device *dev) > > +static int __maybe_unused tegra_mc_resume(struct device *dev) > > { > > struct tegra_mc *mc =3D dev_get_drvdata(dev); > > - int err; > > =20 > > - if (IS_ENABLED(CONFIG_TEGRA_IOMMU_GART) && mc->gart) { > > - err =3D tegra_gart_resume(mc->gart); > > - if (err) > > - return err; > > - } > > + if (mc->soc->ops && mc->soc->ops->resume) > > + return mc->soc->ops->resume(mc); > > =20 > > return 0; > > } > > diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/teg= ra186.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) > > } > > } > > =20 > > +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 =3D { > > + .resume =3D tegra186_mc_resume, > > +}; > > + > > #if defined(CONFIG_ARCH_TEGRA_186_SOC) > > static const struct tegra_mc_client tegra186_mc_clients[] =3D { > > { > > @@ -701,6 +712,7 @@ static const struct tegra_mc_client tegra186_mc_cli= ents[] =3D { > > static const struct tegra_mc_soc tegra186_mc_soc =3D { > > .num_clients =3D ARRAY_SIZE(tegra186_mc_clients), > > .clients =3D tegra186_mc_clients, > > + .ops =3D &tegra186_mc_ops, > > }; > > #endif > > =20 > > @@ -1909,6 +1921,7 @@ static const struct tegra_mc_client tegra194_mc_c= lients[] =3D { > > static const struct tegra_mc_soc tegra194_mc_soc =3D { > > .num_clients =3D ARRAY_SIZE(tegra194_mc_clients), > > .clients =3D tegra194_mc_clients, > > + .ops =3D &tegra186_mc_ops, > > }; > > #endif > > =20 > > @@ -1961,22 +1974,28 @@ static const struct of_device_id tegra186_mc_of= _match[] =3D { > > }; > > MODULE_DEVICE_TABLE(of, tegra186_mc_of_match); > > =20 > > -static int __maybe_unused tegra186_mc_suspend(struct device *dev) > > +static int __maybe_unused tegra_mc_suspend(struct device *dev) > > { > > + struct tegra_mc *mc =3D dev_get_drvdata(dev); > > + > > + if (mc->soc->ops && mc->soc->ops->suspend) > > + return mc->soc->ops->suspend(mc); > > + > > return 0; > > } > > =20 > > -static int __maybe_unused tegra186_mc_resume(struct device *dev) > > +static int __maybe_unused tegra_mc_resume(struct device *dev) > > { > > struct tegra_mc *mc =3D dev_get_drvdata(dev); > > =20 > > - tegra186_mc_program_sid(mc); > > + if (mc->soc->ops && mc->soc->ops->resume) > > + return mc->soc->ops->resume(mc); > > =20 > > return 0; > > } > > =20 > > static const struct dev_pm_ops tegra186_mc_pm_ops =3D { > > - SET_SYSTEM_SLEEP_PM_OPS(tegra186_mc_suspend, tegra186_mc_resume) > > + SET_SYSTEM_SLEEP_PM_OPS(tegra_mc_suspend, tegra_mc_resume) >=20 > 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()). This confusion is merely temporary. The idea here is to make the Tegra186 driver the same as the "legacy" driver in this regard so that the unification patch becomes just a removal of this code. If I didn't first make this the same, I would've had to push this set of changes into the unification patch, making it more difficult to understand. Thierry --QyCVhmrfZDBfN07X Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmCGsQ4ACgkQ3SOs138+ s6HBKA/8CInhxbN7i16ucd8J8UklQ/GmGaA2S8RUt/wHUCnR7DZhty0ysl7tSApS xlj0D+VLkT21+FLiEaFcyGdOtEVguSHJzfdi0gqpvzLFESSq77Jc1pWpVc2qakYp IdhyoHx2ytgwAyThVSG4I0Os2P2Nle+6x+ZvyK/YtFErSIE6iSLLUXs1jinBT9Fi 0RIpX/j5Rs28XCmO6Gb0+3rJJyPWwXotwMmciobPwjkdpmDi6EsZX1KWNSxZ1LNh XhQY6fAS5dWawAWadQCJ+Zy2PGeFUEnpkHKiiZxa1+WvhRmu5jbyX8sZgNSkql97 HWBrqf6x5mkgEP5GYKhuJGI3x6LzrCHVCYXZvgSLI6yWPsFOY38Z6NaTEpo56/0d 7DTzICiIQ+uB4QOmbmjmJaWmEEg2v8ltPShBqxekvIXKgrdFxUf6cjfMtyf0FW4Z AlqJvzLXhi20IZfZkc7VZZwMz3JaVThSkjnwqdjoxrfTM7x6vedbAw87YZtBnnlY KOVR3WLasuPAsbBsG5NI6OPuBtK8EHnO2tLPcoyqXaO/YqpoX8waalZcfRbYxtlj +MFjuhuXs8BmAr4KWwdTfwq/XkiaxkPA9LqBueYLSrLmritAAP2sOvjn44SV/LZ3 aPkvwEl9npIR2mXPyZrIYbwkvemRGO8/Vz4GS211kMK4NmgTfko= =SmRg -----END PGP SIGNATURE----- --QyCVhmrfZDBfN07X-- --===============9153591086171595993== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============9153591086171595993==--