2022-01-21 20:03:33

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH v20 5/5] arm64: dts: qcom: sc7180-trogdor: Add nodes for onboard USB hub

Add nodes for the onboard USB hub on trogdor devices. Remove the
'always-on' property from the hub regulator, since the regulator
is now managed by the onboard_usb_hub driver.

Signed-off-by: Matthias Kaehlcke <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
---

Changes in v20:
- renamed hub labels to 'usb_hub_2/3_x'
- added comment for 'regulator-boot-on' of 'pp3300_hub'
- added 'Reviewed-by' tags from Stephen and Doug

Changes in v19:
- none

Changes in v18:
- also adjust config for pompom rev1

Changes in v17:
- none

Changes in v16:
- none

Changes in v15:
- none

Changes in v14:
- none

Changes in v13:
- none

Changes in v12:
- none

Changes in v11:
- rebased on qcom/arm64-for-5.14 (with the rest of the series)

Changes in v10:
- keep 'regulator-boot-on' property
- updated commit message

Changes in v9:
- none

Changes in v8:
- none

Changes in v7:
- rebased on qcom/arm64-for-5.13 (with the rest of the series)

Changes in v6:
- added 'companion-hub' entry to both USB devices
- added 'vdd-supply' also to hub@2

Changes in v5:
- patch added to the series

.../boot/dts/qcom/sc7180-trogdor-lazor-r0.dts | 19 ++++++++----------
.../boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 12 +++++------
.../dts/qcom/sc7180-trogdor-pompom-r1.dts | 11 ++++------
.../arm64/boot/dts/qcom/sc7180-trogdor-r1.dts | 19 ++++++++----------
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 20 ++++++++++++++++++-
5 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
index 30e3e769d2b4..b69590a17cab 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
@@ -14,17 +14,6 @@ / {
compatible = "google,lazor-rev0", "qcom,sc7180";
};

