| Message ID | 1345186799-18229-1-git-send-email-ldewangan@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 08/17/2012 12:59 AM, Laxman Dewangan wrote: > Harmony uses a TPS6586x regulator. Instantiate this, and hook up a > couple of fixed GPIO-controlled regulators too. > > Based on Ventana regulator patch by Stephen Warren <swarren@nvidia.com> > and converted to Harmony. Thanks, applied to Tegra's for-3.7/dt branch. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/17/2012 01:27 PM, Stephen Warren wrote: > On 08/17/2012 12:59 AM, Laxman Dewangan wrote: >> Harmony uses a TPS6586x regulator. Instantiate this, and hook up a >> couple of fixed GPIO-controlled regulators too. >> >> Based on Ventana regulator patch by Stephen Warren <swarren@nvidia.com> >> and converted to Harmony. > > Thanks, applied to Tegra's for-3.7/dt branch. Oh hang on, this adds regulators to DT, but doesn't do anything to remove the board-file registration of Harmony's regulators from board-dt-tegra20.c, which end up conflicting, and preventing the PCIe driver from being registered. So, this patch really wants to remove the legacy code to, but doing so will be problematic; to avoid runtime git bisect failures, we'd need to merge the regulator tree in first, or wait until next kernel release or something... Thoughts? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 18 August 2012 01:25 AM, Stephen Warren wrote: > On 08/17/2012 01:27 PM, Stephen Warren wrote: >> On 08/17/2012 12:59 AM, Laxman Dewangan wrote: >>> Harmony uses a TPS6586x regulator. Instantiate this, and hook up a >>> couple of fixed GPIO-controlled regulators too. >>> >>> Based on Ventana regulator patch by Stephen Warren<swarren@nvidia.com> >>> and converted to Harmony. >> Thanks, applied to Tegra's for-3.7/dt branch. > Oh hang on, this adds regulators to DT, but doesn't do anything to > remove the board-file registration of Harmony's regulators from > board-dt-tegra20.c, which end up conflicting, and preventing the PCIe > driver from being registered. > > So, this patch really wants to remove the legacy code to, but doing so > will be problematic; to avoid runtime git bisect failures, we'd need to > merge the regulator tree in first, or wait until next kernel release or > something... Thoughts? Why not just remove the regulator registration from board files along with the dt entry (in one patch). That will resolve the issue of bisect. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/20/2012 11:22 AM, Laxman Dewangan wrote: > On Saturday 18 August 2012 01:25 AM, Stephen Warren wrote: >> On 08/17/2012 01:27 PM, Stephen Warren wrote: >>> On 08/17/2012 12:59 AM, Laxman Dewangan wrote: >>>> Harmony uses a TPS6586x regulator. Instantiate this, and hook up a >>>> couple of fixed GPIO-controlled regulators too. >>>> >>>> Based on Ventana regulator patch by Stephen Warren<swarren@nvidia.com> >>>> and converted to Harmony. >>> Thanks, applied to Tegra's for-3.7/dt branch. >> Oh hang on, this adds regulators to DT, but doesn't do anything to >> remove the board-file registration of Harmony's regulators from >> board-dt-tegra20.c, which end up conflicting, and preventing the PCIe >> driver from being registered. >> >> So, this patch really wants to remove the legacy code to, but doing so >> will be problematic; to avoid runtime git bisect failures, we'd need to >> merge the regulator tree in first, or wait until next kernel release or >> something... Thoughts? > > Why not just remove the regulator registration from board files along > with the dt entry (in one patch). That will resolve the issue of bisect. The problem there is that will prevent any regulators from being registered at all, until the patch is merged with the regulator tree. That will also prevent PCIe working on Harmony. Those both work right now, so that's a bisection breakage. I suppose the solution here is to merge the regulator tree into the Tegra branch before the Harmony regulator patch. Mark, can we do that? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 20, 2012 at 11:49:11AM -0600, Stephen Warren wrote: > The problem there is that will prevent any regulators from being > registered at all, until the patch is merged with the regulator tree. > That will also prevent PCIe working on Harmony. Those both work right > now, so that's a bisection breakage. > I suppose the solution here is to merge the regulator tree into the > Tegra branch before the Harmony regulator patch. Mark, can we do that? I've no idea what you're talking about here, sorry. What are "this" and "the patch"? Surely you can just add whatever DT changes you need to add without the thing that uses the bindings?
On 08/20/2012 11:54 AM, Mark Brown wrote: > On Mon, Aug 20, 2012 at 11:49:11AM -0600, Stephen Warren wrote: > >> The problem there is that will prevent any regulators from being >> registered at all, until the patch is merged with the regulator >> tree. That will also prevent PCIe working on Harmony. Those both >> work right now, so that's a bisection breakage. > >> I suppose the solution here is to merge the regulator tree into >> the Tegra branch before the Harmony regulator patch. Mark, can we >> do that? > > I've no idea what you're talking about here, sorry. What are > "this" and "the patch"? Surely you can just add whatever DT > changes you need to add without the thing that uses the bindings? When booting Harmony using device tree, there's a special case that registers all the regulators using the remnants of the Harmony board file. Hence, regulators work right now even without the DT instantiating them. The patch in this thread adds all the required regulators to the DT file. However, it should also remove the special case so the regulators don't get registered twice, once from the board file and once from DT. However, doing that would prevent regulators getting registered at all, since v3.6-rc* don't have all the required (TPS6586x) regulator (driver) patches to get the regulators instantiated from DT. Everything will only work in linux-next or sometime during the 3.7 merge window. Hence, git bisect would be broken. One solution to this is to bring the regulator tree into the Tegra tree as a dependency, and then apply a patch which adds the regulators to DT, and removes the special case from the board file. I'm asking for your OK to do that. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 20, 2012 at 12:08:45PM -0600, Stephen Warren wrote: > The patch in this thread adds all the required regulators to the DT > file. However, it should also remove the special case so the > regulators don't get registered twice, once from the board file and > once from DT. However, doing that would prevent regulators getting > registered at all, since v3.6-rc* don't have all the required > (TPS6586x) regulator (driver) patches to get the regulators > instantiated from DT. Everything will only work in linux-next or > sometime during the 3.7 merge window. Hence, git bisect would be broken. > One solution to this is to bring the regulator tree into the Tegra > tree as a dependency, and then apply a patch which adds the regulators > to DT, and removes the special case from the board file. I'm asking > for your OK to do that. Why not just pull the patch in via the regulator tree? The idea of merging the entire regulator drivers branch into the Tegra tree doesn't seem awesome...
On 08/20/2012 12:14 PM, Mark Brown wrote: > On Mon, Aug 20, 2012 at 12:08:45PM -0600, Stephen Warren wrote: > >> The patch in this thread adds all the required regulators to the >> DT file. However, it should also remove the special case so the >> regulators don't get registered twice, once from the board file >> and once from DT. However, doing that would prevent regulators >> getting registered at all, since v3.6-rc* don't have all the >> required (TPS6586x) regulator (driver) patches to get the >> regulators instantiated from DT. Everything will only work in >> linux-next or sometime during the 3.7 merge window. Hence, git >> bisect would be broken. > >> One solution to this is to bring the regulator tree into the >> Tegra tree as a dependency, and then apply a patch which adds the >> regulators to DT, and removes the special case from the board >> file. I'm asking for your OK to do that. > > Why not just pull the patch in via the regulator tree? The idea > of merging the entire regulator drivers branch into the Tegra tree > doesn't seem awesome... I think that'd end up causing some annoying conflicts with other changes that also touch arch/arm/match-tegra/board-dt-tegra20.c. Perhaps they're manageable; they probably aren't that bad, but it seems better to avoid them. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 20, 2012 at 12:37:44PM -0600, Stephen Warren wrote: > On 08/20/2012 12:14 PM, Mark Brown wrote: > > Why not just pull the patch in via the regulator tree? The idea > > of merging the entire regulator drivers branch into the Tegra tree > > doesn't seem awesome... > I think that'd end up causing some annoying conflicts with other > changes that also touch arch/arm/match-tegra/board-dt-tegra20.c. > Perhaps they're manageable; they probably aren't that bad, but it > seems better to avoid them. Ugh, right. I guess I'd best rebase the commits out of the drivers branch. Can you let me know what you depend on and I'll build a mergable branch for you to pull in?
On 08/22/2012 01:07 PM, Mark Brown wrote: > On Mon, Aug 20, 2012 at 12:37:44PM -0600, Stephen Warren wrote: >> On 08/20/2012 12:14 PM, Mark Brown wrote: > >>> Why not just pull the patch in via the regulator tree? The >>> idea of merging the entire regulator drivers branch into the >>> Tegra tree doesn't seem awesome... > >> I think that'd end up causing some annoying conflicts with other >> changes that also touch arch/arm/match-tegra/board-dt-tegra20.c. >> Perhaps they're manageable; they probably aren't that bad, but >> it seems better to avoid them. > > Ugh, right. I guess I'd best rebase the commits out of the > drivers branch. Can you let me know what you depend on and I'll > build a mergable branch for you to pull in? I believe the only commit the Tegra tree might need is: b93fffb regulator: tps6586x: add support for SYS rail There is one other commit that touches this driver queued up for 3.7, but I've validated that the two commits can merge together without any conflicts, so I don't think we actually need this in the Tegra tree: 4c79c8d regulator: tps6586x: Convert to regulator_[enable|disable|is_enabled|get_voltage_sel]_regmap A few more notes on the overall situation: a) As background, regulators on the Harmony board are broken in v3.6-rc2, but fixed in v3.6-rc3. Specifically, commit 798bd59 "ARM: tegra: more regulator fixes for Harmony" is required. b) Even on top of v3.6-rc3, commit b93fffb "regulator: tps6586x: add support for SYS rail" introduces a regression on Harmony. I did give a Tested-by tag for this commit, but had tested it on a different board. To fix this, the commit needs to also edit arch/arm/mach-tegra/board-harmony-power.c and s/vdd_sys/REG-SYS/. c) I tested all of next-20120822, regulator's for-next, and Tegra's for-next. In all cases, if I fix the issues above, then merge Laxman's change to instantiate all Harmony's regulators from DT, without removing use of board-harmony-power.c, then all the regulator stuff does in fact work OK; I had previously thought it didn't, but I believe that was because of (a) and (b) above, not the .dts regulator patch. However, because the regulators are now registered from DT first, and the regulator names in the DT are different to the regulator names in board-harmony-power.c, Laxman's .dts regulator patch does break PCIe on Harmony, since it can't obtain the regulators it needs. Overall, I think the solution is as follows: 1) Remove b93fffb "regulator: tps6586x: add support for SYS rail" from the regulator tree entirely. 2) Fix that patch to s/vdd_sys/REG-SYS/ in board-harmony-power.c, and apply it to the Tegra tree. 3) Fix Laxman's regulator .dts patch to solve the PCIe problem, possibly also remove board-harmony-power.c while we're at it, and apply it to the Tegra tree. If it makes life easier, I'm quite happy to move 4c79c8d "regulator: tps6586x: Convert to regulator_[enable|disable|is_enabled|get_voltage_sel]_regmap" to the Tegra tree too. This might be useful; I know that Laxman has another patch for the TPS6586x driver queued up to move the regulator-specific DT parsing from the MFD driver into the regulator driver. I have no idea, but it seems quite likely that would conflict with this patch, and perhaps make everything easier to go through one tree? An alternative would be: 1) Remove b93fffb "regulator: tps6586x: add support for SYS rail" from the regulator tree since it causes a regression. 2) Fix that patch to s/vdd_sys/REG-SYS/ in board-harmony-power.c, and re-apply it to its own branch in the regulator tree. It would be nice to base this branch on v3.6-rc3 so Harmony regulators work before the patch, so the patch can be checked for regressions. 3) Merge that branch into both regulator and Tegra. 4) Apply all future TPS6586x patches to the regulator tree. 5) Apply Laxman's patch to add Harmony regulators to the device tree to the Tegra tree, after fixing the PCIe interaction issue. Let me know which way you want to go. Sorry this driver is causing pain. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/tegra20-harmony.dts b/arch/arm/boot/dts/tegra20-harmony.dts index f146dbf..0add02b 100644 --- a/arch/arm/boot/dts/tegra20-harmony.dts +++ b/arch/arm/boot/dts/tegra20-harmony.dts @@ -275,6 +275,152 @@ i2c@7000d000 { status = "okay"; clock-frequency = <400000>; + + pmic: tps6586x@34 { + compatible = "ti,tps6586x"; + reg = <0x34>; + interrupts = <0 86 0x4>; + + #gpio-cells = <2>; + gpio-controller; + + sys-supply = <&vdd_5v0_reg>; + vin-sm0-supply = <&sys_reg>; + vin-sm1-supply = <&sys_reg>; + vin-sm2-supply = <&sys_reg>; + vinldo01-supply = <&sm2_reg>; + vinldo23-supply = <&sm2_reg>; + vinldo4-supply = <&sm2_reg>; + vinldo678-supply = <&sm2_reg>; + vinldo9-supply = <&sm2_reg>; + + regulators { + #address-cells = <1>; + #size-cells = <0>; + + sys_reg: regulator@0 { + reg = <0>; + regulator-compatible = "sys"; + regulator-name = "vdd_sys"; + regulator-always-on; + }; + + regulator@1 { + reg = <1>; + regulator-compatible = "sm0"; + regulator-name = "vdd_sm0,vdd_core"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + }; + + regulator@2 { + reg = <2>; + regulator-compatible = "sm1"; + regulator-name = "vdd_sm1,vdd_cpu"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + }; + + sm2_reg: regulator@3 { + reg = <3>; + regulator-compatible = "sm2"; + regulator-name = "vdd_sm2,vin_ldo*"; + regulator-min-microvolt = <3700000>; + regulator-max-microvolt = <3700000>; + regulator-always-on; + }; + + /* LDO0 is not connected to anything */ + + regulator@5 { + reg = <5>; + regulator-compatible = "ldo1"; + regulator-name = "vdd_ldo1,avdd_pll*"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + regulator-always-on; + }; + + regulator@6 { + reg = <6>; + regulator-compatible = "ldo2"; + regulator-name = "vdd_ldo2,vdd_rtc"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + + regulator@7 { + reg = <7>; + regulator-compatible = "ldo3"; + regulator-name = "vdd_ldo3,avdd_usb*"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + + regulator@8 { + reg = <8>; + regulator-compatible = "ldo4"; + regulator-name = "vdd_ldo4,avdd_osc,vddio_sys"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + }; + + regulator@9 { + reg = <9>; + regulator-compatible = "ldo5"; + regulator-name = "vdd_ldo5,vcore_mmc"; + regulator-min-microvolt = <2850000>; + regulator-max-microvolt = <2850000>; + regulator-always-on; + }; + + regulator@10 { + reg = <10>; + regulator-compatible = "ldo6"; + regulator-name = "vdd_ldo6,avdd_vdac"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + regulator@11 { + reg = <11>; + regulator-compatible = "ldo7"; + regulator-name = "vdd_ldo7,avdd_hdmi,vdd_fuse"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + }; + + regulator@12 { + reg = <12>; + regulator-compatible = "ldo8"; + regulator-name = "vdd_ldo8,avdd_hdmi_pll"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + regulator@13 { + reg = <13>; + regulator-compatible = "ldo9"; + regulator-name = "vdd_ldo9,avdd_2v85,vdd_ddr_rx"; + regulator-min-microvolt = <2850000>; + regulator-max-microvolt = <2850000>; + regulator-always-on; + }; + + regulator@14 { + reg = <14>; + regulator-compatible = "ldo_rtc"; + regulator-name = "vdd_rtc_out,vdd_cell"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + }; + }; }; pmc { @@ -310,6 +456,70 @@ bus-width = <8>; }; + regulators { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <0>; + + vdd_5v0_reg: regulator@0 { + compatible = "regulator-fixed"; + reg = <0>; + regulator-name = "vdd_5v0"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + regulator-always-on; + }; + + regulator@1 { + compatible = "regulator-fixed"; + reg = <1>; + regulator-name = "vdd_1v5"; + regulator-min-microvolt = <1500000>; + regulator-max-microvolt = <1500000>; + gpio = <&pmic 0 0>; + }; + + regulator@2 { + compatible = "regulator-fixed"; + reg = <2>; + regulator-name = "vdd_1v2"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + gpio = <&pmic 1 0>; + enable-active-high; + }; + + regulator@3 { + compatible = "regulator-fixed"; + reg = <3>; + regulator-name = "vdd_1v05"; + regulator-min-microvolt = <10500000>; + regulator-max-microvolt = <10500000>; + gpio = <&pmic 2 0>; + enable-active-high; + }; + + regulator@4 { + compatible = "regulator-fixed"; + reg = <4>; + regulator-name = "vdd_pnl"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + gpio = <&gpio 22 0>; /* gpio PC6 */ + enable-active-high; + }; + + regulator@5 { + compatible = "regulator-fixed"; + reg = <5>; + regulator-name = "vdd_bl"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + gpio = <&gpio 176 0>; /* gpio PW0 */ + enable-active-high; + }; + }; + sound { compatible = "nvidia,tegra-audio-wm8903-harmony", "nvidia,tegra-audio-wm8903";
Harmony uses a TPS6586x regulator. Instantiate this, and hook up a couple of fixed GPIO-controlled regulators too. Based on Ventana regulator patch by Stephen Warren <swarren@nvidia.com> and converted to Harmony. Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- arch/arm/boot/dts/tegra20-harmony.dts | 210 +++++++++++++++++++++++++++++++++ 1 files changed, 210 insertions(+), 0 deletions(-)