linux-kernel - Re: [PATCH v1 8/8] drm/etnaviv: Add a helper to get a pointer to the first available node

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]

Date:   Mon, 17 Jul 2023 18:20:16 +0800
From:   suijingfeng <suijingfeng@...ngson.cn>
To:     Lucas Stach <l.stach@...gutronix.de>,
        Sui Jingfeng <sui.jingfeng@...ux.dev>,
        Russell King <linux+etnaviv@...linux.org.uk>,
        Christian Gmeiner <christian.gmeiner@...il.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>
Cc:     loongson-kernel@...ts.loongnix.cn, etnaviv@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 8/8] drm/etnaviv: Add a helper to get a pointer to the
 first available node

Hi,

On 2023/7/17 18:07, Lucas Stach wrote:
> Am Freitag, dem 23.06.2023 um 18:08 +0800 schrieb Sui Jingfeng:
>> From: Sui Jingfeng <suijingfeng@...ngson.cn>
>>
>> This make the code in etnaviv_pdev_probe() less twisted, drop the reference
>> to device node after finished. Also kill a double blank line.
>>
> I can't spot the double blank line you claim to remove.
>
>> Signed-off-by: Sui Jingfeng <suijingfeng@...ngson.cn>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_drv.c | 32 ++++++++++++++++++---------
>>   1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> index 7d0eeab3e8b7..3446f8eabf59 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> @@ -27,6 +27,19 @@
>>    * DRM operations:
>>    */
>>   
>> +/* If the DT contains at least one available GPU, return a pointer to it */
>> +

Here is the double blank line my patch remove, it (a blank line) is occupied by
the comment of "/* If the DT contains at least one available GPU, return a pointer to it */"


> I think the code in the function is simple enough that we don't need a
> comment explaining what it does.

Ok, then I'll remove the comment at the next version. Thanks


> Regards,
> Lucas
>
>> +static struct device_node *etnaviv_of_first_node(void)
>> +{
>> +	struct device_node *np;
>> +
>> +	for_each_compatible_node(np, NULL, "vivante,gc") {
>> +		if (of_device_is_available(np))
>> +			return np;
>> +	}
>> +
>> +	return NULL;
>> +}
>>   
>>   static void load_gpu(struct drm_device *dev)
>>   {
>> @@ -587,7 +600,7 @@ static const struct component_master_ops etnaviv_master_ops = {
>>   static int etnaviv_pdev_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>> -	struct device_node *first_node = NULL;
>> +	struct device_node *first_node;
>>   	struct component_match *match = NULL;
>>   
>>   	if (!dev->platform_data) {
>> @@ -597,11 +610,10 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
>>   			if (!of_device_is_available(core_node))
>>   				continue;
>>   
>> -			if (!first_node)
>> -				first_node = core_node;
>> -
>>   			drm_of_component_match_add(&pdev->dev, &match,
>>   						   component_compare_of, core_node);
>> +
>> +			of_node_put(core_node);
>>   		}
>>   	} else {
>>   		char **names = dev->platform_data;
>> @@ -634,8 +646,11 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
>>   	 * device as the GPU we found. This assumes that all Vivante
>>   	 * GPUs in the system share the same DMA constraints.
>>   	 */
>> -	if (first_node)
>> +	first_node = etnaviv_of_first_node();
>> +	if (first_node) {
>>   		of_dma_configure(&pdev->dev, first_node, true);
>> +		of_node_put(first_node);
>> +	}
>>   
>>   	return component_master_add_with_match(dev, &etnaviv_master_ops, match);
>>   }
>> @@ -709,17 +724,14 @@ static int __init etnaviv_init(void)
>>   	 * If the DT contains at least one available GPU device, instantiate
>>   	 * the DRM platform device.
>>   	 */
>> -	for_each_compatible_node(np, NULL, "vivante,gc") {
>> -		if (!of_device_is_available(np))
>> -			continue;
>> +	np = etnaviv_of_first_node();
>> +	if (np) {
>>   		of_node_put(np);
>>   
>>   		ret = etnaviv_create_platform_device("etnaviv",
>>   						     &etnaviv_platform_device);
>>   		if (ret)
>>   			goto unregister_platform_driver;
>> -
>> -		break;
>>   	}
>>   
>>   	return 0;