Re: [RFC PATCHv3] usb: USB Type-C Connector Class
On Sat, Jun 11, 2016 at 11:03:53AM -0700, Guenter Roeck wrote:
> 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(>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.
Works for me.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3] usb: USB Type-C Connector Class
On Sat, Jun 11, 2016 at 09:05:46AM +0200, 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(>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.
>
> And, even more importantly, this offeres no way to cancel
> a preference.
Well, no way that would actually work. I'll fix that.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3] usb: USB Type-C Connector Class
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(>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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3] usb: USB Type-C Connector Class
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(>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.
And, even more importantly, this offeres no way to cancel
a preference.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
