| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <db95dba7-c749-48d3-bbba-01a45dfb9815@kernel.org>
Date: Wed, 7 Feb 2024 14:42:14 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Andrew Davis <afd@...com>, 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 06/02/2024 19:34, Andrew Davis wrote:
> 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?
Yes.
>
> We should move these pinctrl to the instance node in the other
> dts files at some point then.
>
> Reviewed-by: Andrew Davis <afd@...com>
Thanks!
>
>>> Andrew
>>>
>>>> +};
>>>> +
>>>> &mcasp1 {
>>>> status = "okay";
>>>> #sound-dai-cells = <0>;
>>
--
cheers,
-roger
Powered by blists - more mailing lists