Introduce binding and driver for the Maxim MAX20411, and wire these up
on the Qualcomm SA8295P ADP.
Bjorn Andersson (3):
regulator: dt-bindings: Describe Maxim MAX20411
regulator: Introduce Maxim MAX20411 Step-Down converter
arm64: dts: qcom: sa8295p-adp: Add max20411 on i2c12
.../bindings/regulator/maxim,max20411.yaml | 58 +++++++
arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 41 +++++
drivers/regulator/Kconfig | 8 +
drivers/regulator/Makefile | 1 +
drivers/regulator/max20411-regulator.c | 163 ++++++++++++++++++
5 files changed, 271 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/maxim,max20411.yaml
create mode 100644 drivers/regulator/max20411-regulator.c
--
2.25.1
From: Bjorn Andersson <[email protected]>
The SA8295P ADP has a Maxim max20411 step-down converter on i2c12.
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v1:
- i2c node had changed name
arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 41 ++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
index bb4270e8f551..642000d95812 100644
--- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
@@ -266,6 +266,27 @@ &dispcc1 {
status = "okay";
};
+&i2c12 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c12_state>;
+
+ status = "okay";
+
+ vdd_gfx: regulator@39 {
+ compatible = "maxim,max20411";
+ reg = <0x39>;
+
+ regulator-name = "vdd_gfx";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <968750>;
+
+ enable-gpios = <&pmm8540a_gpios 2 GPIO_ACTIVE_HIGH>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&vdd_gfx_enable_state>;
+ };
+};
+
&mdss0 {
status = "okay";
};
@@ -476,6 +497,10 @@ &pcie4_phy {
status = "okay";
};
+&qup1 {
+ status = "okay";
+};
+
&qup2 {
status = "okay";
};
@@ -636,7 +661,23 @@ &xo_board_clk {
/* PINCTRL */
+&pmm8540a_gpios {
+ vdd_gfx_enable_state: vdd-gfx-enable-state {
+ pins = "gpio2";
+ function = "normal";
+ output-enable;
+ };
+};
+
&tlmm {
+ i2c12_state: i2c12-state {
+ pins = "gpio0", "gpio1";
+ function = "qup12";
+
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
pcie2a_default: pcie2a-default-state {
clkreq-n-pins {
pins = "gpio142";
--
2.25.1
On Tue, 24 Jan 2023 10:44:37 -0800, Bjorn Andersson wrote:
> Introduce binding and driver for the Maxim MAX20411, and wire these up
> on the Qualcomm SA8295P ADP.
>
> Bjorn Andersson (3):
> regulator: dt-bindings: Describe Maxim MAX20411
> regulator: Introduce Maxim MAX20411 Step-Down converter
> arm64: dts: qcom: sa8295p-adp: Add max20411 on i2c12
>
> [...]
Applied to
broonie/regulator.git for-next
Thanks!
[1/3] regulator: dt-bindings: Describe Maxim MAX20411
commit: c1bf8de25d0aa6e399399d6410b1140d4402c2e0
[2/3] regulator: Introduce Maxim MAX20411 Step-Down converter
commit: 047ebaffd8171a47eb5462aec0f6006416fbe62e
[3/3] arm64: dts: qcom: sa8295p-adp: Add max20411 on i2c12
(no commit info)
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
On Tue, Jan 24, 2023 at 10:44:40AM -0800, Bjorn Andersson wrote:
> From: Bjorn Andersson <[email protected]>
>
> The SA8295P ADP has a Maxim max20411 step-down converter on i2c12.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>
> Changes since v1:
> - i2c node had changed name
>
> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 41 ++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
I realized today this has to do with the comment over at:
https://lore.kernel.org/all/[email protected]/
and I just didn't realize that the schematic I've started looking at
black boxes the SOM/SIP which holds this... darn I thought I could see
more than I could :(
I took a similiar patch for a spin on sa8540p-ride (which I'll later
submit), and things worked fine (I'm not really consuming the output of
the regulator mind you).
Downstream devicetree indicates all of this looks ok except for possibly
the below comment:
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> index bb4270e8f551..642000d95812 100644
> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> @@ -266,6 +266,27 @@ &dispcc1 {
> status = "okay";
> };
>
> +&i2c12 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c12_state>;
> +
> + status = "okay";
> +
> + vdd_gfx: regulator@39 {
> + compatible = "maxim,max20411";
> + reg = <0x39>;
> +
> + regulator-name = "vdd_gfx";
> + regulator-min-microvolt = <800000>;
Is there a reason you chose this instead of the 500000 I see downstream?
> + regulator-max-microvolt = <968750>;
Likewise, I see in this brief description of the regulator
that the upper bound is higher than this (1.275 V). I am not sure if
the values in the devicetree are supposed to describe the
min/max of the regulator itself, or of what your board can really
handle/needs (the latter I guess makes more sense since you wouldn't want to
accidentally request a current draw that could melt something.. that can
be fun). I do see you've got that min/max in the driver itself (now that
I peaked at that patch).
https://www.analog.com/en/products/MAX20411.html#product-overview
For what it is worth, I also see a SIP document that states vdd_gfx min/max
is 0.56/1.03 V, which is ultimately what you'd feed this into. The
downstream devicetree uses the max value you provide though.
No idea how much faith I should put into the SIP document's bounds, or
downstream, but I thought I should at least highlight them.
Thanks,
Andrew
On 26.01.2023 23:54, Andrew Halaney wrote:
> On Tue, Jan 24, 2023 at 10:44:40AM -0800, Bjorn Andersson wrote:
>> From: Bjorn Andersson <[email protected]>
>>
>> The SA8295P ADP has a Maxim max20411 step-down converter on i2c12.
>>
>> Signed-off-by: Bjorn Andersson <[email protected]>
>> Signed-off-by: Bjorn Andersson <[email protected]>
>> ---
>>
>> Changes since v1:
>> - i2c node had changed name
>>
>> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 41 ++++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>
> I realized today this has to do with the comment over at:
>
> https://lore.kernel.org/all/[email protected]/
>
> and I just didn't realize that the schematic I've started looking at
> black boxes the SOM/SIP which holds this... darn I thought I could see
> more than I could :(
>
> I took a similiar patch for a spin on sa8540p-ride (which I'll later
> submit), and things worked fine (I'm not really consuming the output of
> the regulator mind you).
>
> Downstream devicetree indicates all of this looks ok except for possibly
> the below comment:
>
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
>> index bb4270e8f551..642000d95812 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
>> @@ -266,6 +266,27 @@ &dispcc1 {
>> status = "okay";
>> };
>>
>> +&i2c12 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&i2c12_state>;
>> +
>> + status = "okay";
>> +
>> + vdd_gfx: regulator@39 {
>> + compatible = "maxim,max20411";
>> + reg = <0x39>;
>> +
>> + regulator-name = "vdd_gfx";
>> + regulator-min-microvolt = <800000>;
>
> Is there a reason you chose this instead of the 500000 I see downstream?
>
>> + regulator-max-microvolt = <968750>;
>
> Likewise, I see in this brief description of the regulator
> that the upper bound is higher than this (1.275 V). I am not sure if
> the values in the devicetree are supposed to describe the
> min/max of the regulator itself, or of what your board can really
> handle/needs (the latter I guess makes more sense since you wouldn't want to
> accidentally request a current draw that could melt something.. that can
> be fun). I do see you've got that min/max in the driver itself (now that
> I peaked at that patch).
Yes, your suspicions are correct and the DT sets the actual ranges
for the voltage regulators on this specific board while the
hardware reachable ranges are defined in the .c driver.
Konrad
>
> https://www.analog.com/en/products/MAX20411.html#product-overview
>
> For what it is worth, I also see a SIP document that states vdd_gfx min/max
> is 0.56/1.03 V, which is ultimately what you'd feed this into. The
> downstream devicetree uses the max value you provide though.
>
> No idea how much faith I should put into the SIP document's bounds, or
> downstream, but I thought I should at least highlight them.
>
> Thanks,
> Andrew
>
On Fri, Jan 27, 2023 at 12:35:37AM +0100, Konrad Dybcio wrote:
>
>
> On 26.01.2023 23:54, Andrew Halaney wrote:
> > On Tue, Jan 24, 2023 at 10:44:40AM -0800, Bjorn Andersson wrote:
> >> From: Bjorn Andersson <[email protected]>
> >>
> >> The SA8295P ADP has a Maxim max20411 step-down converter on i2c12.
> >>
> >> Signed-off-by: Bjorn Andersson <[email protected]>
> >> Signed-off-by: Bjorn Andersson <[email protected]>
> >> ---
> >>
> >> Changes since v1:
> >> - i2c node had changed name
> >>
> >> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 41 ++++++++++++++++++++++++
> >> 1 file changed, 41 insertions(+)
> >
> > I realized today this has to do with the comment over at:
> >
> > https://lore.kernel.org/all/[email protected]/
> >
> > and I just didn't realize that the schematic I've started looking at
> > black boxes the SOM/SIP which holds this... darn I thought I could see
> > more than I could :(
> >
> > I took a similiar patch for a spin on sa8540p-ride (which I'll later
> > submit), and things worked fine (I'm not really consuming the output of
> > the regulator mind you).
> >
> > Downstream devicetree indicates all of this looks ok except for possibly
> > the below comment:
> >
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> >> index bb4270e8f551..642000d95812 100644
> >> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> >> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> >> @@ -266,6 +266,27 @@ &dispcc1 {
> >> status = "okay";
> >> };
> >>
> >> +&i2c12 {
> >> + pinctrl-names = "default";
> >> + pinctrl-0 = <&i2c12_state>;
> >> +
> >> + status = "okay";
> >> +
> >> + vdd_gfx: regulator@39 {
> >> + compatible = "maxim,max20411";
> >> + reg = <0x39>;
> >> +
> >> + regulator-name = "vdd_gfx";
> >> + regulator-min-microvolt = <800000>;
> >
> > Is there a reason you chose this instead of the 500000 I see downstream?
> >
> >> + regulator-max-microvolt = <968750>;
> >
> > Likewise, I see in this brief description of the regulator
> > that the upper bound is higher than this (1.275 V). I am not sure if
> > the values in the devicetree are supposed to describe the
> > min/max of the regulator itself, or of what your board can really
> > handle/needs (the latter I guess makes more sense since you wouldn't want to
> > accidentally request a current draw that could melt something.. that can
> > be fun). I do see you've got that min/max in the driver itself (now that
> > I peaked at that patch).
> Yes, your suspicions are correct and the DT sets the actual ranges
> for the voltage regulators on this specific board while the
> hardware reachable ranges are defined in the .c driver.
>
> Konrad
Thanks Konrad, then I think:
Reviewed-by: Andrew Halaney <[email protected]>
Tested-by: Andrew Halaney <[email protected]>
is appropriate since things are within range on all accounts. I would
appreciate an explanation on the current min/max values though if possible!
On Tue, Jan 24, 2023 at 10:44:40AM -0800, Bjorn Andersson wrote:
> From: Bjorn Andersson <[email protected]>
>
> The SA8295P ADP has a Maxim max20411 step-down converter on i2c12.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>
> Changes since v1:
> - i2c node had changed name
>
> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 41 ++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> index bb4270e8f551..642000d95812 100644
> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> @@ -266,6 +266,27 @@ &dispcc1 {
> status = "okay";
> };
>
> +&i2c12 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c12_state>;
> +
> + status = "okay";
> +
> + vdd_gfx: regulator@39 {
Nit: Should the label be named 'vreg_gfx' (or 'vreg_vdd_gfx)') for
consistency with rest of the file?
> + compatible = "maxim,max20411";
> + reg = <0x39>;
> +
> + regulator-name = "vdd_gfx";
> + regulator-min-microvolt = <800000>;
> + regulator-max-microvolt = <968750>;
> +
> + enable-gpios = <&pmm8540a_gpios 2 GPIO_ACTIVE_HIGH>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&vdd_gfx_enable_state>;
> + };
> +};
> +
> &mdss0 {
> status = "okay";
> };
> @@ -476,6 +497,10 @@ &pcie4_phy {
> status = "okay";
> };
>
> +&qup1 {
> + status = "okay";
> +};
> +
> &qup2 {
> status = "okay";
> };
> @@ -636,7 +661,23 @@ &xo_board_clk {
>
> /* PINCTRL */
>
> +&pmm8540a_gpios {
> + vdd_gfx_enable_state: vdd-gfx-enable-state {
For consistency with the rest of sc8280xp, can you rename this
vdd_gfx_en: vdd-gfx-en-state {
(i.e. drop the 'state' from the label and shorten 'enable')?
> + pins = "gpio2";
> + function = "normal";
> + output-enable;
> + };
> +};
> +
> &tlmm {
> + i2c12_state: i2c12-state {
Similar here, this should be
i2c12_default: i2c12-default-state {
> + pins = "gpio0", "gpio1";
> + function = "qup12";
> +
And this newline can be removed.
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> pcie2a_default: pcie2a-default-state {
> clkreq-n-pins {
> pins = "gpio142";
Johan
On Fri, Jan 27, 2023 at 10:55:21AM +0100, Johan Hovold wrote:
> On Tue, Jan 24, 2023 at 10:44:40AM -0800, Bjorn Andersson wrote:
> > From: Bjorn Andersson <[email protected]>
> >
> > The SA8295P ADP has a Maxim max20411 step-down converter on i2c12.
> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > ---
> >
> > Changes since v1:
> > - i2c node had changed name
> >
> > arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 41 ++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > index bb4270e8f551..642000d95812 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > @@ -266,6 +266,27 @@ &dispcc1 {
> > status = "okay";
> > };
> >
> > +&i2c12 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&i2c12_state>;
> > +
> > + status = "okay";
> > +
> > + vdd_gfx: regulator@39 {
>
> Nit: Should the label be named 'vreg_gfx' (or 'vreg_vdd_gfx)') for
> consistency with rest of the file?
>
I will investigate what the appropriate name is, consistency would be
nice though.
> > + compatible = "maxim,max20411";
> > + reg = <0x39>;
> > +
> > + regulator-name = "vdd_gfx";
> > + regulator-min-microvolt = <800000>;
> > + regulator-max-microvolt = <968750>;
> > +
> > + enable-gpios = <&pmm8540a_gpios 2 GPIO_ACTIVE_HIGH>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&vdd_gfx_enable_state>;
> > + };
> > +};
> > +
> > &mdss0 {
> > status = "okay";
> > };
> > @@ -476,6 +497,10 @@ &pcie4_phy {
> > status = "okay";
> > };
> >
> > +&qup1 {
> > + status = "okay";
> > +};
> > +
> > &qup2 {
> > status = "okay";
> > };
> > @@ -636,7 +661,23 @@ &xo_board_clk {
> >
> > /* PINCTRL */
> >
> > +&pmm8540a_gpios {
> > + vdd_gfx_enable_state: vdd-gfx-enable-state {
>
> For consistency with the rest of sc8280xp, can you rename this
>
> vdd_gfx_en: vdd-gfx-en-state {
>
> (i.e. drop the 'state' from the label and shorten 'enable')?
>
Will do.
> > + pins = "gpio2";
> > + function = "normal";
> > + output-enable;
> > + };
> > +};
> > +
> > &tlmm {
> > + i2c12_state: i2c12-state {
>
> Similar here, this should be
>
> i2c12_default: i2c12-default-state {
>
Sounds reasonable.
> > + pins = "gpio0", "gpio1";
> > + function = "qup12";
> > +
>
> And this newline can be removed.
>
Sure
> > + drive-strength = <2>;
> > + bias-pull-up;
> > + };
> > +
> > pcie2a_default: pcie2a-default-state {
> > clkreq-n-pins {
> > pins = "gpio142";
>
> Johan
Thanks Johan,
Bjorn
On Thu, Jan 26, 2023 at 05:43:54PM -0600, Andrew Halaney wrote:
> On Fri, Jan 27, 2023 at 12:35:37AM +0100, Konrad Dybcio wrote:
> >
> >
> > On 26.01.2023 23:54, Andrew Halaney wrote:
> > > On Tue, Jan 24, 2023 at 10:44:40AM -0800, Bjorn Andersson wrote:
> > >> From: Bjorn Andersson <[email protected]>
> > >>
> > >> The SA8295P ADP has a Maxim max20411 step-down converter on i2c12.
> > >>
> > >> Signed-off-by: Bjorn Andersson <[email protected]>
> > >> Signed-off-by: Bjorn Andersson <[email protected]>
> > >> ---
> > >>
> > >> Changes since v1:
> > >> - i2c node had changed name
> > >>
> > >> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 41 ++++++++++++++++++++++++
> > >> 1 file changed, 41 insertions(+)
> > >
> > > I realized today this has to do with the comment over at:
> > >
> > > https://lore.kernel.org/all/[email protected]/
> > >
> > > and I just didn't realize that the schematic I've started looking at
> > > black boxes the SOM/SIP which holds this... darn I thought I could see
> > > more than I could :(
> > >
> > > I took a similiar patch for a spin on sa8540p-ride (which I'll later
> > > submit), and things worked fine (I'm not really consuming the output of
> > > the regulator mind you).
> > >
> > > Downstream devicetree indicates all of this looks ok except for possibly
> > > the below comment:
> > >
> > >>
> > >> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > >> index bb4270e8f551..642000d95812 100644
> > >> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > >> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > >> @@ -266,6 +266,27 @@ &dispcc1 {
> > >> status = "okay";
> > >> };
> > >>
> > >> +&i2c12 {
> > >> + pinctrl-names = "default";
> > >> + pinctrl-0 = <&i2c12_state>;
> > >> +
> > >> + status = "okay";
> > >> +
> > >> + vdd_gfx: regulator@39 {
> > >> + compatible = "maxim,max20411";
> > >> + reg = <0x39>;
> > >> +
> > >> + regulator-name = "vdd_gfx";
> > >> + regulator-min-microvolt = <800000>;
> > >
> > > Is there a reason you chose this instead of the 500000 I see downstream?
> > >
> > >> + regulator-max-microvolt = <968750>;
> > >
> > > Likewise, I see in this brief description of the regulator
> > > that the upper bound is higher than this (1.275 V). I am not sure if
> > > the values in the devicetree are supposed to describe the
> > > min/max of the regulator itself, or of what your board can really
> > > handle/needs (the latter I guess makes more sense since you wouldn't want to
> > > accidentally request a current draw that could melt something.. that can
> > > be fun). I do see you've got that min/max in the driver itself (now that
> > > I peaked at that patch).
> > Yes, your suspicions are correct and the DT sets the actual ranges
> > for the voltage regulators on this specific board while the
> > hardware reachable ranges are defined in the .c driver.
> >
> > Konrad
>
> Thanks Konrad, then I think:
>
> Reviewed-by: Andrew Halaney <[email protected]>
> Tested-by: Andrew Halaney <[email protected]>
>
> is appropriate since things are within range on all accounts. I would
> appreciate an explanation on the current min/max values though if possible!
>
I will add a line about the range as I resubmit the patch.
Thanks,
Bjorn