Re: [PATCH v4 2/4] device property: Add fwnode_is_compatible() and device_is_compatible() helpers

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Oct 19, 2020 at 05:03:13PM +0800, Li Jun wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> Since there are also some ACPI platforms where the
> "compatible" property is used, introducing a generic helper
> function fwnode_is_compatible() that can be used with
> DT, ACPI and swnodes, and a wrapper function
> device_is_compatible() with it.
> 
> The function calls of_device_is_comaptible() with OF nodes,
> and with ACPI and swnodes it matches the given string
> against the "compatible" string property array.

...

> + * Match the compatible strings of @fwnode against @compat. Returns positive
> + * value on match, and 0 when no matching compatible string is found.

Please move Returns... to a separate paragraph.

Btw, this is not true...

> +int fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compat)
> +{
> +	int ret;
> +
> +	if (is_of_node(fwnode))
> +		return of_device_is_compatible(to_of_node(fwnode), compat);
> +
> +	ret = fwnode_property_match_string(fwnode, "compatible", compat);
> +

> +	return ret < 0 ? 0 : 1;

...and this is at least strange after all.

> +}

> + * Match the compatible strings of @dev against @compat. Returns positive value
> + * on match, and 0 when no matching compatible string is found.

So does this.

...

> +int fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compat);
> +int device_is_compatible(struct device *dev, const char *compat);

Please, group them rather device_is_compatible() with device_* and fwnode_*
ones respectively.

-- 
With Best Regards,
Andy Shevchenko