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