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

[Jun Li <jun.li@nxp.com>]

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",
};

Thanks
Li Jun
> 
> 
> Thanks,
> 
> --
> heikki