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

[Jun Li <jun.li@nxp.com>]

> -----Original Message-----
> From: dmitry.torokhov@gmail.com <dmitry.torokhov@gmail.com>
> Sent: Thursday, October 22, 2020 2:56 AM
> To: Jun Li <jun.li@nxp.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>;
> heikki.krogerus@linux.intel.com; robh+dt@kernel.org; rafael@kernel.org;
> gregkh@linuxfoundation.org; hdegoede@redhat.com; lee.jones@linaro.org;
> mika.westerberg@linux.intel.com;
> prabhakar.mahadev-lad.rj@bp.renesas.com;
> laurent.pinchart+renesas@ideasonboard.com; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Peter Chen
> <peter.chen@nxp.com>
> Subject: Re: [PATCH v4 2/4] device property: Add fwnode_is_compatible() and
> device_is_compatible() helpers
> 
> On Tue, Oct 20, 2020 at 11:13:47AM +0000, Jun Li wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Sent: Monday, October 19, 2020 8:25 PM
> > > To: Jun Li <jun.li@nxp.com>
> > > Cc: heikki.krogerus@linux.intel.com; robh+dt@kernel.org;
> > > rafael@kernel.org; gregkh@linuxfoundation.org; hdegoede@redhat.com;
> > > lee.jones@linaro.org; mika.westerberg@linux.intel.com;
> > > dmitry.torokhov@gmail.com; prabhakar.mahadev-lad.rj@bp.renesas.com;
> > > laurent.pinchart+renesas@ideasonboard.com;
> > > linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx
> > > <linux-imx@nxp.com>; Peter Chen <peter.chen@nxp.com>
> > > Subject: Re: [PATCH v4 2/4] device property: Add
> > > fwnode_is_compatible() and
> > > device_is_compatible() helpers
> > >
> > > 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.
> >
> > OK, will change.
> >
> > >
> > > 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.
> >
> > of_device_is_compatible() will return positive value on match, and 0
> > when no matching,
> > fwnode_property_match_string() will return 0 if the property was found
> > (success); and negative error code on failure. so the return
> > conversion of fwnode_property_match_string () should work here.
> 
> Yes, but please make the function return bool instead of int.
> of_device_is_compatible() returns "score", so it is int, but here we only
> want yes/no.
> 
> So
> 
> 	return fwnode_property_match_string(...) == 0;

My understanding this is to keep this new API fwnode_is_compatible() consistent
with of_device_is_compatible(). I would wait patch author Heikki to comment this.

thanks
Li Jun 
> 
> Thanks.
> 
> --
> Dmitry