Thanks Stephen for your comments, incorporated them.
From: Poonam Aggrwal <[email protected]>
This patch makes necessary changes in the QE and UCC framework to support
TDM. It also adds support to configure the BRG properly through device
tree entries. Includes the device tree changes for UCC TDM driver as well.
It also includes device tree entries for UCC TDM driver.
Tested on MPC8323ERDB platform.
Signed-off-by: Poonam Aggrwal <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
Signed-off-by: Kim Phillips <[email protected]>
Signed-off-by: Michael Barkowski <[email protected]>
---
arch/powerpc/boot/dts/mpc832x_rdb.dts | 58 +++++++
arch/powerpc/sysdev/qe_lib/qe.c | 184 +++++++++++++++++++++--
arch/powerpc/sysdev/qe_lib/ucc.c | 265 +++++++++++++++++++++++++++++++++
arch/powerpc/sysdev/qe_lib/ucc_fast.c | 37 +++++
include/asm-powerpc/qe.h | 8 +
include/asm-powerpc/ucc.h | 4 +
include/asm-powerpc/ucc_fast.h | 4 +
7 files changed, 548 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/boot/dts/mpc832x_rdb.dts
index 388c8a7..c0e6283 100644
--- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
@@ -105,6 +105,17 @@
device_type = "par_io";
num-ports = <7>;
+ ucc1pio:ucc_pin@01 {
+ pio-map = <
+ /* port pin dir open_drain assignment has_irq */
+ 0 e 2 0 1 0 /* CLK11 */
+ 3 16 1 0 2 0 /* BRG9 */
+ 3 1b 1 0 2 0 /* BRG3 */
+ 0 0 3 0 2 0 /* TDMATxD0 */
+ 0 4 3 0 2 0 /* TDMARxD0 */
+ 3 1b 2 0 1 0>; /* CLK1 */
+ };
+
ucc2pio:ucc_pin@02 {
pio-map = <
/* port pin dir open_drain assignment has_irq */
@@ -169,6 +180,36 @@
};
};
+ clocks {
+ compatible = "fsl,cpm-clocks";
+ /* clock freqs in Hz(for CLK1~CLK24).
+ * CLK11 is 1024KHz,
+ * all other clocks unused
+ * #clock-cells define number of cells
+ * used by the clock-frequency.
+ * right now only #clock cells=1 is
+ * implemented. Provision is there to
+ * handle frequencies >4Gig
+ */
+ #clock-cells = <1>;
+ clock-frequency = <0 0 0 0 0 0
+ 0 0 0 0 d#1024000 0
+ 0 0 0 0 0 0
+ 0 0 0 0 0 0>;
+ };
+
+ brg@640 {
+ compatible = "fsl,cpm-brg";
+ /* input clock sources for all the 16 BRGs.
+ * 1-24 for CLK1 to CLK24.
+ * BRG9 uses CLK11,BRG1 and BRG2-8 use
+ * the QE clock.
+ */
+ fsl,brg-sources = <0 0 0 0 0 0 0 0
+ b 0 0 0 0 0 0 0>;
+ reg = <640 7f>;
+ };
+
spi@4c0 {
device_type = "spi";
compatible = "fsl_spi";
@@ -187,6 +228,23 @@
mode = "cpu";
};
+ ucc@2000 {
+ device_type = "tdm";
+ compatible = "fsl,ucc-tdm";
+ model = "UCC";
+ device-id = <1>;
+ fsl,tdm-num = <1>;
+ fsl,si-num = <1>;
+ fsl,tdm-tx-clk = "CLK1";
+ fsl,tdm-rx-clk = "CLK1";
+ fsl,tdm-tx-sync = "BRG9";
+ fsl,tdm-rx-sync = "BRG9";
+ reg = <2000 200>;
+ interrupts = <20>;
+ interrupt-parent = <&qeic>;
+ pio-handle = <&ucc1pio>;
+ };
+
ucc@3000 {
device_type = "network";
compatible = "ucc_geth";
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index 1df3b4a..9b9971d 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -149,20 +149,170 @@ EXPORT_SYMBOL(qe_issue_cmd);
*/
static unsigned int brg_clk = 0;
-unsigned int get_brg_clk(void)
+u32 get_brg_clk(enum qe_clock brgclk, enum qe_clock *brg_source)
{
- struct device_node *qe;
- if (brg_clk)
- return brg_clk;
+ struct device_node *qe, *brg, *clocks;
+ enum qe_clock brg_src;
+ u32 brg_input_freq = 0;
+ u32 brg_num;
+ int ret;
+ const unsigned int *prop;
- qe = of_find_node_by_type(NULL, "qe");
- if (qe) {
+ *brg_source = 0;
+
+ brg_num = brgclk - QE_BRG1;
+ brg = of_find_compatible_node(NULL, NULL, "fsl,cpm-brg");
+ if (!brg) {
+ printk(KERN_ERR "%s: no brg node in device tree\n",
+ __FUNCTION__);
+ return -EINVAL;
+ }
+ unsigned int size;
+ prop = of_get_property(brg, "fsl,brg-sources", &size);
+ of_node_put(brg);
+
+ if (!prop) {
+ printk(KERN_ERR "%s: invalid fsl,brg-sources in device tree \n"
+ , __FUNCTION__);
+ return -EINVAL;
+ }
+ brg_src = *(prop + brg_num);
+ if (brg_src == 0) {
+ *brg_source = 0;
+ if (brg_clk > 0)
+ return brg_clk;
+ qe = of_find_node_by_type(NULL, "qe");
+ if (!qe) {
+ printk(KERN_ERR "%s: no qe node in device tree \n"
+ , __FUNCTION__);
+ ret = EINVAL;
+ goto err;
+ }
unsigned int size;
- const u32 *prop = of_get_property(qe, "brg-frequency", &size);
- brg_clk = *prop;
+ prop = of_get_property(qe, "brg-frequency", &size);
+ if (!prop) {
+ printk(KERN_ERR "%s: QE brg-frequency"
+ "not present in device tree\n", __FUNCTION__);
+ ret = -EINVAL;
+ of_node_put(qe);
+ goto err;
+ }
+ if (*prop) {
+ of_node_put(qe);
+ brg_clk = *prop;
+ return *prop;
+ }
+ /*
+ * Older versions of U-Boot do not initialize
+ * the brg-frequency property, so in this case
+ * we assume the BRG frequency is half the QE
+ * bus frequency.
+ */
+ prop = of_get_property(qe, "bus-frequency", NULL);
of_node_put(qe);
- };
- return brg_clk;
+ if (!prop) {
+ printk(KERN_ERR "%s:QE bus-frequency not"
+ " present in device tree\n", __FUNCTION__);
+ ret = -EINVAL;
+ goto err;
+ }
+ if (*prop) {
+ brg_clk = *prop / 2;
+ return brg_clk;
+ }
+ printk(KERN_ERR "%s: invalid QE bus-frequency in device tree\n",
+ __FUNCTION__);
+ ret = -EINVAL;
+ goto err;
+ } else {
+ *brg_source = brg_src + QE_CLK1 - 1;
+ clocks = of_find_compatible_node(NULL, NULL, "fsl,cpm-clocks");
+ if (!clocks) {
+ printk(KERN_ERR "%s: no clocks node in device tree \n",
+ __FUNCTION__);
+ ret = -EINVAL;
+ goto err;
+ }
+ prop = of_get_property(clocks, "#clock-cells", &size);
+ /*
+ * clock-cells = 1 only supported right now.
+ */
+ if (!prop || *prop != 1) {
+ printk(KERN_ERR "%s: invalid #clock-cells value"
+ "in device tree \n", __FUNCTION__);
+ of_node_put(clocks);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ prop = of_get_property(clocks, "clock-frequency", &size);
+ if (!prop) {
+ printk(KERN_ERR "%s:no #clock-frequency prop in"
+ "device tree\n", __FUNCTION__);
+ of_node_put(clocks);
+ ret = -EINVAL;
+ goto err;
+ }
+ brg_input_freq = *(prop+(brg_src - 1));
+ of_node_put(clocks);
+ return brg_input_freq;
+ }
+err:
+ return ret;
+}
+
+u32 qe_brg_src(int brg_num, enum qe_clock brg_src)
+{
+ u32 clock_bits, shift;
+
+ clock_bits = 0;
+
+ switch (brg_num) {
+ case 1:
+ case 2:
+ case 5:
+ case 6:
+ switch (brg_src) {
+ case QE_CLK3: clock_bits = 1; break;
+ case QE_CLK5: clock_bits = 2; break;
+ default: break;
+ }
+ break;
+ case 3:
+ case 4:
+ case 7:
+ case 8:
+ switch (brg_src) {
+ case QE_CLK9: clock_bits = 1; break;
+ case QE_CLK15: clock_bits = 2; break;
+ default: break;
+ }
+ break;
+ case 9:
+ case 10:
+ switch (brg_src) {
+ case QE_CLK11: clock_bits = 1; break;
+ default: break;
+ }
+ break;
+ case 11:
+ case 15:
+ case 16:
+ switch (brg_src) {
+ case QE_CLK13: clock_bits = 1; break;
+ default: break;
+ }
+ break;
+ default: clock_bits = 0; break;
+ }
+ shift = 14;
+
+ if (!clock_bits)
+ return -ENOENT;
+
+ clock_bits <<= shift;
+
+ return clock_bits;
}
/* Program the BRG to the given sampling rate and multiplier
@@ -177,11 +327,18 @@ int qe_setbrg(enum qe_clock brg, unsigned int rate, unsigned int multiplier)
{
u32 divisor, tempval;
u32 div16 = 0;
+ u32 brg_clock;
+ enum qe_clock brgsrc;
+ u32 src_bits = 0;
if ((brg < QE_BRG1) || (brg > QE_BRG16))
return -EINVAL;
- divisor = get_brg_clk() / (rate * multiplier);
+ brg_clock = get_brg_clk(brg, &brgsrc);
+ if (brg_clock < 0)
+ return -EINVAL;
+
+ divisor = brg_clock / (rate * multiplier);
if (divisor > QE_BRGC_DIVISOR_MAX + 1) {
div16 = QE_BRGC_DIV16;
@@ -194,8 +351,11 @@ int qe_setbrg(enum qe_clock brg, unsigned int rate, unsigned int multiplier)
if (!div16 && (divisor & 1))
divisor++;
+ if (brgsrc > 0)
+ src_bits = qe_brg_src(brg - QE_BRG1 + 1, brgsrc);
+
tempval = ((divisor - 1) << QE_BRGC_DIVISOR_SHIFT) |
- QE_BRGC_ENABLE | div16;
+ QE_BRGC_ENABLE | div16 | src_bits;
out_be32(&qe_immr->brg.brgc[brg - QE_BRG1], tempval);
diff --git a/arch/powerpc/sysdev/qe_lib/ucc.c b/arch/powerpc/sysdev/qe_lib/ucc.c
index 0e348d9..f2de0ed 100644
--- a/arch/powerpc/sysdev/qe_lib/ucc.c
+++ b/arch/powerpc/sysdev/qe_lib/ucc.c
@@ -213,3 +213,268 @@ int ucc_set_qe_mux_rxtx(unsigned int ucc_num, enum qe_clock clock,
return 0;
}
+
+int ucc_set_tdm_rxtx_clk(int tdm_num, char *clk_src, enum comm_dir mode)
+{
+ enum qe_clock clock;
+ u32 clock_bits, shift;
+ struct qe_mux *qe_mux_reg = NULL;
+
+ clock_bits = 0;
+ qe_mux_reg = &qe_immr->qmx;
+
+ if ((tdm_num > 3 || tdm_num < 0))
+ return -EINVAL;
+
+ /* The communications direction must be RX or TX */
+ if (!((mode == COMM_DIR_RX) || (mode == COMM_DIR_TX)))
+ return -EINVAL;
+
+ clock = qe_clock_source(clk_src);
+ switch (mode) {
+ case COMM_DIR_RX:
+ switch (tdm_num) {
+ case 0:
+ switch (clock) {
+ case QE_BRG3: clock_bits = 1; break;
+ case QE_BRG4: clock_bits = 2; break;
+ case QE_CLK1: clock_bits = 4; break;
+ case QE_CLK2: clock_bits = 5; break;
+ case QE_CLK3: clock_bits = 6; break;
+ case QE_CLK8: clock_bits = 7; break;
+ default: break;
+ }
+ shift = 28;
+ break;
+ case 1:
+ switch (clock) {
+ case QE_BRG3: clock_bits = 1; break;
+ case QE_BRG4: clock_bits = 2; break;
+ case QE_CLK1: clock_bits = 4; break;
+ case QE_CLK2: clock_bits = 5; break;
+ case QE_CLK5: clock_bits = 6; break;
+ case QE_CLK10: clock_bits = 7; break;
+ default: break;
+ }
+ shift = 24;
+ break;
+ case 2:
+ switch (clock) {
+ case QE_BRG3: clock_bits = 1; break;
+ case QE_BRG4: clock_bits = 2; break;
+ case QE_CLK1: clock_bits = 4; break;
+ case QE_CLK2: clock_bits = 5; break;
+ case QE_CLK7: clock_bits = 6; break;
+ case QE_CLK12: clock_bits = 7; break;
+ default: break;
+ }
+ shift = 20;
+ break;
+ case 3:
+ switch (clock) {
+ case QE_BRG3: clock_bits = 1; break;
+ case QE_BRG4: clock_bits = 2; break;
+ case QE_CLK1: clock_bits = 4; break;
+ case QE_CLK2: clock_bits = 5; break;
+ case QE_CLK9: clock_bits = 6; break;
+ case QE_CLK14: clock_bits = 7; break;
+ default: break;
+ }
+ shift = 16;
+ break;
+ default:
+ break;
+ }
+ break;
+ case COMM_DIR_TX:
+ switch (tdm_num) {
+ case 0:
+ switch (clock) {
+ case QE_BRG3: clock_bits = 1; break;
+ case QE_BRG4: clock_bits = 2; break;
+ case QE_CLK1: clock_bits = 4; break;
+ case QE_CLK2: clock_bits = 5; break;
+ case QE_CLK4: clock_bits = 6; break;
+ case QE_CLK9: clock_bits = 7; break;
+ default: break;
+ }
+ shift = 12;
+ break;
+ case 1:
+ switch (clock) {
+ case QE_BRG3: clock_bits = 1; break;
+ case QE_BRG4: clock_bits = 2; break;
+ case QE_CLK1: clock_bits = 4; break;
+ case QE_CLK2: clock_bits = 5; break;
+ case QE_CLK6: clock_bits = 6; break;
+ case QE_CLK11: clock_bits = 7; break;
+ default: break;
+ }
+ shift = 8;
+ break;
+ case 2:
+ switch (clock) {
+ case QE_BRG3: clock_bits = 1; break;
+ case QE_BRG4: clock_bits = 2; break;
+ case QE_CLK1: clock_bits = 4; break;
+ case QE_CLK2: clock_bits = 5; break;
+ case QE_CLK8: clock_bits = 6; break;
+ case QE_CLK13: clock_bits = 7; break;
+ default: break;
+ }
+ shift = 4;
+ break;
+ case 3:
+ switch (clock) {
+ case QE_BRG3: clock_bits = 1; break;
+ case QE_BRG4: clock_bits = 2; break;
+ case QE_CLK1: clock_bits = 4; break;
+ case QE_CLK2: clock_bits = 5; break;
+ case QE_CLK10: clock_bits = 6; break;
+ case QE_CLK15: clock_bits = 7; break;
+ default: break;
+ }
+ shift = 0;
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ if (!clock_bits)
+ return -ENOENT;
+
+ clock_bits <<= shift;
+
+ qe_mux_reg->cmxsi1cr_l |= clock_bits;
+
+ return 0;
+}
+
+int ucc_set_tdm_rxtx_sync(int tdm_num, char *sync_src, enum comm_dir mode)
+{
+ enum qe_clock clock;
+ u32 shift, clock_bits;
+ struct qe_mux *qe_mux_reg = NULL;
+ int source;
+
+ source = -1;
+ qe_mux_reg = &qe_immr->qmx;
+
+ if ((tdm_num > 3 || tdm_num < 0))
+ return -EINVAL;
+
+ /* The communications direction must be RX or TX */
+ if (!((mode == COMM_DIR_RX) || (mode == COMM_DIR_TX)))
+ return -EINVAL;
+
+ switch (mode) {
+ case COMM_DIR_RX:
+ if (strcasecmp("RSYNC", sync_src) == 0) {
+ source = 0;
+ shift = 0;
+ break;
+ }
+ clock = qe_clock_source(sync_src);
+ switch (tdm_num) {
+ case 0:
+ switch (clock) {
+ case QE_BRG9: source = 1; break;
+ case QE_BRG10: source = 2; break;
+ default: source = -1; break;
+ }
+ shift = 30;
+ break;
+ case 1:
+ switch (clock) {
+ case QE_BRG9: source = 1; break;
+ case QE_BRG10: source = 2; break;
+ default: source = -1; break;
+ }
+ shift = 28;
+ break;
+ case 2:
+ switch (clock) {
+ case QE_BRG9: source = 1; break;
+ case QE_BRG11: source = 2; break;
+ default: source = -1; break;
+ }
+ shift = 26;
+ break;
+ case 3:
+ switch (clock) {
+ case QE_BRG9: source = 1; break;
+ case QE_BRG11: source = 2; break;
+ default: source = -1; break;
+ }
+ shift = 24;
+ break;
+ default:
+ source = -1;
+ break;
+ }
+ break;
+ case COMM_DIR_TX:
+ if (strcasecmp("TSYNC", sync_src) == 0) {
+ source = 0;
+ shift = 0;
+ break;
+ }
+ clock = qe_clock_source(sync_src);
+ switch (tdm_num) {
+ case 0:
+ switch (clock) {
+ case QE_BRG9: source = 1; break;
+ case QE_BRG10: source = 2; break;
+ default: source = -1; break;
+ }
+ shift = 14;
+ break;
+ case 1:
+ switch (clock) {
+ case QE_BRG9: source = 1; break;
+ case QE_BRG10: source = 2; break;
+ default: source = -1; break;
+ }
+ shift = 12;
+ break;
+ case 2:
+ switch (clock) {
+ case QE_BRG9: source = 1; break;
+ case QE_BRG11: source = 2; break;
+ default: source = -1; break;
+ }
+ shift = 10;
+ break;
+ case 3:
+ switch (clock) {
+ case QE_BRG9: source = 1; break;
+ case QE_BRG11: source = 2; break;
+ default: source = -1; break;
+ }
+ shift = 8;
+ break;
+ default:
+ source = -1;
+ break;
+ }
+ break;
+ default:
+ source = -1;
+ break;
+ }
+
+ if (source == -1)
+ return -ENOENT;
+
+ clock_bits = (u32) source;
+ clock_bits <<= shift;
+
+
+ qe_mux_reg->cmxsi1syr |= clock_bits;
+
+ return 0;
+}
diff --git a/arch/powerpc/sysdev/qe_lib/ucc_fast.c b/arch/powerpc/sysdev/qe_lib/ucc_fast.c
index 3223acb..9c8559f 100644
--- a/arch/powerpc/sysdev/qe_lib/ucc_fast.c
+++ b/arch/powerpc/sysdev/qe_lib/ucc_fast.c
@@ -327,6 +327,43 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct ucc_fast_private ** ucc
ucc_fast_free(uccf);
return -EINVAL;
}
+ } else {
+ /* TDM Rx clock routing */
+ if ((uf_info->tdm_rx_clk != NULL) &&
+ ucc_set_tdm_rxtx_clk(uf_info->ucc_num,
+ uf_info->tdm_rx_clk, COMM_DIR_RX)) {
+ printk(KERN_ERR "%s: illegal value for TDM RX clock",
+ __FUNCTION__);
+ ucc_fast_free(uccf);
+ return -EINVAL;
+ }
+ /* TDM Tx clock routing */
+ if ((uf_info->tdm_tx_clk != NULL) &&
+ ucc_set_tdm_rxtx_clk(uf_info->ucc_num,
+ uf_info->tdm_tx_clk, COMM_DIR_TX)) {
+ printk(KERN_ERR "%s: illegal value for TDM TX clock",
+ __FUNCTION__);
+ ucc_fast_free(uccf);
+ return -EINVAL;
+ }
+ /* TDM Rx sync routing */
+ if ((uf_info->tdm_rx_sync != NULL) &&
+ ucc_set_tdm_rxtx_sync(uf_info->ucc_num,
+ uf_info->tdm_rx_sync, COMM_DIR_RX)) {
+ printk(KERN_ERR "%s: illegal value for TDM RX"
+ "Frame sync", __FUNCTION__);
+ ucc_fast_free(uccf);
+ return -EINVAL;
+ }
+ /* TDM Tx sync routing */
+ if ((uf_info->tdm_tx_sync != NULL) &&
+ ucc_set_tdm_rxtx_sync(uf_info->ucc_num,
+ uf_info->tdm_tx_sync, COMM_DIR_TX)) {
+ printk(KERN_ERR "%s: illegal value for TDM TX"
+ "Frame sync", __FUNCTION__);
+ ucc_fast_free(uccf);
+ return -EINVAL;
+ }
}
/* Set interrupt mask register at UCC level. */
diff --git a/include/asm-powerpc/qe.h b/include/asm-powerpc/qe.h
index bcf60be..51de236 100644
--- a/include/asm-powerpc/qe.h
+++ b/include/asm-powerpc/qe.h
@@ -497,6 +497,14 @@ struct ucc_slow_pram {
#define UCC_GETH_UCCE_RXF1 0x00000002
#define UCC_GETH_UCCE_RXF0 0x00000001
+/* Transparent UCC Event Register (UCCE) */
+#define UCC_TRANS_UCCE_GRA 0x0080
+#define UCC_TRANS_UCCE_TXE 0x0010
+#define UCC_TRANS_UCCE_RXF 0x0008
+#define UCC_TRANS_UCCE_BSY 0x0004
+#define UCC_TRANS_UCCE_TXB 0x0002
+#define UCC_TRANS_UCCE_RXB 0x0001
+
/* UPSMR, when used as a UART */
#define UCC_UART_UPSMR_FLC 0x8000
#define UCC_UART_UPSMR_SL 0x4000
diff --git a/include/asm-powerpc/ucc.h b/include/asm-powerpc/ucc.h
index 46b09ba..153db97 100644
--- a/include/asm-powerpc/ucc.h
+++ b/include/asm-powerpc/ucc.h
@@ -42,6 +42,10 @@ int ucc_set_qe_mux_mii_mng(unsigned int ucc_num);
int ucc_set_qe_mux_rxtx(unsigned int ucc_num, enum qe_clock clock,
enum comm_dir mode);
+int ucc_set_tdm_rxtx_clk(int tdm_num, char *clk_src, enum comm_dir mode);
+
+int ucc_set_tdm_rxtx_sync(int tdm_num, char *clk_src, enum comm_dir mode);
+
int ucc_mux_set_grant_tsa_bkpt(unsigned int ucc_num, int set, u32 mask);
/* QE MUX clock routing for UCC
diff --git a/include/asm-powerpc/ucc_fast.h b/include/asm-powerpc/ucc_fast.h
index f529f70..cf35553 100644
--- a/include/asm-powerpc/ucc_fast.h
+++ b/include/asm-powerpc/ucc_fast.h
@@ -152,6 +152,10 @@ struct ucc_fast_info {
enum ucc_fast_rx_decoding_method renc;
enum ucc_fast_transparent_tcrc tcrc;
enum ucc_fast_sync_len synl;
+ const char *tdm_rx_clk;
+ const char *tdm_tx_clk;
+ const char *tdm_rx_sync;
+ const char *tdm_tx_sync;
};
struct ucc_fast_private {
--
1.5.2.4
Hello Poonam,
On Thu, Jan 24, 2008 at 04:00:06PM +0530, Poonam_Aggrwal-b10812 wrote:
> Thanks Stephen for your comments, incorporated them.
> From: Poonam Aggrwal <[email protected]>
>
> This patch makes necessary changes in the QE and UCC framework to support
> TDM. It also adds support to configure the BRG properly through device
> tree entries. Includes the device tree changes for UCC TDM driver as well.
> It also includes device tree entries for UCC TDM driver.
>
> Tested on MPC8323ERDB platform.
>
> Signed-off-by: Poonam Aggrwal <[email protected]>
> Signed-off-by: Ashish Kalra <[email protected]>
> Signed-off-by: Kim Phillips <[email protected]>
> Signed-off-by: Michael Barkowski <[email protected]>
> ---
> arch/powerpc/boot/dts/mpc832x_rdb.dts | 58 +++++++
> arch/powerpc/sysdev/qe_lib/qe.c | 184 +++++++++++++++++++++--
> arch/powerpc/sysdev/qe_lib/ucc.c | 265 +++++++++++++++++++++++++++++++++
> arch/powerpc/sysdev/qe_lib/ucc_fast.c | 37 +++++
> include/asm-powerpc/qe.h | 8 +
> include/asm-powerpc/ucc.h | 4 +
> include/asm-powerpc/ucc_fast.h | 4 +
> 7 files changed, 548 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/boot/dts/mpc832x_rdb.dts
> index 388c8a7..c0e6283 100644
> --- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
> +++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
> @@ -105,6 +105,17 @@
> device_type = "par_io";
> num-ports = <7>;
>
> + ucc1pio:ucc_pin@01 {
> + pio-map = <
> + /* port pin dir open_drain assignment has_irq */
> + 0 e 2 0 1 0 /* CLK11 */
> + 3 16 1 0 2 0 /* BRG9 */
> + 3 1b 1 0 2 0 /* BRG3 */
> + 0 0 3 0 2 0 /* TDMATxD0 */
> + 0 4 3 0 2 0 /* TDMARxD0 */
> + 3 1b 2 0 1 0>; /* CLK1 */
> + };
> +
Can we not introduce new pio-maps in the device trees? There
were debates regarding this, and if I understood everything
correctly, pio-maps considered as a bad taste. Better
do bunch of par_io_config_pin() in the board file. Better
yet fixup the firmware (u-boot) to set up dedicated pins
correctly.
Thanks,
--
Anton Vorontsov
email: [email protected]
backup email: [email protected]
irc://irc.freenode.net/bd2
Anton Vorontsov wrote:
> Can we not introduce new pio-maps in the device trees? There
> were debates regarding this, and if I understood everything
> correctly, pio-maps considered as a bad taste. Better
> do bunch of par_io_config_pin() in the board file. Better
> yet fixup the firmware (u-boot) to set up dedicated pins
> correctly.
I'm on the fence with respect to pio-maps vs. par_io_config_pin() calls. The
problem is that the configuration of these pins is board-specific, but pins are
used by devices. A device driver can't call par_io_config_pin(), because the
calls are different depending on which SoC and which UCC you're using. The
platform code can't call par_io_config_pin(), because that configuration depends
on which drivers are loaded.
In other words, the pin configurations are dependent on the UCC configurations,
and the UCC configurations are stored in the device tree. So it makes sense to
put the pin configurations in the device tree, too.
--
Timur Tabi
Linux kernel developer at Freescale
On Thu, Jan 24, 2008 at 09:55:31AM -0600, Timur Tabi wrote:
> Anton Vorontsov wrote:
>
> >Can we not introduce new pio-maps in the device trees? There
> >were debates regarding this, and if I understood everything
> >correctly, pio-maps considered as a bad taste. Better
> >do bunch of par_io_config_pin() in the board file. Better
> >yet fixup the firmware (u-boot) to set up dedicated pins
> >correctly.
>
> I'm on the fence with respect to pio-maps vs. par_io_config_pin() calls.
> The problem is that the configuration of these pins is board-specific, but
> pins are used by devices. A device driver can't call par_io_config_pin(),
> because the calls are different depending on which SoC and which UCC you're
> using. The platform code can't call par_io_config_pin(), because that
> configuration depends on which drivers are loaded.
Are you saying that TDM is sharing same pins with the other QE device,
and we can choose to use/not use some device depending on which driver
is loaded? I think this particular board and patch isn't that case.
Even if someday there will be the case when drivers are mutually
exclusive, i.e. presence of some driver should trigger pins
reconfiguration, then anyway this should be handled differently.
That is, we should not _register_ two mutually exclusive devices
in the first place, so drivers will not probe them. That's board
setup code authority, and pins configuration still should happen
there.
[ Irrelevant to UCCs and this particular case: lately I've
encountered one interesting case of Par IO usage. FHCI USB needs
switching between pin's dedicated functions and GPIO functions.
So, firstly it is using pins as dedicated, and later (at the bus
reset) driver turns them to act as GPIOs. This is still handled
without pio-map though, via gpios = <> property for that driver. ]
> In other words, the pin configurations are dependent on the UCC
> configurations, and the UCC configurations are stored in the device tree.
> So it makes sense to put the pin configurations in the device tree, too.
In that particular case UCC configuration is static, for every UCC.
So, we can set up all pins in the firmware/board file.
Please correct me if I'm wrong.
--
Anton Vorontsov
email: [email protected]
backup email: [email protected]
irc://irc.freenode.net/bd2
Anton Vorontsov wrote:
> Are you saying that TDM is sharing same pins with the other QE device,
> and we can choose to use/not use some device depending on which driver
> is loaded?
No. I'd have to closely examine the DTS, but I don't think that UCC devices
share pins at all. But that isn't my point.
> In that particular case UCC configuration is static, for every UCC.
> So, we can set up all pins in the firmware/board file.
Yes, but deciding what the UCC does might not be static. At what point do we
declare, "UCC5 is for eth0 and eth0 only"?
The advantage of putting the pin configurations in the device tree is that they
now become configurable. I can envision a scenario where UCC5 could be either
an Ethernet or a UART, depending on the setting of some jumpers on the board.
That's what the QE was designed for: any UCC can do any task, and you can even
have a UCC change its purpose while the system is running. So I don't want the
pin configurations hard-coded into the kernel. Having them in the device tree
gives me some flexibility.
For instance, I have a plan (that I keep postponing) to introduce a new feature
in U-Boot where U-Boot can determine the settings of some board jumpers and
modify the device tree accordingly. The instructions on how to modify the
device tree would be embedded in the tree itself. I can't support this feature
if the kernel calls par_io_config_pin() regardless of what's in the device tree.
--
Timur Tabi
Linux kernel developer at Freescale
On Thu, Jan 24, 2008 at 10:33:47AM -0600, Timur Tabi wrote:
> Anton Vorontsov wrote:
>
> >Are you saying that TDM is sharing same pins with the other QE device,
> >and we can choose to use/not use some device depending on which driver
> >is loaded?
>
> No. I'd have to closely examine the DTS, but I don't think that UCC
> devices share pins at all. But that isn't my point.
>
> >In that particular case UCC configuration is static, for every UCC.
> >So, we can set up all pins in the firmware/board file.
>
> Yes, but deciding what the UCC does might not be static. At what point do
> we declare, "UCC5 is for eth0 and eth0 only"?
>
> The advantage of putting the pin configurations in the device tree is that
> they now become configurable. I can envision a scenario where UCC5 could
> be either an Ethernet or a UART, depending on the setting of some jumpers
> on the board. That's what the QE was designed for: any UCC can do any task,
> and you can even have a UCC change its purpose while the system is running.
> So I don't want the pin configurations hard-coded into the kernel. Having
> them in the device tree gives me some flexibility.
If hardware configuration is selected at the bootup time, by jumpers
or switches, it's even easier to do it right. Without pio-map.
> For instance, I have a plan (that I keep postponing) to introduce a new
> feature in U-Boot where U-Boot can determine the settings of some board
> jumpers and modify the device tree accordingly. The instructions on how to
> modify the device tree would be embedded in the tree itself.
Why you need to modify the device tree for that? Let the U-Boot simply
setup pins for the kernel. Regarding kernel overwriting pins
configuration...
> I can't
> support this feature if the kernel calls par_io_config_pin() regardless of
> what's in the device tree.
What I've understood from the previous debates, is that ideally kernel
should not touch pins' configuration. Today we're using pio-map solely
to fix up some old firmware misconfiguration. And we can do this in the
board file still. To determine if we need to fixup the firmware or not,
we can use some device tree property instead (firmware version?).
p.s.
I'm neither for pio-map nor against. I just want some consequence
regarding this. Last thread ended with consequence that pio-map is a
bad thing to use...
--
Anton Vorontsov
email: [email protected]
backup email: [email protected]
irc://irc.freenode.net/bd2
On Thu, Jan 24, 2008 at 10:33:47AM -0600, Timur Tabi wrote: Yes, but
> deciding what the UCC does might not be static. At what point do we
> declare, "UCC5 is for eth0 and eth0 only"?
When the board designer decides to hook eth0 up to UCC5.
If the board designer decides to hook multiple devices up to UCC5, we first
smack the board designer, and then set it up to whichever configuration has
been jumpered.
-Scott
Hello Anton/Tabi
I am not sure which is the best place to configure the pins. Because
some drivers do it in one way and some in the other.
I actually tried to make the driver similar to ucc_geth because it is a
QE driver. The driver has no platform code in the platform files similar
to ucc_geth. It is probed along with all the QE devices thorugh
of_platform_bus_probe.
And the pins are configured for all the QE devices using par_io_init.
I thought this to be the most consistent way at that time.
How should we close this point?
Can we go ahead with the pio-map?
Infact the discussion in this thread was very good and I got to know a
lot of rationales behind this.
Please suggest.
Thanks and Regards
Poonam
From: Anton Vorontsov [mailto:[email protected]]
Sent: Thursday, January 24, 2008 10:53 PM
To: Tabi Timur
Cc: Aggrwal Poonam; Gala Kumar; [email protected];
[email protected]; [email protected];
[email protected]; [email protected]; Barkowski Michael;
Cutler Richard; Kalra Ashish
Subject: Re: [PATCH UCC TDM 1/3 Updated] Platform changes for UCC TDM
driver for MPC8323eRDB. Also includes related QE changes and dts
entries.
On Thu, Jan 24, 2008 at 10:33:47AM -0600, Timur Tabi wrote:
> Anton Vorontsov wrote:
>
> >Are you saying that TDM is sharing same pins with the other QE
> >device, and we can choose to use/not use some device depending on
> >which driver is loaded?
>
> No. I'd have to closely examine the DTS, but I don't think that UCC
> devices share pins at all. But that isn't my point.
>
> >In that particular case UCC configuration is static, for every UCC.
> >So, we can set up all pins in the firmware/board file.
>
> Yes, but deciding what the UCC does might not be static. At what
> point do we declare, "UCC5 is for eth0 and eth0 only"?
>
> The advantage of putting the pin configurations in the device tree is
> that they now become configurable. I can envision a scenario where
> UCC5 could be either an Ethernet or a UART, depending on the setting
> of some jumpers on the board. That's what the QE was designed for: any
> UCC can do any task, and you can even have a UCC change its purpose
while the system is running.
> So I don't want the pin configurations hard-coded into the kernel.
> Having them in the device tree gives me some flexibility.
If hardware configuration is selected at the bootup time, by jumpers or
switches, it's even easier to do it right. Without pio-map.
> For instance, I have a plan (that I keep postponing) to introduce a
> new feature in U-Boot where U-Boot can determine the settings of some
> board jumpers and modify the device tree accordingly. The instructions
> on how to modify the device tree would be embedded in the tree itself.
Why you need to modify the device tree for that? Let the U-Boot simply
setup pins for the kernel. Regarding kernel overwriting pins
configuration...
> I can't
> support this feature if the kernel calls par_io_config_pin()
> regardless of what's in the device tree.
What I've understood from the previous debates, is that ideally kernel
should not touch pins' configuration. Today we're using pio-map solely
to fix up some old firmware misconfiguration. And we can do this in the
board file still. To determine if we need to fixup the firmware or not,
we can use some device tree property instead (firmware version?).
p.s.
I'm neither for pio-map nor against. I just want some consequence
regarding this. Last thread ended with consequence that pio-map is a bad
thing to use...
--
Anton Vorontsov
email: [email protected]
backup email: [email protected]
irc://irc.freenode.net/bd2