On Sat, Jun 17, 2023 at 02:29:34AM +0200, Jakob Hauser wrote: > For the regulators, apply the same settings as in the downstream > devicetree [1], including the "regulator-always-on" for the SAFE_LDO. > For the voltage of SAFE_LDO, however, there is only one voltage of 4.9 V > available in the mainline driver [2][3]. > > The values of the battery data evolve from following sources: > - precharge current: 450 mA corresponds to the default value of the chip. It > doesn't get changed by the downstream Android driver. Therefore let's stick > to this value. > - constant charge current: The 1000 mA are taken from the downstream devicetree > of the serranove battery. It's not easy to spot. The value is in the line > "input_current_limit" [4]. The rows are according to the power supply type, > the 4th value stands for "main supply" [5]. That's the value used by the > Android driver when a charging cable is plugged into the device. > - charge termination current: In the downstream devicetree of the battery > that's the line "full_check_current_1st", which contains the 150 mA [6]. > - precharge voltage: This one doesn't get set in the downstream Android driver. > The chip's default is 2.8 V. That seemed too low to have a notable effect of > handling the battery gentle. The chosen value of 3.5 V is a bit arbitrary > and possibly rather high. As the device is already several years old and > therefore most batteries too, a value on the safe side seems reasonable. > - constant charge voltage: The value of 4.35 V is set in the line > "chg_float_voltage" of the downstream battery devicetree [7]. > > The "connector" sub-node in the extcon node, the "battery" node in the > general section and the line "power-supplies" in the fuel-gauge node result > from the way of implementation documented in the dt-bindings of > rt5033-charger [8] and mfd rt5033 [9]. > > [1] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/arch/arm/boot/dts/samsung/msm8916/msm8916-sec-serranovelte-eur-r03.dtsi#L135-L181 > [2] https://github.com/torvalds/linux/blob/v6.3/include/linux/mfd/rt5033-private.h#L211-L212 > [3] https://github.com/torvalds/linux/blob/v6.3/drivers/regulator/rt5033-regulator.c#L83 > [4] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/arch/arm/boot/dts/samsung/msm8916/msm8916-sec-serranovelte-battery-r01.dtsi#L100 > [5] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/include/linux/power_supply.h#L173-L177 > [6] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/arch/arm/boot/dts/samsung/msm8916/msm8916-sec-serranovelte-battery-r01.dtsi#L102 > [7] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/arch/arm/boot/dts/samsung/msm8916/msm8916-sec-serranovelte-battery-r01.dtsi#L95 > [8] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml?h=next-20230616 > [9] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml?h=next-20230616 > > Signed-off-by: Jakob Hauser <jahau@xxxxxxxxxxxxxx> > --- > The patch is based on linux-next "next-20230616". > > The driver rt5033-charger was just recently added to linux-next. > > RESEND because I used an outdated e-mail address of Bjorn before. > > .../dts/qcom/msm8916-samsung-serranove.dts | 67 ++++++++++++++++++- > 1 file changed, 66 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts > index 15dc246e84e2..2114d26548db 100644 > --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts > [...] > @@ -261,6 +278,46 @@ touchscreen@20 { > }; > }; > > +&blsp_i2c6 { > + status = "okay"; > + > + pmic@34 { > + compatible = "richtek,rt5033"; > + reg = <0x34>; > + > + interrupt-parent = <&tlmm>; > + interrupts = <62 IRQ_TYPE_EDGE_FALLING>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_int_default>; > + > + regulators { > + safe_ldo_reg: SAFE_LDO { > + regulator-name = "SAFE_LDO"; > + regulator-min-microvolt = <4900000>; > + regulator-max-microvolt = <4900000>; > + regulator-always-on; > + }; > + ldo_reg: LDO { > + regulator-name = "LDO"; > + regulator-min-microvolt = <2800000>; > + regulator-max-microvolt = <2800000>; > + }; > + buck_reg: BUCK { > + regulator-name = "BUCK"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1200000>; > + }; The "regulator-name"s here don't really seem useful, since they're just the same as the ones already declared in the driver. Can you drop them? Alternatively you could assign more useful board-specific names, such as the CAM_SENSOR_A2.8V that was used downstream. Also, I think it would be slightly clearer to prefix the regulator labels (safe_ldo_reg, ldo_reg etc) with rt5033_. Perhaps "rt5033_ldo_reg" or "rt5033_reg_ldo"? Thanks, Stephan
