* [PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata"
2022-06-29 0:02 [PATCH 0/2] BUG, more dw8259 ACPI crashes Jeremy Linton
@ 2022-06-29 0:02 ` Jeremy Linton
2022-06-29 6:08 ` Greg KH
2022-06-29 0:02 ` [PATCH 2/2] serial: 8250: dw: Fix NULL pointer dereference Jeremy Linton
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Jeremy Linton @ 2022-06-29 0:02 UTC (permalink / raw)
To: linux-serial
Cc: andriy.shevchenko, gregkh, jirislaby, miquel.raynal,
phil.edworthy, kernel, linux-kernel, Jeremy Linton
pdata is only setup by DT machines, leaving ACPI machines
with null pdata. Since I don't know the exact mapping of
ACPI ID's to dw 8250 variations I can't add pdata to them
without possibly breaking something. As such the simplest
fix here is probably just to revert this commit.
This reverts commit ffd381445eac2aa624e49bab5a811451e8351008.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/tty/serial/8250/8250_dw.c | 9 +++------
drivers/tty/serial/8250/8250_dwlib.h | 2 +-
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index f57bbd32ef11..53e488beb8c2 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -275,7 +275,7 @@ static int dw8250_handle_irq(struct uart_port *p)
if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
/* Clear the USR */
- (void)p->serial_in(p, d->pdata->usr_reg);
+ (void)p->serial_in(p, d->usr_reg);
return 1;
}
@@ -447,6 +447,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
p->serial_out = dw8250_serial_outq;
p->flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_FIXED_TYPE;
p->type = PORT_OCTEON;
+ data->usr_reg = OCTEON_UART_USR;
data->skip_autocfg = true;
}
#endif
@@ -536,6 +537,7 @@ static int dw8250_probe(struct platform_device *pdev)
return -ENOMEM;
data->data.dma.fn = dw8250_fallback_dma_filter;
+ data->usr_reg = DW_UART_USR;
data->pdata = device_get_match_data(p->dev);
p->private_data = &data->data;
@@ -738,27 +740,22 @@ static const struct dev_pm_ops dw8250_pm_ops = {
};
static const struct dw8250_platform_data dw8250_dw_apb = {
- .usr_reg = DW_UART_USR,
};
static const struct dw8250_platform_data dw8250_octeon_3860_data = {
- .usr_reg = OCTEON_UART_USR,
.quirks = DW_UART_QUIRK_OCTEON,
};
static const struct dw8250_platform_data dw8250_armada_38x_data = {
- .usr_reg = DW_UART_USR,
.quirks = DW_UART_QUIRK_ARMADA_38X,
};
static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
- .usr_reg = DW_UART_USR,
.cpr_val = 0x00012f32,
.quirks = DW_UART_QUIRK_IS_DMA_FC,
};
static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
- .usr_reg = DW_UART_USR,
.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
};
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 055bfdc87985..9870ad0d54e4 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -26,7 +26,6 @@ struct dw8250_port_data {
};
struct dw8250_platform_data {
- u8 usr_reg;
u32 cpr_val;
unsigned int quirks;
};
@@ -35,6 +34,7 @@ struct dw8250_data {
struct dw8250_port_data data;
const struct dw8250_platform_data *pdata;
+ u8 usr_reg;
int msr_mask_on;
int msr_mask_off;
struct clk *clk;
--
2.35.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata"
2022-06-29 0:02 ` [PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata" Jeremy Linton
@ 2022-06-29 6:08 ` Greg KH
2022-06-29 14:38 ` Jeremy Linton
0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2022-06-29 6:08 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-serial, andriy.shevchenko, jirislaby, miquel.raynal,
phil.edworthy, kernel, linux-kernel
On Tue, Jun 28, 2022 at 07:02:31PM -0500, Jeremy Linton wrote:
> pdata is only setup by DT machines, leaving ACPI machines
> with null pdata. Since I don't know the exact mapping of
> ACPI ID's to dw 8250 variations I can't add pdata to them
> without possibly breaking something. As such the simplest
> fix here is probably just to revert this commit.
>
> This reverts commit ffd381445eac2aa624e49bab5a811451e8351008.
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Didn't checkpatch complain that you need a blank line before your
signed-off-by line?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata"
2022-06-29 6:08 ` Greg KH
@ 2022-06-29 14:38 ` Jeremy Linton
0 siblings, 0 replies; 8+ messages in thread
From: Jeremy Linton @ 2022-06-29 14:38 UTC (permalink / raw)
To: Greg KH
Cc: linux-serial, andriy.shevchenko, jirislaby, miquel.raynal,
phil.edworthy, kernel, linux-kernel
Hi,
On 6/29/22 01:08, Greg KH wrote:
> On Tue, Jun 28, 2022 at 07:02:31PM -0500, Jeremy Linton wrote:
>> pdata is only setup by DT machines, leaving ACPI machines
>> with null pdata. Since I don't know the exact mapping of
>> ACPI ID's to dw 8250 variations I can't add pdata to them
>> without possibly breaking something. As such the simplest
>> fix here is probably just to revert this commit.
>>
>> This reverts commit ffd381445eac2aa624e49bab5a811451e8351008.
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>
> Didn't checkpatch complain that you need a blank line before your
> signed-off-by line?
No, it, and I apparently thought that the revert was part of the tagging.
Lets kill this set anyway, as the patch you already have to add pdata to
ACPI in tty-linus solves all these problems. I guess I missed that
posting, so sorry about the noise.
Thanks,
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] serial: 8250: dw: Fix NULL pointer dereference
2022-06-29 0:02 [PATCH 0/2] BUG, more dw8259 ACPI crashes Jeremy Linton
2022-06-29 0:02 ` [PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata" Jeremy Linton
@ 2022-06-29 0:02 ` Jeremy Linton
2022-06-29 8:01 ` Ilpo Järvinen
2022-06-29 7:43 ` [PATCH 0/2] BUG, more dw8259 ACPI crashes Miquel Raynal
2022-06-29 14:37 ` Jeremy Linton
3 siblings, 1 reply; 8+ messages in thread
From: Jeremy Linton @ 2022-06-29 0:02 UTC (permalink / raw)
To: linux-serial
Cc: andriy.shevchenko, gregkh, jirislaby, miquel.raynal,
phil.edworthy, kernel, linux-kernel, Jeremy Linton
Similarly to e6a08c6949cb, ACPI machines don't have
a pdata setup, leading to a null pointer dereference
when dw8250_handle_irq runs.
Fixes: aa63d786cea2 ("serial: 8250: dw: Add support for DMA flow controlling devices")
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/tty/serial/8250/8250_dw.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 53e488beb8c2..06f80090cbb9 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -237,10 +237,13 @@ static int dw8250_handle_irq(struct uart_port *p)
struct dw8250_data *d = to_dw8250_data(p->private_data);
unsigned int iir = p->serial_in(p, UART_IIR);
bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
- unsigned int quirks = d->pdata->quirks;
+ unsigned int quirks = 0;
unsigned int status;
unsigned long flags;
+ if (d->pdata)
+ quirks = d->pdata->quirks;
+
/*
* There are ways to get Designware-based UARTs into a state where
* they are asserting UART_IIR_RX_TIMEOUT but there is no actual
--
2.35.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] serial: 8250: dw: Fix NULL pointer dereference
2022-06-29 0:02 ` [PATCH 2/2] serial: 8250: dw: Fix NULL pointer dereference Jeremy Linton
@ 2022-06-29 8:01 ` Ilpo Järvinen
0 siblings, 0 replies; 8+ messages in thread
From: Ilpo Järvinen @ 2022-06-29 8:01 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-serial, Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby,
miquel.raynal, phil.edworthy, kernel, LKML
Hi,
Does cd16044d7c38 ("serial: 8250: dw: enable using pdata with ACPI") work
for you? (Currently in tty-linus branch).
--
i.
On Tue, 28 Jun 2022, Jeremy Linton wrote:
> Similarly to e6a08c6949cb, ACPI machines don't have
> a pdata setup, leading to a null pointer dereference
> when dw8250_handle_irq runs.
>
> Fixes: aa63d786cea2 ("serial: 8250: dw: Add support for DMA flow controlling devices")
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/tty/serial/8250/8250_dw.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 53e488beb8c2..06f80090cbb9 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -237,10 +237,13 @@ static int dw8250_handle_irq(struct uart_port *p)
> struct dw8250_data *d = to_dw8250_data(p->private_data);
> unsigned int iir = p->serial_in(p, UART_IIR);
> bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
> - unsigned int quirks = d->pdata->quirks;
> + unsigned int quirks = 0;
> unsigned int status;
> unsigned long flags;
>
> + if (d->pdata)
> + quirks = d->pdata->quirks;
> +
> /*
> * There are ways to get Designware-based UARTs into a state where
> * they are asserting UART_IIR_RX_TIMEOUT but there is no actual
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] BUG, more dw8259 ACPI crashes
2022-06-29 0:02 [PATCH 0/2] BUG, more dw8259 ACPI crashes Jeremy Linton
2022-06-29 0:02 ` [PATCH 1/2] Revert "serial: 8250: dw: Move the USR register to pdata" Jeremy Linton
2022-06-29 0:02 ` [PATCH 2/2] serial: 8250: dw: Fix NULL pointer dereference Jeremy Linton
@ 2022-06-29 7:43 ` Miquel Raynal
2022-06-29 14:37 ` Jeremy Linton
3 siblings, 0 replies; 8+ messages in thread
From: Miquel Raynal @ 2022-06-29 7:43 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-serial, andriy.shevchenko, gregkh, jirislaby, phil.edworthy,
kernel, linux-kernel
Hi Jeremy,
jeremy.linton@arm.com wrote on Tue, 28 Jun 2022 19:02:30 -0500:
> 5.19rc4 crashes with:
>
> [ 4.441703] Internal error: Oops: 96000004 [#1] SMP
> [ 4.446601] Modules linked in:
> [ 4.449668] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.19.0-0.rc4.33.fc37.aarch64 #1
> [ 4.457532] Hardware name: Marvell Armada 7k/8k Family Board /Armada 7k/8k Family Board , BIOS EDK II Jun 4 2019
> [ 4.470800] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 4.477791] pc : dw8250_handle_irq+0x58/0x190
> [ 4.482170] lr : dw8250_handle_irq+0x38/0x190
> [ 4.486544] sp : ffff800008003e40
> [ 4.489869] x29: ffff800008003e40 x28: ffffbe13be183980 x27: 00000000b46bd3d0
> [ 4.497036] x26: ffffbe13bdbc3008 x25: ffff6f1447910f80 x24: ffff6f1447910f94
> [ 4.504204] x23: 0000000000000012 x22: ffff6f144218fc80 x21: 0000000000000000
> [ 4.511372] x20: 0000000000000007 x19: ffffbe13be7628d0 x18: ffff80000918b528
> [ 4.518540] x17: ffffb103c0343000 x16: ffff800008004000 x15: 0000000000004000
> [ 4.525706] x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
> [ 4.532872] x11: ffffbe13be16f6d8 x10: 0000000000001d90 x9 : ffffbe13bc6b4f0c
> [ 4.540040] x8 : ffffbe13be185770 x7 : 0000000000000000 x6 : 00000000296b5f4a
> [ 4.547206] x5 : 00ffffffffffffff x4 : 0000000000010002 x3 : ffffbe13be7628d0
> [ 4.554373] x2 : 0000000000000007 x1 : 0000000000000000 x0 : 0000000000000000
> [ 4.561539] Call trace:
> [ 4.563994] dw8250_handle_irq+0x58/0x190
> [ 4.568020] serial8250_interrupt+0x68/0x140
> [ 4.572307] __handle_irq_event_percpu+0x68/0x230
> [ 4.577034] handle_irq_event+0x58/0x120
> [ 4.580973] handle_fasteoi_irq+0xcc/0x1c4
> [ 4.585086] generic_handle_domain_irq+0x38/0x50
> [ 4.589723] gic_handle_irq+0x50/0xe0
> [ 4.593399] call_on_irq_stack+0x2c/0x38
> [ 4.597338] do_interrupt_handler+0xdc/0xe0
> [ 4.601539] el1_interrupt+0x34/0x70
> [ 4.605131] el1h_64_irq_handler+0x18/0x24
> [ 4.609244] el1h_64_irq+0x68/0x6c
> [ 4.612658] arch_cpu_idle+0x18/0x2c
> [ 4.616246] default_idle_call+0x34/0x18c
> [ 4.620273] cpuidle_idle_call+0x154/0x1a0
> [ 4.624388] do_idle+0xa4/0xf4
> [ 4.627455] cpu_startup_entry+0x34/0x3c
> [ 4.631394] kernel_init+0x0/0x150
> [ 4.634808] arch_post_acpi_subsys_init+0x0/0x30
> [ 4.639445] start_kernel+0x474/0x490
> [ 4.643121] __primary_switched+0xc0/0xc8
> [ 4.647148] Code: 7100305f 1a9f17f5 f100003f 7a400aa4 (b9400817)
> [ 4.653267] ---[ end trace 0000000000000000 ]---
>
> On an ACPI enabled Mcbin. This is again the result
> of the pdata information not being available on an
> ACPI machine. But since there are multiple commits
> I've broken this into to fixes. The first
> simply reverts the USR register change because it
> should be functionality transparent and I don't
> have a good way to tie ACPI ids to the hardware
> and the second fixes the later patch which uses it
> in the interrupt handler.
Sorry for the breakage, the move of the USR register to pdata is not
really necessary anyway, it came up rather late in the review process,
and the other fix lgtm, so:
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> Jeremy Linton (2):
> Revert "serial: 8250: dw: Move the USR register to pdata"
> serial: 8250: dw: Fix NULL pointer dereference
>
> drivers/tty/serial/8250/8250_dw.c | 14 +++++++-------
> drivers/tty/serial/8250/8250_dwlib.h | 2 +-
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] BUG, more dw8259 ACPI crashes
2022-06-29 0:02 [PATCH 0/2] BUG, more dw8259 ACPI crashes Jeremy Linton
` (2 preceding siblings ...)
2022-06-29 7:43 ` [PATCH 0/2] BUG, more dw8259 ACPI crashes Miquel Raynal
@ 2022-06-29 14:37 ` Jeremy Linton
3 siblings, 0 replies; 8+ messages in thread
From: Jeremy Linton @ 2022-06-29 14:37 UTC (permalink / raw)
To: linux-serial
Cc: andriy.shevchenko, gregkh, jirislaby, miquel.raynal,
phil.edworthy, kernel, linux-kernel
Hi,
On 6/28/22 19:02, Jeremy Linton wrote:
> 5.19rc4 crashes with:
>
> [ 4.441703] Internal error: Oops: 96000004 [#1] SMP
> [ 4.446601] Modules linked in:
> [ 4.449668] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.19.0-0.rc4.33.fc37.aarch64 #1
> [ 4.457532] Hardware name: Marvell Armada 7k/8k Family Board /Armada 7k/8k Family Board , BIOS EDK II Jun 4 2019
> [ 4.470800] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 4.477791] pc : dw8250_handle_irq+0x58/0x190
> [ 4.482170] lr : dw8250_handle_irq+0x38/0x190
> [ 4.486544] sp : ffff800008003e40
> [ 4.489869] x29: ffff800008003e40 x28: ffffbe13be183980 x27: 00000000b46bd3d0
> [ 4.497036] x26: ffffbe13bdbc3008 x25: ffff6f1447910f80 x24: ffff6f1447910f94
> [ 4.504204] x23: 0000000000000012 x22: ffff6f144218fc80 x21: 0000000000000000
> [ 4.511372] x20: 0000000000000007 x19: ffffbe13be7628d0 x18: ffff80000918b528
> [ 4.518540] x17: ffffb103c0343000 x16: ffff800008004000 x15: 0000000000004000
> [ 4.525706] x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
> [ 4.532872] x11: ffffbe13be16f6d8 x10: 0000000000001d90 x9 : ffffbe13bc6b4f0c
> [ 4.540040] x8 : ffffbe13be185770 x7 : 0000000000000000 x6 : 00000000296b5f4a
> [ 4.547206] x5 : 00ffffffffffffff x4 : 0000000000010002 x3 : ffffbe13be7628d0
> [ 4.554373] x2 : 0000000000000007 x1 : 0000000000000000 x0 : 0000000000000000
> [ 4.561539] Call trace:
> [ 4.563994] dw8250_handle_irq+0x58/0x190
> [ 4.568020] serial8250_interrupt+0x68/0x140
> [ 4.572307] __handle_irq_event_percpu+0x68/0x230
> [ 4.577034] handle_irq_event+0x58/0x120
> [ 4.580973] handle_fasteoi_irq+0xcc/0x1c4
> [ 4.585086] generic_handle_domain_irq+0x38/0x50
> [ 4.589723] gic_handle_irq+0x50/0xe0
> [ 4.593399] call_on_irq_stack+0x2c/0x38
> [ 4.597338] do_interrupt_handler+0xdc/0xe0
> [ 4.601539] el1_interrupt+0x34/0x70
> [ 4.605131] el1h_64_irq_handler+0x18/0x24
> [ 4.609244] el1h_64_irq+0x68/0x6c
> [ 4.612658] arch_cpu_idle+0x18/0x2c
> [ 4.616246] default_idle_call+0x34/0x18c
> [ 4.620273] cpuidle_idle_call+0x154/0x1a0
> [ 4.624388] do_idle+0xa4/0xf4
> [ 4.627455] cpu_startup_entry+0x34/0x3c
> [ 4.631394] kernel_init+0x0/0x150
> [ 4.634808] arch_post_acpi_subsys_init+0x0/0x30
> [ 4.639445] start_kernel+0x474/0x490
> [ 4.643121] __primary_switched+0xc0/0xc8
> [ 4.647148] Code: 7100305f 1a9f17f5 f100003f 7a400aa4 (b9400817)
> [ 4.653267] ---[ end trace 0000000000000000 ]---
>
> On an ACPI enabled Mcbin. This is again the result
> of the pdata information not being available on an
> ACPI machine. But since there are multiple commits
> I've broken this into to fixes. The first
> simply reverts the USR register change because it
> should be functionality transparent and I don't
> have a good way to tie ACPI ids to the hardware
> and the second fixes the later patch which uses it
> in the interrupt handler.
Hi,
I missed cd16044d7c38 ("serial: 8250: dw: enable using pdata with ACPI")
in tty-linus, which fixes this. So please ignore this set, sorry about
the noise.
Thanks,
^ permalink raw reply [flat|nested] 8+ messages in thread