2024-05-14 19:07:48

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 0/2] serial: 8250_dw: Fix console on Intel Galileo

Console on Intel Galileo is broken. Here is the fix and one revert that
I prefer to have as part of the fix to prevent similar mistakes from
being made.

Andy Shevchenko (2):
serial: 8250_dw: Don't use struct dw8250_data outside of 8250_dw
serial: 8250_dw: Revert "Move definitions to the shared header"

drivers/tty/serial/8250/8250_dw.c | 36 ++++++++++++++++++++++++++--
drivers/tty/serial/8250/8250_dwlib.c | 3 +--
drivers/tty/serial/8250/8250_dwlib.h | 33 +------------------------
3 files changed, 36 insertions(+), 36 deletions(-)

--
2.43.0.rc1.1336.g36b5255a03ac



2024-05-14 19:08:20

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/2] serial: 8250_dw: Don't use struct dw8250_data outside of 8250_dw

The container of the struct dw8250_port_data is private to the actual
driver. In particular, 8250_lpss and 8250_dw use different data types
that are assigned to the UART port private_data. Hence, it must not
be used outside the specific driver.

Currently the only cpr_val is required by the common code, make it
be available via struct dw8250_port_data.

This fixes the UART breakage on Intel Galileo boards.

Fixes: 593dea000bc1 ("serial: 8250: dw: Allow to use a fallback CPR value if not synthesized")
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_dw.c | 9 +++++++--
drivers/tty/serial/8250/8250_dwlib.c | 3 +--
drivers/tty/serial/8250/8250_dwlib.h | 3 ++-
3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 1300c92b8702..94d680e4b535 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -55,6 +55,7 @@
#define DW_UART_QUIRK_SKIP_SET_RATE BIT(2)
#define DW_UART_QUIRK_IS_DMA_FC BIT(3)
#define DW_UART_QUIRK_APMC0D08 BIT(4)
+#define DW_UART_QUIRK_CPR_VALUE BIT(5)

static inline struct dw8250_data *clk_to_dw8250_data(struct notifier_block *nb)
{
@@ -445,6 +446,10 @@ static void dw8250_prepare_rx_dma(struct uart_8250_port *p)
static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
{
unsigned int quirks = data->pdata ? data->pdata->quirks : 0;
+ u32 cpr_value = data->pdata ? data->pdata->cpr_value : 0;
+
+ if (quirks & DW_UART_QUIRK_CPR_VALUE)
+ data->data.cpr_value = cpr_value;

#ifdef CONFIG_64BIT
if (quirks & DW_UART_QUIRK_OCTEON) {
@@ -727,8 +732,8 @@ static const struct dw8250_platform_data dw8250_armada_38x_data = {

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,
+ .cpr_value = 0x00012f32,
+ .quirks = DW_UART_QUIRK_CPR_VALUE | DW_UART_QUIRK_IS_DMA_FC,
};

static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 3e33ddf7bc80..5a2520943dfd 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -242,7 +242,6 @@ static const struct serial_rs485 dw8250_rs485_supported = {
void dw8250_setup_port(struct uart_port *p)
{
struct dw8250_port_data *pd = p->private_data;
- struct dw8250_data *data = to_dw8250_data(pd);
struct uart_8250_port *up = up_to_u8250p(p);
u32 reg, old_dlf;

@@ -278,7 +277,7 @@ void dw8250_setup_port(struct uart_port *p)

reg = dw8250_readl_ext(p, DW_UART_CPR);
if (!reg) {
- reg = data->pdata->cpr_val;
+ reg = pd->cpr_value;
dev_dbg(p->dev, "CPR is not available, using 0x%08x instead\n", reg);
}
if (!reg)
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index f13e91f2cace..794a9014cdac 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -19,6 +19,7 @@ struct dw8250_port_data {
struct uart_8250_dma dma;

/* Hardware configuration */
+ u32 cpr_value;
u8 dlf_size;

/* RS485 variables */
@@ -27,7 +28,7 @@ struct dw8250_port_data {

struct dw8250_platform_data {
u8 usr_reg;
- u32 cpr_val;
+ u32 cpr_value;
unsigned int quirks;
};

--
2.43.0.rc1.1336.g36b5255a03ac


2024-05-14 21:07:44

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/2] serial: 8250_dw: Revert "Move definitions to the shared header"

This reverts commit d9666dfb314e1ffd6eb9c3c4243fe3e094c047a7.

The container of the struct dw8250_port_data is private to the actual
driver. In particular, 8250_lpss and 8250_dw use different data types
that are assigned to the UART port private_data. Hence, it must not
be used outside the specific driver.

Fix the mistake made in the past by moving the respective definitions
to the specific driver.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_dw.c | 27 +++++++++++++++++++++++
drivers/tty/serial/8250/8250_dwlib.h | 32 ----------------------------
2 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 94d680e4b535..a58890fd53e2 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -57,6 +57,33 @@
#define DW_UART_QUIRK_APMC0D08 BIT(4)
#define DW_UART_QUIRK_CPR_VALUE BIT(5)

+struct dw8250_platform_data {
+ u8 usr_reg;
+ u32 cpr_value;
+ unsigned int quirks;
+};
+
+struct dw8250_data {
+ struct dw8250_port_data data;
+ const struct dw8250_platform_data *pdata;
+
+ int msr_mask_on;
+ int msr_mask_off;
+ struct clk *clk;
+ struct clk *pclk;
+ struct notifier_block clk_notifier;
+ struct work_struct clk_work;
+ struct reset_control *rst;
+
+ unsigned int skip_autocfg:1;
+ unsigned int uart_16550_compatible:1;
+};
+
+static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
+{
+ return container_of(data, struct dw8250_data, data);
+}
+
static inline struct dw8250_data *clk_to_dw8250_data(struct notifier_block *nb)
{
return container_of(nb, struct dw8250_data, clk_notifier);
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 794a9014cdac..7dd2a8e7b780 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -2,15 +2,10 @@
/* Synopsys DesignWare 8250 library header file. */

#include <linux/io.h>
-#include <linux/notifier.h>
#include <linux/types.h>
-#include <linux/workqueue.h>

#include "8250.h"

-struct clk;
-struct reset_control;
-
struct dw8250_port_data {
/* Port properties */
int line;
@@ -26,36 +21,9 @@ struct dw8250_port_data {
bool hw_rs485_support;
};

-struct dw8250_platform_data {
- u8 usr_reg;
- u32 cpr_value;
- unsigned int quirks;
-};
-
-struct dw8250_data {
- struct dw8250_port_data data;
- const struct dw8250_platform_data *pdata;
-
- int msr_mask_on;
- int msr_mask_off;
- struct clk *clk;
- struct clk *pclk;
- struct notifier_block clk_notifier;
- struct work_struct clk_work;
- struct reset_control *rst;
-
- unsigned int skip_autocfg:1;
- unsigned int uart_16550_compatible:1;
-};
-
void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, const struct ktermios *old);
void dw8250_setup_port(struct uart_port *p);

-static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
-{
- return container_of(data, struct dw8250_data, data);
-}
-
static inline u32 dw8250_readl_ext(struct uart_port *p, int offset)
{
if (p->iotype == UPIO_MEM32BE)
--
2.43.0.rc1.1336.g36b5255a03ac