Hi,
On Thu, Nov 5, 2020 at 4:37 PM Matthias Kaehlcke <mka@xxxxxxxxxxxx> wrote:
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> index 0a281c24841c..6603f2102233 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> @@ -58,10 +58,23 @@ ap_ts: touchscreen@10 {
> };
> };
>
> +&pp3300_hub {
> + /* pp3300_l7c is used to power the USB hub */
> + /delete-property/regulator-always-on;
> +};
> +
> +&pp3300_l7c {
> + regulator-always-on;
Personally I always end up pairing "always-on" and "boot-on", but that
might just be superstition from many kernel versions ago when there
were weird quirks. The way you have it now you will sometimes have
"boot-on" but not "always-on". Probably what you have is fine,
though.
> +};
> +
> &sdhc_2 {
> status = "okay";
> };
>
> +&usb_hub {
> + vdd-supply = <&pp3300_l7c>;
> +};
> +
> /* PINCTRL - board-specific pinctrl */
>
> &tlmm {
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index bf875589d364..50e733412a7f 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -174,6 +174,25 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
> vin-supply = <&pp3300_a>;
> };
>
> + pp3300_hub: pp3300-hub {
> + compatible = "regulator-fixed";
> + regulator-name = "pp3300_hub";
> +
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + gpio = <&tlmm 84 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + pinctrl-names = "default";
> + pinctrl-0 = <&en_pp3300_hub>;
> +
> + /* AP turns on with en_pp3300_hub; always on for AP */
Delete the above comment. It's obvious based on the properties in
this node. Other similar comments are useful because they describe
how the _EC_ turns on regulators and why a regulator that has an
enable still looks like an "always-on" regulator to the AP (because
it's always on whenever the AP is on).
If you want to add a comment, you could say:
/* Always on until we have a way to specify it can go off in suspend */
> @@ -469,7 +488,6 @@ ppvar_l6c: ldo6 {
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> };
>
> - pp3300_hub:
> pp3300_l7c: ldo7 {
> regulator-min-microvolt = <3304000>;
> regulator-max-microvolt = <3304000>;
Shouldn't you delete the "regulator-always-on;" from ldo7 since you're
adding it for all the older revs?
