linux-kernel - Re: [PATCH RESEND] arm64: dts: qcom: msm8916-samsung-serranove: Add RT5033 PMIC with charger

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ab3e475f-27ae-e718-60bd-cb22f5070942@rocketmail.com>
Date:   Sun, 18 Jun 2023 18:49:16 +0200
From:   Jakob Hauser <jahau@...ketmail.com>
To:     Stephan Gerhold <stephan@...hold.net>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Sebastian Reichel <sre@...nel.org>, Lee Jones <lee@...nel.org>,
        Raymond Hackley <raymondhackley@...tonmail.com>,
        Henrik Grimler <henrik@...mler.se>,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH RESEND] arm64: dts: qcom: msm8916-samsung-serranove: Add
 RT5033 PMIC with charger

Hi Stephan,

On 17.06.23 16:15, Stephan Gerhold wrote:
> On Sat, Jun 17, 2023 at 02:29:34AM +0200, Jakob Hauser wrote:

...

>> +		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"?

...
About the "regulator-name"s I wasn't really aware. I don't have a strong 
opinion on this.

With the downstream names, it would look like this:

regulators {
	rt5033_reg_safe_ldo: SAFE_LDO {
		regulator-name = "RT5033SafeLDO";
		regulator-min-microvolt = <4900000>;
		regulator-max-microvolt = <4900000>;
		regulator-always-on;
	};
	rt5033_reg_ldo: LDO {
		regulator-name = "CAM_SENSOR_A2.8V";
		regulator-min-microvolt = <2800000>;
		regulator-max-microvolt = <2800000>;
	};
	rt5033_reg_buck: BUCK {
		regulator-name = "CAM_SENSOR_CORE_1.25V";
		regulator-min-microvolt = <1200000>;
		regulator-max-microvolt = <1200000>;
	};

Dropping them would look like this:

regulators {
	rt5033_reg_safe_ldo: SAFE_LDO {
		regulator-min-microvolt = <4900000>;
		regulator-max-microvolt = <4900000>;
		regulator-always-on;
	};
	rt5033_reg_ldo: LDO {
		regulator-min-microvolt = <2800000>;
		regulator-max-microvolt = <2800000>;
	};
	rt5033_reg_buck: BUCK {
		regulator-min-microvolt = <1200000>;
		regulator-max-microvolt = <1200000>;
	};

I would rather drop them. The first name "RT5033SafeLDO" doesn't add 
much information. The other two I'm not fully sure if they provide the 
cam sensor only or if there might be other users as well. Also it add an 
additional set of names. When dropping them, the generic names SAFE_LDO, 
LDO and BUCK are taken from the rt5033-regulator driver.

Unfortunately, I added the example in the dt-bindings with the generic 
names. So this question might come up again when someone else adds 
rt5033-regulators to another device.

For the phandle labels I'd go for rt5033_reg_..., I already changed them 
in the examples above.

Kind regards,
Jakob