Re: [RFC PATCHv3] usb: USB Type-C Connector Class

[Guenter Roeck]

On 06/11/2016 12:05 AM, Oliver Neukum wrote:
> On Fri, 2016-06-10 at 17:34 +0300, Heikki Krogerus wrote:
> > +static ssize_t
> > +preferred_role_store(struct device *dev, struct device_attribute
> > *attr,
> > +                    const char *buf, size_t size)
> > +{
> > +       struct typec_port *port = to_typec_port(dev);
> > +       enum typec_role role;
> > +       int ret;
> > +
> > +       mutex_lock(&port->lock);
> > +
> > +       if (port->cap->type != TYPEC_PORT_DRP) {
> > +               dev_dbg(dev, "Try.SRC/SNK only supported with DRP
> > ports\n");
> > +               ret = -EOPNOTSUPP;
> > +               goto out;
> > +       }
>
> I think Try.SNK with an UFP should fail differently from
> Try.SRC and vice versa.

I think the message should not even mention Try.src or Try.snk
in the first place, since both are optional and really orthogonal
to how the role is set (it can also be changed with pd messages).
On top of that, the specification states that Try.{src/snk} are
optional if there is no PD support, and NA if PD is supported
(see table 4.10). Kind of interesting, and I wonder how this
is supposed to be implemented in practice, since Try.{src/snk}
runs before it is established that/if the partner supports PD.

A better message would be something like "Setting preferred
role only supported with DRP ports", and leave implementation
details to the port driver.

Guenter