Received-SPF: pass (google.com: domain of linux-kernel+bounces-58430-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 8 Feb 2024 11:14:51 -0600
From: Andrew Halaney <ahalaney@redhat.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: neil.armstrong@linaro.org, 
	Krishna Kurapati <quic_kriskura@quicinc.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, 
	Rob Herring <robh+dt@kernel.org>, Bjorn Andersson <andersson@kernel.org>, 
	Konrad Dybcio <konrad.dybcio@linaro.org>, Conor Dooley <conor+dt@kernel.org>, linux-kernel@vger.kernel.org, 
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, quic_ppratap@quicinc.com, 
	quic_jackp@quicinc.com
Subject: Re: Re: [PATCH 3/3] arm64: dts: qcom: sa8540-ride: Enable first port
 of tertiary usb controller
Message-ID: <baw3wxbdvzpkqqb6a7iut2wpt6jgzyqii5uyfkzptzt4ryjvao@4tpee6nqup5w>
References: <20240206114745.1388491-1-quic_kriskura@quicinc.com>
 <20240206114745.1388491-4-quic_kriskura@quicinc.com>
 <23824242-1b37-4544-ae9a-0a5a0582580e@linaro.org>
 <CAA8EJpqbXvKMQktGsxMFJnR+fXoOz8hFmm+E3ROPTjjiD0QLvg@mail.gmail.com>
 <6q2ocvrujbli42rjddflyol74xianr7j47jwcgdnnmwjanv25d@uw2da7zulqqd>
 <CAA8EJpr6k8c5C54S9xxQgZvd9NYFoxi5qQrOTz2AMrp0xeZZpw@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAA8EJpr6k8c5C54S9xxQgZvd9NYFoxi5qQrOTz2AMrp0xeZZpw@mail.gmail.com>

On Wed, Feb 07, 2024 at 03:32:03AM +0200, Dmitry Baryshkov wrote:
> On Wed, 7 Feb 2024 at 02:10, Andrew Halaney <ahalaney@redhat.com> wrote:
> >
> > On Tue, Feb 06, 2024 at 03:30:32PM +0200, Dmitry Baryshkov wrote:
> > > On Tue, 6 Feb 2024 at 15:28, <neil.armstrong@linaro.org> wrote:
> > > >
> > > > On 06/02/2024 12:47, Krishna Kurapati wrote:
> > > > > From: Andrew Halaney <ahalaney@redhat.com>
> > > > >
> > > > > There is now support for the multiport USB controller this uses so
> > > > > enable it.
> > > > >
> > > > > The board only has a single port hooked up (despite it being wired up to
> > > > > the multiport IP on the SoC). There's also a USB 2.0 mux hooked up,
> > > > > which by default on boot is selected to mux properly. Grab the gpio
> > > > > controlling that and ensure it stays in the right position so USB 2.0
> > > > > continues to be routed from the external port to the SoC.
> > >
> > > What is connected to the other port of the MUX?
> >
> > /me blows off the dust on the schematic
> >
> > It's a 1:2 mux, and one option is the out the board, the other
> > is a test point I believe if I'm reading things right, although its not
> > labeled so I'm not sure anyone would actually find it on the board.
> 
> Ack, this definitely looks like a static configuration.

Krishna, when you make v2 can you update the wording about the USB 2.0
mux? Maybe something like "which by default on boot is selected to mux
to the external port on the board (with the other option being a test
point)." instead of the wording I originally had? That way the
information Dmitry requested here is easily accessible in the future.

> 
> >
> > >
> > > > >
> > > > > Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
> > > > > Co-developed-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> > > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> > > > > ---
> > > > >   arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 21 +++++++++++++++++++++
> > > > >   1 file changed, 21 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
> > > > > index b04f72ec097c..eed1ddc29bc1 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
> > > > > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
> > > > > @@ -503,6 +503,18 @@ &usb_2_qmpphy0 {
> > > > >       status = "okay";
> > > > >   };
> > > > >
> > > > > +&usb_2 {
> > > > > +     pinctrl-0 = <&usb2_en>;
> > > > > +     pinctrl-names = "default";
> > > > > +
> > > > > +     status = "okay";
> > > > > +};
> > > > > +
> > > > > +&usb_2_dwc3 {
> > > > > +     phy-names = "usb2-port0", "usb3-port0";
> > > > > +     phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>;
> > > > > +};
> > > > > +
> > > > >   &xo_board_clk {
> > > > >       clock-frequency = <38400000>;
> > > > >   };
> > > > > @@ -655,4 +667,13 @@ wake-pins {
> > > > >                       bias-pull-up;
> > > > >               };
> > > > >       };
> > > > > +
> > > > > +     usb2_en: usb2-en-state {
> > > > > +             /* TS3USB221A USB2.0 mux select */
> > > > > +             pins = "gpio24";
> > > > > +             function = "gpio";
> > > > > +             drive-strength = <2>;
> > > > > +             bias-disable;
> > > > > +             output-low;
> > > > > +     };
> > > > >   };
> > > >
> > > > Isn't gpio-hog the preferred way to describe that ?
> > >
> > > That depends. As this pinctrl describes board configuration, I'd agree
> > > with Neil.
> >
> > I unfortunately don't have the experience with gpio-hog to weigh in
> > here, but wouldn't be opposed to Krishna switching it if that's what's
> > recommended for this type of thing.
> 
> Quoting gpio.txt:
> 
> The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
> providing automatic GPIO request and configuration as part of the
> gpio-controller's driver probe function.
> 
> See sdm845-pinctrl.yaml for an example of the gpio-hog node.

Thanks, that seems like the way to go. Krishna please take note of this
for v2!

> 
> >
> > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> > >
> >
> 
> 
> -- 
> With best wishes
> Dmitry
>