* [PATCH V2 0/2] Fix RX cancel command failure
@ 2020-02-25 13:54 satya priya
2020-02-25 13:54 ` [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe satya priya
2020-02-25 13:54 ` [PATCH V2 2/2] tty: serial: qcom_geni_serial: Fix RX cancel command failure satya priya
0 siblings, 2 replies; 8+ messages in thread
From: satya priya @ 2020-02-25 13:54 UTC (permalink / raw)
To: gregkh
Cc: swboyd, mgautam, linux-arm-msm, linux-serial, akashast, rojay,
msavaliy, satya priya
Changes in V2:
- Add patch to allocate port->rx_fifo buffer in probe to resolve
NULL pointer dereference crash reported by stephen.
The crash is caused due to set_termios call starting RX engine and RX engine
is sampling some garbage data from line, and by the time startup is called,
we have few data to read but port->rx_fifo buffer is not allocated causing
NULL pointer dereference.
satya priya (2):
tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe
tty: serial: qcom_geni_serial: Fix RX cancel command failure
drivers/tty/serial/qcom_geni_serial.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe
2020-02-25 13:54 [PATCH V2 0/2] Fix RX cancel command failure satya priya
@ 2020-02-25 13:54 ` satya priya
2020-02-28 22:54 ` Stephen Boyd
2020-02-28 23:01 ` Stephen Boyd
2020-02-25 13:54 ` [PATCH V2 2/2] tty: serial: qcom_geni_serial: Fix RX cancel command failure satya priya
1 sibling, 2 replies; 8+ messages in thread
From: satya priya @ 2020-02-25 13:54 UTC (permalink / raw)
To: gregkh
Cc: swboyd, mgautam, linux-arm-msm, linux-serial, akashast, rojay,
msavaliy, satya priya
To fix the RX cancel command failure, rx_fifo buffer needs to be
flushed in stop_rx() by calling handle_rx().
If set_termios is called before startup, by this time memory is not
allocated to port->rx_fifo buffer, which leads to a NULL pointer
dereference.
To avoid this NULL pointer dereference allocate memory to port->rx_fifo
in probe itself.
Signed-off-by: satya priya <skakit@codeaurora.org>
---
drivers/tty/serial/qcom_geni_serial.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 191abb1..d2a909c 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -858,12 +858,6 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
false, false, true);
geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2);
geni_se_select_mode(&port->se, GENI_SE_FIFO);
- if (!uart_console(uport)) {
- port->rx_fifo = devm_kcalloc(uport->dev,
- port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
- if (!port->rx_fifo)
- return -ENOMEM;
- }
port->setup = true;
return 0;
@@ -1274,6 +1268,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
port->rx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
port->tx_fifo_width = DEF_FIFO_WIDTH_BITS;
+ if (!console) {
+ port->rx_fifo = devm_kcalloc(uport->dev,
+ port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
+ if (!port->rx_fifo)
+ return -ENOMEM;
+ }
+
port->name = devm_kasprintf(uport->dev, GFP_KERNEL,
"qcom_geni_serial_%s%d",
uart_console(uport) ? "console" : "uart", uport->line);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH V2 2/2] tty: serial: qcom_geni_serial: Fix RX cancel command failure
2020-02-25 13:54 [PATCH V2 0/2] Fix RX cancel command failure satya priya
2020-02-25 13:54 ` [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe satya priya
@ 2020-02-25 13:54 ` satya priya
1 sibling, 0 replies; 8+ messages in thread
From: satya priya @ 2020-02-25 13:54 UTC (permalink / raw)
To: gregkh
Cc: swboyd, mgautam, linux-arm-msm, linux-serial, akashast, rojay,
msavaliy, satya priya
RX cancel command fails when BT is switched on and off multiple times.
To handle this, poll for the cancel bit in SE_GENI_S_IRQ_STATUS register
instead of SE_GENI_S_CMD_CTRL_REG.
As per the HPG update, handle the RX last bit after cancel command
and flush out the RX FIFO buffer.
Signed-off-by: satya priya <skakit@codeaurora.org>
---
drivers/tty/serial/qcom_geni_serial.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index d2a909c..22d99b0 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -129,6 +129,7 @@ static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop);
static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop);
static unsigned int qcom_geni_serial_tx_empty(struct uart_port *port);
static void qcom_geni_serial_stop_rx(struct uart_port *uport);
+static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop);
static const unsigned long root_freq[] = {7372800, 14745600, 19200000, 29491200,
32000000, 48000000, 64000000, 80000000,
@@ -599,7 +600,7 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport)
u32 irq_en;
u32 status;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
- u32 irq_clear = S_CMD_DONE_EN;
+ u32 s_irq_status;
irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
irq_en &= ~(S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN);
@@ -615,10 +616,19 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport)
return;
geni_se_cancel_s_cmd(&port->se);
- qcom_geni_serial_poll_bit(uport, SE_GENI_S_CMD_CTRL_REG,
- S_GENI_CMD_CANCEL, false);
+ qcom_geni_serial_poll_bit(uport, SE_GENI_S_IRQ_STATUS,
+ S_CMD_CANCEL_EN, true);
+ /*
+ * If timeout occurs secondary engine remains active
+ * and Abort sequence is executed.
+ */
+ s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
+ /* Flush the Rx buffer */
+ if (s_irq_status & S_RX_FIFO_LAST_EN)
+ qcom_geni_serial_handle_rx(uport, true);
+ writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
+
status = readl(uport->membase + SE_GENI_STATUS);
- writel(irq_clear, uport->membase + SE_GENI_S_IRQ_CLEAR);
if (status & S_GENI_CMD_ACTIVE)
qcom_geni_serial_abort_rx(uport);
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe
2020-02-25 13:54 ` [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe satya priya
@ 2020-02-28 22:54 ` Stephen Boyd
2020-03-04 13:38 ` skakit
2020-02-28 23:01 ` Stephen Boyd
1 sibling, 1 reply; 8+ messages in thread
From: Stephen Boyd @ 2020-02-28 22:54 UTC (permalink / raw)
To: gregkh, satya priya
Cc: mgautam, linux-arm-msm, linux-serial, akashast, rojay, msavaliy,
satya priya
Quoting satya priya (2020-02-25 05:54:21)
> To fix the RX cancel command failure, rx_fifo buffer needs to be
> flushed in stop_rx() by calling handle_rx().
>
> If set_termios is called before startup, by this time memory is not
> allocated to port->rx_fifo buffer, which leads to a NULL pointer
> dereference.
>
> To avoid this NULL pointer dereference allocate memory to port->rx_fifo
> in probe itself.
>
> Signed-off-by: satya priya <skakit@codeaurora.org>
Please give me reported-by credit.
Reported-by: Stephen Boyd <swboyd@chromium.org>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 191abb1..d2a909c 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -858,12 +858,6 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
> false, false, true);
> geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2);
> geni_se_select_mode(&port->se, GENI_SE_FIFO);
> - if (!uart_console(uport)) {
> - port->rx_fifo = devm_kcalloc(uport->dev,
> - port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
> - if (!port->rx_fifo)
> - return -ENOMEM;
> - }
> port->setup = true;
>
> return 0;
> @@ -1274,6 +1268,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> port->rx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
> port->tx_fifo_width = DEF_FIFO_WIDTH_BITS;
>
> + if (!console) {
> + port->rx_fifo = devm_kcalloc(uport->dev,
> + port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
> + if (!port->rx_fifo)
> + return -ENOMEM;
> + }
Is there any reason the rx_fifo pointer is a u32 instead of a u8 or void
pointer? ioread32_rep() doesn't care what the pointer is and then we
have to cast it, so it seems like we should do something like below too.
-----8<-----
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 191abb18fc2a..b4875dfef6aa 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -113,7 +113,7 @@ struct qcom_geni_serial_port {
unsigned int baud;
unsigned int tx_bytes_pw;
unsigned int rx_bytes_pw;
- u32 *rx_fifo;
+ u8 *rx_fifo;
u32 loopback;
bool brk;
@@ -504,7 +504,6 @@ static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
{
- unsigned char *buf;
struct tty_port *tport;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
u32 num_bytes_pw = port->tx_fifo_width / BITS_PER_BYTE;
@@ -516,8 +515,7 @@ static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
if (drop)
return 0;
- buf = (unsigned char *)port->rx_fifo;
- ret = tty_insert_flip_string(tport, buf, bytes);
+ ret = tty_insert_flip_string(tport, port->rx_fifo, bytes);
if (ret != bytes) {
dev_err(uport->dev, "%s:Unable to push data ret %d_bytes %d\n",
__func__, ret, bytes);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe
2020-02-25 13:54 ` [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe satya priya
2020-02-28 22:54 ` Stephen Boyd
@ 2020-02-28 23:01 ` Stephen Boyd
2020-03-04 13:34 ` skakit
1 sibling, 1 reply; 8+ messages in thread
From: Stephen Boyd @ 2020-02-28 23:01 UTC (permalink / raw)
To: gregkh, satya priya
Cc: mgautam, linux-arm-msm, linux-serial, akashast, rojay, msavaliy,
satya priya
Quoting satya priya (2020-02-25 05:54:21)
> To fix the RX cancel command failure, rx_fifo buffer needs to be
> flushed in stop_rx() by calling handle_rx().
>
> If set_termios is called before startup, by this time memory is not
> allocated to port->rx_fifo buffer, which leads to a NULL pointer
> dereference.
Also, clearly set_termios() isn't being called in the warning stack that
I sent last round:
pc : handle_rx_uart+0x64/0x278
lr : qcom_geni_serial_handle_rx+0x84/0x90
sp : ffffff814348f960
x29: ffffff814348f960 x28: ffffffd01ac24288
x27: 0000000000000018 x26: 0000000000000002
x25: 0000000000000001 x24: ffffff8146341348
x23: ffffff8146341000 x22: ffffffd01accc978
x21: ffffff8146341000 x20: 0000000000000001
x19: 0000000000000001 x18: ffffffd01b22d000
x17: 0000000000008000 x16: 00000000000000b0
x15: ffffffd01afdbdd0 x14: ffffffd01b3edde0
x13: ffffffd01b7fb000 x12: 0000000000000001
x11: 0000000000000000 x10: 0000000000000000
x9 : ffffffd010344780 x8 : 0000000000000000
x7 : ffffffd019d8e768 x6 : 0000000000000000
x5 : ffffffd01adbb000 x4 : 0000000000008004
x3 : 0000000000000001 x2 : 0000000000000001
x1 : 0000000000000001 x0 : ffffffd01accc978
Call trace:
handle_rx_uart+0x64/0x278
qcom_geni_serial_handle_rx+0x84/0x90
qcom_geni_serial_stop_rx+0x110/0x180
qcom_geni_serial_port_setup+0x68/0x1b0
qcom_geni_serial_startup+0x24/0x70
uart_startup+0x164/0x28c
uart_port_activate+0x6c/0xbc
tty_port_open+0xa8/0x114
uart_open+0x28/0x38
ttyport_open+0x7c/0x164
serdev_device_open+0x38/0xe4
hci_uart_register_device+0x54/0x2e8 [hci_uart]
qca_serdev_probe+0x1c4/0x374 [hci_uart]
serdev_drv_probe+0x3c/0x64
really_probe+0x144/0x3f8
driver_probe_device+0x70/0x140
__driver_attach_async_helper+0x7c/0xa8
async_run_entry_fn+0x60/0x178
process_one_work+0x33c/0x640
worker_thread+0x2a0/0x470
kthread+0x128/0x138
ret_from_fork+0x10/0x18
Code: 1aca096a 911e0129 b940012b 7100054a (b800450b)
This shows that uart_startup() is the one that is calling
qcom_geni_serial_startup() and that's running the newly added cancel
path. So even if we allocate the buffer in probe vs. in startup we're
going to flip a buffer full of junk that we're trying to cancel out of
the fifo into the tty layer. That seems wrong. We should have a
different qcom_geni_serial_stop_rx() function that knows we're starting
up vs. handling a normal rx event and call something besides handle_rx()
because that pushes bytes up into the tty layer.
>
> To avoid this NULL pointer dereference allocate memory to port->rx_fifo
> in probe itself.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe
2020-02-28 23:01 ` Stephen Boyd
@ 2020-03-04 13:34 ` skakit
2020-03-04 17:48 ` Stephen Boyd
0 siblings, 1 reply; 8+ messages in thread
From: skakit @ 2020-03-04 13:34 UTC (permalink / raw)
To: Stephen Boyd
Cc: gregkh, mgautam, linux-arm-msm, linux-serial, akashast, rojay, msavaliy
On 2020-02-29 04:31, Stephen Boyd wrote:
> Quoting satya priya (2020-02-25 05:54:21)
>> To fix the RX cancel command failure, rx_fifo buffer needs to be
>> flushed in stop_rx() by calling handle_rx().
>>
>> If set_termios is called before startup, by this time memory is not
>> allocated to port->rx_fifo buffer, which leads to a NULL pointer
>> dereference.
>
> Also, clearly set_termios() isn't being called in the warning stack
> that
> I sent last round:
>
> pc : handle_rx_uart+0x64/0x278
>
>
> lr : qcom_geni_serial_handle_rx+0x84/0x90
>
>
> sp : ffffff814348f960
>
>
> x29: ffffff814348f960 x28: ffffffd01ac24288
>
>
> x27: 0000000000000018 x26: 0000000000000002
>
>
> x25: 0000000000000001 x24: ffffff8146341348
>
>
> x23: ffffff8146341000 x22: ffffffd01accc978
>
>
> x21: ffffff8146341000 x20: 0000000000000001
>
>
> x19: 0000000000000001 x18: ffffffd01b22d000
>
>
> x17: 0000000000008000 x16: 00000000000000b0
>
>
> x15: ffffffd01afdbdd0 x14: ffffffd01b3edde0
>
>
> x13: ffffffd01b7fb000 x12: 0000000000000001
>
>
> x11: 0000000000000000 x10: 0000000000000000
>
>
> x9 : ffffffd010344780 x8 : 0000000000000000
>
>
> x7 : ffffffd019d8e768 x6 : 0000000000000000
>
>
> x5 : ffffffd01adbb000 x4 : 0000000000008004
>
>
> x3 : 0000000000000001 x2 : 0000000000000001
>
>
> x1 : 0000000000000001 x0 : ffffffd01accc978
>
>
> Call trace:
>
>
> handle_rx_uart+0x64/0x278
>
>
> qcom_geni_serial_handle_rx+0x84/0x90
>
>
> qcom_geni_serial_stop_rx+0x110/0x180
>
>
> qcom_geni_serial_port_setup+0x68/0x1b0
>
>
> qcom_geni_serial_startup+0x24/0x70
>
>
> uart_startup+0x164/0x28c
>
>
> uart_port_activate+0x6c/0xbc
>
>
> tty_port_open+0xa8/0x114
>
>
> uart_open+0x28/0x38
>
>
> ttyport_open+0x7c/0x164
>
>
> serdev_device_open+0x38/0xe4
>
>
> hci_uart_register_device+0x54/0x2e8 [hci_uart]
>
>
> qca_serdev_probe+0x1c4/0x374 [hci_uart]
>
>
> serdev_drv_probe+0x3c/0x64
>
>
> really_probe+0x144/0x3f8
>
>
> driver_probe_device+0x70/0x140
>
>
> __driver_attach_async_helper+0x7c/0xa8
>
>
> async_run_entry_fn+0x60/0x178
>
>
> process_one_work+0x33c/0x640
>
>
> worker_thread+0x2a0/0x470
>
>
> kthread+0x128/0x138
>
>
> ret_from_fork+0x10/0x18
>
>
> Code: 1aca096a 911e0129 b940012b 7100054a (b800450b)
>
>
> This shows that uart_startup() is the one that is calling
> qcom_geni_serial_startup() and that's running the newly added cancel
> path. So even if we allocate the buffer in probe vs. in startup we're
> going to flip a buffer full of junk that we're trying to cancel out of
> the fifo into the tty layer. That seems wrong. We should have a
> different qcom_geni_serial_stop_rx() function that knows we're starting
> up vs. handling a normal rx event and call something besides
> handle_rx()
> because that pushes bytes up into the tty layer.
>
As we mentioned in the V1 patch, we are passing drop="true" to handle_rx
function so it will read and discard whatever data present in RX FIFO,
it won't send to upper layers.
static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
{
....
ioread32_rep(uport->membase + SE_GENI_RX_FIFOn, port->rx_fifo,
words);
if (drop)
return 0;
....
}
In general uart_startup() is called before set_termios() ,but as per the
crash logs shared, it seems RX engine is active(which can only happen
from set_termios) before startup() is called.So, if we allocate
port->rx_fifo in probe we can overcome this crash.
>>
>> To avoid this NULL pointer dereference allocate memory to
>> port->rx_fifo
>> in probe itself.
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe
2020-02-28 22:54 ` Stephen Boyd
@ 2020-03-04 13:38 ` skakit
0 siblings, 0 replies; 8+ messages in thread
From: skakit @ 2020-03-04 13:38 UTC (permalink / raw)
To: Stephen Boyd
Cc: gregkh, mgautam, linux-arm-msm, linux-serial, akashast, rojay, msavaliy
On 2020-02-29 04:24, Stephen Boyd wrote:
> Quoting satya priya (2020-02-25 05:54:21)
>> To fix the RX cancel command failure, rx_fifo buffer needs to be
>> flushed in stop_rx() by calling handle_rx().
>>
>> If set_termios is called before startup, by this time memory is not
>> allocated to port->rx_fifo buffer, which leads to a NULL pointer
>> dereference.
>>
>> To avoid this NULL pointer dereference allocate memory to
>> port->rx_fifo
>> in probe itself.
>>
>> Signed-off-by: satya priya <skakit@codeaurora.org>
>
> Please give me reported-by credit.
>
> Reported-by: Stephen Boyd <swboyd@chromium.org>
Ok.
>
>> ---
>> drivers/tty/serial/qcom_geni_serial.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c
>> b/drivers/tty/serial/qcom_geni_serial.c
>> index 191abb1..d2a909c 100644
>> --- a/drivers/tty/serial/qcom_geni_serial.c
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -858,12 +858,6 @@ static int qcom_geni_serial_port_setup(struct
>> uart_port *uport)
>> false, false, true);
>> geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2);
>> geni_se_select_mode(&port->se, GENI_SE_FIFO);
>> - if (!uart_console(uport)) {
>> - port->rx_fifo = devm_kcalloc(uport->dev,
>> - port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
>> - if (!port->rx_fifo)
>> - return -ENOMEM;
>> - }
>> port->setup = true;
>>
>> return 0;
>> @@ -1274,6 +1268,13 @@ static int qcom_geni_serial_probe(struct
>> platform_device *pdev)
>> port->rx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
>> port->tx_fifo_width = DEF_FIFO_WIDTH_BITS;
>>
>> + if (!console) {
>> + port->rx_fifo = devm_kcalloc(uport->dev,
>> + port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
>> + if (!port->rx_fifo)
>> + return -ENOMEM;
>> + }
>
> Is there any reason the rx_fifo pointer is a u32 instead of a u8 or
> void
> pointer? ioread32_rep() doesn't care what the pointer is and then we
> have to cast it, so it seems like we should do something like below
> too.
>
Yes, we can use void instead of u32, will add this change in next patch.
> -----8<-----
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c
> b/drivers/tty/serial/qcom_geni_serial.c
> index 191abb18fc2a..b4875dfef6aa 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -113,7 +113,7 @@ struct qcom_geni_serial_port {
> unsigned int baud;
> unsigned int tx_bytes_pw;
> unsigned int rx_bytes_pw;
> - u32 *rx_fifo;
> + u8 *rx_fifo;
> u32 loopback;
> bool brk;
>
> @@ -504,7 +504,6 @@ static int handle_rx_console(struct uart_port
> *uport, u32 bytes, bool drop)
>
> static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool
> drop)
> {
> - unsigned char *buf;
> struct tty_port *tport;
> struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
> u32 num_bytes_pw = port->tx_fifo_width / BITS_PER_BYTE;
> @@ -516,8 +515,7 @@ static int handle_rx_uart(struct uart_port *uport,
> u32 bytes, bool drop)
> if (drop)
> return 0;
>
> - buf = (unsigned char *)port->rx_fifo;
> - ret = tty_insert_flip_string(tport, buf, bytes);
> + ret = tty_insert_flip_string(tport, port->rx_fifo, bytes);
> if (ret != bytes) {
> dev_err(uport->dev, "%s:Unable to push data ret %d_bytes %d\n",
> __func__, ret, bytes);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe
2020-03-04 13:34 ` skakit
@ 2020-03-04 17:48 ` Stephen Boyd
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2020-03-04 17:48 UTC (permalink / raw)
To: skakit
Cc: gregkh, mgautam, linux-arm-msm, linux-serial, akashast, rojay, msavaliy
Quoting skakit@codeaurora.org (2020-03-04 05:34:20)
> As we mentioned in the V1 patch, we are passing drop="true" to handle_rx
> function so it will read and discard whatever data present in RX FIFO,
> it won't send to upper layers.
> static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
> {
> ....
> ioread32_rep(uport->membase + SE_GENI_RX_FIFOn, port->rx_fifo,
> words);
> if (drop)
> return 0;
> ....
> }
> In general uart_startup() is called before set_termios() ,but as per the
> crash logs shared, it seems RX engine is active(which can only happen
> from set_termios) before startup() is called.So, if we allocate
> port->rx_fifo in probe we can overcome this crash.
Ok. Thanks for clarifying. Can you please mention in the commit text
that the fifo contents are read into rx_fifo but then dropped?
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-04 17:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 13:54 [PATCH V2 0/2] Fix RX cancel command failure satya priya
2020-02-25 13:54 ` [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe satya priya
2020-02-28 22:54 ` Stephen Boyd
2020-03-04 13:38 ` skakit
2020-02-28 23:01 ` Stephen Boyd
2020-03-04 13:34 ` skakit
2020-03-04 17:48 ` Stephen Boyd
2020-02-25 13:54 ` [PATCH V2 2/2] tty: serial: qcom_geni_serial: Fix RX cancel command failure satya priya
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.