-&pp3300_hub {
- /* pp3300_l7c is used to power the USB hub */
- /delete-property/regulator-always-on;
- /delete-property/regulator-boot-on;
-};
-
-&pp3300_l7c {
- regulator-always-on;
- regulator-boot-on;
-};
-
&sn65dsi86_out {
/*
* Lane 0 was incorrectly mapped on the cable, but we've now decided
@@ -33,3 +22,11 @@ &sn65dsi86_out {
*/
lane-polarities = <1 0>;
};
+
+&usb_hub_2_x {
+ vdd-supply = <&pp3300_l7c>;
+};
+
+&usb_hub_3_x {
+ vdd-supply = <&pp3300_l7c>;
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
index c2ef06367baf..25f178c3c6eb 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
@@ -14,13 +14,11 @@ / {
compatible = "google,lazor-rev1", "google,lazor-rev2", "qcom,sc7180";
};

-&pp3300_hub {
- /* pp3300_l7c is used to power the USB hub */
- /delete-property/regulator-always-on;
- /delete-property/regulator-boot-on;
+
+&usb_hub_2_x {
+ vdd-supply = <&pp3300_l7c>;
};

-&pp3300_l7c {
- regulator-always-on;
- regulator-boot-on;
+&usb_hub_3_x {
+ vdd-supply = <&pp3300_l7c>;
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts
index 76a130bad60a..8467ff41e6d5 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts
@@ -34,13 +34,10 @@ &pm6150_adc_tm {
/delete-node/ charger-thermistor@0;
};

-&pp3300_hub {
- /* pp3300_l7c is used to power the USB hub */
- /delete-property/regulator-always-on;
- /delete-property/regulator-boot-on;
+&usb_hub_2_x {
+ vdd-supply = <&pp3300_l7c>;
};

-&pp3300_l7c {
- regulator-always-on;
- regulator-boot-on;
+&usb_hub_3_x {
+ vdd-supply = <&pp3300_l7c>;
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
index 457c25499863..0cbb7a68d58b 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
@@ -43,17 +43,6 @@ &panel {
compatible = "auo,b116xa01";
};

-&pp3300_hub {
- /* pp3300_l7c is used to power the USB hub */
- /delete-property/regulator-always-on;
- /delete-property/regulator-boot-on;
-};
-
-&pp3300_l7c {
- regulator-always-on;
- regulator-boot-on;
-};
-
&sdhc_2 {
status = "okay";
};
@@ -62,6 +51,14 @@ &trackpad {
interrupts = <58 IRQ_TYPE_EDGE_FALLING>;
};

+&usb_hub_2_x {
+ vdd-supply = <&pp3300_l7c>;
+};
+
+&usb_hub_3_x {
+ vdd-supply = <&pp3300_l7c>;
+};
+
/* PINCTRL - modifications to sc7180-trogdor.dtsi */

&trackpad_int_1v8_odl {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index d4f4441179fc..be06938872ca 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -202,7 +202,7 @@ pp3300_hub: pp3300-hub {
pinctrl-names = "default";
pinctrl-0 = <&en_pp3300_hub>;

- regulator-always-on;
+ /* The BIOS leaves this regulator on */
regulator-boot-on;

vin-supply = <&pp3300_a>;
@@ -839,6 +839,24 @@ &usb_1 {

&usb_1_dwc3 {
dr_mode = "host";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* 2.x hub on port 1 */
+ usb_hub_2_x: hub@1 {
+ compatible = "usbbda,5411";
+ reg = <1>;
+ vdd-supply = <&pp3300_hub>;
+ companion-hub = <&usb_hub_3_x>;
+ };
+
+ /* 3.x hub on port 2 */
+ usb_hub_3_x: hub@2 {
+ compatible = "usbbda,411";
+ reg = <2>;
+ vdd-supply = <&pp3300_hub>;
+ companion-hub = <&usb_hub_2_x>;
+ };
};

&usb_1_hsphy {
--
2.34.1.703.g22d0c6ccf7-goog


2022-02-09 11:40:00

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v20 5/5] arm64: dts: qcom: sc7180-trogdor: Add nodes for onboard USB hub

On Tue, Feb 08, 2022 at 11:57:20AM +0100, Greg Kroah-Hartman wrote:
> On Wed, Jan 19, 2022 at 12:43:45PM -0800, Matthias Kaehlcke wrote:
> > Add nodes for the onboard USB hub on trogdor devices. Remove the
> > 'always-on' property from the hub regulator, since the regulator
> > is now managed by the onboard_usb_hub driver.
> >
> > Signed-off-by: Matthias Kaehlcke <[email protected]>
> > Reviewed-by: Stephen Boyd <[email protected]>
> > Reviewed-by: Douglas Anderson <[email protected]>
> > ---
>
> No DT maintainer approval yet? :(

Bjorn usually just picks DT changes into the QCOM tree when they are
ready, so I wouldn't interpret anything into the lack of an explicit
Ack.

2022-02-09 12:13:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v20 5/5] arm64: dts: qcom: sc7180-trogdor: Add nodes for onboard USB hub

On Wed, Jan 19, 2022 at 12:43:45PM -0800, Matthias Kaehlcke wrote:
> Add nodes for the onboard USB hub on trogdor devices. Remove the
> 'always-on' property from the hub regulator, since the regulator
> is now managed by the onboard_usb_hub driver.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>
> Reviewed-by: Douglas Anderson <[email protected]>
> ---

No DT maintainer approval yet? :(


2022-02-16 03:02:47

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v20 5/5] arm64: dts: qcom: sc7180-trogdor: Add nodes for onboard USB hub

On Tue, Feb 15, 2022 at 09:54:54AM -0800, Doug Anderson wrote:
> Hi,
>
> On Tue, Feb 8, 2022 at 11:21 AM Matthias Kaehlcke <[email protected]> wrote:
> >
> > On Tue, Feb 08, 2022 at 11:57:20AM +0100, Greg Kroah-Hartman wrote:
> > > On Wed, Jan 19, 2022 at 12:43:45PM -0800, Matthias Kaehlcke wrote:
> > > > Add nodes for the onboard USB hub on trogdor devices. Remove the
> > > > 'always-on' property from the hub regulator, since the regulator
> > > > is now managed by the onboard_usb_hub driver.
> > > >
> > > > Signed-off-by: Matthias Kaehlcke <[email protected]>
> > > > Reviewed-by: Stephen Boyd <[email protected]>
> > > > Reviewed-by: Douglas Anderson <[email protected]>
> > > > ---
> > >
> > > No DT maintainer approval yet? :(
> >
> > Bjorn usually just picks DT changes into the QCOM tree when they are
> > ready, so I wouldn't interpret anything into the lack of an explicit
> > Ack.
>
> Right, so the expectation is that this patch wouldn't land through the
> USB tree but would instead land through the Qualcomm tree, probably a
> revision after the code lands in the USB tree to avoid dependency
> problems.

But our tools pick up the whole series. I can't just do "i will pick
patches 1-4 only" easily, and neither can any other maintainer.

Why not just get their ack so that I know it can come through the USB
tree? That's what normally happens for other changes like this where a
driver change is required first.

thanks,

greg k-h

2022-02-16 08:03:38

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v20 5/5] arm64: dts: qcom: sc7180-trogdor: Add nodes for onboard USB hub

On 15/02/2022 19:55, Greg Kroah-Hartman wrote:
> On Tue, Feb 15, 2022 at 09:54:54AM -0800, Doug Anderson wrote:
>> Hi,
>>
>> On Tue, Feb 8, 2022 at 11:21 AM Matthias Kaehlcke <[email protected]> wrote:
>>>
>>> On Tue, Feb 08, 2022 at 11:57:20AM +0100, Greg Kroah-Hartman wrote:
>>>> On Wed, Jan 19, 2022 at 12:43:45PM -0800, Matthias Kaehlcke wrote:
>>>>> Add nodes for the onboard USB hub on trogdor devices. Remove the
>>>>> 'always-on' property from the hub regulator, since the regulator
>>>>> is now managed by the onboard_usb_hub driver.
>>>>>
>>>>> Signed-off-by: Matthias Kaehlcke <[email protected]>
>>>>> Reviewed-by: Stephen Boyd <[email protected]>
>>>>> Reviewed-by: Douglas Anderson <[email protected]>
>>>>> ---
>>>>
>>>> No DT maintainer approval yet? :(
>>>
>>> Bjorn usually just picks DT changes into the QCOM tree when they are
>>> ready, so I wouldn't interpret anything into the lack of an explicit
>>> Ack.
>>
>> Right, so the expectation is that this patch wouldn't land through the
>> USB tree but would instead land through the Qualcomm tree, probably a
>> revision after the code lands in the USB tree to avoid dependency
>> problems.
>
> But our tools pick up the whole series. I can't just do "i will pick
> patches 1-4 only" easily, and neither can any other maintainer.

I don't have problems picking individual patches - either b4 am on each
patch or on entire series and dropping later unneeded commits.

>
> Why not just get their ack so that I know it can come through the USB
> tree? That's what normally happens for other changes like this where a
> driver change is required first.

DTS is a description of the hardware and we take it via separate
branches of SoC-fami0ly repositories. These are always separated from
the driver changes. Always. For several reasons:
1. By convention,
2. To be sure there is no dependency on driver code thus an ABI break,
3. To have a nice and clean history of DTS changes, properly organized.

What is more, if this was coming via my Samsung SoC tree towards SoC
folks, I could not take it in one branch. I would need to physically
split it, otherwise Arnd/Olof would bounce back my pull request saying I
am mixing DTS with driver. Of course you do not have such requirement -
I am just saying that splitting DTS is quite common and proper way.


Best regards,
Krzysztof

2022-02-17 01:20:20

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v20 5/5] arm64: dts: qcom: sc7180-trogdor: Add nodes for onboard USB hub

On Tue 15 Feb 12:55 CST 2022, Greg Kroah-Hartman wrote:

> On Tue, Feb 15, 2022 at 09:54:54AM -0800, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Feb 8, 2022 at 11:21 AM Matthias Kaehlcke <[email protected]> wrote:
> > >
> > > On Tue, Feb 08, 2022 at 11:57:20AM +0100, Greg Kroah-Hartman wrote:
> > > > On Wed, Jan 19, 2022 at 12:43:45PM -0800, Matthias Kaehlcke wrote:
> > > > > Add nodes for the onboard USB hub on trogdor devices. Remove the
> > > > > 'always-on' property from the hub regulator, since the regulator
> > > > > is now managed by the onboard_usb_hub driver.
> > > > >
> > > > > Signed-off-by: Matthias Kaehlcke <[email protected]>
> > > > > Reviewed-by: Stephen Boyd <[email protected]>
> > > > > Reviewed-by: Douglas Anderson <[email protected]>
> > > > > ---
> > > >
> > > > No DT maintainer approval yet? :(
> > >
> > > Bjorn usually just picks DT changes into the QCOM tree when they are
> > > ready, so I wouldn't interpret anything into the lack of an explicit
> > > Ack.
> >
> > Right, so the expectation is that this patch wouldn't land through the
> > USB tree but would instead land through the Qualcomm tree, probably a
> > revision after the code lands in the USB tree to avoid dependency
> > problems.
>
> But our tools pick up the whole series. I can't just do "i will pick
> patches 1-4 only" easily, and neither can any other maintainer.
>

Most other maintainers uses -P to selectively pick the patches that
applies to their subsystem. That said, I really do recognize the
inconvenience on your part and the number of patches being sent your
way.

> Why not just get their ack so that I know it can come through the USB
> tree? That's what normally happens for other changes like this where a
> driver change is required first.
>

Because while the change looks good I don't think it's fine to take it
through the USB tree - the dts tree typically looks like a shotgun hit
across the dts files. And you and I have already seen several times that
dts changes do conflict when you take some of them in the USB tree.

Unfortunately I see only two ways around this problem, either you start
picking selectively or I manage to convince all contributors that they
must split their series to keep dts changes separate (which isn't a bad
idea in itself).

Regards,
Bjorn