This series adds the missing devicetree binding for SC8280XP, clean up
the current bindings somewhat and fixes a related driver warning when an
optional wakeup interrupt is missing.
The last four patches fix up the SC8280XP devicetree that's currently in
Bjorn's tree and addresses some DT schema warnings in other Qualcomm
devicetrees and are only included here for completeness.
The binding and driver updates are expected to go though the USB tree,
while the devicetree updates can be picked up by Bjorn once the binding
updates have been merged.
Johan
Johan Hovold (7):
dt-bindings: usb: qcom,dwc3: add SC8280XP binding
dt-bindings: usb: qcom,dwc3: refine interrupt requirements
usb: dwc3: qcom: fix missing optional irq warnings
arm64: dts: qcom: sc8280xp: fix USB clock order
arm64: dts: qcom: sc8280xp: fix USB interrupts
arm64: dts: qcom: sc7280: reorder USB interrupts
arm64: dts: qcom: reorder USB interrupts
.../devicetree/bindings/usb/qcom,dwc3.yaml | 152 ++++++++++++++++--
arch/arm/boot/dts/qcom-sdx65.dtsi | 10 +-
arch/arm64/boot/dts/qcom/sc7280.dtsi | 15 +-
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 28 ++--
arch/arm64/boot/dts/qcom/sm8250.dtsi | 20 ++-
arch/arm64/boot/dts/qcom/sm8350.dtsi | 20 ++-
drivers/usb/dwc3/dwc3-qcom.c | 4 +-
7 files changed, 194 insertions(+), 55 deletions(-)
--
2.35.1
Fix the USB controller clock order and naming so that they match the
devicetree binding.
Note that the driver currently simply enables all clocks in the order
that they are specified in the devicetree. Reordering the clocks as per
the binding means that the only explicit ordering constraint found in
the vendor driver, that cfg_noc should be enabled before the core_clk,
is now honoured.
Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 285a9828c250..45cc7d714fd2 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1855,16 +1855,16 @@ usb_0: usb@a6f8800 {
#size-cells = <2>;
ranges;
- clocks = <&gcc GCC_USB30_PRIM_MASTER_CLK>,
- <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
+ clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
+ <&gcc GCC_USB30_PRIM_MASTER_CLK>,
<&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
- <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
<&gcc GCC_USB30_PRIM_SLEEP_CLK>,
+ <&gcc GCC_USB30_PRIM_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 = "core", "iface", "bus_aggr", "utmi", "sleep",
+ clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi",
"noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys";
assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
@@ -1905,16 +1905,16 @@ usb_1: usb@a8f8800 {
#size-cells = <2>;
ranges;
- clocks = <&gcc GCC_USB30_SEC_MASTER_CLK>,
- <&gcc GCC_CFG_NOC_USB3_SEC_AXI_CLK>,
+ clocks = <&gcc GCC_CFG_NOC_USB3_SEC_AXI_CLK>,
+ <&gcc GCC_USB30_SEC_MASTER_CLK>,
<&gcc GCC_AGGRE_USB3_SEC_AXI_CLK>,
- <&gcc GCC_USB30_SEC_MOCK_UTMI_CLK>,
<&gcc GCC_USB30_SEC_SLEEP_CLK>,
+ <&gcc GCC_USB30_SEC_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 = "core", "iface", "bus_aggr", "utmi", "sleep",
+ clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi",
"noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys";
assigned-clocks = <&gcc GCC_USB30_SEC_MOCK_UTMI_CLK>,
--
2.35.1
Three SoCs did not follow the interrupt order specified by the USB
controller binding.
While keeping the non-SuperSpeed interrupts together seems natural,
reorder the interrupts to match the binding.
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm/boot/dts/qcom-sdx65.dtsi | 10 ++++++----
arch/arm64/boot/dts/qcom/sm8250.dtsi | 20 ++++++++++++--------
arch/arm64/boot/dts/qcom/sm8350.dtsi | 20 ++++++++++++--------
3 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/arch/arm/boot/dts/qcom-sdx65.dtsi b/arch/arm/boot/dts/qcom-sdx65.dtsi
index 7a193678b4f5..8daefd50217a 100644
--- a/arch/arm/boot/dts/qcom-sdx65.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx65.dtsi
@@ -372,11 +372,13 @@ usb: usb@a6f8800 {
assigned-clock-rates = <19200000>, <200000000>;
interrupts-extended = <&intc GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
- <&pdc 19 IRQ_TYPE_EDGE_BOTH>,
<&pdc 76 IRQ_TYPE_LEVEL_HIGH>,
- <&pdc 18 IRQ_TYPE_EDGE_BOTH>;
- interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
- "ss_phy_irq", "dm_hs_phy_irq";
+ <&pdc 18 IRQ_TYPE_EDGE_BOTH>,
+ <&pdc 19 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "hs_phy_irq",
+ "ss_phy_irq",
+ "dm_hs_phy_irq",
+ "dp_hs_phy_irq";
power-domains = <&gcc USB30_GDSC>;
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 7ac8aa110f81..65be7f3ec74c 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3026,11 +3026,13 @@ usb_1: usb@a6f8800 {
assigned-clock-rates = <19200000>, <200000000>;
interrupts-extended = <&intc GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
- <&pdc 14 IRQ_TYPE_EDGE_BOTH>,
+ <&pdc 17 IRQ_TYPE_LEVEL_HIGH>,
<&pdc 15 IRQ_TYPE_EDGE_BOTH>,
- <&pdc 17 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
- "dm_hs_phy_irq", "ss_phy_irq";
+ <&pdc 14 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "hs_phy_irq",
+ "ss_phy_irq",
+ "dm_hs_phy_irq",
+ "dp_hs_phy_irq";
power-domains = <&gcc USB30_PRIM_GDSC>;
@@ -3081,11 +3083,13 @@ usb_2: usb@a8f8800 {
assigned-clock-rates = <19200000>, <200000000>;
interrupts-extended = <&intc GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>,
- <&pdc 12 IRQ_TYPE_EDGE_BOTH>,
+ <&pdc 16 IRQ_TYPE_LEVEL_HIGH>,
<&pdc 13 IRQ_TYPE_EDGE_BOTH>,
- <&pdc 16 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
- "dm_hs_phy_irq", "ss_phy_irq";
+ <&pdc 12 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "hs_phy_irq",
+ "ss_phy_irq",
+ "dm_hs_phy_irq",
+ "dp_hs_phy_irq";
power-domains = <&gcc USB30_SEC_GDSC>;
diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index 65c7fe54613d..e72a04411888 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -2461,11 +2461,13 @@ usb_1: usb@a6f8800 {
assigned-clock-rates = <19200000>, <200000000>;
interrupts-extended = <&intc GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
- <&pdc 14 IRQ_TYPE_EDGE_BOTH>,
+ <&pdc 17 IRQ_TYPE_LEVEL_HIGH>,
<&pdc 15 IRQ_TYPE_EDGE_BOTH>,
- <&pdc 17 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
- "dm_hs_phy_irq", "ss_phy_irq";
+ <&pdc 14 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "hs_phy_irq",
+ "ss_phy_irq",
+ "dm_hs_phy_irq",
+ "dp_hs_phy_irq";
power-domains = <&gcc USB30_PRIM_GDSC>;
@@ -2509,11 +2511,13 @@ usb_2: usb@a8f8800 {
assigned-clock-rates = <19200000>, <200000000>;
interrupts-extended = <&intc GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>,
- <&pdc 12 IRQ_TYPE_EDGE_BOTH>,
+ <&pdc 16 IRQ_TYPE_LEVEL_HIGH>,
<&pdc 13 IRQ_TYPE_EDGE_BOTH>,
- <&pdc 16 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
- "dm_hs_phy_irq", "ss_phy_irq";
+ <&pdc 12 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "hs_phy_irq",
+ "ss_phy_irq",
+ "dm_hs_phy_irq",
+ "dp_hs_phy_irq";
power-domains = <&gcc USB30_SEC_GDSC>;
--
2.35.1
The two single-port SC8280XP USB controllers do not have an hs_phy_irq
interrupt. Instead they have a pwr_event interrupt which is distinct
from the former and not yet supported by the driver.
Fix the USB node interrupt names so that they match the devicetree
binding.
Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 45cc7d714fd2..4a7aa9992f3a 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1875,8 +1875,10 @@ usb_0: usb@a6f8800 {
<&pdc 14 IRQ_TYPE_EDGE_BOTH>,
<&pdc 15 IRQ_TYPE_EDGE_BOTH>,
<&pdc 138 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
- "dm_hs_phy_irq", "ss_phy_irq";
+ interrupt-names = "pwr_event",
+ "dp_hs_phy_irq",
+ "dm_hs_phy_irq",
+ "ss_phy_irq";
power-domains = <&gcc USB30_PRIM_GDSC>;
@@ -1925,8 +1927,10 @@ usb_1: usb@a8f8800 {
<&pdc 12 IRQ_TYPE_EDGE_BOTH>,
<&pdc 13 IRQ_TYPE_EDGE_BOTH>,
<&pdc 136 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
- "dm_hs_phy_irq", "ss_phy_irq";
+ interrupt-names = "pwr_event",
+ "dp_hs_phy_irq",
+ "dm_hs_phy_irq",
+ "ss_phy_irq";
power-domains = <&gcc USB30_SEC_GDSC>;
--
2.35.1
On Wed, Jul 13, 2022 at 03:13:38PM +0200, Johan Hovold wrote:
> The two single-port SC8280XP USB controllers do not have an hs_phy_irq
> interrupt. Instead they have a pwr_event interrupt which is distinct
> from the former and not yet supported by the driver.
>
> Fix the USB node interrupt names so that they match the devicetree
> binding.
>
> Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 45cc7d714fd2..4a7aa9992f3a 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -1875,8 +1875,10 @@ usb_0: usb@a6f8800 {
> <&pdc 14 IRQ_TYPE_EDGE_BOTH>,
> <&pdc 15 IRQ_TYPE_EDGE_BOTH>,
> <&pdc 138 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
> - "dm_hs_phy_irq", "ss_phy_irq";
> + interrupt-names = "pwr_event",
> + "dp_hs_phy_irq",
> + "dm_hs_phy_irq",
> + "ss_phy_irq";
>
> power-domains = <&gcc USB30_PRIM_GDSC>;
>
> @@ -1925,8 +1927,10 @@ usb_1: usb@a8f8800 {
> <&pdc 12 IRQ_TYPE_EDGE_BOTH>,
> <&pdc 13 IRQ_TYPE_EDGE_BOTH>,
> <&pdc 136 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
> - "dm_hs_phy_irq", "ss_phy_irq";
> + interrupt-names = "pwr_event",
> + "dp_hs_phy_irq",
> + "dm_hs_phy_irq",
> + "ss_phy_irq";
For this specific change to pwr_event:
Reviewed-by: Andrew Halaney <[email protected]>
That being said, I was reviewing this against the (fairly old)
downstream release I have, and the IRQs defined there look like this:
interrupts-extended = <&pdc 12 IRQ_TYPE_EDGE_RISING>,
<&intc GIC_SPI 811 IRQ_TYPE_LEVEL_HIGH>,
<&pdc 136 IRQ_TYPE_LEVEL_HIGH>,
<&pdc 13 IRQ_TYPE_EDGE_RISING>;
interrupt-names = "dp_hs_phy_irq", "pwr_event_irq",
"ss_phy_irq", "dm_hs_phy_irq";
The part I want to highlight is that the "pwr_event" irq downstream maps
to <&intc GIC_SPI 811 IRQ_TYPE_LEVEL_HIGH>, but the current upstream
devicetree I'm looking at has it mapped to <&intc GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>
Do you happen to have any source you can also check to confirm if this
is a bug or not?
Thanks,
Andrew
>
> power-domains = <&gcc USB30_SEC_GDSC>;
>
> --
> 2.35.1
>
On Wed, Jul 13, 2022 at 09:12:28AM -0500, Andrew Halaney wrote:
> On Wed, Jul 13, 2022 at 03:13:38PM +0200, Johan Hovold wrote:
> > The two single-port SC8280XP USB controllers do not have an hs_phy_irq
> > interrupt. Instead they have a pwr_event interrupt which is distinct
> > from the former and not yet supported by the driver.
> >
> > Fix the USB node interrupt names so that they match the devicetree
> > binding.
> >
> > Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
> > Signed-off-by: Johan Hovold <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > index 45cc7d714fd2..4a7aa9992f3a 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > @@ -1875,8 +1875,10 @@ usb_0: usb@a6f8800 {
> > <&pdc 14 IRQ_TYPE_EDGE_BOTH>,
> > <&pdc 15 IRQ_TYPE_EDGE_BOTH>,
> > <&pdc 138 IRQ_TYPE_LEVEL_HIGH>;
> > - interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
> > - "dm_hs_phy_irq", "ss_phy_irq";
> > + interrupt-names = "pwr_event",
> > + "dp_hs_phy_irq",
> > + "dm_hs_phy_irq",
> > + "ss_phy_irq";
> >
> > power-domains = <&gcc USB30_PRIM_GDSC>;
> >
> > @@ -1925,8 +1927,10 @@ usb_1: usb@a8f8800 {
> > <&pdc 12 IRQ_TYPE_EDGE_BOTH>,
> > <&pdc 13 IRQ_TYPE_EDGE_BOTH>,
> > <&pdc 136 IRQ_TYPE_LEVEL_HIGH>;
> > - interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
> > - "dm_hs_phy_irq", "ss_phy_irq";
> > + interrupt-names = "pwr_event",
> > + "dp_hs_phy_irq",
> > + "dm_hs_phy_irq",
> > + "ss_phy_irq";
>
> For this specific change to pwr_event:
>
> Reviewed-by: Andrew Halaney <[email protected]>
>
> That being said, I was reviewing this against the (fairly old)
> downstream release I have, and the IRQs defined there look like this:
>
> interrupts-extended = <&pdc 12 IRQ_TYPE_EDGE_RISING>,
> <&intc GIC_SPI 811 IRQ_TYPE_LEVEL_HIGH>,
> <&pdc 136 IRQ_TYPE_LEVEL_HIGH>,
> <&pdc 13 IRQ_TYPE_EDGE_RISING>;
> interrupt-names = "dp_hs_phy_irq", "pwr_event_irq",
> "ss_phy_irq", "dm_hs_phy_irq";
>
> The part I want to highlight is that the "pwr_event" irq downstream maps
> to <&intc GIC_SPI 811 IRQ_TYPE_LEVEL_HIGH>, but the current upstream
> devicetree I'm looking at has it mapped to <&intc GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>
>
> Do you happen to have any source you can also check to confirm if this
> is a bug or not?
Good catch! I believe this is another copy-paste error when creating
base dtsi based on some vendor source (or perhaps an error carried over
from an earlier version).
The vendor devicetree I have access to also has 811 here, which matches
the pattern for usb_0 (dwc3 interrupt + 1).
Do you mind if I fold a fix for that into a v2 of this patch?
Thanks for reviewing!
Johan
On Wed, Jul 13, 2022 at 09:43:10AM -0500, Andrew Halaney wrote:
> On Wed, Jul 13, 2022 at 04:35:22PM +0200, Johan Hovold wrote:
> > Do you mind if I fold a fix for that into a v2 of this patch?
> Sounds good, feel free to add my R-B with that change as well!
Done. I'll wait a bit for any further comments before reposting.
Thanks again.
Johan
On Wed, Jul 13, 2022 at 04:35:22PM +0200, Johan Hovold wrote:
> On Wed, Jul 13, 2022 at 09:12:28AM -0500, Andrew Halaney wrote:
> > On Wed, Jul 13, 2022 at 03:13:38PM +0200, Johan Hovold wrote:
> > > The two single-port SC8280XP USB controllers do not have an hs_phy_irq
> > > interrupt. Instead they have a pwr_event interrupt which is distinct
> > > from the former and not yet supported by the driver.
> > >
> > > Fix the USB node interrupt names so that they match the devicetree
> > > binding.
> > >
> > > Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
> > > Signed-off-by: Johan Hovold <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 12 ++++++++----
> > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > > index 45cc7d714fd2..4a7aa9992f3a 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > > @@ -1875,8 +1875,10 @@ usb_0: usb@a6f8800 {
> > > <&pdc 14 IRQ_TYPE_EDGE_BOTH>,
> > > <&pdc 15 IRQ_TYPE_EDGE_BOTH>,
> > > <&pdc 138 IRQ_TYPE_LEVEL_HIGH>;
> > > - interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
> > > - "dm_hs_phy_irq", "ss_phy_irq";
> > > + interrupt-names = "pwr_event",
> > > + "dp_hs_phy_irq",
> > > + "dm_hs_phy_irq",
> > > + "ss_phy_irq";
> > >
> > > power-domains = <&gcc USB30_PRIM_GDSC>;
> > >
> > > @@ -1925,8 +1927,10 @@ usb_1: usb@a8f8800 {
> > > <&pdc 12 IRQ_TYPE_EDGE_BOTH>,
> > > <&pdc 13 IRQ_TYPE_EDGE_BOTH>,
> > > <&pdc 136 IRQ_TYPE_LEVEL_HIGH>;
> > > - interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
> > > - "dm_hs_phy_irq", "ss_phy_irq";
> > > + interrupt-names = "pwr_event",
> > > + "dp_hs_phy_irq",
> > > + "dm_hs_phy_irq",
> > > + "ss_phy_irq";
> >
> > For this specific change to pwr_event:
> >
> > Reviewed-by: Andrew Halaney <[email protected]>
> >
> > That being said, I was reviewing this against the (fairly old)
> > downstream release I have, and the IRQs defined there look like this:
> >
> > interrupts-extended = <&pdc 12 IRQ_TYPE_EDGE_RISING>,
> > <&intc GIC_SPI 811 IRQ_TYPE_LEVEL_HIGH>,
> > <&pdc 136 IRQ_TYPE_LEVEL_HIGH>,
> > <&pdc 13 IRQ_TYPE_EDGE_RISING>;
> > interrupt-names = "dp_hs_phy_irq", "pwr_event_irq",
> > "ss_phy_irq", "dm_hs_phy_irq";
> >
> > The part I want to highlight is that the "pwr_event" irq downstream maps
> > to <&intc GIC_SPI 811 IRQ_TYPE_LEVEL_HIGH>, but the current upstream
> > devicetree I'm looking at has it mapped to <&intc GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>
> >
> > Do you happen to have any source you can also check to confirm if this
> > is a bug or not?
>
> Good catch! I believe this is another copy-paste error when creating
> base dtsi based on some vendor source (or perhaps an error carried over
> from an earlier version).
>
> The vendor devicetree I have access to also has 811 here, which matches
> the pattern for usb_0 (dwc3 interrupt + 1).
>
> Do you mind if I fold a fix for that into a v2 of this patch?
>
> Thanks for reviewing!
Sounds good, feel free to add my R-B with that change as well!
>
> Johan
>
On 13/07/2022 15:13, Johan Hovold wrote:
> Three SoCs did not follow the interrupt order specified by the USB
> controller binding.
>
> While keeping the non-SuperSpeed interrupts together seems natural,
> reorder the interrupts to match the binding.
>
> Signed-off-by: Johan Hovold <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 13/07/2022 15:13, Johan Hovold wrote:
> The two single-port SC8280XP USB controllers do not have an hs_phy_irq
> interrupt. Instead they have a pwr_event interrupt which is distinct
> from the former and not yet supported by the driver.
>
> Fix the USB node interrupt names so that they match the devicetree
> binding.
>
> Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 13/07/2022 15:13, Johan Hovold wrote:
> Fix the USB controller clock order and naming so that they match the
> devicetree binding.
>
> Note that the driver currently simply enables all clocks in the order
> that they are specified in the devicetree. Reordering the clocks as per
> the binding means that the only explicit ordering constraint found in
> the vendor driver, that cfg_noc should be enabled before the core_clk,
> is now honoured.
>
> Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 285a9828c250..45cc7d714fd2 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -1855,16 +1855,16 @@ usb_0: usb@a6f8800 {
> #size-cells = <2>;
> ranges;
>
> - clocks = <&gcc GCC_USB30_PRIM_MASTER_CLK>,
> - <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
> + clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
> + <&gcc GCC_USB30_PRIM_MASTER_CLK>,
> <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
> - <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
> <&gcc GCC_USB30_PRIM_SLEEP_CLK>,
> + <&gcc GCC_USB30_PRIM_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 = "core", "iface", "bus_aggr", "utmi", "sleep",
> + clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi",
> "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys";
Your commit title should also include change of naming.
Best regards,
Krzysztof