Re: [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: schowdhu@codeaurora.org
Cc: Rob Herring <robh@kernel.org>,
	linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
	devicetree@vger.kernel.org,
	Bryan O'Donoghue <pure.logic@nexus-software.ie>,
	Greg KH <greg@kroah.com>,
	linux-kernel@vger.kernel.org, ckadabi@codeaurora.org,
	tsoni@codeaurora.org, bryanh@codeaurora.org,
	psodagud@codeaurora.org, satyap@codeaurora.org,
	pheragu@codeaurora.org, Rajendra Nayak <rnayak@codeaurora.org>,
	Sibi Sankar <sibis@codeaurora.org>,
	Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Subject: Re: [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector
Date: Tue, 5 Oct 2021 09:37:39 -0700	[thread overview]
Message-ID: <YVx/U+w8zS6/P6oa@ripper> (raw)
In-Reply-To: <b3d10d7b874c11462604a5f78bc0e8cf@codeaurora.org>

On Tue 05 Oct 06:11 PDT 2021, schowdhu@codeaurora.org wrote:

> On 2021-10-04 22:07, Rob Herring wrote:
> > On Mon, Oct 04, 2021 at 04:46:19PM +0530, Souradeep Chowdhury wrote:
> > > Added the property for EUD(Embedded USB Debug) connector.Added
> > > the "reg" and "interrupts" property which is needed for EUD.
> > 
> > You are going to need a better explanation of this h/w.
> 
> Ack. Will update this with the detailed hardware description
> in the next version.
> 
> > 
> > > 
> > > Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> > > ---
> > >  .../devicetree/bindings/connector/usb-connector.yaml      | 15
> > > +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > index 7eb8659..908129f 100644
> > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > @@ -30,6 +30,21 @@ properties:
> > >            - const: samsung,usb-connector-11pin
> > >            - const: usb-b-connector
> > > 
> > > +      - items:
> > > +          - enum:
> > > +              - qcom,sc7280-usb-connector-eud
> > > +          - const: qcom,usb-connector-eud
> > > +          - const: usb-c-connector
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: EUD Base Register Region
> > > +      - description: EUD Mode Manager Region
> > 
> > A connector node represents the physical connector on a board. That
> > can't really be an MMIO peripheral. Maybe you need a node for EUD and
> > then it should have a connector child node? Don't really know without
> > understanding this h/w.
> 
> As per the previous discussion on the EUD, it was agreed upon to map EUD
> as a type C connector and use Role-Switch to change the USB role instead
> of extcon interface that was being used previously. The link for the same
> is as follows:-
> 
> https://lore.kernel.org/lkml/5db1a666-62ec-c850-6626-ad33d337b452@codeaurora.org/
> 

Not using extcon is the right thing, but perhaps we should make the EUD
a role_switch provider and client, so that we can describe how it sits
inbetween the connector and the controller.

That way it has the power to pass through or override requests from the
upstream role-switcher, based on the status of EUD.

That said, I'm still curious to what happens if I renegotiate the roles
dynamically in a Type-C environment, while enabling EUD. How would the
device on the other end of the cable know that it's supposed to be a
host? Or there's simply a reset of the link when this happens?

Thanks,
Bjorn

> > 
> > > +
> > > +  interrupts:
> > > +    description:
> > > +      EUD interrupt
> > > +
> > >    label:
> > >      description: Symbolic name for the connector.
> > > 
> > > --
> > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> > > member
> > > of Code Aurora Forum, hosted by The Linux Foundation
> > > 
> > > 