Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5E937C433FE for ; Mon, 29 Nov 2021 22:23:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229732AbhK2W1L (ORCPT ); Mon, 29 Nov 2021 17:27:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58472 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232990AbhK2WXZ (ORCPT ); Mon, 29 Nov 2021 17:23:25 -0500 Received: from mail-ua1-x935.google.com (mail-ua1-x935.google.com [IPv6:2607:f8b0:4864:20::935]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DE911C09676F for ; Mon, 29 Nov 2021 12:38:59 -0800 (PST) Received: by mail-ua1-x935.google.com with SMTP id i6so36752171uae.6 for ; Mon, 29 Nov 2021 12:38:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=UONcs7N6LGxVu2k8VNIBFR8w78/QJ3sPH/s1Z5GJbro=; b=noH8p60w0Jb98Gun1YSBguriftRk73erhwNTOPAf0Xyfpd3cQ8YQLdk0dHdFCR1L/V VrbMrAhXcw1OzPppMd1vEqZMILJq4Be4I+3XY4YrVFi+ez8sbi2I10v75Hzf1oAKt7Md Q+cvz1xKXfTgGvcmKkzceKwtuG2XPov5wncs00SjT30lSAPi0nz1rgj5LYqDrXjXbwrd 7qJ2K/OlJwBdQB+TZLO+Uy7e1hft0Au/MPm+8zOCJqUVXLzTHyR5Ted8dz9vzXFYRmAv pFLYaHJJ3SzU0UlR3r5WXbyN0td2n1zWE0TOOJKSdPvYoBjSfIBXLpFKtsOyF1bjh5D1 IQuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=UONcs7N6LGxVu2k8VNIBFR8w78/QJ3sPH/s1Z5GJbro=; b=ytnNp1RGQHh8moqybXFdqsuCr3t31i7ue/L1I01kt+md2j+cglCh69xHHr7+d2+6v7 TGDyE6idIPlIjpKe0sHKzFMwdlfuePGTQkGH+Ks2PRRBXnTozRr7IKv5w7k+xNqL588+ CoO8fujev2Z/AF4/IONKEnXqATiLsqYm/kj3va/EvtfNzG4tLDh3HWCWRyfIcHksJ+ZQ +oV+pS43tEtgVciWQzDSi/156JzoURKkpz+/2wt2e/wEC3o3MfgwzgDhWhzTvugdrlMD ik4uPXana7FVoGw5jOeMhjgB/V0/VcVb1cz2TFoYsoPToyZohAK4QfZ0y0C9SelyKwkr +JpA== X-Gm-Message-State: AOAM532J2ExI2fcez0ogEKckwa8beZqP5psplmPv6NbCnrc07JDsXOT8 XasSW7fEYHjVGxHZsqBzj7mOXfE9tYBg+TU4J9Kb3g== X-Google-Smtp-Source: ABdhPJzhebEttD8OzDSAnlep6FeEbByTFuexl48l99GbNggwLSd6/VU+oEF2UMCz1+jFsh7LSdV7Dujt31QbGYN5BvA= X-Received: by 2002:a67:be0f:: with SMTP id x15mr36219046vsq.86.1638218338933; Mon, 29 Nov 2021 12:38:58 -0800 (PST) MIME-Version: 1.0 References: <20211127223253.19098-1-semen.protsenko@linaro.org> <20211127223253.19098-6-semen.protsenko@linaro.org> <9a51b37b-d2c4-fb73-bd3f-447c94a66c82@canonical.com> In-Reply-To: From: Sam Protsenko Date: Mon, 29 Nov 2021 22:38:47 +0200 Message-ID: Subject: Re: [PATCH 5/8] tty: serial: samsung: Enable console as module To: Krzysztof Kozlowski Cc: Rob Herring , Mark Brown , Greg Kroah-Hartman , Jaewon Kim , Chanho Park , David Virag , Youngmin Nam , devicetree@vger.kernel.org, linux-spi@vger.kernel.org, linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-i2c@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 29 Nov 2021 at 22:18, Sam Protsenko wrote: > > On Mon, 29 Nov 2021 at 10:52, Krzysztof Kozlowski > wrote: > > > > On 27/11/2021 23:32, Sam Protsenko wrote: > > > Enable serial driver to be built as a module. To do so, init the console > > > support on driver/module load instead of using console_initcall(). > > > > > > This is needed for proper support of USIv2 driver (which can be built as > > > a module, which in turn makes SERIAL_SAMSUNG be a module too). It also > > > might be useful for Android GKI modularization efforts. > > > > > > Inspired by commit 87a0b9f98ac5 ("tty: serial: meson: enable console as > > > module"). > > > > > > Signed-off-by: Sam Protsenko > > > --- > > > drivers/tty/serial/Kconfig | 2 +- > > > drivers/tty/serial/samsung_tty.c | 21 +++++++++++++++++++-- > > > 2 files changed, 20 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > > > index fc543ac97c13..0e5ccb25bdb1 100644 > > > --- a/drivers/tty/serial/Kconfig > > > +++ b/drivers/tty/serial/Kconfig > > > @@ -263,7 +263,7 @@ config SERIAL_SAMSUNG_UARTS > > > > > > config SERIAL_SAMSUNG_CONSOLE > > > bool "Support for console on Samsung SoC serial port" > > > - depends on SERIAL_SAMSUNG=y > > > + depends on SERIAL_SAMSUNG > > > select SERIAL_CORE_CONSOLE > > > select SERIAL_EARLYCON > > > help > > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > > > index f986a9253dc8..92a63e9392ed 100644 > > > --- a/drivers/tty/serial/samsung_tty.c > > > +++ b/drivers/tty/serial/samsung_tty.c > > > @@ -1720,10 +1720,10 @@ static int __init s3c24xx_serial_console_init(void) > > > register_console(&s3c24xx_serial_console); > > > return 0; > > > } > > > -console_initcall(s3c24xx_serial_console_init); > > > > > > #define S3C24XX_SERIAL_CONSOLE &s3c24xx_serial_console > > > #else > > > +static inline int s3c24xx_serial_console_init(void) { return 0; } > > > #define S3C24XX_SERIAL_CONSOLE NULL > > > #endif > > > > > > @@ -2898,7 +2898,24 @@ static struct platform_driver samsung_serial_driver = { > > > }, > > > }; > > > > > > -module_platform_driver(samsung_serial_driver); > > > +static int __init samsung_serial_init(void) > > > +{ > > > + int ret; > > > + > > > + ret = s3c24xx_serial_console_init(); > > > + if (ret) > > > + return ret; > > > > This will trigger warns on module re-loading, won't it? Either suppress > > unbind or cleanup in module exit. > > > > I guess that's already taken care of in samsung_serial_remove(): it's > doing uart_remove_one_port(), which in turn does unregister_console(). > So I don't think anything extra should be done on module exit. Or I'm > missing something? > > That case (unload/load) actually doesn't work well in my case: serial > console doesn't work after doing "modprobe -r samsung_tty; modprobe > samsung_tty" (but it works fine e.g. in case of i2c_exynos5 driver). > Not sure what is wrong, but I can see that my board keeps running > (heartbeat LED is still blinking). Not even sure if that use case > (unload/load) was ever functional before. > > Anyway, please let me know if you think something should be done about > this particular patch. Right now I don't see anything missing. > ...But I'll actually add proper error path handling in samsung_serial_init(), i.e. unregister console if platform_driver_register() fails. And I'll add the same console unregister in samsung_serial_exit(), just in case. > > > + > > > + return platform_driver_register(&samsung_serial_driver); > > > +} > > > + > > > +static void __exit samsung_serial_exit(void) > > > +{ > > > + platform_driver_unregister(&samsung_serial_driver); > > > +} > > > + > > > +module_init(samsung_serial_init); > > > +module_exit(samsung_serial_exit); > > > > > > #ifdef CONFIG_SERIAL_SAMSUNG_CONSOLE > > > /* > > > > > > > > > Best regards, > > Krzysztof