Re: [PATCH 7/9] arm64: dts: qcom: ipq6018: Flatten usb controller nodes

[George Moussalem]

On 8/29/26 20:48, Krishna Kurapati wrote:
> 
> 
> On 8/25/2026 4:12 PM, George Moussalem via B4 Relay wrote:
>> From: George Moussalem <george.moussalem@xxxxxxxxxxx>
>>
>> Flatten usb controller nodes and update to using latest bindings and
>> flattened driver approach.
>>
>> This SoC has two USB controllers of which the USB3 node had a ref clock
>> populated in the DWC3 child node. Add property to the USB3 node only to
>> select UTMI as reference clock for the DWC3 core to calculate the right
>> values for the period and frame length adjustment.
>>
>> Signed-off-by: George Moussalem <george.moussalem@xxxxxxxxxxx>
> 
> [...]
> 
>>   -            interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
>> +            interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
>>                        <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>;
>> -            interrupt-names = "pwr_event",
>> +            interrupt-names = "dwc_usb3",
>> +                      "pwr_event",
>>                         "qusb2_phy";
>>   
> 
> You are adding pwr_event irq here. Do mention that in the commit message
> or send this particular interrupt addition as a separate patch.

I think you may have misread, I'm not adding the pwr_event irq. It was
already there. I'm simply moving up the dwc_usb3 irq from the dwc node.

> 
>>               resets = <&gcc GCC_USB1_BCR>;
>> -            status = "disabled";
>>   -            dwc_1: usb@7000000 {
>> -                compatible = "snps,dwc3";
>> -                reg = <0x0 0x07000000 0x0 0xcd00>;
>> -                interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>;
>> -                phys = <&qusb_phy_1>;
>> -                phy-names = "usb2-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";
>> -            };
>> +            phys = <&qusb_phy_1>;
>> +            phy-names = "usb2-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";
>> +
> 
> If this is HS only, do we need to add "qcom,select-utmi-as-pipe-clk" ?

No, I've not removed it, I've flattened the node as is.

> 
> [...]
> 
>> +        usb3: usb@8a00000 {
>> +            compatible = "qcom,ipq6018-dwc3", "qcom,snps-dwc3";
>> +            reg = <0x0 0x08a00000 0x0 0xfc100>;
>>                 clocks = <&gcc GCC_SYS_NOC_USB0_AXI_CLK>,
>> -                <&gcc GCC_USB0_MASTER_CLK>,
>> -                <&gcc GCC_USB0_SLEEP_CLK>,
>> -                <&gcc GCC_USB0_MOCK_UTMI_CLK>;
>> +                 <&gcc GCC_USB0_MASTER_CLK>,
>> +                 <&gcc GCC_USB0_SLEEP_CLK>,
>> +                 <&gcc GCC_USB0_MOCK_UTMI_CLK>;
>>               clock-names = "cfg_noc",
>> -                "core",
>> -                "sleep",
>> -                "mock_utmi";
>> +                      "core",
>> +                      "sleep",
>> +                      "mock_utmi";
>>                 assigned-clocks = <&gcc GCC_SYS_NOC_USB0_AXI_CLK>,
>>                         <&gcc GCC_USB0_MASTER_CLK>,
>> @@ -692,32 +686,31 @@ usb3: usb@8af8800 {
>>                              <133330000>,
>>                              <24000000>;
>>   -            interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
>> +            interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
>>                        <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>,
>>                        <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
>> -            interrupt-names = "pwr_event",
>> +            interrupt-names = "dwc_usb3",
>> +                      "pwr_event",
>>                         "qusb2_phy",
>>                         "ss_phy_irq";
> 
> Same comment as above.

Same reply as above.
> 
>>                 resets = <&gcc GCC_USB0_BCR>;
>> -            status = "disabled";
>>   -            dwc_0: usb@8a00000 {
>> -                compatible = "snps,dwc3";
>> -                reg = <0x0 0x08a00000 0x0 0xcd00>;
>> -                interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
>> -                phys = <&qusb_phy_0>, <&ssphy_0>;
>> -                phy-names = "usb2-phy", "usb3-phy";
>> -                clocks = <&xo>;
>> -                clock-names = "ref";
>> -                tx-fifo-resize;
>> -                snps,parkmode-disable-ss-quirk;
>> -                snps,is-utmi-l1-suspend;
>> -                snps,hird-threshold = /bits/ 8 <0x0>;
>> -                snps,dis_u2_susphy_quirk;
>> -                snps,dis_u3_susphy_quirk;
>> -                dr_mode = "host";
>> -            };
>> +            phys = <&qusb_phy_0>, <&ssphy_0>;
>> +            phy-names = "usb2-phy", "usb3-phy";
>> +
>> +            qcom,select-utmi-as-ref-clk;
>> +
>> +            tx-fifo-resize;
>> +            snps,parkmode-disable-ss-quirk;
>> +            snps,is-utmi-l1-suspend;
>> +            snps,hird-threshold = /bits/ 8 <0x0>;
>> +            snps,dis_u2_susphy_quirk;
>> +            snps,dis_u3_susphy_quirk;
>> +            dr_mode = "host";
> 
> This controller is DRD capable. Please move this dr_mode property to DTS
> file.

dr_mode was already there. Removing may break existing implementations
and should require a separate patch.

> 
> Regards,
> Krishna,