Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Heikki Krogerus]

Hi Oliver,

> > +static ssize_t alternate_mode_store(struct device *dev,
> > +				    struct device_attribute *attr,
> > +				    const char *buf, size_t size)
> > +{
> > +	struct typec_port *port = to_typec_port(dev);
> > +	struct typec_alt_mode alt_mode;
> > +	int ret;
> > +
> > +	if (!port->cap->set_alt_mode) {
> > +		dev_warn(dev, "entering Alternate Modes not supported\n");
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	if (!port->connected)
> > +		return -ENXIO;
> 
> Doesn't this need locking?

Yes, I need to fix the locking.

> And why wouldn't user space want to preselect a mode?

That is tricky, as we would need to keep a list of the preselected
modes and for all SVIDs the connector supports. I don't think it would
be practical to do from this file as we would then use it differently
when connected and not connected, so the preselected modes would
probable be better to give from a separate file.

That is certainly doable, but is it really useful? I not really
against adding that support, but I would like to keep this interface
as simple as possible.


Thanks,

-- 
heikki