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
