Skip to contentSkip to conversation

tegra: pinmux: fix FUNCMUX_NDFLASH_KBC_8_BIT

PATCH New Marcel Ziswiler 2 participants 5 messages Mar 24, 2015 – Mar 25, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tegra: pinmux: fix FUNCMUX_NDFLASH_KBC_8_BIT

From: Lucas Stach <[email protected]>

Even the 8-bit case needs KBCB configured, as pin D7 is located in this pingroup. Also pingroup ATC seems to come out of reset with config set to NAND, so we need to explictly configure some other function to this group in order to avoid clashing settings.

---
 arch/arm/mach-tegra/tegra20/funcmux.c | 8 ++++++++
 1 file changed, 8 insertions(+)
arch/arm/mach-tegra/tegra20/funcmux.c +8 -0
index 0df4a07..f9b6214 100644
@@ -252,17 +252,25 @@ int funcmux_select(enum periph_id id, int config)
 			break;
 		case FUNCMUX_NDFLASH_KBC_8_BIT:
 			pinmux_set_func(PMUX_PINGRP_KBCA, PMUX_FUNC_NAND);
+			pinmux_set_func(PMUX_PINGRP_KBCB, PMUX_FUNC_NAND);
 			pinmux_set_func(PMUX_PINGRP_KBCC, PMUX_FUNC_NAND);
 			pinmux_set_func(PMUX_PINGRP_KBCD, PMUX_FUNC_NAND);
 			pinmux_set_func(PMUX_PINGRP_KBCE, PMUX_FUNC_NAND);
 			pinmux_set_func(PMUX_PINGRP_KBCF, PMUX_FUNC_NAND);
 
 			pinmux_tristate_disable(PMUX_PINGRP_KBCA);
+			pinmux_tristate_disable(PMUX_PINGRP_KBCB);
 			pinmux_tristate_disable(PMUX_PINGRP_KBCC);
 			pinmux_tristate_disable(PMUX_PINGRP_KBCD);
 			pinmux_tristate_disable(PMUX_PINGRP_KBCE);
 			pinmux_tristate_disable(PMUX_PINGRP_KBCF);
 
+			/*
+			 * configure pingroup ATC to something unrelated to
+			 * avoid ATC overriding KBC
+			 */
+			pinmux_set_func(PMUX_PINGRP_ATC, PMUX_FUNC_GMI);
+
 			bad_config = 0;
 			break;
 		}
Signature · 1 line
1.9.3
#1 lore raw

PATCH 1/1 tegra: pinmux: fix FUNCMUX_NDFLASH_KBC_8_BIT

Quoted 6 lines · Marcel Ziswiler

From: Lucas Stach <[email protected]>

Even the 8-bit case needs KBCB configured, as pin D7 is located in this pingroup. Also pingroup ATC seems to come out of reset with config set to NAND, so we need to explictly configure some other function to this group in order to avoid clashing settings.

I would rather not touch ATC like this; see below.

arch/arm/mach-tegra/tegra20/funcmux.c

Quoted 1 line
arch/arm/mach-tegra/tegra20/funcmux.c +0 -0

Note that this will conflict with:

09f455dca749 ARM: tegra: collect SoC sources into mach-tegra

... which moved that file.

Quoted 1 line
arch/arm/mach-tegra/tegra20/funcmux.c +0 -0
@@ -252,17 +252,25 @@ int funcmux_select(enum periph_id id, int config)

...

Quoted 1 line

case FUNCMUX_NDFLASH_KBC_8_BIT:

...

arch/arm/mach-tegra/tegra20/funcmux.c int funcmux_select(enum periph_id id, int config) · line 272

Quoted 5 lines
arch/arm/mach-tegra/tegra20/funcmux.c +5 -0
+			/*
+			 * configure pingroup ATC to something unrelated to
+			 * avoid ATC overriding KBC
+			 */
+			pinmux_set_func(PMUX_PINGRP_ATC, PMUX_FUNC_GMI);

