From: jonathanh@nvidia•com (Jon Hunter)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 2/4] drm/tegra: Add VIC support
Date: Mon, 20 Jul 2015 10:56:00 +0100 [thread overview]
Message-ID: <55ACC5B0.20703@nvidia.com> (raw)
In-Reply-To: <55ACB6A2.9030906@nvidia.com>
On 20/07/15 09:51, Mikko Perttunen wrote:
> On 07/20/2015 11:28 AM, Jon Hunter wrote:
>> Hi Mikko,
>
> Hi!
>
>> ...
>>> +static int vic_runtime_resume(struct device *dev)
>>> +{
>>> + struct vic *vic = dev_get_drvdata(dev);
>>> + int err;
>>> +
>>> + err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
>>> + vic->clk, vic->rst);
>>
>> I would like to deprecate use of the above function [1]. I have been
>> trying to migrate driver to use a new API instead of the above.
>
> Yes, we will need to coordinate the API change here. I kept the old way
> here to not depend on your series for now.
>
>>
>>> + if (err < 0)
>>> + dev_err(dev, "failed to power up device\n");
>>> +
>>> + return err;
>>> +}
>>> +
>>> ...
>>> +
>>> + pm_runtime_enable(&pdev->dev);
>>> + if (!pm_runtime_enabled(&pdev->dev)) {
>>> + err = vic_runtime_resume(&pdev->dev);
>>> + if (err < 0)
>>> + goto unregister_client;
>>> + }
>>
>> I don't see why pm_runtime_enable() should ever fail to enable
>> pm_runtime here. Hence, the if-statement appears to be redundant. If you
>> are trying to determine whether rpm is supported for the power-domains
>> then you should simply check to see if the DT node for the device has
>> the "power-domains" property. See my series [1].
>
> The intention is that if runtime PM is not enabled, the power domain is
> still brought up. On a cursory look, it seems like with your patch, this
> should indeed work fine without this check if CONFIG_PM is enabled. Now,
> that might always be enabled? If so, then everything would be fine; if
> this lands after your series, we could even just make the power-domains
> prop mandatory and not implement the legacy mode at all. If not, then
> AFAIU we must still keep this path.
Ok, I see you just wish to test it is enabled in the kernel config. Then
yes that would make sense and the above is fine.
Cheers
Jon
next prev parent reply other threads:[~2015-07-20 9:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-20 7:54 [PATCH v2 0/4] Add VIC support for Tegra124 Mikko Perttunen
2015-07-20 7:54 ` [PATCH v2 1/4] drm/tegra: Add falcon helper library Mikko Perttunen
2015-07-20 7:54 ` [PATCH v2 2/4] drm/tegra: Add VIC support Mikko Perttunen
2015-07-20 8:28 ` Jon Hunter
2015-07-20 8:51 ` Mikko Perttunen
2015-07-20 9:56 ` Jon Hunter [this message]
2015-07-20 10:17 ` Thierry Reding
2015-07-20 9:35 ` Thierry Reding
2015-07-20 7:54 ` [PATCH v2 3/4] of: Add NVIDIA Tegra VIC binding Mikko Perttunen
2015-07-20 17:20 ` Emil Velikov
2015-07-21 6:32 ` Mikko Perttunen
2015-07-20 7:54 ` [PATCH v2 4/4] ARM: tegra: Add VIC for Tegra124 Mikko Perttunen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55ACC5B0.20703@nvidia.com \
--to=jonathanh@nvidia$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox