[PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code

Ravi Patel posted 3 patches 3 weeks ago
[PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
Posted by Ravi Patel 3 weeks ago
Since ARTPEC-8 is using exynos8895 driver data, remove the unused
artpec-8 specific driver data.

ARTPEC-8 is using exynos4210 for earlycon, so earlycon code
for ARTPEC-8 is also not required.

Signed-off-by: Ravi Patel <ravi.patel@samsung.com>
---
NOTE: This is exactly the revert of the below commit.
commit 1db536f95d0264a2b83fb032d5b057ba0113e622
Author: Vincent Whitchurch <vincent.whitchurch@axis.com>
Date:   Fri Mar 11 10:45:15 2022 +0100

tty: serial: samsung: Add ARTPEC-8 support
---
 drivers/tty/serial/Kconfig       |  2 +-
 drivers/tty/serial/samsung_tty.c | 38 --------------------------------
 2 files changed, 1 insertion(+), 39 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 44427415a80d..6f4d6f44d997 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -219,7 +219,7 @@ config SERIAL_CLPS711X_CONSOLE

 config SERIAL_SAMSUNG
 	tristate "Samsung SoC serial support"
-	depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || ARCH_APPLE || ARCH_ARTPEC || COMPILE_TEST
+	depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || ARCH_APPLE || COMPILE_TEST
 	select SERIAL_CORE
 	help
 	  Support for the on-chip UARTs on the Samsung
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 2fb58c626daf..322ab280a59e 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -2577,37 +2577,6 @@ static const struct s3c24xx_serial_drv_data s5l_serial_drv_data = {
 #define S5L_SERIAL_DRV_DATA NULL
 #endif

-#if defined(CONFIG_ARCH_ARTPEC)
-static const struct s3c24xx_serial_drv_data artpec8_serial_drv_data = {
-	.info = {
-		.name		= "Axis ARTPEC-8 UART",
-		.type		= TYPE_S3C6400,
-		.port_type	= PORT_S3C6400,
-		.iotype		= UPIO_MEM,
-		.fifosize	= 64,
-		.has_divslot	= true,
-		.rx_fifomask	= S5PV210_UFSTAT_RXMASK,
-		.rx_fifoshift	= S5PV210_UFSTAT_RXSHIFT,
-		.rx_fifofull	= S5PV210_UFSTAT_RXFULL,
-		.tx_fifofull	= S5PV210_UFSTAT_TXFULL,
-		.tx_fifomask	= S5PV210_UFSTAT_TXMASK,
-		.tx_fifoshift	= S5PV210_UFSTAT_TXSHIFT,
-		.def_clk_sel	= S3C2410_UCON_CLKSEL0,
-		.num_clks	= 1,
-		.clksel_mask	= 0,
-		.clksel_shift	= 0,
-	},
-	.def_cfg = {
-		.ucon		= S5PV210_UCON_DEFAULT,
-		.ufcon		= S5PV210_UFCON_DEFAULT,
-		.has_fracval	= 1,
-	}
-};
-#define ARTPEC8_SERIAL_DRV_DATA (&artpec8_serial_drv_data)
-#else
-#define ARTPEC8_SERIAL_DRV_DATA (NULL)
-#endif
-
 static const struct platform_device_id s3c24xx_serial_driver_ids[] = {
 	{
 		.name		= "s3c6400-uart",
@@ -2627,9 +2596,6 @@ static const struct platform_device_id s3c24xx_serial_driver_ids[] = {
 	}, {
 		.name		= "exynos850-uart",
 		.driver_data	= (kernel_ulong_t)EXYNOS850_SERIAL_DRV_DATA,
-	}, {
-		.name		= "artpec8-uart",
-		.driver_data	= (kernel_ulong_t)ARTPEC8_SERIAL_DRV_DATA,
 	}, {
 		.name		= "gs101-uart",
 		.driver_data	= (kernel_ulong_t)GS101_SERIAL_DRV_DATA,
@@ -2655,8 +2621,6 @@ static const struct of_device_id s3c24xx_uart_dt_match[] = {
 		.data = S5L_SERIAL_DRV_DATA },
 	{ .compatible = "samsung,exynos850-uart",
 		.data = EXYNOS850_SERIAL_DRV_DATA },
-	{ .compatible = "axis,artpec8-uart",
-		.data = ARTPEC8_SERIAL_DRV_DATA },
 	{ .compatible = "google,gs101-uart",
 		.data = GS101_SERIAL_DRV_DATA },
 	{ .compatible = "samsung,exynos8895-uart",
@@ -2828,8 +2792,6 @@ OF_EARLYCON_DECLARE(s5pv210, "samsung,s5pv210-uart",
 			s5pv210_early_console_setup);
 OF_EARLYCON_DECLARE(exynos4210, "samsung,exynos4210-uart",
 			s5pv210_early_console_setup);
-OF_EARLYCON_DECLARE(artpec8, "axis,artpec8-uart",
-			s5pv210_early_console_setup);

 static int __init gs101_early_console_setup(struct earlycon_device *device,
 					    const char *opt)
--
2.17.1
Re: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
Posted by Geert Uytterhoeven 3 weeks ago
Hi Ravi,

On Thu, 11 Sept 2025 at 16:17, Ravi Patel <ravi.patel@samsung.com> wrote:
> Since ARTPEC-8 is using exynos8895 driver data, remove the unused
> artpec-8 specific driver data.
>
> ARTPEC-8 is using exynos4210 for earlycon, so earlycon code
> for ARTPEC-8 is also not required.
>
> Signed-off-by: Ravi Patel <ravi.patel@samsung.com>

Thanks for your patch!

> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c

> @@ -2655,8 +2621,6 @@ static const struct of_device_id s3c24xx_uart_dt_match[] = {
>                 .data = S5L_SERIAL_DRV_DATA },
>         { .compatible = "samsung,exynos850-uart",
>                 .data = EXYNOS850_SERIAL_DRV_DATA },
> -       { .compatible = "axis,artpec8-uart",
> -               .data = ARTPEC8_SERIAL_DRV_DATA },
>         { .compatible = "google,gs101-uart",
>                 .data = GS101_SERIAL_DRV_DATA },
>         { .compatible = "samsung,exynos8895-uart",
> @@ -2828,8 +2792,6 @@ OF_EARLYCON_DECLARE(s5pv210, "samsung,s5pv210-uart",
>                         s5pv210_early_console_setup);
>  OF_EARLYCON_DECLARE(exynos4210, "samsung,exynos4210-uart",
>                         s5pv210_early_console_setup);
> -OF_EARLYCON_DECLARE(artpec8, "axis,artpec8-uart",
> -                       s5pv210_early_console_setup);
>
>  static int __init gs101_early_console_setup(struct earlycon_device *device,
>                                             const char *opt)

Removing these breaks backwards-compatibility with existing DTBs,
which lack the new "samsung,exynos8895-uart" fallback compatible value.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
Posted by Krzysztof Kozlowski 3 weeks ago
On 11/09/2025 16:27, Geert Uytterhoeven wrote:
> Hi Ravi,
> 
> On Thu, 11 Sept 2025 at 16:17, Ravi Patel <ravi.patel@samsung.com> wrote:
>> Since ARTPEC-8 is using exynos8895 driver data, remove the unused
>> artpec-8 specific driver data.
>>
>> ARTPEC-8 is using exynos4210 for earlycon, so earlycon code
>> for ARTPEC-8 is also not required.
>>
>> Signed-off-by: Ravi Patel <ravi.patel@samsung.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/tty/serial/samsung_tty.c
>> +++ b/drivers/tty/serial/samsung_tty.c
> 
>> @@ -2655,8 +2621,6 @@ static const struct of_device_id s3c24xx_uart_dt_match[] = {
>>                 .data = S5L_SERIAL_DRV_DATA },
>>         { .compatible = "samsung,exynos850-uart",
>>                 .data = EXYNOS850_SERIAL_DRV_DATA },
>> -       { .compatible = "axis,artpec8-uart",
>> -               .data = ARTPEC8_SERIAL_DRV_DATA },
>>         { .compatible = "google,gs101-uart",
>>                 .data = GS101_SERIAL_DRV_DATA },
>>         { .compatible = "samsung,exynos8895-uart",
>> @@ -2828,8 +2792,6 @@ OF_EARLYCON_DECLARE(s5pv210, "samsung,s5pv210-uart",
>>                         s5pv210_early_console_setup);
>>  OF_EARLYCON_DECLARE(exynos4210, "samsung,exynos4210-uart",
>>                         s5pv210_early_console_setup);
>> -OF_EARLYCON_DECLARE(artpec8, "axis,artpec8-uart",
>> -                       s5pv210_early_console_setup);
>>
>>  static int __init gs101_early_console_setup(struct earlycon_device *device,
>>                                             const char *opt)
> 
> Removing these breaks backwards-compatibility with existing DTBs,
> which lack the new "samsung,exynos8895-uart" fallback compatible value.

This was just applied, so ABI break would be fine. It should be however
clearly expressed in the commit msg.

I have a feeling that not much testing was happening in Samsung around
this patchset and only now - after I applied it - some things happen.
But it is damn too late, my tree is already closed which means this is
going to be the ABI.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds


Best regards,
Krzysztof
Re: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
Posted by Krzysztof Kozlowski 3 weeks ago
On 11/09/2025 16:29, Krzysztof Kozlowski wrote:
>>> --- a/drivers/tty/serial/samsung_tty.c
>>> +++ b/drivers/tty/serial/samsung_tty.c
>>
>>> @@ -2655,8 +2621,6 @@ static const struct of_device_id s3c24xx_uart_dt_match[] = {
>>>                 .data = S5L_SERIAL_DRV_DATA },
>>>         { .compatible = "samsung,exynos850-uart",
>>>                 .data = EXYNOS850_SERIAL_DRV_DATA },
>>> -       { .compatible = "axis,artpec8-uart",
>>> -               .data = ARTPEC8_SERIAL_DRV_DATA },
>>>         { .compatible = "google,gs101-uart",
>>>                 .data = GS101_SERIAL_DRV_DATA },
>>>         { .compatible = "samsung,exynos8895-uart",
>>> @@ -2828,8 +2792,6 @@ OF_EARLYCON_DECLARE(s5pv210, "samsung,s5pv210-uart",
>>>                         s5pv210_early_console_setup);
>>>  OF_EARLYCON_DECLARE(exynos4210, "samsung,exynos4210-uart",
>>>                         s5pv210_early_console_setup);
>>> -OF_EARLYCON_DECLARE(artpec8, "axis,artpec8-uart",
>>> -                       s5pv210_early_console_setup);
>>>
>>>  static int __init gs101_early_console_setup(struct earlycon_device *device,
>>>                                             const char *opt)
>>
>> Removing these breaks backwards-compatibility with existing DTBs,
>> which lack the new "samsung,exynos8895-uart" fallback compatible value.
> 
> This was just applied, so ABI break would be fine. It should be however
> clearly expressed in the commit msg.
> 
> I have a feeling that not much testing was happening in Samsung around
> this patchset and only now - after I applied it - some things happen.
> But it is damn too late, my tree is already closed which means this is
> going to be the ABI.

Ah, no, I mixed up patches with recent DTS for Artpec-8. This serial ABI
was accepted three years ago (!!!), so you are Geert absolutely right -
that's ABI break.

Folks in Samsung, maybe try to organize some weekly sessions sharing
knowledge after upstreaming reviews/feedbacks? I feel like you are
repeating same mistakes.

Best regards,
Krzysztof
RE: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
Posted by Ravi Patel 3 weeks ago

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 11 September 2025 20:05
...
> Subject: Re: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
> 
> On 11/09/2025 16:29, Krzysztof Kozlowski wrote:
> >>> --- a/drivers/tty/serial/samsung_tty.c
> >>> +++ b/drivers/tty/serial/samsung_tty.c
> >>
> >>> @@ -2655,8 +2621,6 @@ static const struct of_device_id s3c24xx_uart_dt_match[] = {
> >>>                 .data = S5L_SERIAL_DRV_DATA },
> >>>         { .compatible = "samsung,exynos850-uart",
> >>>                 .data = EXYNOS850_SERIAL_DRV_DATA },
> >>> -       { .compatible = "axis,artpec8-uart",
> >>> -               .data = ARTPEC8_SERIAL_DRV_DATA },
> >>>         { .compatible = "google,gs101-uart",
> >>>                 .data = GS101_SERIAL_DRV_DATA },
> >>>         { .compatible = "samsung,exynos8895-uart",
> >>> @@ -2828,8 +2792,6 @@ OF_EARLYCON_DECLARE(s5pv210, "samsung,s5pv210-uart",
> >>>                         s5pv210_early_console_setup);
> >>>  OF_EARLYCON_DECLARE(exynos4210, "samsung,exynos4210-uart",
> >>>                         s5pv210_early_console_setup);
> >>> -OF_EARLYCON_DECLARE(artpec8, "axis,artpec8-uart",
> >>> -                       s5pv210_early_console_setup);
> >>>
> >>>  static int __init gs101_early_console_setup(struct earlycon_device *device,
> >>>                                             const char *opt)
> >>
> >> Removing these breaks backwards-compatibility with existing DTBs,
> >> which lack the new "samsung,exynos8895-uart" fallback compatible value.
> >
> > This was just applied, so ABI break would be fine. It should be however
> > clearly expressed in the commit msg.
> >
> > I have a feeling that not much testing was happening in Samsung around
> > this patchset and only now - after I applied it - some things happen.
> > But it is damn too late, my tree is already closed which means this is
> > going to be the ABI.
> 
> Ah, no, I mixed up patches with recent DTS for Artpec-8. This serial ABI
> was accepted three years ago (!!!), so you are Geert absolutely right -
> that's ABI break.

Thank you for your review.

The DTS patches for ARTPEC-8 is added recently (https://lore.kernel.org/linux-samsung-soc/20250901051926.59970-1-ravi.patel@samsung.com/)
Before that, there was no user (in DT) of "axis,artpec8-uart" compatible.
So I am not convinced of ABI break (considering patch #1 and #2 goes first with review comment fixes)

My intension here is to optimize the driver code (by removing kind of duplicated code)
And also preparation of upcoming ARTPEC-9 patch series where ARTPEC-9 uart is
exactly same as ARTPEC-8.

Please let me know if I misunderstood anything here.

Thanks,
Ravi

> 
> Folks in Samsung, maybe try to organize some weekly sessions sharing
> knowledge after upstreaming reviews/feedbacks? I feel like you are
> repeating same mistakes.
> 
> Best regards,
> Krzysztof
Re: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
Posted by Krzysztof Kozlowski 3 weeks ago
On 11/09/2025 18:04, Ravi Patel wrote:
>>>>> -OF_EARLYCON_DECLARE(artpec8, "axis,artpec8-uart",
>>>>> -                       s5pv210_early_console_setup);
>>>>>
>>>>>  static int __init gs101_early_console_setup(struct earlycon_device *device,
>>>>>                                             const char *opt)
>>>>
>>>> Removing these breaks backwards-compatibility with existing DTBs,
>>>> which lack the new "samsung,exynos8895-uart" fallback compatible value.
>>>
>>> This was just applied, so ABI break would be fine. It should be however
>>> clearly expressed in the commit msg.
>>>
>>> I have a feeling that not much testing was happening in Samsung around
>>> this patchset and only now - after I applied it - some things happen.
>>> But it is damn too late, my tree is already closed which means this is
>>> going to be the ABI.
>>
>> Ah, no, I mixed up patches with recent DTS for Artpec-8. This serial ABI
>> was accepted three years ago (!!!), so you are Geert absolutely right -
>> that's ABI break.
> 
> Thank you for your review.
> 
> The DTS patches for ARTPEC-8 is added recently (https://lore.kernel.org/linux-samsung-soc/20250901051926.59970-1-ravi.patel@samsung.com/)
> Before that, there was no user (in DT) of "axis,artpec8-uart" compatible.
> So I am not convinced of ABI break (considering patch #1 and #2 goes first with review comment fixes)


ABI is defined by bindings and implemented by kernel. Having DTS user is
irrelevant to fact whether ABI is or is not broken.

Having DTS user determines the known impact of known ABI breakage.

Best regards,
Krzysztof
RE: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
Posted by Ravi Patel 2 weeks, 6 days ago

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 11 September 2025 22:52
> To: Ravi Patel <ravi.patel@samsung.com>; 'Geert Uytterhoeven' <geert@linux-m68k.org>
> Cc: gregkh@linuxfoundation.org; jirislaby@kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> jesper.nilsson@axis.com; lars.persson@axis.com; alim.akhtar@samsung.com; arnd@kernel.org; andriy.shevchenko@linux.intel.com;
> geert+renesas@glider.be; thierry.bultel.yh@bp.renesas.com; dianders@chromium.org; robert.marko@sartura.hr; schnelle@linux.ibm.com;
> kkartik@nvidia.com; linux-kernel@vger.kernel.org; linux-serial@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-arm-kernel@axis.com; ksk4725@coasia.com; kenkim@coasia.com;
> smn1196@coasia.com; pjsin865@coasia.com; shradha.t@samsung.com
> Subject: Re: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
> 
> On 11/09/2025 18:04, Ravi Patel wrote:
> >>>>> -OF_EARLYCON_DECLARE(artpec8, "axis,artpec8-uart",
> >>>>> -                       s5pv210_early_console_setup);
> >>>>>
> >>>>>  static int __init gs101_early_console_setup(struct earlycon_device *device,
> >>>>>                                             const char *opt)
> >>>>
> >>>> Removing these breaks backwards-compatibility with existing DTBs,
> >>>> which lack the new "samsung,exynos8895-uart" fallback compatible value.
> >>>
> >>> This was just applied, so ABI break would be fine. It should be however
> >>> clearly expressed in the commit msg.
> >>>
> >>> I have a feeling that not much testing was happening in Samsung around
> >>> this patchset and only now - after I applied it - some things happen.
> >>> But it is damn too late, my tree is already closed which means this is
> >>> going to be the ABI.
> >>
> >> Ah, no, I mixed up patches with recent DTS for Artpec-8. This serial ABI
> >> was accepted three years ago (!!!), so you are Geert absolutely right -
> >> that's ABI break.
> >
> > Thank you for your review.
> >
> > The DTS patches for ARTPEC-8 is added recently (https://lore.kernel.org/linux-samsung-soc/20250901051926.59970-1-
> ravi.patel@samsung.com/)
> > Before that, there was no user (in DT) of "axis,artpec8-uart" compatible.
> > So I am not convinced of ABI break (considering patch #1 and #2 goes first with review comment fixes)
> 
> 
> ABI is defined by bindings and implemented by kernel. Having DTS user is
> irrelevant to fact whether ABI is or is not broken.
> 
> Having DTS user determines the known impact of known ABI breakage.

OK. So does that mean if someone adds the ABI then it cannot be reverted,
because of it breaks backword compatibility (users are using ABI in their local DTB) ?

Please suggest what should be the proper way.

Thanks,
Ravi

> 
> Best regards,
> Krzysztof
Re: [PATCH 3/3] tty: serial: samsung: Remove unused artpec-8 specific code
Posted by Krzysztof Kozlowski 2 weeks, 6 days ago
On 12/09/2025 07:19, Ravi Patel wrote:
>>>> Ah, no, I mixed up patches with recent DTS for Artpec-8. This serial ABI
>>>> was accepted three years ago (!!!), so you are Geert absolutely right -
>>>> that's ABI break.
>>>
>>> Thank you for your review.
>>>
>>> The DTS patches for ARTPEC-8 is added recently (https://lore.kernel.org/linux-samsung-soc/20250901051926.59970-1-
>> ravi.patel@samsung.com/)
>>> Before that, there was no user (in DT) of "axis,artpec8-uart" compatible.
>>> So I am not convinced of ABI break (considering patch #1 and #2 goes first with review comment fixes)
>>
>>
>> ABI is defined by bindings and implemented by kernel. Having DTS user is
>> irrelevant to fact whether ABI is or is not broken.
>>
>> Having DTS user determines the known impact of known ABI breakage.
> 
> OK. So does that mean if someone adds the ABI then it cannot be reverted,
> because of it breaks backword compatibility (users are using ABI in their local DTB) ?
> 
> Please suggest what should be the proper way.


Three years ago you (as in plural) submitted the Artpec-8 bindings and
driver with purpose. I think it was also claimed that DTS might come or
might not come ever, so the purpose was to supported out of tree users.

This means you made a contract in the kernel for that support.

You cannot change that ABI, because otherwise the contract you made
three years ago meant nothing.

You need to keep full backwards compatibility in the kernel. Look at
mailing list or git log how others dealt with it.

Best regards,
Krzysztof