RE: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config

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

 



Hi Heikki,

> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.krogerus@xxxxxxxxxxxxxxx]
> Sent: 2018年5月21日 21:13
> To: Jun Li <jun.li@xxxxxxx>
> Cc: Mats Karrman <mats.dev.list@xxxxxxxxx>; robh+dt@xxxxxxxxxx;
> gregkh@xxxxxxxxxxxxxxxxxxx; linux@xxxxxxxxxxxx; a.hajda@xxxxxxxxxxx;
> cw00.choi@xxxxxxxxxxx; shufan_lee@xxxxxxxxxxx; Peter Chen
> <peter.chen@xxxxxxx>; gsomlo@xxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-usb@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> 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.krogerus@xxxxxxxxxxxxxxx]
> > > Sent: 2018??5??17?? 22:24
> > > To: Jun Li <jun.li@xxxxxxx>
> > > Cc: Mats Karrman <mats.dev.list@xxxxxxxxx>; robh+dt@xxxxxxxxxx;
> > > gregkh@xxxxxxxxxxxxxxxxxxx; linux@xxxxxxxxxxxx; a.hajda@xxxxxxxxxxx;
> > > cw00.choi@xxxxxxxxxxx; shufan_lee@xxxxxxxxxxx; Peter Chen
> > > <peter.chen@xxxxxxx>; gsomlo@xxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> > > linux-usb@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> > > 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
?韬{.n?????%??檩??w?{.n???{炳???骅w*jg????????G??⒏⒎?:+v????????????"??????



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux