| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <036a04e1-3cb7-44c3-8836-3640f640b4a4@gocontroll.com>
Date: Thu, 30 Oct 2025 15:45:14 +0100
From: Maud Spierings <maudspierings@...ontroll.com>
To: Lothar Waßmann <LW@...O-electronics.de>
Cc: Matti Vaittinen <mazziesaccount@...il.com>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 3/5] arm64: dts: freescale: add Ka-Ro Electronics
tx8m-1610 COM
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:
regulator use open bypass opmode voltage current
min max
---------------------------------------------------------------------------------------
regulator-dummy 1 1 0 unknown 0mV
0mA 0mV 0mV
2-0014-vled 0
0mA 0mV 0mV
6v4 1 0 0 unknown 6400mV
0mA 6400mV 6400mV
3v3-etn 1 1 0 unknown 3300mV
0mA 3300mV 3300mV
30be0000.ethernet-phy 1
0mA 0mV 0mV
3v3-m.2 3 3 0 unknown 3300mV
0mA 3300mV 3300mV
30b50000.mmc-vmmc 1
0mA 3300mV 3400mV
serial0-0-vddio 1
0mA 0mV 0mV
serial0-0-vbat 1
0mA 0mV 0mV
5v0 2 1 0 unknown 5000mV
0mA 5000mV 5000mV
32e50000.usb-vbus 1
0mA 0mV 0mV
can1-stby 1 1 0 unknown 3300mV
0mA 3300mV 3300mV
spi0.2-xceiver 1
0mA 0mV 0mV
can2-stby 1 1 0 unknown 3300mV
0mA 3300mV 3300mV
spi0.3-xceiver 1
0mA 0mV 0mV
can3-stby 1 1 0 unknown 3300mV
0mA 3300mV 3300mV
spi1.2-xceiver 1
0mA 0mV 0mV
can4-stby 1 1 0 unknown 3300mV
0mA 3300mV 3300mV
spi1.3-xceiver 1
0mA 0mV 0mV
buck1 1 0 0 unknown 900mV
0mA 780mV 900mV
buck2 2 1 0 unknown 850mV
0mA 810mV 950mV
cpu0-cpu 1
0mA 850mV 850mV
buck3 1 0 0 unknown 900mV
0mA 850mV 900mV
buck4 7 6 0 unknown 3300mV
0mA 3300mV 3300mV
30b40000.mmc-vmmc 1
0mA 3300mV 3400mV
spi1.3-vdd 1
0mA 0mV 0mV
spi1.2-vdd 1
0mA 0mV 0mV
spi0.3-vdd 1
0mA 0mV 0mV
spi0.2-vdd 1
0mA 0mV 0mV
spi2.4-vref 1
0mA 0mV 0mV
buck5 3 2 0 unknown 1800mV
0mA 1755mV 1950mV
30b40000.mmc-vqmmc 1
0mA 0mV 0mV
ldo6 1 0 0 unknown 1200mV
0mA 1200mV 1200mV
buck6 1 0 0 unknown 1349mV
0mA 1288mV 1410mV
ldo1 1 0 0 unknown 1800mV
0mA 1700mV 1900mV
ldo2 1 0 0 unknown 800mV
0mA 800mV 900mV
ldo3 1 0 0 unknown 1800mV
0mA 1800mV 1800mV
ldo4 1 0 0 unknown 900mV
0mA 900mV 1000mV
ldo5 0 0 0 unknown 3300mV
0mA 1800mV 3300mV
buck6 at 1349 mV
Kind regards,
Maud
Powered by blists - more mailing lists