| Message ID | 1603098195-9923-2-git-send-email-jun.li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/4] dt-bindings: usb: add documentation for typec switch simple driver | expand |
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.
> -----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. > > > +} > > > + * 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. Yes, will change. > > ... > > > +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. Ok, will move them in next version. Thanks Li Jun > > -- > With Best Regards, > Andy Shevchenko >
On Tue, Oct 20, 2020 at 2:35 PM Jun Li <jun.li@nxp.com> wrote: > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Sent: Monday, October 19, 2020 8:25 PM > > On Mon, Oct 19, 2020 at 05:03:13PM +0800, Li Jun wrote: ... > 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. Thanks for checking this!
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; Thanks.
> -----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
diff --git a/drivers/base/property.c b/drivers/base/property.c index d58aa98..32e3a3e 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -1184,3 +1184,38 @@ const void *device_get_match_data(struct device *dev) return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); } EXPORT_SYMBOL_GPL(device_get_match_data); + +/** + * fwnode_is_compatible - Check does fwnode have the given compatible string + * @fwnode: fwnode with the "compatible" property + * @compat: The compatible string + * + * Match the compatible strings of @fwnode against @compat. Returns positive + * value on match, and 0 when no matching compatible string is found. + */ +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; +} +EXPORT_SYMBOL_GPL(fwnode_is_compatible); + +/** + * device_is_compatible - Check does a device have the given compatible string + * @dev: Device with the "compatible" property + * @compat: The compatible string + * + * Match the compatible strings of @dev against @compat. Returns positive value + * on match, and 0 when no matching compatible string is found. + */ +int device_is_compatible(struct device *dev, const char *compat) +{ + return fwnode_is_compatible(dev_fwnode(dev), compat); +} +EXPORT_SYMBOL_GPL(device_is_compatible); diff --git a/include/linux/property.h b/include/linux/property.h index 9f805c4..4a84325 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -418,6 +418,9 @@ fwnode_graph_get_endpoint_by_id(const struct fwnode_handle *fwnode, int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode, struct fwnode_endpoint *endpoint); +int fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compat); +int device_is_compatible(struct device *dev, const char *compat); + /* -------------------------------------------------------------------------- */ /* Software fwnode support - when HW description is incomplete or missing */