looks like there was a typo while checking validatity of tx_dma_addr, the
code was checking rx instead of tx.
This can potentially lead to memory leak, this patch fixes the typo.
Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
Signed-off-by: Srinivas Kandagatla <[email protected]>
Reviewed-by: Bartosz Golaszewski <[email protected]>
---
drivers/tty/serial/qcom_geni_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index d69592e5e2ec..5972b5c317d3 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -596,7 +596,7 @@ static void qcom_geni_serial_stop_tx_dma(struct uart_port *uport)
if (!qcom_geni_serial_main_active(uport))
return;
- if (port->rx_dma_addr) {
+ if (port->tx_dma_addr) {
geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr,
port->tx_remaining);
port->tx_dma_addr = 0;
--
2.21.0
Check if there are valid length of bytes to transfer before starting dma.
without this check we can see below kernel warning when we try to map a zero size buffers.
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at drivers/iommu/dma-iommu.c:1046 iommu_dma_unmap_page+0xe0/0xfc
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.3.0-rc1-dirty #347
Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
pstate: 804000c5 (Nzcv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : iommu_dma_unmap_page+0xe0/0xfc
lr : iommu_dma_unmap_page+0x38/0xfc
...
Call trace:
iommu_dma_unmap_page+0xe0/0xfc
dma_unmap_page_attrs+0x30/0x1ec
geni_se_tx_dma_unprep+0x58/0x80
qcom_geni_serial_isr+0x350/0x750
__handle_irq_event_percpu+0x58/0x148
handle_irq_event_percpu+0x18/0x4c
handle_irq_event+0x48/0x88
handle_fasteoi_irq+0xb0/0x130
generic_handle_domain_irq+0x2c/0x44
gic_handle_irq+0xd4/0x140
call_on_irq_stack+0x24/0x4c
do_interrupt_handler+0x80/0x84
el1_interrupt+0x38/0x6c
el1h_64_irq_handler+0x18/0x24
el1h_64_irq+0x64/0x68
cpuidle_enter_state+0x1e4/0x310
cpuidle_enter+0x3c/0x54
call_cpuidle+0x1c/0x40
do_idle+0x204/0x260
cpu_startup_entry+0x28/0x2c
kernel_init+0x0/0x12c
arch_post_acpi_subsys_init+0x0/0x8
start_kernel+0x3cc/0x74c
__primary_switched+0xbc/0xc4
Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/tty/serial/qcom_geni_serial.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 5972b5c317d3..bb63a00f4c07 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -637,6 +637,9 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
+ if (!xmit_size)
+ return;
+
qcom_geni_serial_setup_tx(uport, xmit_size);
ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
--
2.21.0
On 7.03.2023 16:55, Srinivas Kandagatla wrote:
> looks like there was a typo while checking validatity of tx_dma_addr, the
> code was checking rx instead of tx.
> This can potentially lead to memory leak, this patch fixes the typo.
>
> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> Reviewed-by: Bartosz Golaszewski <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
> drivers/tty/serial/qcom_geni_serial.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index d69592e5e2ec..5972b5c317d3 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -596,7 +596,7 @@ static void qcom_geni_serial_stop_tx_dma(struct uart_port *uport)
> if (!qcom_geni_serial_main_active(uport))
> return;
>
> - if (port->rx_dma_addr) {
> + if (port->tx_dma_addr) {
> geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr,
> port->tx_remaining);
> port->tx_dma_addr = 0;
On 7.03.2023 16:55, Srinivas Kandagatla wrote:
> Check if there are valid length of bytes to transfer before starting dma.
>
> without this check we can see below kernel warning when we try to map a zero size buffers.
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at drivers/iommu/dma-iommu.c:1046 iommu_dma_unmap_page+0xe0/0xfc
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.3.0-rc1-dirty #347
> Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> pstate: 804000c5 (Nzcv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : iommu_dma_unmap_page+0xe0/0xfc
> lr : iommu_dma_unmap_page+0x38/0xfc
> ...
> Call trace:
> iommu_dma_unmap_page+0xe0/0xfc
> dma_unmap_page_attrs+0x30/0x1ec
> geni_se_tx_dma_unprep+0x58/0x80
> qcom_geni_serial_isr+0x350/0x750
> __handle_irq_event_percpu+0x58/0x148
> handle_irq_event_percpu+0x18/0x4c
> handle_irq_event+0x48/0x88
> handle_fasteoi_irq+0xb0/0x130
> generic_handle_domain_irq+0x2c/0x44
> gic_handle_irq+0xd4/0x140
> call_on_irq_stack+0x24/0x4c
> do_interrupt_handler+0x80/0x84
> el1_interrupt+0x38/0x6c
> el1h_64_irq_handler+0x18/0x24
> el1h_64_irq+0x64/0x68
> cpuidle_enter_state+0x1e4/0x310
> cpuidle_enter+0x3c/0x54
> call_cpuidle+0x1c/0x40
> do_idle+0x204/0x260
> cpu_startup_entry+0x28/0x2c
> kernel_init+0x0/0x12c
> arch_post_acpi_subsys_init+0x0/0x8
> start_kernel+0x3cc/0x74c
> __primary_switched+0xbc/0xc4
>
> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> Signed-off-by: Srinivas Kandagatla <[email protected]>
S-o-b but no C-d-b? Weird..
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
Anyway, the change is good!
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
> drivers/tty/serial/qcom_geni_serial.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 5972b5c317d3..bb63a00f4c07 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -637,6 +637,9 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
>
> xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
>
> + if (!xmit_size)
> + return;
> +
> qcom_geni_serial_setup_tx(uport, xmit_size);
>
> ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
On Tue, 7 Mar 2023 at 17:20, Konrad Dybcio <[email protected]> wrote:
>
>
>
> On 7.03.2023 16:55, Srinivas Kandagatla wrote:
> > Check if there are valid length of bytes to transfer before starting dma.
> >
> > without this check we can see below kernel warning when we try to map a zero size buffers.
> >
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 0 at drivers/iommu/dma-iommu.c:1046 iommu_dma_unmap_page+0xe0/0xfc
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.3.0-rc1-dirty #347
> > Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> > pstate: 804000c5 (Nzcv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > pc : iommu_dma_unmap_page+0xe0/0xfc
> > lr : iommu_dma_unmap_page+0x38/0xfc
> > ...
> > Call trace:
> > iommu_dma_unmap_page+0xe0/0xfc
> > dma_unmap_page_attrs+0x30/0x1ec
> > geni_se_tx_dma_unprep+0x58/0x80
> > qcom_geni_serial_isr+0x350/0x750
> > __handle_irq_event_percpu+0x58/0x148
> > handle_irq_event_percpu+0x18/0x4c
> > handle_irq_event+0x48/0x88
> > handle_fasteoi_irq+0xb0/0x130
> > generic_handle_domain_irq+0x2c/0x44
> > gic_handle_irq+0xd4/0x140
> > call_on_irq_stack+0x24/0x4c
> > do_interrupt_handler+0x80/0x84
> > el1_interrupt+0x38/0x6c
> > el1h_64_irq_handler+0x18/0x24
> > el1h_64_irq+0x64/0x68
> > cpuidle_enter_state+0x1e4/0x310
> > cpuidle_enter+0x3c/0x54
> > call_cpuidle+0x1c/0x40
> > do_idle+0x204/0x260
> > cpu_startup_entry+0x28/0x2c
> > kernel_init+0x0/0x12c
> > arch_post_acpi_subsys_init+0x0/0x8
> > start_kernel+0x3cc/0x74c
> > __primary_switched+0xbc/0xc4
> >
> > Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> > Signed-off-by: Srinivas Kandagatla <[email protected]>
> S-o-b but no C-d-b? Weird..
>
It was supposed to be Reviewed-by actually.
Bart
> > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > ---
> Anyway, the change is good!
>
> Reviewed-by: Konrad Dybcio <[email protected]>
>
> Konrad
> > drivers/tty/serial/qcom_geni_serial.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> > index 5972b5c317d3..bb63a00f4c07 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -637,6 +637,9 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
> >
> > xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
> >
> > + if (!xmit_size)
> > + return;
> > +
> > qcom_geni_serial_setup_tx(uport, xmit_size);
> >
> > ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
On 07/03/2023 16:20, Konrad Dybcio wrote:
>
>
> On 7.03.2023 16:55, Srinivas Kandagatla wrote:
>> Check if there are valid length of bytes to transfer before starting dma.
>>
>> without this check we can see below kernel warning when we try to map a zero size buffers.
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 0 at drivers/iommu/dma-iommu.c:1046 iommu_dma_unmap_page+0xe0/0xfc
>> Modules linked in:
>> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.3.0-rc1-dirty #347
>> Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
>> pstate: 804000c5 (Nzcv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : iommu_dma_unmap_page+0xe0/0xfc
>> lr : iommu_dma_unmap_page+0x38/0xfc
>> ...
>> Call trace:
>> iommu_dma_unmap_page+0xe0/0xfc
>> dma_unmap_page_attrs+0x30/0x1ec
>> geni_se_tx_dma_unprep+0x58/0x80
>> qcom_geni_serial_isr+0x350/0x750
>> __handle_irq_event_percpu+0x58/0x148
>> handle_irq_event_percpu+0x18/0x4c
>> handle_irq_event+0x48/0x88
>> handle_fasteoi_irq+0xb0/0x130
>> generic_handle_domain_irq+0x2c/0x44
>> gic_handle_irq+0xd4/0x140
>> call_on_irq_stack+0x24/0x4c
>> do_interrupt_handler+0x80/0x84
>> el1_interrupt+0x38/0x6c
>> el1h_64_irq_handler+0x18/0x24
>> el1h_64_irq+0x64/0x68
>> cpuidle_enter_state+0x1e4/0x310
>> cpuidle_enter+0x3c/0x54
>> call_cpuidle+0x1c/0x40
>> do_idle+0x204/0x260
>> cpu_startup_entry+0x28/0x2c
>> kernel_init+0x0/0x12c
>> arch_post_acpi_subsys_init+0x0/0x8
>> start_kernel+0x3cc/0x74c
>> __primary_switched+0xbc/0xc4
>>
>> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
> S-o-b but no C-d-b? Weird..
>
Yes, I did mess up this part.
thanks for spotting this, Will fix this in next spin!
--srini
>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>> ---
> Anyway, the change is good!
>
> Reviewed-by: Konrad Dybcio <[email protected]>
>
> Konrad
>> drivers/tty/serial/qcom_geni_serial.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>> index 5972b5c317d3..bb63a00f4c07 100644
>> --- a/drivers/tty/serial/qcom_geni_serial.c
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -637,6 +637,9 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
>>
>> xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
>>
>> + if (!xmit_size)
>> + return;
>> +
>> qcom_geni_serial_setup_tx(uport, xmit_size);
>>
>> ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],