Re: [PATCH v2] ufs: host: ufschd-pltfrm: Hold reference returned by of_parse_phandle()

[Bart Van Assche <bvanassche@acm.org>]

On 7/16/22 20:03, Liang He wrote:
> At 2022-07-16 21:50:08, "Bart Van Assche" <bvanassche@acm.org> wrote:
>> On 7/14/22 17:17, Liang He wrote:
>>> +static bool phandle_exists(const struct device_node *np,
>>> +						const char *phandle_name,
>>> +						int index)
>>
>> Indentation of the arguments now looks really odd :-(
> 
> Yes, Bart, I also wonder this coding style, however I learned that
> from the definition of 'of_parse_phandle' in of.h.
> 
> Is it OK if I put all of them in one line？

No. From Documentation/process/coding-style.rst (please read that 
document in its entirety): "The preferred limit on the length of a 
single line is 80 columns. [...] A very commonly used style
is to align descendants to a function open parenthesis."

Consider to use the following formatting:

static bool phandle_exists(const struct device_node *np,
			   const char *phandle_name, int index)
{
	[ ... ]
}

>>> +{
>>> +	struct device_node *parse_np = of_parse_phandle(np, phandle_name, index);
>>> +	bool ret = false;
>>> +
>>> +	if (parse_np) {
>>> +		ret = true;
>>> +		of_node_put(parse_np);
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>
>> The 'ret' variable is not necessary. If "return ret" is changed into
>> "return parse_np" then the variable 'ret' can be left out.
>>
> 
> OK, I will use 'return parse_np' in new version when you confirm above coding style.

You may want to use "return parse_np != NULL" if you want to be sure 
that nobody else will complain about an implicit conversion of a pointer 
to a boolean type.

Thanks,

Bart.