Re: [PATCH net-next] net: wan: framer: Simplify API framer_provider_simple_of_xlate() implementation

[Zijun Hu]

On 2024/12/13 00:41, Simon Horman wrote:
>>  struct framer *framer_provider_simple_of_xlate(struct device *dev,
>>  					       const struct of_phandle_args *args)
>>  {
>> -	struct class_dev_iter iter;
>> -	struct framer *framer;
>> -
>> -	class_dev_iter_init(&iter, &framer_class, NULL, NULL);
>> -	while ((dev = class_dev_iter_next(&iter))) {
>> -		framer = dev_to_framer(dev);
>> -		if (args->np != framer->dev.of_node)
>> -			continue;
>> +	struct device *target_dev;
>>  
>> -		class_dev_iter_exit(&iter);
>> -		return framer;
>> +	target_dev = class_find_device_by_of_node(&framer_class, args->np);
>> +	if (target_dev) {
>> +		put_device(target_dev);
>> +		return dev_to_framer(target_dev);
>>  	}
>>  
>> -	class_dev_iter_exit(&iter);
>>  	return ERR_PTR(-ENODEV);
> Hi Zijun Hu,
> 
> FWIIW, I think it would be more idiomatic to have the non-error path in the
> main flow of execution, something like this (completely untested!):
> 
> 	target_dev = class_find_device_by_of_node(&framer_class, args->np);
> 	if (!target_dev)
> 		return ERR_PTR(-ENODEV);
> 
> 	put_device(target_dev);
> 	return dev_to_framer(target_dev);
> 
thank you Simon for code review.
good suggestion. let me take it in v2.

> Also, is it safe to put_device(target_dev) before
> passing target_dev to dev_to_framer() ?

Successful class_find_device_by_of_node() invocation will increase the
refcount of @target_dev, so i put_device() here to keep the same logic
as original.

thank you.