[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8f9e8ec5-fc78-4b51-b36f-3ba06768ce62@ti.com>
Date: Tue, 6 Feb 2024 11:34:48 -0600
From: Andrew Davis <afd@...com>
To: Roger Quadros <rogerq@...nel.org>, <nm@...com>, <vigneshr@...com>
CC: <kristo@...nel.org>, <robh+dt@...nel.org>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<srk@...com>, <r-gunasekaran@...com>, <b-liu@...com>,
<linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: k3-am62p: add the USB sub-system
On 2/6/24 6:30 AM, Roger Quadros wrote:
>
>
> On 05/02/2024 19:34, Andrew Davis wrote:
>> On 2/5/24 7:59 AM, Roger Quadros wrote:
>>> There are two USB instances available on the am62p5 starter kit. Include
>>> and enable them for use on the board.
>>>
>>> Signed-off-by: Vignesh Raghavendra <vigneshr@...com>
>>> Signed-off-by: Roger Quadros <rogerq@...nel.org>
>>> ---
>>>
>>> Notes:
>>> Changelog:
>>> v4 - no change
>>> v3 - no change
>>> https://lore.kernel.org/all/20240201120332.4811-4-rogerq@kernel.org/
>>> v2:
>>> - added USB PHY CTRL node changes here
>>> - changed USB wrapper node names to usb@
>>> - changed Type-C chip node name to usb-power-control@
>>>
>>> arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 46 ++++++++++++++
>>> arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 10 +++
>>> arch/arm64/boot/dts/ti/k3-am62p5-sk.dts | 67 +++++++++++++++++++++
>>> 3 files changed, 123 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>>> index 4c51bae06b57..17d28390d587 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>>> @@ -560,6 +560,52 @@ sdhci2: mmc@...0000 {
>>> status = "disabled";
>>> };
>>> + usbss0: usb@...0000 {
>>> + compatible = "ti,am62-usb";
>>> + reg = <0x00 0x0f900000 0x00 0x800>;
>>> + clocks = <&k3_clks 161 3>;
>>> + clock-names = "ref";
>>> + ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + power-domains = <&k3_pds 178 TI_SCI_PD_EXCLUSIVE>;
>>> + ranges;
>>> + status = "disabled";
>>> +
>>> + usb0: usb@...00000 {
>>> + compatible = "snps,dwc3";
>>> + reg = <0x00 0x31000000 0x00 0x50000>;
>>> + interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>, /* irq.0 */
>>> + <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>; /* irq.0 */
>>> + interrupt-names = "host", "peripheral";
>>> + maximum-speed = "high-speed";
>>> + dr_mode = "otg";
>>> + };
>>> + };
>>> +
>>> + usbss1: usb@...0000 {
>>> + compatible = "ti,am62-usb";
>>> + reg = <0x00 0x0f910000 0x00 0x800>;
>>> + clocks = <&k3_clks 162 3>;
>>> + clock-names = "ref";
>>> + ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>;
>>> + ranges;
>>> + status = "disabled";
>>> +
>>> + usb1: usb@...00000 {
>>> + compatible = "snps,dwc3";
>>> + reg = <0x00 0x31100000 0x00 0x50000>;
>>> + interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>, /* irq.0 */
>>> + <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>; /* irq.0 */
>>> + interrupt-names = "host", "peripheral";
>>> + maximum-speed = "high-speed";
>>> + dr_mode = "otg";
>>> + };
>>> + };
>>> +
>>> fss: bus@...0000 {
>>> compatible = "simple-bus";
>>> reg = <0x00 0x0fc00000 0x00 0x70000>;
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
>>> index 19f42b39394e..00dd38b02a52 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
>>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
>>> @@ -18,6 +18,16 @@ chipid: chipid@14 {
>>> reg = <0x14 0x4>;
>>> bootph-all;
>>> };
>>> +
>>> + usb0_phy_ctrl: syscon@...8 {
>>> + compatible = "ti,am62-usb-phy-ctrl", "syscon";
>>> + reg = <0x4008 0x4>;
>>> + };
>>> +
>>> + usb1_phy_ctrl: syscon@...8 {
>>> + compatible = "ti,am62-usb-phy-ctrl", "syscon";
>>> + reg = <0x4018 0x4>;
>>> + };
>>> };
>>> wkup_uart0: serial@...00000 {
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
>>> index 1773c05f752c..80be56c0a4e0 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
>>> @@ -27,6 +27,8 @@ aliases {
>>> spi0 = &ospi0;
>>> ethernet0 = &cpsw_port1;
>>> ethernet1 = &cpsw_port2;
>>> + usb0 = &usb0;
>>> + usb1 = &usb1;
>>> };
>>> chosen {
>>> @@ -297,6 +299,12 @@ AM62PX_IOPAD(0x01b0, PIN_OUTPUT, 2) /* (G20) MCASP0_ACLKR.UART1_TXD */
>>> bootph-all;
>>> };
>>> + main_usb1_pins_default: main-usb1-default-pins {
>>> + pinctrl-single,pins = <
>>> + AM62PX_IOPAD(0x0258, PIN_INPUT, 0) /* (G21) USB1_DRVVBUS */
>>> + >;
>>> + };
>>> +
>>> main_wlirq_pins_default: main-wlirq-default-pins {
>>> pinctrl-single,pins = <
>>> AM62PX_IOPAD(0x0128, PIN_INPUT, 7) /* (K25) MMC2_SDWP.GPIO0_72 */
>>> @@ -340,6 +348,36 @@ AM62PX_IOPAD(0x0124, PIN_INPUT, 7) /* (J25) MMC2_SDCD.GPIO0_71 */
>>> };
>>> };
>>> +&main_i2c0 {
>>> + status = "okay";
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&main_i2c0_pins_default>;
>>> + clock-frequency = <400000>;
>>> +
>>> + typec_pd0: usb-power-controller@3f {
>>> + compatible = "ti,tps6598x";
>>> + reg = <0x3f>;
>>> +
>>> + connector {
>>> + compatible = "usb-c-connector";
>>> + label = "USB-C";
>>> + self-powered;
>>> + data-role = "dual";
>>> + power-role = "sink";
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + port@0 {
>>> + reg = <0>;
>>> + usb_con_hs: endpoint {
>>> + remote-endpoint = <&usb0_hs_ep>;
>>> + };
>>> + };
>>> + };
>>> + };
>>> + };
>>> +};
>>> +
>>> &main_i2c1 {
>>> status = "okay";
>>> pinctrl-names = "default";
>>> @@ -460,6 +498,35 @@ cpsw3g_phy1: ethernet-phy@1 {
>>> };
>>> };
>>> +&usbss0 {
>>> + status = "okay";
>>> + ti,vbus-divider;
>>> +};
>>> +
>>> +&usbss1 {
>>> + status = "okay";
>>> + ti,vbus-divider;
>>> +};
>>> +
>>> +&usb0 {
>>> + usb-role-switch;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + usb0_hs_ep: endpoint {
>>> + remote-endpoint = <&usb_con_hs>;
>>> + };
>>> + };
>>> +};
>>> +
>>> +&usb1 {
>>> + dr_mode = "host";
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&main_usb1_pins_default>;
>>
>> I'm not super familiar with USB, but I see this pinmux for the
>> "DRVVBUS" pin is usually added the the parent USB subsystem node (usbss).
>> Does this pin belong to the subsystem or the specific USB instance?
>>
> There is only 1 USB instance per USB sub-system.
> The "DRVVBUS" pin is a control signal to enable the VBUS regulator
> while in host mode.
>
> Just probing the usbss1 driver has no use of "DRVVBUS".
> I think usb1 is the right place as it is used there.
>
Seems reasonable, so if there ever was more than 1 instance
per sub-system, each instance would need its own pin?
We should move these pinctrl to the instance node in the other
dts files at some point then.
Reviewed-by: Andrew Davis <afd@...com>
>> Andrew
>>
>>> +};
>>> +
>>> &mcasp1 {
>>> status = "okay";
>>> #sound-dai-cells = <0>;
>
Powered by blists - more mailing lists