Hi, On Tue, Mar 13, 2018 at 05:34:30PM +0800, Li Jun wrote: > +/** > + * typec_get_power_type - Get the typec port power type > + * @fwnode: Firmware node to get the property of > + * > + * This routine is used by typec hardware driver to read property > + * port power type from the device firmware description. > + * > + * Returns typec_port_type if success, otherwise negative error code. > + */ > +int typec_get_power_type(struct fwnode_handle *fwnode) > +{ > + const char *type_str; > + int ret; > + > + if (!fwnode) > + return -EINVAL; > + > + ret = fwnode_property_read_string(fwnode, "power-type", &type_str); > + if (ret < 0) > + return ret; > + > + return match_string(typec_port_types, ARRAY_SIZE(typec_port_types), > + type_str); > +} > +EXPORT_SYMBOL_GPL(typec_get_power_type); > > +/** > + * typec_get_preferred_role - Get the typec preferred power role > + * @fwnode: Firmware node to get the property of > + * > + * This routine is used by typec hardware driver to read property > + * try-power-role from the device firmware description to decide > + * if the port can support Try.SRC or Try.SNK. > + * > + * Returns typec_role if success, otherwise negative error code. > + */ > +int typec_get_preferred_role(struct fwnode_handle *fwnode) > +{ > + const char *type_str; > + int ret; > + > + if (!fwnode) > + return -EINVAL; > + > + ret = fwnode_property_read_string(fwnode, "try-power-role", &type_str); > + if (ret < 0) > + return ret; > + > + return match_string(typec_roles, ARRAY_SIZE(typec_roles), type_str); > +} > +EXPORT_SYMBOL_GPL(typec_get_preferred_role); > + > +/** > + * typec_get_data_type - Get the typec port data type > + * @fwnode: Firmware node to get the property of > + * > + * This routine is used by typec hardware driver to read property > + * port data type from the device firmware description. > + * > + * Returns typec_data_type if success, otherwise negative error code. > + */ > +int typec_get_data_type(struct fwnode_handle *fwnode) > +{ > + const char *type_str; > + int ret; > + > + if (!fwnode) > + return -EINVAL; > + > + ret = fwnode_property_read_string(fwnode, "data-type", &type_str); > + if (ret < 0) > + return ret; > + > + return match_string(typec_data_types, ARRAY_SIZE(typec_data_types), > + type_str); > +} > +EXPORT_SYMBOL_GPL(typec_get_data_type); I been thinking about this. Perhaps it's better that we don't read any properties in these helpers. These helpers become more useful that way, and we can use the in other places as well if needed. So my proposal is that the callers of these functions are responsible of reading the property values. Here we just convert the strings to values: int typec_find_preferred_role(const char *name) { return match_string(typec_roles, ARRAY_SIZE(typec_roles), name); } EXPORT_SYMBOL_GPL(typec_find_preferred_role); int typec_find_power_role(const char *name) { return match_string(typec_port_types, ARRAY_SIZE(typec_data_types), name); } EXPORT_SYMBOL_GPL(typec_find_data_role); int typec_find_data_role(const char *name) { return match_string(typec_data_types, ARRAY_SIZE(typec_data_types), name); } EXPORT_SYMBOL_GPL(typec_find_data_role); Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html