| 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: <20200410221748.GD20625@builder.lan>
Date: Fri, 10 Apr 2020 15:17:48 -0700
From: Bjorn Andersson <bjorn.andersson@...aro.org>
To: Sivaprakash Murugesan <sivaprak@...eaurora.org>
Cc: agross@...nel.org, kishon@...com, robh+dt@...nel.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org,
Balaji Prakash J <bjagadee@...eaurora.org>
Subject: Re: [PATCH 3/3] arm64: dts: ipq8074: enable USB support
On Fri 10 Apr 11:29 PDT 2020, Sivaprakash Murugesan wrote:
> IPQ8074 has two super speed usb ports, add phy and dwc3 nodes
> to enable them.
>
Thanks Sivaprakash, your patch looks good, just some comments on the
style below.
> Co-developed-by: Balaji Prakash J <bjagadee@...eaurora.org>
> Signed-off-by: Balaji Prakash J <bjagadee@...eaurora.org>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@...eaurora.org>
> ---
> arch/arm64/boot/dts/qcom/ipq8074-hk01.dts | 24 +++++
> arch/arm64/boot/dts/qcom/ipq8074.dtsi | 168 ++++++++++++++++++++++++++++++
> 2 files changed, 192 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq8074-hk01.dts b/arch/arm64/boot/dts/qcom/ipq8074-hk01.dts
> index 70be3f9..dd27d84 100644
> --- a/arch/arm64/boot/dts/qcom/ipq8074-hk01.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq8074-hk01.dts
> @@ -26,6 +26,22 @@
> };
>
> soc {
> + ssphy@...00 {
Please reference these by label, like we do in e.g. sdm845-mtp.dts.
> + status = "ok";
> + };
> +
> + qusb@...00 {
> + status = "ok";
> + };
> +
> + ssphy@...00 {
> + status = "ok";
> + };
> +
> + qusb@...00 {
> + status = "ok";
> + };
> +
> serial@...3000 {
> status = "ok";
> };
> @@ -65,6 +81,14 @@
> };
> };
>
> + usb3@...0000 {
> + status = "ok";
> + };
> +
> + usb3@...0000 {
> + status = "ok";
> + };
> +
> phy@...00 {
> status = "ok";
> };
> diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> index 2b31823..47bb9ad 100644
> --- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> @@ -16,6 +16,92 @@
> ranges = <0 0 0 0xffffffff>;
> compatible = "simple-bus";
>
> + ssphy_1: ssphy@...00 {
Please use the generic name of "phy" here (i.e. ssphy_1: phy@...00 {)
> + compatible = "qcom,ipq8074-qmp-usb3-phy";
> + reg = <0x00058000 0x1c4>;
> + status = "disabled";
> + #clock-cells = <1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + clocks = <&gcc GCC_USB1_AUX_CLK>,
> + <&gcc GCC_USB1_PHY_CFG_AHB_CLK>,
> + <&xo>;
> + clock-names = "aux", "cfg_ahb", "ref";
> +
> + resets = <&gcc GCC_USB1_PHY_BCR>,
> + <&gcc GCC_USB3PHY_1_PHY_BCR>;
> + reset-names = "phy","common";
> +
> + usb1_ssphy: lane@...00 {
> + reg = <0x00058200 0x130>, /* Tx */
> + <0x00058400 0x200>, /* Rx */
> + <0x00058800 0x1F8>, /* PCS */
> + <0x00058600 0x044>; /* PCS misc */
> + #phy-cells = <0>;
> + clocks = <&gcc GCC_USB1_PIPE_CLK>;
> + clock-names = "pipe0";
> + clock-output-names = "gcc_usb1_pipe_clk_src";
> + };
> + };
> +
> + qusb_phy_1: qusb@...00 {
phy@
> + compatible = "qcom,msm8996-qusb2-phy";
Please add and use a ipq8074 compatible to the driver (.data can point
to msm8996_phy_cfg still).
> + reg = <0x00059000 0x180>;
> + status = "disabled";
> + #phy-cells = <0>;
> +
> + clocks = <&gcc GCC_USB1_PHY_CFG_AHB_CLK>,
> + <&xo>;
> + clock-names = "cfg_ahb", "ref";
> +
> + resets = <&gcc GCC_QUSB2_1_PHY_BCR>;
> + };
> +
> + ssphy_0: ssphy@...00 {
phy@
> + compatible = "qcom,ipq8074-qmp-usb3-phy";
> + reg = <0x00078000 0x1c4>;
> + status = "disabled";
> + #clock-cells = <1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + clocks = <&gcc GCC_USB0_AUX_CLK>,
> + <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> + <&xo>;
> + clock-names = "aux", "cfg_ahb", "ref";
> +
> + resets = <&gcc GCC_USB0_PHY_BCR>,
> + <&gcc GCC_USB3PHY_0_PHY_BCR>;
> + reset-names = "phy","common";
> +
> + usb0_ssphy: lane@...00 {
> + reg = <0x00078200 0x130>, /* Tx */
> + <0x00078400 0x200>, /* Rx */
> + <0x00078800 0x1F8>, /* PCS */
> + <0x00078600 0x044>; /* PCS misc */
> + #phy-cells = <0>;
> + clocks = <&gcc GCC_USB0_PIPE_CLK>;
> + clock-names = "pipe0";
> + clock-output-names = "gcc_usb0_pipe_clk_src";
> + };
> + };
> +
> + qusb_phy_0: qusb@...00 {
phy@
> + compatible = "qcom,msm8996-qusb2-phy";
> + reg = <0x00079000 0x180>;
> + status = "disabled";
> + #phy-cells = <0>;
> +
> + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> + <&xo>;
> + clock-names = "cfg_ahb", "ref";
> +
> + resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> + };
> +
> tlmm: pinctrl@...0000 {
> compatible = "qcom,ipq8074-pinctrl";
> reg = <0x1000000 0x300000>;
> @@ -272,6 +358,88 @@
> status = "disabled";
> };
>
> + usb3_0: usb3@...0000 {
usb@ and please lower case and make sure the unit address matches the
reg.
> + compatible = "qcom,dwc3";
> + reg = <0x08af8800 0x400>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + clocks = <&gcc GCC_SYS_NOC_USB0_AXI_CLK>,
> + <&gcc GCC_USB0_MASTER_CLK>,
> + <&gcc GCC_USB0_SLEEP_CLK>,
> + <&gcc GCC_USB0_MOCK_UTMI_CLK>;
> + clock-names = "sys_noc_axi",
> + "master",
> + "sleep",
> + "mock_utmi";
> +
> + assigned-clocks = <&gcc GCC_SYS_NOC_USB0_AXI_CLK>,
> + <&gcc GCC_USB0_MASTER_CLK>,
> + <&gcc GCC_USB0_MOCK_UTMI_CLK>;
> + assigned-clock-rates = <133330000>,
> + <133330000>,
> + <19200000>;
> +
> + resets = <&gcc GCC_USB0_BCR>;
> + status = "disabled";
> +
> + dwc_0: dwc3@...0000 {
Please lowercase the address
> + compatible = "snps,dwc3";
> + reg = <0x8A00000 0xcd00>;
Ditto.
> + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&qusb_phy_0>, <&usb0_ssphy>;
> + 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";
> + };
> + };
> +
> + usb3_1: usb3@...0000 {
usb@, lowercase and match reg.
> + compatible = "qcom,dwc3";
> + reg = <0x08cf8800 0x400>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + clocks = <&gcc GCC_SYS_NOC_USB1_AXI_CLK>,
> + <&gcc GCC_USB1_MASTER_CLK>,
> + <&gcc GCC_USB1_SLEEP_CLK>,
> + <&gcc GCC_USB1_MOCK_UTMI_CLK>;
> + clock-names = "sys_noc_axi",
> + "master",
> + "sleep",
> + "mock_utmi";
> +
> + assigned-clocks = <&gcc GCC_SYS_NOC_USB1_AXI_CLK>,
> + <&gcc GCC_USB1_MASTER_CLK>,
> + <&gcc GCC_USB1_MOCK_UTMI_CLK>;
> + assigned-clock-rates = <133330000>,
> + <133330000>,
> + <19200000>;
> +
> + resets = <&gcc GCC_USB1_BCR>;
> + status = "disabled";
> +
> + dwc_1: dwc3@...0000 {
Please lowercase
> + compatible = "snps,dwc3";
> + reg = <0x8C00000 0xcd00>;
Ditto.
> + interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&qusb_phy_1>, <&usb1_ssphy>;
> + 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";
> + };
> + };
> +
> pcie_phy0: phy@...00 {
> compatible = "qcom,ipq8074-qmp-pcie-phy";
> reg = <0x86000 0x1000>;
If you could send a separate patch (after this is merged is okay) that
sort the nodes in this file by address, it would be much appreciated.
Regards,
Bjorn
> --
> 2.7.4
>
Powered by blists - more mailing lists