mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Maud Spierings <maudspierings@gocontroll.com>
To: "Matti Vaittinen" <mazziesaccount@gmail.com>,
	"Lothar Waßmann" <LW@KARO-electronics.de>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 3/5] arm64: dts: freescale: add Ka-Ro Electronics tx8m-1610 COM
Date: Wed, 29 Oct 2025 16:35:25 +0100	[thread overview]
Message-ID: <0667e026-99f3-4233-b3f6-e38273961d49@gocontroll.com> (raw)
In-Reply-To: <d05c62c9-7ed7-46e4-aa4d-27172741b5ee@gmail.com>

Hi Matti,

On 10/29/25 11:05, Matti Vaittinen wrote:
> On 29/10/2025 11:48, Lothar Waßmann wrote:
>> Hi,
>>
>> On Wed, 29 Oct 2025 10:42:17 +0200 Matti Vaittinen wrote:
>>> On 29/10/2025 09:11, Lothar Waßmann wrote:
>>>> Hi,
>>>>
>>>> On Tue, 28 Oct 2025 14:10:04 +0100 Maud Spierings wrote:
>>>>> On 10/28/25 13:42, Maud Spierings wrote:
>>>>>> On 10/28/25 13:15, Matti Vaittinen wrote:
>>>> [...]
>>>>>>> Could/Should this be described using the:
>>>>>>> 'rohm,feedback-pull-up-r1-ohms' and
>>>>>>> 'rohm,feedback-pull-up-r2-ohms'? If I understand the comment
>>>>>>> correctly, that might allow the driver to be able to use correctly
>>>>>>> scaled voltages.
>>>>>>>
>>>>>>> https://elixir.bootlin.com/linux/v6.18-rc1/source/Documentation/
>>>>>>> devicetree/bindings/regulator/rohm,bd71837-regulator.yaml#L108
>>>>>>
>>>>>> Ah I didn't know those existed, should've checked the bindings in 
>>>>>> more
>>>>>> detail, thanks for the hint!
>>>>>>
>>>>>> I will have to investigate this carefully, since I don't have 
>>>>>> access to
>>>>>> the actual design of the COM, so I don't know exactly what is there.
>>>>>
>>>>> So I am not yet entirely sure if this works out, I used the 
>>>>> calculation
>>>>> in the driver:
>>>>>
>>>>> /*
>>>>>     * Setups where regulator (especially the buck8) output voltage 
>>>>> is scaled
>>>>>     * by adding external connection where some other regulator 
>>>>> output is
>>>>> connected
>>>>>     * to feedback-pin (over suitable resistors) is getting popular 
>>>>> amongst
>>>>> users
>>>>>     * of BD71837. (This allows for example scaling down the buck8 
>>>>> voltages
>>>>> to suit
>>>>>     * lover GPU voltages for projects where buck8 is (ab)used to 
>>>>> supply power
>>>>>     * for GPU. Additionally some setups do allow DVS for buck8 but 
>>>>> as this do
>>>>>     * produce voltage spikes the HW must be evaluated to be able to
>>>>> survive this
>>>>>     * - hence I keep the DVS disabled for non DVS bucks by default. I
>>>>> don't want
>>>>>     * to help you burn your proto board)
>>>>>     *
>>>>>     * So we allow describing this external connection from DT and 
>>>>> scale the
>>>>>     * voltages accordingly. This is what the connection should look 
>>>>> like:
>>>>>     *
>>>>>     * |------------|
>>>>>     * |    buck 8  |-------+----->Vout
>>>>>     * |        |    |
>>>>>     * |------------|    |
>>>>>     *    | FB pin    |
>>>>>     *    |        |
>>>>>     *    +-------+--R2---+
>>>>>     *        |
>>>>>     *        R1
>>>>>     *        |
>>>>>     *    V FB-pull-up
>>>>>     *
>>>>>     *    Here the buck output is sifted according to formula:
>>>>>     *
>>>>>     * Vout_o = Vo - (Vpu - Vo)*R2/R1
>>>>>     * Linear_step = step_orig*(R1+R2)/R1
>>>>>     *
>>>>>     * where:
>>>>>     * Vout_o is adjusted voltage output at vsel reg value 0
>>>>>     * Vo is original voltage output at vsel reg value 0
>>>>>     * Vpu is the pull-up voltage V FB-pull-up in the picture
>>>>>     * R1 and R2 are resistor values.
>>>>>     *
>>>>>     * As a real world example for buck8 and a specific GPU:
>>>>>     * VLDO = 1.6V (used as FB-pull-up)
>>>>>     * R1 = 1000ohms
>>>>>     * R2 = 150ohms
>>>>>     * VSEL 0x0 => 0.8V – (VLDO – 0.8) * R2 / R1 = 0.68V
>>>>>     * Linear Step = 10mV * (R1 + R2) / R1 = 11.5mV
>>>>>     */
>>>>>
>>>>> Because I do not know the pull up voltage, and I am not sure if it 
>>>>> is a
>>>>> pull up.
>>>>>
>>>>> So:
>>>>> Vout_o = 1.35V
>>>>> Vo = 1.1V
>>>>> Vpu = unknown
>>>>> R2 = 499 Ohm
>>>>> R1 = 2200 Ohm
>>>>> Gives:
>>>>> Vpu = ~0V
>>>>>
>>>>> And:
>>>>> Vout_o = 1.35V
>>>>> Vo = 1.1V
>>>>> Vpu = unknown
>>>>> R2 = 2200 Ohm
>>>>> R1 = 499 Ohm
>>>>> Gives:
>>>>> Vpu = ~1.04V
>>>>>
>>>>> I am not quite sure which resistor is R1 and which is R2 but having
>>>>> there be a pull down to 0V seems the most logical answer?
>>>>>
>>>>> I am adding Lothar from Ka-Ro to the CC maybe he can shed some 
>>>>> light on
>>>>> this setup.
>>>> R2 is connected to GND, so Vpu = 0.
>>>> With:
>>>>     regulator-min-microvolt = <1350000>;
>>>>     regulator-max-microvolt = <1350000>;
>>>>     rohm,fb-pull-up-microvolt = <0>;
>>>>     rohm,feedback-pull-up-r1-ohms = <2200>;
>>>>     rohm,feedback-pull-up-r2-ohms = <499>;
>>>> the correct voltage should be produced on the BUCK8 output, but a quick
>>>> test with these parameters led to:
>>>> |failed to get the current voltage: -EINVAL
>>>> |bd718xx-pmic bd71847-pmic.3.auto: error -EINVAL: failed to register 
>>>> buck6 regulator
>>>> |bd718xx-pmic: probe of bd71847-pmic.3.auto failed with error -22
>>>>
>>>> Apparently noone has ever tested this feature in real life.
>>>
>>> Thanks for trying it out Lothar. I am positive this was tested - but
>>> probably the use-case has been using a pull-up. I assume having the zero
>>> pull-up voltage causes the driver to calculate some bogus values. I
>>> think fixing the computation in the driver might not be that big of a
>>> task(?) The benefit of doing it would be that the correct voltages would
>>> be calculated by the driver.
>>>
>>> If real voltages aren't matching what is calculated by the driver, then
>>> the voltages requested by regulator consumers will cause wrong voltages
>>> to be applied. Debug interfaces will also show wrong voltages, and the
>>> safety limits set in the device-tree will not be really respected.
>>>
>>> I think this would be well worth fixing.
>>>
>> Before doing the real-life test I did the same calculation that's done
>> in the driver to be sure that it will generate the correct values:
>> bc 1.07.1
>> Copyright 1991-1994, 1997, 1998, 2000, 2004, 2006, 2008, 2012-2017 
>> Free Software Foundation, Inc.
>> This is free software with ABSOLUTELY NO WARRANTY.
>> For details type `warranty'.
>> fb_uv=0
>> r1=2200
>> r2=499
>> min=800000
>> step=10000
>> # default voltage without divider
>> min+30*step
>> 1100000
>> min=min-(fb_uv-min)*r2/r1
>> step=step*(r1+r2)/r1
>> min
>> 981454
>> step
>> 12268
>> # default voltage with divider
>> min+30*step
>> 1349494
>>
>> Probably we need to use this value rather than the nominal 135000 as
>> the target voltage in the DTB.
> 
> Yes. When the driver calculates the voltages which match the actual 
> voltages, then you should also use the actual voltages in the device-tree.
> 

Think I've got it:

diff --git a/drivers/regulator/bd718x7-regulator.c 
b/drivers/regulator/bd718x7-regulator.c
index 022d98f3c32a2..ea9c4058ee6a5 100644
--- a/drivers/regulator/bd718x7-regulator.c
+++ b/drivers/regulator/bd718x7-regulator.c
@@ -1613,6 +1613,8 @@ static int setup_feedback_loop(struct device *dev, 
struct device_node *np,
                                 step /= r1;

                                 new[j].min = min;
+                               new[j].min_sel = 
desc->linear_ranges[j].min_sel;
+                               new[j].max_sel = 
desc->linear_ranges[j].max_sel;
                                 new[j].step = step;

                                 dev_dbg(dev, "%s: old range min %d, 
step %d\n",


the min_sel and max_sel fields were uninitialized in the new 
linear_range, copying them over from the old one (they refer to the 
register range if I understand correctly so they should not change) 
initializes them.

Then setting 1349494 as the actual voltage makes it fully work. 
Otherwise it complains:
buck6: failed to apply 1350000-1350000uV constraint: -EINVAL

Final debug output now:
[    0.327807] rohm-bd718x7 0-004b: buck6: old range min 800000, step 10000
[    0.327813] rohm-bd718x7 0-004b: new range min 981454, step 12268
[    0.327819] rohm-bd718x7 0-004b: regulator 'buck6' has FB pull-up 
configured

I will add this fix to the next version of this patchset and include 
your requested change in the dts.

Kind regards,
Maud

  reply	other threads:[~2025-10-29 15:35 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-22  7:22 [PATCH v2 0/5] arm64: dts: freescale: add support for the GOcontroll Moduline IV/Mini Maud Spierings via B4 Relay
2025-10-22  7:22 ` [PATCH v2 1/5] dt-bindings: arm: fsl: Add " Maud Spierings via B4 Relay
2025-10-22  7:22 ` [PATCH v2 2/5] arm64: dts: imx8mm: Add pinctrl config definitions Maud Spierings via B4 Relay
2025-10-22  7:22 ` [PATCH v2 3/5] arm64: dts: freescale: add Ka-Ro Electronics tx8m-1610 COM Maud Spierings via B4 Relay
2025-10-28 12:15   ` Matti Vaittinen
2025-10-28 12:42     ` Maud Spierings
2025-10-28 13:10       ` Maud Spierings
2025-10-29  7:11         ` Lothar Waßmann
2025-10-29  8:42           ` Matti Vaittinen
2025-10-29  9:00             ` Maud Spierings
2025-10-29  9:33               ` Matti Vaittinen
2025-10-29  9:48             ` Lothar Waßmann
2025-10-29 10:05               ` Matti Vaittinen
2025-10-29 15:35                 ` Maud Spierings [this message]
2025-10-29 15:51                   ` Maud Spierings
2025-10-29 19:04                     ` Matti Vaittinen
2025-10-30  8:54                   ` Lothar Waßmann
2025-10-30 11:01                     ` Matti Vaittinen
2025-10-30 12:00                       ` Lothar Waßmann
2025-10-31 12:07                         ` Matti Vaittinen
2025-10-30 14:45                     ` Maud Spierings
2025-10-31  5:36                       ` Lothar Waßmann
2025-10-31  7:26                         ` Maud Spierings
2025-10-22  7:22 ` [PATCH v2 4/5] arm64: dts: freescale: Add the GOcontroll Moduline IV Maud Spierings via B4 Relay
2025-10-22  7:57   ` Marc Kleine-Budde
2025-10-22  8:23     ` Maud Spierings
2025-10-22 10:18   ` Maud Spierings
2025-10-22  7:22 ` [PATCH v2 5/5] arm64: dts: freescale: Add the GOcontroll Moduline Mini Maud Spierings via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0667e026-99f3-4233-b3f6-e38273961d49@gocontroll.com \
    --to=maudspierings@gocontroll.com \
    --cc=LW@KARO-electronics.de \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mazziesaccount@gmail.com \
    --cc=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®