linux-kernel - Re: [PATCHv3 2/4] drm/tegra: Add runtime pm support for gr2d

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <524BB1B9.1040806@nvidia.com>
Date:	Wed, 2 Oct 2013 08:40:09 +0300
From:	Arto Merilainen <amerilainen@...dia.com>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	Terje Bergstrom <tbergstrom@...dia.com>,
	Thierry Reding <treding@...dia.com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Mayuresh Kulkarni <mkulkarni@...dia.com>
Subject: Re: [PATCHv3 2/4] drm/tegra: Add runtime pm support for gr2d

On 10/01/2013 09:14 PM, Stephen Warren wrote:
> On 09/24/2013 06:05 AM, Arto Merilainen wrote:
>> diff --git a/drivers/gpu/host1x/drm/gr2d.c b/drivers/gpu/host1x/drm/gr2d.c
>
>> @@ -327,11 +336,48 @@ static int __exit gr2d_remove(struct platform_device *pdev)
>>   		host1x_syncpt_free(gr2d->client.syncpts[i]);
>>
>>   	host1x_channel_free(gr2d->channel);
>> +
>> +	if (pm_runtime_enabled(&pdev->dev))
>> +		pm_runtime_disable(&pdev->dev);
>> +	else
>> +		gr2d_runtime_suspend(&pdev->dev);
>
> This code is slightly different to the code in e.g.
> sound/soc/tegra/tegra20_i2s.c:remove(), whereas the code in probe() is
> identical. I'm not sure whether there's some advantage in this version?
> If so, perhaps the sound drivers should be updated to be consistent. If
> not, perhaps this driver should do the same thing as the I2S driver, so
> we keep the drivers consistent, and provide the same "example" code
> everywhere.

Hmm. I cannot immediately see any advantage in this version... I will 
update the patch.

>
>> +static int gr2d_runtime_suspend(struct device *dev)
>> +{
>> +	struct gr2d *gr2d;
>> +
>> +	gr2d = dev_get_drvdata(dev);
>> +	if (!gr2d)
>> +		return -EINVAL;
>
> Presumably, gr2d will never be NULL here, unless there's some chronic
> bug. Can't we re-write those last 5 lines as simply:
>
> 	struct gr2d *grd2 = dev_get_drvdata(dev);
>
> If that's not valid, we should probably update the audio drivers (and
> perhaps others) too.

I think you are correct. I cannot see any reason why that check is required.

- Arto
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/