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

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

[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 <gpramod@codeaurora.org>
---
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(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b73889c..99aba04 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -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;
 }
 
diff --git a/drivers/tty/serial/msm_serial.h b/drivers/tty/serial/msm_serial.h
index 3e1c713..caf5363 100644
--- a/drivers/tty/serial/msm_serial.h
+++ b/drivers/tty/serial/msm_serial.h
@@ -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
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Pramod Gurav]

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 <gpramod@codeaurora.org>
---
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(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 99aba04..c32b088 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -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);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Pramod Gurav]

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

Signed-off-by: Pramod Gurav <gpramod@codeaurora.org>
---
This is a new patch added in this series.

 drivers/tty/serial/msm_serial.h | 58 ++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.h b/drivers/tty/serial/msm_serial.h
index caf5363..f663822 100644
--- a/drivers/tty/serial/msm_serial.h
+++ b/drivers/tty/serial/msm_serial.h
@@ -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
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Fabio Estevam]

On Fri, Apr 10, 2015 at 9:19 AM, Pramod Gurav <gpramod@codeaurora.org> wrote:

> -#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.

> -#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

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

[Stephen Boyd]

On 04/10/15 05:19, Pramod Gurav wrote:
> 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 <gpramod@codeaurora.org>

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

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

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

[Stephen Boyd]

On 04/10/15 05:19, Pramod Gurav wrote:
> @@ -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?

>  		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

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

[Greg KH]

On Fri, Apr 10, 2015 at 05:49:54PM +0530, Pramod Gurav wrote:
> 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 <gpramod@codeaurora.org>
> ---
> 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

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

[Pramod Gurav]

Thanks Stephen for review.

On Fri, April 10, 2015 11:33 pm, Stephen Boyd wrote:
> On 04/10/15 05:19, Pramod Gurav wrote:
>> @@ -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?

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?
>
>>  		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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


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

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

[Stephen Boyd]

On 04/29/15 08:45, Pramod Gurav wrote:
> Thanks Stephen for review.
>
> On Fri, April 10, 2015 11:33 pm, Stephen Boyd wrote:
>> On 04/10/15 05:19, Pramod Gurav wrote:
>>> @@ -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?
> 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().

>
> 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.

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

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

[Stephen Boyd]

On 04/10/15 05:19, Pramod Gurav wrote:
> Replaces (1 << x) with BIT(x) macro
>
> Signed-off-by: Pramod Gurav <gpramod@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

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