Re: [PATCH 06/12] tty: serial: samsung: Add Exynos850 SoC data

On 31/07/2021 01:10, Sam Protsenko wrote:

On Fri, 30 Jul 2021 at 19:05, Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 30/07/2021 16:49, Sam Protsenko wrote:

quoted

Add serial driver data for Exynos850 SoC. This driver data is basically
reusing EXYNOS_COMMON_SERIAL_DRV_DATA, which is common for all Exynos
chips, but also enables USI init, which was added in previous commit:
"tty: serial: samsung: Init USI to keep clocks running".

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
 drivers/tty/serial/samsung_tty.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 75ccbb08df4a..d059b516a0f4 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c

@@ -2814,11 +2814,19 @@ static struct s3c24xx_serial_drv_data exynos5433_serial_drv_data = {
      .fifosize = { 64, 256, 16, 256 },
 };

+static struct s3c24xx_serial_drv_data exynos850_serial_drv_data = {
+     EXYNOS_COMMON_SERIAL_DRV_DATA_USI(1),
+     .fifosize = { 0, },

This does not look correct. You rely on samsung,uart-fifosize property
but it is optional.

Good point. I will replace fifosize elements (in patch series v2) with
this code (the reasoning is below):

    .fifosize = { 256, 64, 64, 64 }

TRM mentions that USI block has configurable FIFO of 16/32/64/128/256
byte. In vendor kernel they are setting default values in dtsi instead
of driver, that's where fifosize = { 0 } appeared from. And in vendor
dtsi they set 256 for serial_0 (USI UART instance), 64 for serial_1
(CMGP0 UART instance) and 64 for serial_2 (CMGP1 UART instance). I
tested 256 and 64 for serial_0 (which is used for serial console)

As for fifosize array elements count: though it's possible to
configure up to 7 UARTs in Exynos850 (it has 5 USI blocks and 2 CMGP
blocks, which can be configured as USIs), in a regular case it's only
3 UARTs (1 in USI and 2 in CMGP). This is how it's done in vendor's
device tree, and I doubt someone is going to need more than 3 serials
anyway, looks like very specific case for a mobile SoC. But
CONFIG_SERIAL_SAMSUNG_UARTS_4=y is set by default when using arm64
defconfig, and I'd like to keep minimal delta for this defconfig for
now.

Hope you are ok with this?

Yes, sounds good.


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel