diff mbox series

[v2,RESEND,3/5] tty: serial: samsung: Remove USI initialization

Message ID 20211130111325.29328-4-semen.protsenko@linaro.org (mailing list archive)
State New, archived
Headers show
Series soc: samsung: Add USI driver | expand

Commit Message

Sam Protsenko Nov. 30, 2021, 11:13 a.m. UTC
USI control is now extracted to dedicated USI driver. Remove USI related
code from serial driver to avoid conflicts and code duplication.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
  - (none)

 drivers/tty/serial/samsung_tty.c | 36 ++++----------------------------
 include/linux/serial_s3c.h       |  9 --------
 2 files changed, 4 insertions(+), 41 deletions(-)

Comments

Andy Shevchenko Dec. 1, 2021, 10:54 a.m. UTC | #1
On Wed, Dec 1, 2021 at 12:42 AM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
>
> USI control is now extracted to dedicated USI driver. Remove USI related

the dedicated

> code from serial driver to avoid conflicts and code duplication.

Would it break run-time bisectability?
If so, why is it not a problem?
Sam Protsenko Dec. 3, 2021, 4:22 p.m. UTC | #2
On Wed, 1 Dec 2021 at 12:54, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Wed, Dec 1, 2021 at 12:42 AM Sam Protsenko
> <semen.protsenko@linaro.org> wrote:
> >
> > USI control is now extracted to dedicated USI driver. Remove USI related
>
> the dedicated
>
> > code from serial driver to avoid conflicts and code duplication.
>
> Would it break run-time bisectability?
> If so, why is it not a problem?
>

It shouldn't. This patch is [3/5], and USI driver (which takes the
control over the USI registers) is [2/5]. As for Device Tree, the only
platform using "samsung,exynos850-uart" right now is Exynos Auto V9
SADK (serial node is declared in exynosautov9.dtsi). I don't have
Exynos Auto V9 datasheet, so I can't really add the USI node properly
there, nor I can test that. I guess it should be done separately from
this patch series.

Chanho, Krzysztof:

Guys, what are your thoughts on this? Basically with this patch series
applied, Exynos Auto V9 serial might become not functional. New USI
node should be added for UART case in Exynos Auto V9 dtsi (providing
correct sysreg, SW_CONF offset, clocks, etc), and serial node should
be encapsulated inside of that USI node. Also, USI node should be
referenced and enabled in SADK dts, providing also "clkreq-on"
property. More details can be found in [PATCH 1/5]. Do you think it's
ok to take this series as is, and add that later? Because otherwise we
might need to collaborate to add that Exynos Auto V9 enablement into
this patch series, which might take more time...

Thanks!

>
> --
> With Best Regards,
> Andy Shevchenko
Krzysztof Kozlowski Dec. 4, 2021, 11:22 a.m. UTC | #3
On 03/12/2021 17:22, Sam Protsenko wrote:
> On Wed, 1 Dec 2021 at 12:54, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>
>> On Wed, Dec 1, 2021 at 12:42 AM Sam Protsenko
>> <semen.protsenko@linaro.org> wrote:
>>>
>>> USI control is now extracted to dedicated USI driver. Remove USI related
>>
>> the dedicated
>>
>>> code from serial driver to avoid conflicts and code duplication.
>>
>> Would it break run-time bisectability?
>> If so, why is it not a problem?
>>
> 
> It shouldn't. This patch is [3/5], and USI driver (which takes the
> control over the USI registers) is [2/5]. As for Device Tree, the only
> platform using "samsung,exynos850-uart" right now is Exynos Auto V9
> SADK (serial node is declared in exynosautov9.dtsi). I don't have
> Exynos Auto V9 datasheet, so I can't really add the USI node properly
> there, nor I can test that. I guess it should be done separately from
> this patch series.
> 
> Chanho, Krzysztof:
> 
> Guys, what are your thoughts on this? Basically with this patch series
> applied, Exynos Auto V9 serial might become not functional. New USI
> node should be added for UART case in Exynos Auto V9 dtsi (providing
> correct sysreg, SW_CONF offset, clocks, etc), and serial node should
> be encapsulated inside of that USI node. Also, USI node should be
> referenced and enabled in SADK dts, providing also "clkreq-on"
> property. More details can be found in [PATCH 1/5]. Do you think it's
> ok to take this series as is, and add that later? Because otherwise we
> might need to collaborate to add that Exynos Auto V9 enablement into
> this patch series, which might take more time...

The patch in current state will probably break Exynos Auto v9 boards,
including the in-tree one, unless bootloader sets the USI to serial. The
trouble is that. Changing the Exynos Auto v9 DTSI in these series would
solve it only partially, because the kernel still won't be bisectable.

Breaking Auto v9 serial within a kernel is okay for me, because the
board was added recently, I don't expect products using it and it is
still development phase. This of course assuming that it's users agree,
so the question is to Chanho and other folks.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index ca084c10d0bb..f986a9253dc8 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -65,7 +65,6 @@  enum s3c24xx_port_type {
 struct s3c24xx_uart_info {
 	char			*name;
 	enum s3c24xx_port_type	type;
-	bool			has_usi;
 	unsigned int		port_type;
 	unsigned int		fifosize;
 	unsigned long		rx_fifomask;
@@ -1357,28 +1356,6 @@  static int apple_s5l_serial_startup(struct uart_port *port)
 	return ret;
 }
 
-static void exynos_usi_init(struct uart_port *port)
-{
-	struct s3c24xx_uart_port *ourport = to_ourport(port);
-	struct s3c24xx_uart_info *info = ourport->info;
-	unsigned int val;
-
-	if (!info->has_usi)
-		return;
-
-	/* Clear the software reset of USI block (it's set at startup) */
-	val = rd_regl(port, USI_CON);
-	val &= ~USI_CON_RESET_MASK;
-	wr_regl(port, USI_CON, val);
-	udelay(1);
-
-	/* Continuously provide the clock to USI IP w/o gating (for Rx mode) */
-	val = rd_regl(port, USI_OPTION);
-	val &= ~USI_OPTION_HWACG_MASK;
-	val |= USI_OPTION_HWACG_CLKREQ_ON;
-	wr_regl(port, USI_OPTION, val);
-}
-
 /* power power management control */
 
 static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level,
@@ -1405,8 +1382,6 @@  static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level,
 
 		if (!IS_ERR(ourport->baudclk))
 			clk_prepare_enable(ourport->baudclk);
-
-		exynos_usi_init(port);
 		break;
 	default:
 		dev_err(port->dev, "s3c24xx_serial: unknown pm %d\n", level);
@@ -2130,8 +2105,6 @@  static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
 	if (ret)
 		pr_warn("uart: failed to enable baudclk\n");
 
-	exynos_usi_init(port);
-
 	/* Keep all interrupts masked and cleared */
 	switch (ourport->info->type) {
 	case TYPE_S3C6400:
@@ -2780,11 +2753,10 @@  static struct s3c24xx_serial_drv_data s5pv210_serial_drv_data = {
 #endif
 
 #if defined(CONFIG_ARCH_EXYNOS)
-#define EXYNOS_COMMON_SERIAL_DRV_DATA(_has_usi)			\
+#define EXYNOS_COMMON_SERIAL_DRV_DATA()				\
 	.info = &(struct s3c24xx_uart_info) {			\
 		.name		= "Samsung Exynos UART",	\
 		.type		= TYPE_S3C6400,			\
-		.has_usi	= _has_usi,			\
 		.port_type	= PORT_S3C6400,			\
 		.has_divslot	= 1,				\
 		.rx_fifomask	= S5PV210_UFSTAT_RXMASK,	\
@@ -2805,17 +2777,17 @@  static struct s3c24xx_serial_drv_data s5pv210_serial_drv_data = {
 	}							\
 
 static struct s3c24xx_serial_drv_data exynos4210_serial_drv_data = {
-	EXYNOS_COMMON_SERIAL_DRV_DATA(false),
+	EXYNOS_COMMON_SERIAL_DRV_DATA(),
 	.fifosize = { 256, 64, 16, 16 },
 };
 
 static struct s3c24xx_serial_drv_data exynos5433_serial_drv_data = {
-	EXYNOS_COMMON_SERIAL_DRV_DATA(false),
+	EXYNOS_COMMON_SERIAL_DRV_DATA(),
 	.fifosize = { 64, 256, 16, 256 },
 };
 
 static struct s3c24xx_serial_drv_data exynos850_serial_drv_data = {
-	EXYNOS_COMMON_SERIAL_DRV_DATA(true),
+	EXYNOS_COMMON_SERIAL_DRV_DATA(),
 	.fifosize = { 256, 64, 64, 64 },
 };
 
diff --git a/include/linux/serial_s3c.h b/include/linux/serial_s3c.h
index cf0de4a86640..f6c3323fc4c5 100644
--- a/include/linux/serial_s3c.h
+++ b/include/linux/serial_s3c.h
@@ -27,15 +27,6 @@ 
 #define S3C2410_UERSTAT	  (0x14)
 #define S3C2410_UFSTAT	  (0x18)
 #define S3C2410_UMSTAT	  (0x1C)
-#define USI_CON		  (0xC4)
-#define USI_OPTION	  (0xC8)
-
-#define USI_CON_RESET			(1<<0)
-#define USI_CON_RESET_MASK		(1<<0)
-
-#define USI_OPTION_HWACG_CLKREQ_ON	(1<<1)
-#define USI_OPTION_HWACG_CLKSTOP_ON	(1<<2)
-#define USI_OPTION_HWACG_MASK		(3<<1)
 
 #define S3C2410_LCON_CFGMASK	  ((0xF<<3)|(0x3))