From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966245AbdAFPsI (ORCPT ); Fri, 6 Jan 2017 10:48:08 -0500 Received: from bh-25.webhostbox.net ([208.91.199.152]:42920 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966031AbdAFPrZ (ORCPT ); Fri, 6 Jan 2017 10:47:25 -0500 Date: Fri, 6 Jan 2017 07:47:22 -0800 From: Guenter Roeck To: Heikki Krogerus Cc: Mika Westerberg , Oliver Neukum , Greg KH , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCHv14 2/3] usb: USB Type-C connector class Message-ID: <20170106154722.GA5378@roeck-us.net> References: <20170105110119.87401-1-heikki.krogerus@linux.intel.com> <20170105110119.87401-3-heikki.krogerus@linux.intel.com> <20170105155402.GG3353@lahna.fi.intel.com> <20170106105405.GA31031@kuha.fi.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170106105405.GA31031@kuha.fi.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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