What if ATC is actually used for some purpose other than GMI? This will corrupt the pinmux for that pingroup, and prevent the other peripheral from working. For example, PERIPH_ID_SDMMC4/FUNCMUX_SDMMC4_ATC_ATD_8BIT actively uses ATC, and this change might break it if both are used together on one board.

The best approach is to simply not use funcmux at all, but rather program the entire pinmux from a table. That guarantees no conflicts. Boards using later SoCs take this approach.

Alternatively, have the board code (rather than funcmux) select some other value for ATC. This allows the board code to select the exact correct value for that pingroup once (thus avoiding multiple changes to the pinmux for that pingroup, which could cause glitches), and guarantees that the common code will never corrupt that setting.

#2 lore raw

PATCH 1/1 tegra: pinmux: fix FUNCMUX_NDFLASH_KBC_8_BIT

Quoted 8 lines · Stephen Warren

Review

Quoted 1 line
(quoted diff) +0 -0

b/arch/arm/mach-tegra/tegra20/funcmux.c

Note that this will conflict with:

09f455dca749 ARM: tegra: collect SoC sources into mach-tegra

... which moved that file.

OK, sorry. Haven't seen that one yet. Will rebase accordingly.

Quoted 7 lines

What if ATC is actually used for some purpose other than GMI? This will

corrupt the pinmux for that pingroup, and prevent the other peripheral from working. For example, PERIPH_ID_SDMMC4/FUNCMUX_SDMMC4_ATC_ATD_8BIT

actively uses ATC, and this change might break it if both are used together on one board.

Yes, agreed.

Quoted 3 lines

The best approach is to simply not use funcmux at all, but rather program the entire pinmux from a table. That guarantees no conflicts. Boards using later SoCs take this approach.

Yes, that one I knew but nobody does on T20 (yet) plus T20 is kind of special due to its pin groups spanning multiple pins and such.

Quoted 6 lines

Alternatively, have the board code (rather than funcmux) select some other value for ATC. This allows the board code to select the exact correct value for that pingroup once (thus avoiding multiple changes to

the pinmux for that pingroup, which could cause glitches), and guarantees that the common code will never corrupt that setting.

That one for sure would work if above table approach proves to be too cumbersome to do.

Thanks Stephen

Signature · 1 line
Sent from my Android device with K-9 Mail. Please excuse my brevity.
#3 lore raw

PATCH 1/1 tegra: pinmux: fix FUNCMUX_NDFLASH_KBC_8_BIT

In reply to Stephen Warren
Quoted 7 lines · Stephen Warren

arch/arm/mach-tegra/tegra20/funcmux.c

Quoted 1 line
arch/arm/mach-tegra/tegra20/funcmux.c +0 -0

Note that this will conflict with:

09f455dca749 ARM: tegra: collect SoC sources into mach-tegra

... which moved that file.

I had another look but believe above mentioned commit actually did the following:

arch/arm/cpu/tegra20-common/* -> arch/arm/mach-tegra/tegra20/*

And my commit was already rebased to latest master and therefore already took that into account or am I missing something here?

#4 lore raw

PATCH 1/1 tegra: pinmux: fix FUNCMUX_NDFLASH_KBC_8_BIT

Quoted 9 lines · Marcel Ziswiler
Quoted 7 lines · Stephen Warren

arch/arm/mach-tegra/tegra20/funcmux.c

Quoted 1 line
arch/arm/mach-tegra/tegra20/funcmux.c +0 -0

Note that this will conflict with:

09f455dca749 ARM: tegra: collect SoC sources into mach-tegra

... which moved that file.

Quoted 7 lines

I had another look but believe above mentioned commit actually did the following:

arch/arm/cpu/tegra20-common/* -> arch/arm/mach-tegra/tegra20/*

And my commit was already rebased to latest master and therefore already took that into account or am I missing something here?

Yes, I think I got it backwards; I was confused by the Tegra210 file having been added in the old location after the move, or something.