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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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.

> 
> > +}
> 
> > + * 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
> 





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux