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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
