| Message ID | 20240206114745.1388491-4-quic_kriskura@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 | expand |
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 >
> 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. > >> >>> [...] >>>>>> }; >>>>> >>>>> 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! > Hi Andrew, Can you help test the following patch. It is just an add-on to your original one. I don't have a SA8540P Ride at the moment and getting one might take time. Incase you can confirm this patch is working. I can push v2 of this series. diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml index ed344deaf8b9..aa42ac5a3197 100644 --- a/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml @@ -36,6 +36,10 @@ patternProperties: $ref: "#/$defs/qcom-sc8280xp-tlmm-state" additionalProperties: false + "-hog(-[0-9]+)?$": + required: + - gpio-hog + $defs: qcom-sc8280xp-tlmm-state: type: object diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts index b04f72ec097c..aa0cec0b4cc2 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_state>; + 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,19 @@ wake-pins { bias-pull-up; }; }; + + usb2-en-hog { + gpio-hog; + gpios = <24 GPIO_ACTIVE_LOW>; + output-low; + }; + + usb2_en_state: usb2-en-state { + /* TS3USB221A USB2.0 mux select */ + pins = "gpio24"; + function = "gpio"; + drive-strength = <2>; + bias-disable; + output-low; + }; Regards, Krishna,
On Sat, 10 Feb 2024 at 12:44, Krishna Kurapati PSSNV <quic_kriskura@quicinc.com> wrote: > > > 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. > > > >> > >>> > > [...] > > >>>>>> }; > >>>>> > >>>>> 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! > > > > Hi Andrew, > > Can you help test the following patch. It is just an add-on to your > original one. I don't have a SA8540P Ride at the moment and getting one > might take time. Incase you can confirm this patch is working. I can > push v2 of this series. > > > diff --git > a/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml > b/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml > index ed344deaf8b9..aa42ac5a3197 100644 > --- a/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml > +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml > @@ -36,6 +36,10 @@ patternProperties: > $ref: "#/$defs/qcom-sc8280xp-tlmm-state" > additionalProperties: false > > + "-hog(-[0-9]+)?$": > + required: > + - gpio-hog > + > $defs: > qcom-sc8280xp-tlmm-state: > type: object > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > index b04f72ec097c..aa0cec0b4cc2 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_state>; > + 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,19 @@ wake-pins { > bias-pull-up; > }; > }; > + > + usb2-en-hog { > + gpio-hog; > + gpios = <24 GPIO_ACTIVE_LOW>; > + output-low; > + }; > + > + usb2_en_state: usb2-en-state { If you are using gpio-hog, you don't need this state. The pinctrl / gpio core will use the hog instead. > + /* TS3USB221A USB2.0 mux select */ > + pins = "gpio24"; > + function = "gpio"; > + drive-strength = <2>; > + bias-disable; > + output-low; > + }; > > > Regards, > Krishna,
On Sat, Feb 10, 2024 at 04:13:51PM +0530, Krishna Kurapati PSSNV wrote: > > 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. > > > > > > > > > > > [...] > > > > > > > > }; > > > > > > > > > > > > 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! > > > > Hi Andrew, > > Can you help test the following patch. It is just an add-on to your > original one. I don't have a SA8540P Ride at the moment and getting one > might take time. Incase you can confirm this patch is working. I can push v2 > of this series. I just realized that unfortunately I no longer have access to a sa8540p-ride, and I'm not sure if I'll regain access. So I would not be opposed to dropping this patch altogether and someone dealing with sa8540p-ride when they can test it :/ Sorry, Andrew > > > diff --git > a/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml > b/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml > index ed344deaf8b9..aa42ac5a3197 100644 > --- a/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml > +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc8280xp-tlmm.yaml > @@ -36,6 +36,10 @@ patternProperties: > $ref: "#/$defs/qcom-sc8280xp-tlmm-state" > additionalProperties: false > > + "-hog(-[0-9]+)?$": > + required: > + - gpio-hog > + > $defs: > qcom-sc8280xp-tlmm-state: > type: object > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > index b04f72ec097c..aa0cec0b4cc2 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_state>; > + 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,19 @@ wake-pins { > bias-pull-up; > }; > }; > + > + usb2-en-hog { > + gpio-hog; > + gpios = <24 GPIO_ACTIVE_LOW>; > + output-low; > + }; > + > + usb2_en_state: usb2-en-state { > + /* TS3USB221A USB2.0 mux select */ > + pins = "gpio24"; > + function = "gpio"; > + drive-strength = <2>; > + bias-disable; > + output-low; > + }; > > > Regards, > Krishna, >
On 2/13/2024 12:47 AM, Andrew Halaney wrote: >> >> Hi Andrew, >> >> Can you help test the following patch. It is just an add-on to your >> original one. I don't have a SA8540P Ride at the moment and getting one >> might take time. Incase you can confirm this patch is working. I can push v2 >> of this series. > > I just realized that unfortunately I no longer have access to a > sa8540p-ride, and I'm not sure if I'll regain access. > > So I would not be opposed to dropping this patch altogether and someone > dealing with sa8540p-ride when they can test it :/ > Hi Andrew, It would take time for me to get my hands on one of them. I can take up this patch once I get access to hw. In the meantime I can push the first two and get this series with. Regards, Krishna,
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; + }; };