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

Lothar Waßmann
2025-10-31T05:36:33Z

Hi,

On Thu, 30 Oct 2025 15:45:14 +0100 Maud Spierings wrote:
> Hi Lothar,
>
> On 10/30/25 09:54, Lothar Waßmann wrote:
> > Hi,
> >
> > On Wed, 29 Oct 2025 16:35:25 +0100 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.
> >>
> > Does it also work with min/max settings in the DTS that are taken from
> > the designated value +/- 5% tolerance margin, so that the DTS contains
> > reasonable values determined by the HW requirements, rather than some
> > artificial number that is enforced by the SW behaviour?
> > E.g.:
> > regulator-min-microvolts = <(135000 - 6750)>;
> > regulator-min-microvolts = <(135000 + 6750)>;
> > Thus, the nominal value of the voltage is explicitly shown in the DTS
> > file.
>
> Setting that range seems to work:
>
I just checked the datasheet of the DRAM powered by the regulator. The voltage
tolerance of the 1.35V supply is -.067 V + .1 V.
I.e. the voltage settings should be:
regulator-min-microvolts = <(135000 - 6700)>;
regulator-max-microvolts = <(135000 + 10000)>;
to match the actual requirements of the consumer for this regulator.

Lothar Waßmann