Skip to contentSkip to conversation

tty: serial: msm: Add mask value for UART_DM registers

PATCH In review Pramod Gurav 4 participants 10 messages Apr 10, 2015 – May 7, 2015

Subsystems (2)
Arm Qualcomm mailing list TTY layer and serial drivers
lore
Share & tools
lore

Pramod Gurav

author
#0 lore raw

PATCH v3 1/3 tty: serial: msm: Add mask value for UART_DM registers

The bit masks for RFR_LEVEL1 and STALE_TIMEOUT_MSB values in MR1 and IPR registers respectively are different for UART and UART_DM hardware cores. We have been using UART core mask values for these. Add the same for UART_DM core.

There is no bit setting as UART_IPR_RXSTALE_LAST for UART_DM core so do it only for UART core.

Signed-off-by Pramod Gurav <[email protected]>

---
Changes since v2:

- Changed the sequnce of code to avoid duplication of code.

 drivers/tty/serial/msm_serial.c | 26 ++++++++++++++++++++------
 drivers/tty/serial/msm_serial.h |  2 ++
 2 files changed, 22 insertions(+), 6 deletions(-)
drivers/tty/serial/msm_serial.c +20 -6
index b73889c..99aba04 100644
@@ -421,7 +421,7 @@ msm_find_best_baud(struct uart_port *port, unsigned int baud)
 
 static int msm_set_baud_rate(struct uart_port *port, unsigned int baud)
 {
-	unsigned int rxstale, watermark;
+	unsigned int rxstale, watermark, mask;
 	struct msm_port *msm_port = UART_TO_MSM(port);
 	const struct msm_baud_map *entry;
 

@@ -432,8 +432,15 @@ static int msm_set_baud_rate(struct uart_port *port, unsigned int baud)
 	/* RX stale watermark */
 	rxstale = entry->rxstale;
 	watermark = UART_IPR_STALE_LSB & rxstale;
-	watermark |= UART_IPR_RXSTALE_LAST;
-	watermark |= UART_IPR_STALE_TIMEOUT_MSB & (rxstale << 2);
+	if (msm_port->is_uartdm)
+		mask = UART_DM_IPR_STALE_TIMEOUT_MSB;
+	else {
+		watermark |= UART_IPR_RXSTALE_LAST;
+		mask = UART_IPR_STALE_TIMEOUT_MSB;
+	}
+
+	watermark |= mask & (rxstale << 2);
+
 	msm_write(port, watermark, UART_IPR);
 
 	/* set RX watermark */

@@ -476,7 +483,7 @@ static void msm_init_clock(struct uart_port *port)
 static int msm_startup(struct uart_port *port)
 {
 	struct msm_port *msm_port = UART_TO_MSM(port);
-	unsigned int data, rfr_level;
+	unsigned int data, rfr_level, mask;
 	int ret;
 
 	snprintf(msm_port->name, sizeof(msm_port->name),

@@ -496,11 +503,18 @@ static int msm_startup(struct uart_port *port)
 
 	/* set automatic RFR level */
 	data = msm_read(port, UART_MR1);
-	data &= ~UART_MR1_AUTO_RFR_LEVEL1;
+
+	if (msm_port->is_uartdm)
+		mask = UART_DM_MR1_AUTO_RFR_LEVEL1;
+	else
+		mask = UART_MR1_AUTO_RFR_LEVEL1;
+
+	data &= ~mask;
 	data &= ~UART_MR1_AUTO_RFR_LEVEL0;
-	data |= UART_MR1_AUTO_RFR_LEVEL1 & (rfr_level << 2);
+	data |= mask & (rfr_level << 2);
 	data |= UART_MR1_AUTO_RFR_LEVEL0 & rfr_level;
 	msm_write(port, data, UART_MR1);
+
 	return 0;
 }
drivers/tty/serial/msm_serial.h +2 -0
index 3e1c713..caf5363 100644
@@ -20,6 +20,7 @@
 
 #define UART_MR1_AUTO_RFR_LEVEL0	0x3F
 #define UART_MR1_AUTO_RFR_LEVEL1	0x3FF00
+#define UART_DM_MR1_AUTO_RFR_LEVEL1	0xFFFFFF00
 #define UART_MR1_RX_RDY_CTL    		(1 << 7)
 #define UART_MR1_CTS_CTL       		(1 << 6)
 

@@ -78,6 +79,7 @@
 #define UART_IPR_RXSTALE_LAST		0x20
 #define UART_IPR_STALE_LSB		0x1F
 #define UART_IPR_STALE_TIMEOUT_MSB	0x3FF80
+#define UART_DM_IPR_STALE_TIMEOUT_MSB	0xFFFFFF80
 
 #define UART_IPR	0x0018
 #define UART_TFWR	0x001C
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#1 lore raw

PATCH v3 2/3 tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown

clk_prepare_enable/clk_disable_unprepare is already done in .pm function of uart_ops hence get rid of them from msm_startup and msm_shutdown.

Signed-off-by Pramod Gurav <[email protected]>

---
Changes since last version:
- Decided to drop calls to clk operations

 drivers/tty/serial/msm_serial.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/tty/serial/msm_serial.c +1 -6
index 99aba04..c32b088 100644
@@ -494,8 +494,6 @@ static int msm_startup(struct uart_port *port)
 	if (unlikely(ret))
 		return ret;
 
-	msm_init_clock(port);
-
 	if (likely(port->fifosize > 12))
 		rfr_level = port->fifosize - 12;
 	else

@@ -525,8 +523,6 @@ static void msm_shutdown(struct uart_port *port)
 	msm_port->imr = 0;
 	msm_write(port, 0, UART_IMR); /* disable interrupts */
 
-	clk_disable_unprepare(msm_port->clk);
-
 	free_irq(port->irq, port);
 }
 

@@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port, unsigned int state,
 
 	switch (state) {
 	case 0:
-		clk_prepare_enable(msm_port->clk);
-		clk_prepare_enable(msm_port->pclk);
+		msm_init_clock(port);
 		break;
 	case 3:
 		clk_disable_unprepare(msm_port->clk);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#2 lore raw

Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown

drivers/tty/serial/msm_serial.c static void msm_power(struct uart_port *port, unsigned int state, · line 682

Quoted 7 lines · Pramod Gurav
drivers/tty/serial/msm_serial.c +1 -2
@@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port, unsigned int state,
 
 	switch (state) {
 	case 0:
-		clk_prepare_enable(msm_port->clk);
-		clk_prepare_enable(msm_port->pclk);
+		msm_init_clock(port);

Hm... now we would call msm_serial_set_mnd_regs() whenever we power on the port? Presumably we only need to do that once when we probe (or when we resume from a sleep state that resets the registers, i.e. hibernation) but I guess we're getting saved by the fact that the if/else if pair in msm_serial_set_mnd_regs_from_uartclk would never be true after the first time we call it?

Quoted 3 lines
drivers/tty/serial/msm_serial.c +0 -0
 		break;
 	case 3:
 		clk_disable_unprepare(msm_port->clk);
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#3 lore raw

Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown

Thanks Stephen for review.

Quoted 4 lines · Stephen Boyd
Quoted 3 lines · Pramod Gurav
(quoted diff) +0 -0
@@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port,
 unsigned int state,
Quoted 12 lines

drivers/tty/serial/msm_serial.c static void msm_power(struct uart_port *port, unsigned int state, · line 682

Quoted 5 lines
drivers/tty/serial/msm_serial.c +1 -2
 	switch (state) {
 	case 0:
-		clk_prepare_enable(msm_port->clk);
-		clk_prepare_enable(msm_port->pclk);
+		msm_init_clock(port);

Hm... now we would call msm_serial_set_mnd_regs() whenever we power on the port? Presumably we only need to do that once when we probe (or when we resume from a sleep state that resets the registers, i.e. hibernation) but I guess we're getting saved by the fact that the if/else if pair in msm_serial_set_mnd_regs_from_uartclk would never be true after the first time we call it?

I tried replacing msm_init_clock() call with msm_serial_set_mnd_regs() in msm_startup() as msm_startup gets called just after msm_power() so that clk_prepare_enable() is followed by mnd settings. But it does not get the kernel booted for some reason.

So, can I get a acked-by for this patch or you still think it can be done in a better way?

Quoted 16 lines
Quoted 4 lines
(quoted diff) +0 -0
 		break;
 	case 3:

clk_disable_unprepare(msm_port->clk);

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#4 lore raw

Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown

Quoted 18 lines · Pramod Gurav

Thanks Stephen for review.

Quoted 4 lines · Stephen Boyd
Quoted 3 lines · Pramod Gurav
(quoted diff) +0 -0
@@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port,
 unsigned int state,
Quoted 11 lines

drivers/tty/serial/msm_serial.c static void msm_power(struct uart_port *port, unsigned int state, · line 682

Quoted 5 lines
drivers/tty/serial/msm_serial.c +1 -2
 	switch (state) {
 	case 0:
-		clk_prepare_enable(msm_port->clk);
-		clk_prepare_enable(msm_port->pclk);
+		msm_init_clock(port);

Hm... now we would call msm_serial_set_mnd_regs() whenever we power on the port? Presumably we only need to do that once when we probe (or when we resume from a sleep state that resets the registers, i.e. hibernation) but I guess we're getting saved by the fact that the if/else if pair in msm_serial_set_mnd_regs_from_uartclk would never be true after the first time we call it?

Quoted 4 lines

I tried replacing msm_init_clock() call with msm_serial_set_mnd_regs() in msm_startup() as msm_startup gets called just after msm_power() so that clk_prepare_enable() is followed by mnd settings. But it does not get the kernel booted for some reason.

That's concerning. Did you also drop the call to msm_init_clock() from msm_power() as is done in this patch? If that's done then it seems that nothing would be different besides the removal of clk_prepare_enable() in msm_startup().

Quoted 3 lines

So, can I get a acked-by for this patch or you still think it can be done in a better way?

Using msm_init_clock() in the msm_power() doesn't look symmetrical so if we can avoid it I would prefer that.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#5 lore raw

PATCH v3 3/3 tty: serial: msm: replaces (1 << x) with BIT(x) macro

In reply to Pramod Gurav

Replaces (1 << x) with BIT(x) macro

Signed-off-by Pramod Gurav <[email protected]>

---
This is a new patch added in this series.

 drivers/tty/serial/msm_serial.h | 58 ++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 29 deletions(-)
drivers/tty/serial/msm_serial.h +29 -29
index caf5363..f663822 100644
@@ -21,11 +21,11 @@
 #define UART_MR1_AUTO_RFR_LEVEL0	0x3F
 #define UART_MR1_AUTO_RFR_LEVEL1	0x3FF00
 #define UART_DM_MR1_AUTO_RFR_LEVEL1	0xFFFFFF00
-#define UART_MR1_RX_RDY_CTL    		(1 << 7)
-#define UART_MR1_CTS_CTL       		(1 << 6)
+#define UART_MR1_RX_RDY_CTL		BIT(7)
+#define UART_MR1_CTS_CTL		BIT(6)
 
 #define UART_MR2			0x0004
-#define UART_MR2_ERROR_MODE		(1 << 6)
+#define UART_MR2_ERROR_MODE		BIT(6)
 #define UART_MR2_BITS_PER_CHAR		0x30
 #define UART_MR2_BITS_PER_CHAR_5	(0x0 << 4)
 #define UART_MR2_BITS_PER_CHAR_6	(0x1 << 4)

@@ -41,8 +41,8 @@
 
 #define UART_CSR			0x0008
 
-#define UART_TF		0x000C
-#define UARTDM_TF	0x0070
+#define UART_TF				0x000C
+#define UARTDM_TF			0x0070
 
 #define UART_CR				0x0010
 #define UART_CR_CMD_NULL		(0 << 4)

@@ -62,29 +62,29 @@
 #define UART_CR_CMD_STALE_EVENT_ENABLE	(80 << 4)
 #define UART_CR_CMD_FORCE_STALE		(4 << 8)
 #define UART_CR_CMD_RESET_TX_READY	(3 << 8)
-#define UART_CR_TX_DISABLE		(1 << 3)
-#define UART_CR_TX_ENABLE		(1 << 2)
-#define UART_CR_RX_DISABLE		(1 << 1)
-#define UART_CR_RX_ENABLE		(1 << 0)
+#define UART_CR_TX_DISABLE		BIT(3)
+#define UART_CR_TX_ENABLE		BIT(2)
+#define UART_CR_RX_DISABLE		BIT(1)
+#define UART_CR_RX_ENABLE		BIT(0)
 #define UART_CR_CMD_RESET_RXBREAK_START	((1 << 11) | (2 << 4))
 
-#define UART_IMR		0x0014
-#define UART_IMR_TXLEV		(1 << 0)
-#define UART_IMR_RXSTALE	(1 << 3)
-#define UART_IMR_RXLEV		(1 << 4)
-#define UART_IMR_DELTA_CTS	(1 << 5)
-#define UART_IMR_CURRENT_CTS	(1 << 6)
-#define UART_IMR_RXBREAK_START	(1 << 10)
+#define UART_IMR			0x0014
+#define UART_IMR_TXLEV			BIT(0)
+#define UART_IMR_RXSTALE		BIT(3)
+#define UART_IMR_RXLEV			BIT(4)
+#define UART_IMR_DELTA_CTS		BIT(5)
+#define UART_IMR_CURRENT_CTS		BIT(6)
+#define UART_IMR_RXBREAK_START		BIT(10)
 
 #define UART_IPR_RXSTALE_LAST		0x20
 #define UART_IPR_STALE_LSB		0x1F
 #define UART_IPR_STALE_TIMEOUT_MSB	0x3FF80
 #define UART_DM_IPR_STALE_TIMEOUT_MSB	0xFFFFFF80
 
-#define UART_IPR	0x0018
-#define UART_TFWR	0x001C
-#define UART_RFWR	0x0020
-#define UART_HCR	0x0024
+#define UART_IPR		0x0018
+#define UART_TFWR		0x001C
+#define UART_RFWR		0x0020
+#define UART_HCR		0x0024
 
 #define UART_MREG		0x0028
 #define UART_NREG		0x002C

@@ -98,20 +98,20 @@
 #define UART_TEST_CTRL		0x0050
 
 #define UART_SR			0x0008
-#define UART_SR_HUNT_CHAR	(1 << 7)
-#define UART_SR_RX_BREAK	(1 << 6)
-#define UART_SR_PAR_FRAME_ERR	(1 << 5)
-#define UART_SR_OVERRUN		(1 << 4)
-#define UART_SR_TX_EMPTY	(1 << 3)
-#define UART_SR_TX_READY	(1 << 2)
-#define UART_SR_RX_FULL		(1 << 1)
-#define UART_SR_RX_READY	(1 << 0)
+#define UART_SR_HUNT_CHAR	BIT(7)
+#define UART_SR_RX_BREAK	BIT(6)
+#define UART_SR_PAR_FRAME_ERR	BIT(5)
+#define UART_SR_OVERRUN		BIT(4)
+#define UART_SR_TX_EMPTY	BIT(3)
+#define UART_SR_TX_READY	BIT(2)
+#define UART_SR_RX_FULL		BIT(1)
+#define UART_SR_RX_READY	BIT(0)
 
 #define UART_RF			0x000C
 #define UARTDM_RF		0x0070
 #define UART_MISR		0x0010
 #define UART_ISR		0x0014
-#define UART_ISR_TX_READY	(1 << 7)
+#define UART_ISR_TX_READY	BIT(7)
 
 #define UARTDM_RXFS		0x50
 #define UARTDM_RXFS_BUF_SHIFT	0x7
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#6 lore raw

Re: [PATCH v3 3/3] tty: serial: msm: replaces (1 << x) with BIT(x) macro

drivers/tty/serial/msm_serial.h line 45

Quoted 4 lines · Pramod Gurav
drivers/tty/serial/msm_serial.h +2 -2
-#define UART_TF                0x000C
-#define UARTDM_TF      0x0070
+#define UART_TF                                0x000C
+#define UARTDM_TF                      0x0070

This is a different change, so it should be part of a different patch.

drivers/tty/serial/msm_serial.h line 87

Quoted 8 lines
drivers/tty/serial/msm_serial.h +4 -4
-#define UART_IPR       0x0018
-#define UART_TFWR      0x001C
-#define UART_RFWR      0x0020
-#define UART_HCR       0x0024
+#define UART_IPR               0x0018
+#define UART_TFWR              0x001C
+#define UART_RFWR              0x0020
+#define UART_HCR               0x0024

Same here.

Regards,

Fabio Estevam

Reviewed-by on v3 1/3
#8 lore raw
In reply to Pramod Gurav
Quoted 9 lines · Pramod Gurav

The bit masks for RFR_LEVEL1 and STALE_TIMEOUT_MSB values in MR1 and IPR registers respectively are different for UART and UART_DM hardware cores. We have been using UART core mask values for these. Add the same for UART_DM core.

There is no bit setting as UART_IPR_RXSTALE_LAST for UART_DM core so do it only for UART core.

Signed-off-by Pramod Gurav <[email protected]>
Reviewed-by Stephen Boyd <[email protected]>
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#9 lore raw
In reply to Pramod Gurav
Quoted 9 lines · Pramod Gurav

The bit masks for RFR_LEVEL1 and STALE_TIMEOUT_MSB values in MR1 and IPR registers respectively are different for UART and UART_DM hardware cores. We have been using UART core mask values for these. Add the same for UART_DM core.

There is no bit setting as UART_IPR_RXSTALE_LAST for UART_DM core so do it only for UART core.

Signed-off-by Pramod Gurav <[email protected]>
Quoted 8 lines

---
Changes since v2:

- Changed the sequnce of code to avoid duplication of code.

 drivers/tty/serial/msm_serial.c | 26 ++++++++++++++++++++------
 drivers/tty/serial/msm_serial.h |  2 ++
 2 files changed, 22 insertions(+), 6 deletions(-)

I only have patch 1/3 in my inbox, please resend all of them, not just individual ones you have redone, as that makes it impossible to keep track of patches in series.

thanks,

greg k-h