diff mbox series

[v6,6/8] arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280

Message ID 20230405125759.4201-7-quic_kriskura@quicinc.com
State Superseded
Headers show
Series [v6,1/8] dt-bindings: usb: Add bindings for multiport properties on DWC3 controller | expand

Commit Message

Krishna Kurapati April 5, 2023, 12:57 p.m. UTC
Add USB and DWC3 node for tertiary port of SC8280 along with multiport
IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride
platforms.

Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
---
Link to v5: https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/

 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

Comments

Andrew Halaney April 14, 2023, 3:45 p.m. UTC | #1
On Wed, Apr 05, 2023 at 06:27:57PM +0530, Krishna Kurapati wrote:
> Add USB and DWC3 node for tertiary port of SC8280 along with multiport
> IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride
> platforms.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> ---
> Link to v5: https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/
> 
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 42bfa9fa5b96..7b81f2b0449d 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint {
>  			};
>  		};
>  
> +		usb_2: usb@a4f8800 {
> +			compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3";
> +			reg = <0 0x0a4f8800 0 0x400>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>,
> +				 <&gcc GCC_USB30_MP_MASTER_CLK>,
> +				 <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>,
> +				 <&gcc GCC_USB30_MP_SLEEP_CLK>,
> +				 <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
> +				 <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>,
> +				 <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>,
> +				 <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>,
> +				 <&gcc GCC_SYS_NOC_USB_AXI_CLK>;
> +			clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi",
> +				      "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys";
> +
> +			assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
> +					  <&gcc GCC_USB30_MP_MASTER_CLK>;
> +			assigned-clock-rates = <19200000>, <200000000>;
> +
> +			interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>,
> +						<&pdc 126 IRQ_TYPE_EDGE_RISING>,
> +						<&pdc 16 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			interrupt-names = "dp_hs_phy_irq", "dm_hs_phy_irq",
> +						"ss_phy_irq";
> +

This is breaking the current schema (with the full series applied),
I am not sure if a pwr_event IRQ exists or but it maybe necessary to
modify qcom,dwc3.yaml in order to explain hardware if it doesn't exist:

(dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] % make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb                                                                                   :(
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb
/home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:0: 'pwr_event' was expected
	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
/home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:1: 'dp_hs_phy_irq' was expected
	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
/home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:2: 'dm_hs_phy_irq' was expected
	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
/home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names: ['dp_hs_phy_irq', 'dm_hs_phy_irq', 'ss_phy_irq'] is too short
	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
/home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupts-extended: [[99, 127, 1], [99, 126, 1], [99, 16, 4]] is too short
	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb  22.61s user 0.54s system 99% cpu 23.172 total
(dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] %

Thanks,
Andrew
Krishna Kurapati April 15, 2023, 7:04 p.m. UTC | #2
On 4/14/2023 9:15 PM, Andrew Halaney wrote:
> On Wed, Apr 05, 2023 at 06:27:57PM +0530, Krishna Kurapati wrote:
>> Add USB and DWC3 node for tertiary port of SC8280 along with multiport
>> IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride
>> platforms.
>>
>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>> ---
>> Link to v5: https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/
>>
>>   arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++
>>   1 file changed, 58 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> index 42bfa9fa5b96..7b81f2b0449d 100644
>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> @@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint {
>>   			};
>>   		};
>>   
>> +		usb_2: usb@a4f8800 {
>> +			compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3";
>> +			reg = <0 0x0a4f8800 0 0x400>;
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> +
>> +			clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>,
>> +				 <&gcc GCC_USB30_MP_MASTER_CLK>,
>> +				 <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>,
>> +				 <&gcc GCC_USB30_MP_SLEEP_CLK>,
>> +				 <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
>> +				 <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>,
>> +				 <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>,
>> +				 <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>,
>> +				 <&gcc GCC_SYS_NOC_USB_AXI_CLK>;
>> +			clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi",
>> +				      "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys";
>> +
>> +			assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
>> +					  <&gcc GCC_USB30_MP_MASTER_CLK>;
>> +			assigned-clock-rates = <19200000>, <200000000>;
>> +
>> +			interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>,
>> +						<&pdc 126 IRQ_TYPE_EDGE_RISING>,
>> +						<&pdc 16 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +			interrupt-names = "dp_hs_phy_irq", "dm_hs_phy_irq",
>> +						"ss_phy_irq";
>> +
> 
> This is breaking the current schema (with the full series applied),
> I am not sure if a pwr_event IRQ exists or but it maybe necessary to
> modify qcom,dwc3.yaml in order to explain hardware if it doesn't exist:
> 
> (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] % make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb                                                                                   :(
>    LINT    Documentation/devicetree/bindings
>    CHKDT   Documentation/devicetree/bindings/processed-schema.json
>    SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>    DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb
> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:0: 'pwr_event' was expected
> 	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:1: 'dp_hs_phy_irq' was expected
> 	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:2: 'dm_hs_phy_irq' was expected
> 	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names: ['dp_hs_phy_irq', 'dm_hs_phy_irq', 'ss_phy_irq'] is too short
> 	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupts-extended: [[99, 127, 1], [99, 126, 1], [99, 16, 4]] is too short
> 	From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb  22.61s user 0.54s system 99% cpu 23.172 total
> (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] %
> 
> Thanks,
> Andrew
> 

Hi Andrew,

  Thanks for pointing it out. Let me check and get back on the 
pwr_event_irq.

Probably I might have missed it 😅. If so, will make sure to add it in 
next version.

Regards,
Krishna,
Krishna Kurapati April 22, 2023, 4:08 p.m. UTC | #3
On 4/16/2023 12:34 AM, Krishna Kurapati PSSNV wrote:
> 
> 
> On 4/14/2023 9:15 PM, Andrew Halaney wrote:
>> On Wed, Apr 05, 2023 at 06:27:57PM +0530, Krishna Kurapati wrote:
>>> Add USB and DWC3 node for tertiary port of SC8280 along with multiport
>>> IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride
>>> platforms.
>>>
>>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>>> ---
>>> Link to v5: 
>>> https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/
>>>
>>>   arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++
>>>   1 file changed, 58 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi 
>>> b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>> index 42bfa9fa5b96..7b81f2b0449d 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>> @@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint {
>>>               };
>>>           };
>>> +        usb_2: usb@a4f8800 {
>>> +            compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3";
>>> +            reg = <0 0x0a4f8800 0 0x400>;
>>> +            #address-cells = <2>;
>>> +            #size-cells = <2>;
>>> +            ranges;
>>> +
>>> +            clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>,
>>> +                 <&gcc GCC_USB30_MP_MASTER_CLK>,
>>> +                 <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>,
>>> +                 <&gcc GCC_USB30_MP_SLEEP_CLK>,
>>> +                 <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
>>> +                 <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>,
>>> +                 <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>,
>>> +                 <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>,
>>> +                 <&gcc GCC_SYS_NOC_USB_AXI_CLK>;
>>> +            clock-names = "cfg_noc", "core", "iface", "sleep", 
>>> "mock_utmi",
>>> +                      "noc_aggr", "noc_aggr_north", 
>>> "noc_aggr_south", "noc_sys";
>>> +
>>> +            assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
>>> +                      <&gcc GCC_USB30_MP_MASTER_CLK>;
>>> +            assigned-clock-rates = <19200000>, <200000000>;
>>> +
>>> +            interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>,
>>> +                        <&pdc 126 IRQ_TYPE_EDGE_RISING>,
>>> +                        <&pdc 16 IRQ_TYPE_LEVEL_HIGH>;
>>> +
>>> +            interrupt-names = "dp_hs_phy_irq", "dm_hs_phy_irq",
>>> +                        "ss_phy_irq";
>>> +
>>
>> This is breaking the current schema (with the full series applied),
>> I am not sure if a pwr_event IRQ exists or but it maybe necessary to
>> modify qcom,dwc3.yaml in order to explain hardware if it doesn't exist:
>>
>> (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] 
>> % make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml 
>> qcom/sa8540p-ride.dtb                                                                                   :(
>>    LINT    Documentation/devicetree/bindings
>>    CHKDT   Documentation/devicetree/bindings/processed-schema.json
>>    SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>>    DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb
>> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:0: 'pwr_event' was expected
>>     From schema: 
>> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:1: 'dp_hs_phy_irq' was expected
>>     From schema: 
>> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:2: 'dm_hs_phy_irq' was expected
>>     From schema: 
>> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names: ['dp_hs_phy_irq', 'dm_hs_phy_irq', 'ss_phy_irq'] is too short
>>     From schema: 
>> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupts-extended: [[99, 127, 1], [99, 126, 1], [99, 16, 4]] is too short
>>     From schema: 
>> /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml 
>> qcom/sa8540p-ride.dtb  22.61s user 0.54s system 99% cpu 23.172 total
>> (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] %
>>
>> Thanks,
>> Andrew
>>
> 
> Hi Andrew,
> 
>   Thanks for pointing it out. Let me check and get back on the 
> pwr_event_irq.
> 
> Probably I might have missed it 😅. If so, will make sure to add it in 
> next version.
> 
> Regards,
> Krishna,


Hi Andrew, Johan,

   I was looking at the pwr_event_irq interrupts for Multiport 
controller and see that there are two of them as per HW specs. All 
targets till date have only 1 pwr_event_irq required.

The reason I thought I missed pwr_event_irq in my patches is because in 
downstream this is a required IRQ for all targets, so I was under 
assumption that we need it for upstream targets as well. But upstream 
qcom driver doesn't have support for this IRQ yet. And this has been 
made a required one only for SC8280 [1]/[2].

Probably we can proceed in one of the following ways:
1. Remove pwr_event_irq in both bindings and DT as driver support is not 
present currently.
2. Update the bindings for SC8280 to include an optional secondary 
pwr_event_irq for multiport controller.

I would prefer option-1 as removing them would be better because they 
are not being used. Please let me know your thoughts on this.

[1]: 
https://lore.kernel.org/all/20220713131340.29401-2-johan+linaro@kernel.org/
[2]: 
https://lore.kernel.org/all/20220713131340.29401-6-johan+linaro@kernel.org/

Regards,
Krishna,
Andrew Halaney April 25, 2023, 8:33 p.m. UTC | #4
On Sat, Apr 22, 2023 at 09:38:44PM +0530, Krishna Kurapati PSSNV wrote:
> 
> 
> On 4/16/2023 12:34 AM, Krishna Kurapati PSSNV wrote:
> > 
> > 
> > On 4/14/2023 9:15 PM, Andrew Halaney wrote:
> > > On Wed, Apr 05, 2023 at 06:27:57PM +0530, Krishna Kurapati wrote:
> > > > Add USB and DWC3 node for tertiary port of SC8280 along with multiport
> > > > IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride
> > > > platforms.
> > > > 
> > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> > > > ---
> > > > Link to v5: https://lore.kernel.org/all/20230310163420.7582-7-quic_kriskura@quicinc.com/
> > > > 
> > > >   arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++
> > > >   1 file changed, 58 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > > > b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > > > index 42bfa9fa5b96..7b81f2b0449d 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > > > @@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint {
> > > >               };
> > > >           };
> > > > +        usb_2: usb@a4f8800 {
> > > > +            compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3";
> > > > +            reg = <0 0x0a4f8800 0 0x400>;
> > > > +            #address-cells = <2>;
> > > > +            #size-cells = <2>;
> > > > +            ranges;
> > > > +
> > > > +            clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>,
> > > > +                 <&gcc GCC_USB30_MP_MASTER_CLK>,
> > > > +                 <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>,
> > > > +                 <&gcc GCC_USB30_MP_SLEEP_CLK>,
> > > > +                 <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
> > > > +                 <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>,
> > > > +                 <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>,
> > > > +                 <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>,
> > > > +                 <&gcc GCC_SYS_NOC_USB_AXI_CLK>;
> > > > +            clock-names = "cfg_noc", "core", "iface", "sleep",
> > > > "mock_utmi",
> > > > +                      "noc_aggr", "noc_aggr_north",
> > > > "noc_aggr_south", "noc_sys";
> > > > +
> > > > +            assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
> > > > +                      <&gcc GCC_USB30_MP_MASTER_CLK>;
> > > > +            assigned-clock-rates = <19200000>, <200000000>;
> > > > +
> > > > +            interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>,
> > > > +                        <&pdc 126 IRQ_TYPE_EDGE_RISING>,
> > > > +                        <&pdc 16 IRQ_TYPE_LEVEL_HIGH>;
> > > > +
> > > > +            interrupt-names = "dp_hs_phy_irq", "dm_hs_phy_irq",
> > > > +                        "ss_phy_irq";
> > > > +
> > > 
> > > This is breaking the current schema (with the full series applied),
> > > I am not sure if a pwr_event IRQ exists or but it maybe necessary to
> > > modify qcom,dwc3.yaml in order to explain hardware if it doesn't exist:
> > > 
> > > (dtschema) ahalaney@halaney-x13s ~/git/linux-next
> > > (git)-[718f2024524f] % make CHECK_DTBS=y
> > > DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml qcom/sa8540p-ride.dtb                                                                                  
> > > :(
> > >    LINT    Documentation/devicetree/bindings
> > >    CHKDT   Documentation/devicetree/bindings/processed-schema.json
> > >    SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> > >    DTC_CHK arch/arm64/boot/dts/qcom/sa8540p-ride.dtb
> > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:0: 'pwr_event' was expected
> > >     From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:1: 'dp_hs_phy_irq' was expected
> > >     From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names:2: 'dm_hs_phy_irq' was expected
> > >     From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupt-names: ['dp_hs_phy_irq', 'dm_hs_phy_irq', 'ss_phy_irq'] is too short
> > >     From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: usb@a4f8800: interrupts-extended: [[99, 127, 1], [99, 126, 1], [99, 16, 4]] is too short
> > >     From schema: /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > make CHECK_DTBS=y DT_SCHEMA_FILES=/usb/qcom,dwc3.yaml
> > > qcom/sa8540p-ride.dtb  22.61s user 0.54s system 99% cpu 23.172 total
> > > (dtschema) ahalaney@halaney-x13s ~/git/linux-next (git)-[718f2024524f] %
> > > 
> > > Thanks,
> > > Andrew
> > > 
> > 
> > Hi Andrew,
> > 
> >   Thanks for pointing it out. Let me check and get back on the
> > pwr_event_irq.
> > 
> > Probably I might have missed it 😅. If so, will make sure to add it in
> > next version.
> > 
> > Regards,
> > Krishna,
> 
> 
> Hi Andrew, Johan,
> 
>   I was looking at the pwr_event_irq interrupts for Multiport controller and
> see that there are two of them as per HW specs. All targets till date have
> only 1 pwr_event_irq required.
> 
> The reason I thought I missed pwr_event_irq in my patches is because in
> downstream this is a required IRQ for all targets, so I was under assumption
> that we need it for upstream targets as well. But upstream qcom driver
> doesn't have support for this IRQ yet. And this has been made a required one
> only for SC8280 [1]/[2].
> 
> Probably we can proceed in one of the following ways:
> 1. Remove pwr_event_irq in both bindings and DT as driver support is not
> present currently.
> 2. Update the bindings for SC8280 to include an optional secondary
> pwr_event_irq for multiport controller.
> 
> I would prefer option-1 as removing them would be better because they are
> not being used. Please let me know your thoughts on this.
> 
> [1]:
> https://lore.kernel.org/all/20220713131340.29401-2-johan+linaro@kernel.org/
> [2]:
> https://lore.kernel.org/all/20220713131340.29401-6-johan+linaro@kernel.org/
> 

Personally, I prefer option 2 since the IRQ does exist technically
(although it isn't currently used), I like it being described... it
makes the dt-binding a more complete description of the hardware.

I am unsure of the rules wrt dt-bindings and usage in drivers, but I
always like to view it as "this is a description of the hardware", and
the driver bit is just nice to have to ensure that whoever is adding the
binding is actually describing things sufficiently.

You could probably add a new compatible string for the multiport
sc8280xp IP as well, and make the second IRQ non-optional in dt-binding
evaluation? That seems more inline with reality, the regular IP has 1
pwr_event_irq, multiport on this platform has 2, they behave slightly
differently and thus they deserve their own string to match on despite
being on the same platform.

Please note, I'm just a contributor -- I would not be surprised to find
out that my view is not aligned with what maintainers here think, but
that's my interpretation of things!

Hope that helps,
Andrew
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 42bfa9fa5b96..7b81f2b0449d 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -3108,6 +3108,64 @@  usb_1_role_switch: endpoint {
 			};
 		};
 
+		usb_2: usb@a4f8800 {
+			compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3";
+			reg = <0 0x0a4f8800 0 0x400>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
+			clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>,
+				 <&gcc GCC_USB30_MP_MASTER_CLK>,
+				 <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>,
+				 <&gcc GCC_USB30_MP_SLEEP_CLK>,
+				 <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
+				 <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>,
+				 <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>,
+				 <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>,
+				 <&gcc GCC_SYS_NOC_USB_AXI_CLK>;
+			clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi",
+				      "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys";
+
+			assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
+					  <&gcc GCC_USB30_MP_MASTER_CLK>;
+			assigned-clock-rates = <19200000>, <200000000>;
+
+			interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>,
+						<&pdc 126 IRQ_TYPE_EDGE_RISING>,
+						<&pdc 16 IRQ_TYPE_LEVEL_HIGH>;
+
+			interrupt-names = "dp_hs_phy_irq", "dm_hs_phy_irq",
+						"ss_phy_irq";
+
+			power-domains = <&gcc USB30_MP_GDSC>;
+
+			resets = <&gcc GCC_USB30_MP_BCR>;
+
+			interconnects = <&aggre1_noc MASTER_USB3_1 0 &mc_virt SLAVE_EBI1 0>,
+					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_1 0>;
+			interconnect-names = "usb-ddr", "apps-usb";
+
+			required-opps = <&rpmhpd_opp_nom>;
+
+			status = "disabled";
+
+			usb_2_dwc3: usb@a400000 {
+				compatible = "snps,dwc3";
+				reg = <0 0x0a400000 0 0xcd00>;
+				interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+				iommus = <&apps_smmu 0x800 0x0>;
+				phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>,
+					<&usb_2_hsphy1>, <&usb_2_qmpphy1>,
+					<&usb_2_hsphy2>,
+					<&usb_2_hsphy3>;
+				phy-names = "usb2-port0", "usb3-port0",
+						"usb2-port1", "usb3-port1",
+						"usb2-port2",
+						"usb2-port3";
+			};
+		};
+
 		mdss0: display-subsystem@ae00000 {
 			compatible = "qcom,sc8280xp-mdss";
 			reg = <0 0x0ae00000 0 0x1000>;