> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com]
> Sent: 2018年2月26日 21:19
> To: Jun Li <jun...@nxp.com>
> Cc: gre...@linuxfoundation.org; robh...@kernel.org; li...@roeck-us.net;
> a.ha...@samsung.com; mark.rutl...@arm.com; yue...@google.com;
> Peter Chen <peter.c...@nxp.com>; garsi...@embeddedor.com;
> o_leve...@orange.fr; shufan_...@richtek.com; linux-usb@vger.kernel.org;
> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> 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...@nxp.com>
> >
> > ---
> > 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�Р骒r��y����b�X�肚�v�^�)藓{.n�+�伐�{焙柒��^n�r■�z���h�ㄨ��&Ⅷ �G���h�(�����茛j"���m����赇z罐��帼f＂�h���~�m�
