Hi Heikki, > -----Original Message----- > From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com] > Sent: 2018年5月21日 21:13 > To: Jun Li <jun...@nxp.com> > Cc: Mats Karrman <mats.dev.l...@gmail.com>; robh...@kernel.org; > gre...@linuxfoundation.org; li...@roeck-us.net; a.ha...@samsung.com; > cw00.c...@samsung.com; shufan_...@richtek.com; Peter Chen > <peter.c...@nxp.com>; gso...@gmail.com; devicet...@vger.kernel.org; > linux-usb@vger.kernel.org; dl-linux-imx <linux-...@nxp.com> > Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port > power > and data config > > Hi Jun, > > Sorry for the delay. > > On Thu, May 17, 2018 at 02:41:31PM +0000, Jun Li wrote: > > Hi > > > -----Original Message----- > > > From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com] > > > Sent: 2018??5??17?? 22:24 > > > To: Jun Li <jun...@nxp.com> > > > Cc: Mats Karrman <mats.dev.l...@gmail.com>; robh...@kernel.org; > > > gre...@linuxfoundation.org; li...@roeck-us.net; a.ha...@samsung.com; > > > cw00.c...@samsung.com; shufan_...@richtek.com; Peter Chen > > > <peter.c...@nxp.com>; gso...@gmail.com; devicet...@vger.kernel.org; > > > linux-usb@vger.kernel.org; dl-linux-imx <linux-...@nxp.com> > > > Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic > > > port power and data config > > > > > > On Thu, May 17, 2018 at 02:05:41PM +0000, Jun Li wrote: > > > > Hi Heikki, > > > > > > > > > > I reread this patch and tried to see it more in the context of > > > > > > the other patches and the existing code. The naming of the > > > > > > existing string tables doesn't help in getting this right, however > > > > > > I have > a proposal: > > > > > > > > > > > > typec_find_port_power_role() to get to TYPEC_PORT_SRC/SNK/DRP > > > > > > typec_find_port_data_role() to get to TYPEC_PORT_DFP/UFP/DRD > > > > > > typec_find_power_role() to get to TYPEC_SINK/SOURCE > > > > > > > > > > > > and sometime, if the use should arise > > > > > > > > > > > > typec_find_data_role() to get to TYPEC_DEVICE/HOST > > > > > > > > > > > > I think it is fairly comprehensible, *_port_* concerns a > > > > > > capability and without *_port_* it is an actual state. Plus it > > > > > > matches the names of the constants. > > > > > > > > > > Well, there are now four things to consider: > > > > > > > > > > 1) the roles (power and data) the port is capable of supporting > > > > > 2) Try.SRC and Try.SNK, i.e. the preferred role > > > > > 3) the current roles > > > > > 4) the role(s) the user want's to limit the use of a port with > > > > > DRP ports > > > > > > > > > > The last one I don't know if it's relevant with these functions. > > > > > It's not information that we would get for example from firmware. > > > > > > > > > > I also don't think we need to use these functions with the > > > > > current roles the port is in. > > > > > > > > > > If the preferred role is "sink" or "source", so just like the > > > > > power role, we don't need separate function for it here. > > > > > > > > > > So isn't two functions all we need here: one for the power and > > > > > one for data role? > > > > > > > > Take power as an example, can we use one function to only look up > > > > typec_port_types[]? as capability(typec_port_type) and > > > > state(typec_role) are different enum, and the allowed strings are > different. > > > > > > > > static const char * const typec_roles[] = { > > > > [TYPEC_SINK] = "sink", > > > > [TYPEC_SOURCE] = "source", > > > > }; > > > > > > > > static const char * const typec_port_types[] = { > > > > [TYPEC_PORT_SRC] = "source", > > > > [TYPEC_PORT_SNK] = "sink", > > > > [TYPEC_PORT_DRP] = "dual", > > > > }; > > > > > > Where out side the class code we need to convert the current role, > > > the "state", with these functions? > > > > Currently, the only call site is getting the preferred power role from > > firmware. > > My point was that if we used enum typec_port_type with the prefered role, we > could use the helper for power role with prefered role. But never mind.
Got your point. So let's follow Mats's suggestion on this, I will update a new version. Thanks Li Jun > > > Thanks, > > -- > heikki
