> -----Original Message----- > From: Heikki Krogerus [mailto:heikki.krogerus@xxxxxxxxxxxxxxx] > Sent: 2018年2月26日 21:19 > To: Jun Li <jun.li@xxxxxxx> > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; linux@xxxxxxxxxxxx; > a.hajda@xxxxxxxxxxx; mark.rutland@xxxxxxx; yueyao@xxxxxxxxxx; > Peter Chen <peter.chen@xxxxxxx>; garsilva@xxxxxxxxxxxxxx; > o_leveque@xxxxxxxxx; shufan_lee@xxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > Subject: Re: [PATCH v2 01/12] usb: typec: add API to get port type and > preferred role > > On Mon, Feb 26, 2018 at 07:49:08PM +0800, Li Jun wrote: > > This patch add 2 APIs to get port type and preferred role from > > firmware description. > > > > Signed-off-by: Li Jun <jun.li@xxxxxxx> > > > > --- > > change for v2 > > - Change the 2 APIs name and input para to be device_node pointer. > > Why? > > You are only dealing with device properties here, so please move back to > using the unified device property API. > Will change it back in next version. > > drivers/usb/typec/typec.c | 46 > > ++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/usb/typec.h | 2 ++ > > 2 files changed, 48 insertions(+) > > > > diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c > > index 735726c..e7b2802 100644 > > --- a/drivers/usb/typec/typec.c > > +++ b/drivers/usb/typec/typec.c > > @@ -9,6 +9,7 @@ > > #include <linux/device.h> > > #include <linux/module.h> > > #include <linux/mutex.h> > > +#include <linux/of.h> > > No need for that. > > > #include <linux/slab.h> > > #include <linux/usb/typec.h> > > > > @@ -1246,6 +1247,51 @@ void typec_set_pwr_opmode(struct typec_port > > *port, } EXPORT_SYMBOL_GPL(typec_set_pwr_opmode); > > > > +/** > > + * typec_of_get_port_type - Get the typec port type > > + * @np: device node from which the property value is to be read. > > + * > > + * This routine is used by typec hardware driver to read property > > +port type > > + * from the device firmware description. > > + * > > + * Returns typec_port_type if success, otherwise negative error code. > > + */ > > +int typec_of_get_port_type(struct device_node *np) > > int typec_get_port_type(struct device *dev) > > I would expect you to have the function like that even if you really were > calling of_* functions in it so we would not need to change the API later > once support for ACPI or some other type of FW interface is added. But as > said, that will not be the case, and you need to use > device_property_* functions instead. > > > +{ > > + const char *type_str; > > + int ret; > > + > > + ret = of_property_read_string(np, "port-type", &type_str); > > ret = device_property_read_string(dev, "port-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_port_type); > > Note that you are still exporting the old function name, but that's fine. Just > change the function name back to the original. > will change it back. > > +/** > > + * typec_of_get_preferred_role - Get the typec preferred role > > + * @np: device node from which the property value is to be read. > > + * > > + * This routine is used by typec hardware driver to read property > > +default role > > + * from the device firmware description. > > + * > > + * Returns typec_role if success, otherwise negative error code. > > + */ > > +int typec_of_get_preferred_role(struct device_node *np) > > int typec_get_preferred_role(struct device dev) > > > +{ > > + const char *power_str; > > + int ret; > > + > > + ret = of_property_read_string(np, "default-role", &power_str); > > ret = device_property_read_string(dev, "default-role", &type_str); > > > + if (ret < 0) > > + return ret; > > + > > + return match_string(typec_roles, ARRAY_SIZE(typec_roles), > > +power_str); } EXPORT_SYMBOL_GPL(typec_get_preferred_role); > > + > > /* --------------------------------------- */ > > Thanks, > > -- > heikki ?韬{.n?????%??檩??w?{.n????z谵{???塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f
