Re: [PATCHv14 2/3] usb: USB Type-C connector class

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 06, 2017 at 12:54:05PM +0200, Heikki Krogerus wrote:
> Hi guys,
> 
> On Thu, Jan 05, 2017 at 05:54:02PM +0200, Mika Westerberg wrote:
> > > +static ssize_t
> > > +typec_altmode_roles_show(struct device *dev, struct device_attribute *attr,
> > > +			 char *buf)
> > > +{
> > > +	struct typec_mode *mode = container_of(attr, struct typec_mode,
> > > +					       roles_attr);
> > > +	ssize_t ret;
> > > +
> > > +	switch (mode->roles) {
> > > +	case TYPEC_PORT_DFP:
> > > +		ret =  sprintf(buf, "source\n");
> > 
> > Extra space after '='.
> > 
> > > +		break;
> > > +	case TYPEC_PORT_UFP:
> > > +		ret = sprintf(buf, "sink\n");
> > > +		break;
> > > +	case TYPEC_PORT_DRP:
> > > +	default:
> > > +		ret = sprintf(buf, "source\nsink\n");
> > 
> > I wonder if "source sink" instead is better?  Along the lines of
> > /sys/power/state.
> > 
> > Then you can print "[source] sink" when source is selected and so on.
> 
> That is more or less how I originally proposed how we list the roles
> in general. I introduced the separate "current_*_role" and
> "supported_*_roles" attribute files because somebody wanted them. I
> don't remember the reason why they were preferred to be in separate
> attribute files.
> 
> Oliver! Guenter! Do we really need to list the current and supported
> roles in separate attribute files? Can't we just have the "power_role"
> and "data_role" attribute files for the ports instead of the separate
> "supported_*_roles" and "current_*_role", and show the current role
> like Mika proposes? I definitely would prefer it that way because it
> is similar style used in other places like Mike pointed out.
> 
Consistency with other drivers/attribute should be preferrable,
but either way is ok with me.

> And since we are talking about the ABI, can we also change the listing
> of the accessory mode back to just "audio" and "debug" like I
> originally had it? I don't remember who and why wanted it to be
> changed to "Audio Adapter Accessory Mode" and "Debug Accessory Mode",
> but it differs from the style we list the other details.
> 

I prefer computer readable attributes over human readable,
so the change is fine with me.

Guenter
--
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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux