Re: [PATCH v2 3/5] arm64: dts: freescale: add Ka-Ro Electronics tx8m-1610 COM

[Maud Spierings <maudspierings@gocontroll.com>]

On 10/29/25 16:35, Maud Spierings wrote:
> 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.

New idea, why don't we just change the values of the original 
linear_range? Do away with the allocation there entirely.

Kind regards,
Maud