Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 17 Nov 2020 09:40:16 -0800
From:   Prashant Malani <pmalani@chromium.org>
To:     Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc:     linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        gregkh@linuxfoundation.org, enric.balletbo@collabora.com
Subject: Re: [PATCH v3 03/11] usb: typec: Add plug num_altmodes sysfs attr
Message-ID: <20201117174016.GA1819103@google.com>
References: <20201116201150.2919178-1-pmalani@chromium.org>
 <20201116201150.2919178-4-pmalani@chromium.org>
 <20201117124143.GI3437448@kuha.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201117124143.GI3437448@kuha.fi.intel.com>
Precedence: bulk

Hi Heikki,

On Tue, Nov 17, 2020 at 02:41:43PM +0200, Heikki Krogerus wrote:
> On Mon, Nov 16, 2020 at 12:11:42PM -0800, Prashant Malani wrote:
> > Add a field to the typec_plug struct to record the number of available
> > altmodes as well as the corresponding sysfs attribute to expose this to
> > userspace.
> > 
> > This allows userspace to determine whether there are any
> > remaining alternate modes left to be registered by the kernel driver. It
> > can begin executing any policy state machine after all available
> > alternate modes have been registered with the connector class framework.
> > 
> > This value is set to "-1" initially, signifying that a valid number of
> > alternate modes haven't been set for the plug. The sysfs file remains
> > hidden as long as the attribute value is -1.
> 
> Why couldn't we just keep it hidden for as long as the number of
> alt modes is 0? If you already explained that, then I apologise, I've
> forgotten.
> 

No worries :)

Succinctly, because 0 is a valid value for "number of altmodes
supported".

If we keep the attribute hidden for 0, then there won't
be a way for userspace to determine that PD discovery is done and we
don't expect any more cable plug altmodes to be registered by the kernel
Type C port driver (it can determine this by comparing
"number_of_altmodes" against the actual number of alt modes registered
by the Type C port driver).

If we keep "number_of_altmodes" hidden even for 0, the userspace cannot
differentiate between "this cable doesn't support any altmodes" and
"it does altmodes, but the PD stack hasn't completed PD Discovery
including DiscoverIdentity yet".

For reference, here is the initial patch and mini-discussion around it
back in July for port-partner altmodes [1] (I've followed a similar
logic here).

Hope this helps the rationale a bit more.

Best regards,

-Prashant

[1]:
https://lore.kernel.org/linux-usb/20200701082230.GF856968@kuha.fi.intel.com/