On Tue, Oct 20, 2020 at 11:13:47AM +0000, Jun Li wrote:
>
>
> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Sent: Monday, October 19, 2020 8:25 PM
> > To: Jun Li <jun.li@xxxxxxx>
> > Cc: heikki.krogerus@xxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> > rafael@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; hdegoede@xxxxxxxxxx;
> > lee.jones@xxxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx;
> > dmitry.torokhov@xxxxxxxxx; prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx;
> > laurent.pinchart+renesas@xxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; Peter Chen
> > <peter.chen@xxxxxxx>
> > 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@xxxxxxxxxxxxxxx>
> > >
> > > 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;
Thanks.
--
Dmitry
