Hi,
On Fri, 12 Feb 2021 at 23:11, AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxxx> wrote:
>
> Il 12/02/21 10:24, Amit Pundir ha scritto:
> > Hi,
> >
> > On Thu, 11 Feb 2021 at 00:25, AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@xxxxxxxxxxxxxx> wrote:
> >>
> >> Il 10/02/21 09:18, Amit Pundir ha scritto:
> >>> From: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> >>>
> >>> Enabling the Display panel for beryllium requires DSI
> >>> labibb regulators and panel dts nodes to be added.
> >>> It is also required to keep some of the regulators as
> >>> always-on.
> >>>
> >>> Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> >>> Signed-off-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
> >>> ---
> >>
> >> Hello!
> >> Your patch looks good, however, I have a few concerns...
> >>
> >>> v3: Addressed Konrad's concerns. Configured labibb regulators
> >>> explicitly based on downstream microvolt values. Display
> >>> comes up fine with default discharge-resistor-kohms and
> >>> soft-start-us properties, so didn't touch them.
> >>> Smoke tested on next-20210209.
> >>> v2: Rebased to mainline (v5.11-rc6) and fixed build warnings.
> >>>
> >>> .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 64 ++++++++++++++++++++++
> >>> 1 file changed, 64 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
> >>> index 86cbae63eaf7..5ac049a247e1 100644
> >>> --- a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
> >>> +++ b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
> >>> @@ -157,6 +157,14 @@
> >>> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> >>> };
> >>>
> >>> + vreg_l14a_1p8: ldo14 {
> >>> + regulator-min-microvolt = <1800000>;
> >>> + regulator-max-microvolt = <1800000>;
> >>> + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> >>> + regulator-boot-on;
> >>> + regulator-always-on;
> >>> + };
> >>> +
> >>> vreg_l17a_1p3: ldo17 {
> >>> regulator-min-microvolt = <1304000>;
> >>> regulator-max-microvolt = <1304000>;
> >>> @@ -191,6 +199,7 @@
> >>> regulator-min-microvolt = <1200000>;
> >>> regulator-max-microvolt = <1200000>;
> >>> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> >>> + regulator-boot-on;
> >>> };
> >>> };
> >>> };
> >>> @@ -200,6 +209,43 @@
> >>> firmware-name = "qcom/sdm845/cdsp.mdt";
> >>> };
> >>>
> >>> +&dsi0 {
> >>> + status = "okay";
> >>> + vdda-supply = <&vreg_l26a_1p2>;
> >>> +
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + panel@0 {
> >>> + compatible = "tianma,fhd-video";
> >>> + reg = <0>;
> >>> + vddi0-supply = <&vreg_l14a_1p8>;
> >>> + vddpos-supply = <&lab>;
> >>> + vddneg-supply = <&ibb>;
> >>> +
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
> >>> +
> >>> + port {
> >>> + tianma_nt36672a_in_0: endpoint {
> >>> + remote-endpoint = <&dsi0_out>;
> >>> + };
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> +&dsi0_out {
> >>> + remote-endpoint = <&tianma_nt36672a_in_0>;
> >>> + data-lanes = <0 1 2 3>;
> >>> +};
> >>> +
> >>> +&dsi0_phy {
> >>> + status = "okay";
> >>> + vdds-supply = <&vreg_l1a_0p875>;
> >>> +};
> >>> +
> >>> &gcc {
> >>> protected-clocks = <GCC_QSPI_CORE_CLK>,
> >>> <GCC_QSPI_CORE_CLK_SRC>,
> >>> @@ -215,6 +261,24 @@
> >>> };
> >>> };
> >>>
> >>> +&ibb {
> >>> + regulator-min-microvolt = <4600000>;
> >>> + regulator-max-microvolt = <6000000>;
> >>> +};
> >>> +
> >>
> >> I think you want to also configure overvoltage and overcurrent
> >> protection values for both LAB and IBB, as these regulators may be a bit
> >> dangerous if used without.
> >
> > Can you point me to the relevant DT properties please. I didn't find
> > any DT properties which set the over voltage/current protection
> > properties explicitly in upstream as well as in downstream kernel.
> > Plus I also do not see "regulator-min/max-microamp" values set
> > downstream, otherwise I'd have used that as well atleast.
> >
>
> The regulator-{min/max}-microamp is the equivalent of the downstream
> "qcom,qpnp-lab-limit-maximum-current" and
> "qcom,qpnp-ibb-limit-maximum-current.
Thank you for the information.
>
> I am sorry if we haven't sent our patch series that will introduce the
> Sony MSM8998 Yoshino and SDM630/636 Nile and Ganges platforms, which
> are actually using the driver that I've sent fully, as these would have
> been a nice reference for you.
>
> In any case, I can point you at our public repositories and their
> downstream equivalents...
>
> Here you find the downstream configuration for LAB/IBB:
> https://github.com/sonyxperiadev/kernel/blob/aosp/LA.UM.7.1.r1/arch/arm64/boot/dts/qcom/msm8998-yoshino-common.dtsi#L3158
>
> ...And here you find the SoMainline upstream stuff for the same:
> https://github.com/SoMainline/linux/blob/SoMainline/v5.11-rc4/arch/arm64/boot/dts/qcom/msm8998-sony-xperia-yoshino.dtsi#L332
>
> https://github.com/SoMainline/linux/blob/SoMainline/v5.11-rc4/arch/arm64/boot/dts/qcom/msm8998-sony-xperia-yoshino-maple.dts#L50
>
> I hope that clears out your doubts and difficulties about implementing
> the protections on labibb.
Exactly what I was looking for. Thanks again.
>
> Yours,
> -- Angelo
>
> > Regards,
> > Amit Pundir
> >
> >> Besides that, even if it wouldn't be that dangerous, since the
> >> protection features are present, it would be nice to configure them
> >> properly as in the rare event that something bad happens, you would be
> >> able to save the hardware (or at least have a chance to!).
> >>
> >>> +&lab {
> >>> + regulator-min-microvolt = <4600000>;
> >>> + regulator-max-microvolt = <6000000>;
> >>> +};
> >>> +
> >>
> >> Same here.
> >>
> >> Yours,
> >> -- Angelo
> >>
> >>> +&mdss {
> >>> + status = "okay";
> >>> +};
> >>> +
> >>> +&mdss_mdp {
> >>> + status = "okay";
> >>> +};
> >>> +
> >>> &mss_pil {
> >>> status = "okay";
> >>> firmware-name = "qcom/sdm845/mba.mbn", "qcom/sdm845/modem.mdt";
> >>>
> >>
>
