On 26/04/2023 12:51, Varadarajan Narayanan wrote:
On Mon, Apr 24, 2023 at 02:17:06PM +0300, Dmitry Baryshkov wrote:On Mon, 24 Apr 2023 at 12:04, Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> wrote:On Sat, Apr 22, 2023 at 12:07:01AM +0300, Dmitry Baryshkov wrote:On 05/04/2023 14:41, Varadarajan Narayanan wrote:Add USB phy and controller related nodes Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> --- Changes in v8: - Change clocks order to match the bindings Changes in v7: - Change com_aux -> cfg_ahb Changes in v6: - Introduce fixed regulators for the phy - Resolved all 'make dtbs_check' messages Changes in v5: - Fix additional comments - Edit nodes to match with qcom,sc8280xp-qmp-usb3-uni-phy.yaml - 'make dtbs_check' giving the following messages since ipq9574 doesn't have power domains. Hope this is ok /local/mnt/workspace/varada/varda-linux/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dtb: phy@7d000: 'power-domains' is a required property From schema: /local/mnt/workspace/varada/varda-linux/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml /local/mnt/workspace/varada/varda-linux/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dtb: usb@8a00000: 'power-domains' is a required property From schema: /local/mnt/workspace/varada/varda-linux/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml Changes in v4: - Use newer bindings without subnodes - Fix coding style issues Changes in v3: - Insert the nodes at proper location Changes in v2: - Fixed issues flagged by Krzysztof - Fix issues reported by make dtbs_check - Remove NOC related clocks (to be added with proper interconnect support) --- arch/arm64/boot/dts/qcom/ipq9574.dtsi | 120 ++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi index 43a3dbe..1242382 100644 --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi @@ -150,6 +150,33 @@ method = "smc"; }; + reg_usb_3p3: s3300 {The node names do not look generic enough. Please take a look at other platforms.Please see below.+ compatible = "regulator-fixed"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-boot-on; + regulator-always-on; + regulator-name = "usb-phy-vdd-dummy";This also doesn't look correct. This regulator should not just fill the gap. Does it represent a generic voltage network on the board? Please do not add 'dummy' voltage regulators if there is no real voltage wire.These are real voltage wires. I used dummy since they are always-on and cannot be increased/decreased (i.e. fixed). Would something along the following lines be appropriate?Still not fully correct. Please use regulator name that corresponds to the power grid on the board schematics. I don't think that you have a separate power grids for USB PHY.vreg_ae10_3p3: s3300 {Naming suggests that these voltages are generated by some PMIC. Is this correct? If so, please describe the PMIC instead.SS PHY needs two supplies and HS PHY needs three supplies. 3.3V and 0.925V are from fixed DC - DC regulators and 1.8V is generated from MP5496 PMIC. Would the following node definitions be ok? usb_hs_vreg0: usb_hs_vreg0 { compatible = "regulator-fixed"; regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; regulator-boot-on; regulator-always-on; regulator-name = "usb-phy-vdd"; }; usb_hs_vreg1: usb_hs_vreg1 { compatible = "regulator-fixed"; regulator-min-microvolt = <925000>; regulator-max-microvolt = <925000>; regulator-boot-on; regulator-always-on; regulator-name = "usb-phy"; };
Again. The voltage rails on the board are not USB-specific, are they? So why are you declaring usb-phy regulators? Would another consumer of 3.3V rail use the same usb-phy-vdd regulator?
&rpm_requests {
regulators {
compatible = "qcom,rpm-mp5496-regulators";
.
.
.
ipq9574_l2: l2 {
mp5496_l2
regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; regulator-boot-on; regulator-always-on; }; }; }; Thanks Varadacompatible = "regulator-fixed"; regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; regulator-boot-on; regulator-always-on; regulator-name = "usb-phy-vdd"; }; vreg_ad8_1p8: s1800 { compatible = "regulator-fixed"; regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; regulator-boot-on; regulator-always-on; regulator-name = "usb-phy-pll"; }; vreg_ad9_0p925: s0925 { compatible = "regulator-fixed"; regulator-min-microvolt = <925000>; regulator-max-microvolt = <925000>; regulator-boot-on; regulator-always-on; regulator-name = "usb-phy"; }; Thanks Varada+ }; + + reg_usb_1p8: s1800 { + compatible = "regulator-fixed"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-boot-on; + regulator-always-on; + regulator-name = "usb-phy-pll-dummy"; + }; + + reg_usb_0p925: s0925 { + compatible = "regulator-fixed"; + regulator-min-microvolt = <925000>; + regulator-max-microvolt = <925000>; + regulator-boot-on; + regulator-always-on; + regulator-name = "usb-phy-dummy"; + }; + reserved-memory { #address-cells = <2>; #size-cells = <2>; @@ -179,6 +206,52 @@ #size-cells = <1>; ranges = <0 0 0 0xffffffff>; + usb_0_qusbphy: phy@7b000 { + compatible = "qcom,ipq9574-qusb2-phy"; + reg = <0x0007b000 0x180>; + #phy-cells = <0>; + + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>, + <&xo_board_clk>; + clock-names = "cfg_ahb", + "ref"; + + vdd-supply = <®_usb_0p925>; + vdda-pll-supply = <®_usb_1p8>; + vdda-phy-dpdm-supply = <®_usb_3p3>; + + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; + status = "disabled"; + }; + + usb_0_qmpphy: phy@7d000 { + compatible = "qcom,ipq9574-qmp-usb3-phy"; + reg = <0x0007d000 0xa00>; + #phy-cells = <0>; + + clocks = <&gcc GCC_USB0_AUX_CLK>, + <&xo_board_clk>, + <&gcc GCC_USB0_PHY_CFG_AHB_CLK>, + <&gcc GCC_USB0_PIPE_CLK>; + clock-names = "aux", + "ref", + "cfg_ahb", + "pipe"; + + resets = <&gcc GCC_USB0_PHY_BCR>, + <&gcc GCC_USB3PHY_0_PHY_BCR>; + reset-names = "phy", + "phy_phy"; + + vdda-pll-supply = <®_usb_1p8>; + vdda-phy-supply = <®_usb_0p925>; + + status = "disabled"; + + #clock-cells = <0>; + clock-output-names = "usb0_pipe_clk"; + }; + pcie0_phy: phy@84000 { compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy"; reg = <0x00084000 0x1000>; @@ -548,6 +621,53 @@ status = "disabled"; }; + usb3: usb@8a00000 { + compatible = "qcom,ipq9574-dwc3", "qcom,dwc3"; + reg = <0x08af8800 0x400>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + clocks = <&gcc GCC_SNOC_USB_CLK>, + <&gcc GCC_USB0_MASTER_CLK>, + <&gcc GCC_ANOC_USB_AXI_CLK>, + <&gcc GCC_USB0_SLEEP_CLK>, + <&gcc GCC_USB0_MOCK_UTMI_CLK>; + + clock-names = "cfg_noc", + "core", + "iface", + "sleep", + "mock_utmi"; + + assigned-clocks = <&gcc GCC_USB0_MASTER_CLK>, + <&gcc GCC_USB0_MOCK_UTMI_CLK>; + assigned-clock-rates = <200000000>, + <24000000>; + + interrupts-extended = <&intc GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "pwr_event"; + + resets = <&gcc GCC_USB_BCR>; + status = "disabled"; + + dwc_0: usb@8a00000 { + compatible = "snps,dwc3"; + reg = <0x8a00000 0xcd00>; + clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>; + clock-names = "ref"; + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>; + phys = <&usb_0_qusbphy>, <&usb_0_qmpphy>; + phy-names = "usb2-phy", "usb3-phy"; + tx-fifo-resize; + snps,is-utmi-l1-suspend; + snps,hird-threshold = /bits/ 8 <0x0>; + snps,dis_u2_susphy_quirk; + snps,dis_u3_susphy_quirk; + dr_mode = "host"; + }; + }; + intc: interrupt-controller@b000000 { compatible = "qcom,msm-qgic2"; reg = <0x0b000000 0x1000>, /* GICD */-- With best wishes Dmitry-- With best wishes Dmitry
-- With best wishes Dmitry
