| Message ID | c9ceead9e3df77b5a33ad654db23c241cd624096.1385508112.git.stefan@agner.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Stefan, Am Mittwoch, den 27.11.2013, 00:45 +0100 schrieb Stefan Agner: > Set the requested SM2 voltage to the correct value of 1.8V. The value > before used to work on TPS658623 since the driver applied a wrong > voltage table too. However, the TPS658643 used on newer devices uses > yet another voltage table and those broke that compatibility. The > regulator driver now has the correct voltage table for both regulator > versions and those the correct voltage can be used in this device > tree. This isn't a global Tegra change, but very specific to the Colibri, so please reword your commit headline to reflect that. Also there are other issues with the regulator setup on Colibri, I sent a patch for this a good while ago, but didn't come around to revise it until now. So if you are going to touch things here, please look up that patch and fold it into this one. I'll take a look at the other patches later today. Regards, Lucas > > Signed-off-by: Stefan Agner <stefan@agner.ch> > --- > arch/arm/boot/dts/tegra20-colibri-512.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi > index d5c9bca..cbe89ff 100644 > --- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi > +++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi > @@ -268,8 +268,8 @@ > reg = <3>; > regulator-compatible = "sm2"; > regulator-name = "vdd_sm2,vin_ldo*"; > - regulator-min-microvolt = <3700000>; > - regulator-max-microvolt = <3700000>; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > regulator-always-on; > }; >
Hi Lucas, Am 2013-11-27 10:59, schrieb Lucas Stach: > This isn't a global Tegra change, but very specific to the Colibri, so > please reword your commit headline to reflect that. Agree, will do that. > Also there are other issues with the regulator setup on Colibri, I sent > a patch for this a good while ago, but didn't come around to revise it > until now. So if you are going to touch things here, please look up that > patch and fold it into this one. I guess you refer to this patchset: http://thread.gmane.org/gmane.linux.ports.tegra/11984 Ok, I will fold patch 4 into my patchset. Not sure whether I should add patch 3 (ULPI phy on Colibri T20) as well, since this one is somewhat unrelated... > I'll take a look at the other patches later today. Thanks -- Stefan
Am Mittwoch, den 27.11.2013, 12:05 +0100 schrieb Stefan Agner: > Hi Lucas, > > Am 2013-11-27 10:59, schrieb Lucas Stach: > > This isn't a global Tegra change, but very specific to the Colibri, so > > please reword your commit headline to reflect that. > Agree, will do that. > > > Also there are other issues with the regulator setup on Colibri, I sent > > a patch for this a good while ago, but didn't come around to revise it > > until now. So if you are going to touch things here, please look up that > > patch and fold it into this one. > I guess you refer to this patchset: > http://thread.gmane.org/gmane.linux.ports.tegra/11984 > > Ok, I will fold patch 4 into my patchset. Not sure whether I should add > patch 3 (ULPI phy on Colibri T20) as well, since this one is somewhat > unrelated... > The ULPI patch is already upstream. At least the first hunk of patch 4 should be folded into your patch, as it's a real regulator bugfix like your correction of the sm2 voltage. Lowering sm0 and sm1 voltages is somewhat an optimization and may be done as a separate patch. Regards, Lucas
On 11/26/2013 04:45 PM, Stefan Agner wrote: > Set the requested SM2 voltage to the correct value of 1.8V. The value > before used to work on TPS658623 since the driver applied a wrong > voltage table too. However, the TPS658643 used on newer devices uses > yet another voltage table and those broke that compatibility. The > regulator driver now has the correct voltage table for both regulator > versions and those the correct voltage can be used in this device > tree. One thing you haven't called out explicitly here is that this series is an incompatible change to the DT, since the old buggy driver used to allow old buggy DT content to accidentally work. I'm not too familiar with who's using mainline on the Colibri boards. Hopefully everyone doing that is using in-kernel DTs, so this incompatible change won't be any issue for anyone. This patch needs to be rolled into patch 2/3 so that "git bisect" isn't broken. You mention there's yet another PMIC version used on later boards. Do we need a new DT for that specfic version of the Colibri board?
Am 2013-11-27 18:13, schrieb Stephen Warren: > On 11/26/2013 04:45 PM, Stefan Agner wrote: >> Set the requested SM2 voltage to the correct value of 1.8V. The value >> before used to work on TPS658623 since the driver applied a wrong >> voltage table too. However, the TPS658643 used on newer devices uses >> yet another voltage table and those broke that compatibility. The >> regulator driver now has the correct voltage table for both regulator >> versions and those the correct voltage can be used in this device >> tree. > > One thing you haven't called out explicitly here is that this series is > an incompatible change to the DT, since the old buggy driver used to > allow old buggy DT content to accidentally work. > > I'm not too familiar with who's using mainline on the Colibri boards. > Hopefully everyone doing that is using in-kernel DTs, so this > incompatible change won't be any issue for anyone. > I don't think there are a lot of users since Toradex ships the NVidia downstream kernel. Two users for sure, Lucas and me. > This patch needs to be rolled into patch 2/3 so that "git bisect" isn't > broken. Will do > You mention there's yet another PMIC version used on later boards. Do we > need a new DT for that specfic version of the Colibri board? The newer device is the TPS658643, which uses a different voltage table, but this change is already in the driver. Maybe I should reword that commit a bit. I just tried to point out why the change is really needed (since we have two regulators with different voltage tables). But there is nothing beyond those two that... Because the DT now states the real, required voltage, a new device (version) would need an appropriate driver change and things should work.
Am Mittwoch, den 27.11.2013, 10:13 -0700 schrieb Stephen Warren: > On 11/26/2013 04:45 PM, Stefan Agner wrote: > > Set the requested SM2 voltage to the correct value of 1.8V. The value > > before used to work on TPS658623 since the driver applied a wrong > > voltage table too. However, the TPS658643 used on newer devices uses > > yet another voltage table and those broke that compatibility. The > > regulator driver now has the correct voltage table for both regulator > > versions and those the correct voltage can be used in this device > > tree. > > One thing you haven't called out explicitly here is that this series is > an incompatible change to the DT, since the old buggy driver used to > allow old buggy DT content to accidentally work. > The current (wrong and potentially dangerous) DT content only worked on the engineering sample models. So this change definitely does improve the situation, even with the risk of breaking a small fraction of working boards. > I'm not too familiar with who's using mainline on the Colibri boards. > Hopefully everyone doing that is using in-kernel DTs, so this > incompatible change won't be any issue for anyone. > I know of some parties using mainline, but I think everyone so far is able to use DTs bundled with the kernel. Regards, Lucas
Am 2013-11-28 10:49, schrieb Lucas Stach: > Am Mittwoch, den 27.11.2013, 10:13 -0700 schrieb Stephen Warren: >> On 11/26/2013 04:45 PM, Stefan Agner wrote: >> > Set the requested SM2 voltage to the correct value of 1.8V. The value >> > before used to work on TPS658623 since the driver applied a wrong >> > voltage table too. However, the TPS658643 used on newer devices uses >> > yet another voltage table and those broke that compatibility. The >> > regulator driver now has the correct voltage table for both regulator >> > versions and those the correct voltage can be used in this device >> > tree. >> >> One thing you haven't called out explicitly here is that this series is >> an incompatible change to the DT, since the old buggy driver used to >> allow old buggy DT content to accidentally work. >> > The current (wrong and potentially dangerous) DT content only worked on > the engineering sample models. So this change definitely does improve > the situation, even with the risk of breaking a small fraction of > working boards. On the released modules, the current value ends in a freeze during boot-up (since the wrong voltage table results in a too low voltage). When I use 1.8V in the device tree, the old drivers refuses to set any voltage. The regulators default/bootloader settings work then better than the wrongfully applied values (tested on a new device). The probe just fails: [ 0.243864] tps6586x-regulator tps6586x-regulator: failed to register regulator REG-SM_2 [ 0.244317] tps6586x-regulator: probe of tps6586x-regulator failed with error -22 So, for newer modules, the incompatible 1.8V DT results in a successful boot. Since we don't have a regulator driver at all, likely other things are broken at that situation... -- Stefan
diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi index d5c9bca..cbe89ff 100644 --- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi +++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi @@ -268,8 +268,8 @@ reg = <3>; regulator-compatible = "sm2"; regulator-name = "vdd_sm2,vin_ldo*"; - regulator-min-microvolt = <3700000>; - regulator-max-microvolt = <3700000>; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; regulator-always-on; };
Set the requested SM2 voltage to the correct value of 1.8V. The value before used to work on TPS658623 since the driver applied a wrong voltage table too. However, the TPS658643 used on newer devices uses yet another voltage table and those broke that compatibility. The regulator driver now has the correct voltage table for both regulator versions and those the correct voltage can be used in this device tree. Signed-off-by: Stefan Agner <stefan@agner.ch> --- arch/arm/boot/dts/tegra20-colibri-512.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)