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
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
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
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
> -----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
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
> -----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
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
© 2016 - 2025 Red Hat, Inc.