[Re: [PATCH] serial: rda-uart: make it explicitly non-modular] On 22/04/2019 (Mon 14:02) Manivannan Sadhasivam wrote:
> Hi Paul,
>
> On Mon, Apr 22, 2019 at 03:06:57AM -0400, Paul Gortmaker wrote:
> > The Kconfig currently controlling compilation of this code is:
> >
> > drivers/tty/serial/Kconfig:config SERIAL_RDA
> > drivers/tty/serial/Kconfig: bool "RDA Micro serial port support"
> >
>
> My bad...This should be "tristate" from the beginning. Eventhough the platform
> is most likely be used with earlycon now, I don't see a harm in building it
> as a module in future.
>
> Is there any specific reason you want this to be built-in always?
I don't want it to be built-in always. I just want the code to be
consistent with the Makefile/Kconfig data in the end.
But I can't just assume everything was intended to be tristate, as that
expands the supported use case and may not be what the author wanted.
And in more complex core devices, like iommu, it may make no sense at
all. There are hundreds of these inconsistencies all over linux.
So, the default action, with limited context, has to be to remove the
dead code and not impact the runtime or the current use case matrix.
If the author and/or users at large want something converted to
tristate instead, that is totally fine with me, and at least the dead
code removal patch starts that discussion.
Will you be submitting a patch changing the Kconfig and test that it
passes the final depmod as =m (and possibly runtime testing too)?
Thanks,
Paul.
--
>
> Thanks,
> Mani
>
> > ...meaning that it currently is not being built as a module by anyone.
> >
> > Lets remove the modular code that is essentially orphaned, so that
> > when reading the driver there is no doubt it is builtin-only.
> >
> > We explicitly disallow a driver unbind, since that doesn't have a
> > sensible use case anyway, and it allows us to drop the ".remove"
> > code for non-modular drivers.
> >
> > Since module_init translates to device_initcall in the non-modular
> > case, the init ordering remains unchanged with this commit.
> >
> > We also delete the MODULE_LICENSE tag etc. since all that information
> > is already contained at the top of the file in the comments.
> >
> > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Jiri Slaby <jslaby@xxxxxxxx>
> > Cc: linux-serial@xxxxxxxxxxxxxxx
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
> >
> > diff --git a/drivers/tty/serial/rda-uart.c b/drivers/tty/serial/rda-uart.c
> > index 284623eefaeb..ce3d9c6fcb91 100644
> > --- a/drivers/tty/serial/rda-uart.c
> > +++ b/drivers/tty/serial/rda-uart.c
> > @@ -4,14 +4,15 @@
> > *
> > * Copyright RDA Microelectronics Company Limited
> > * Copyright (c) 2017 Andreas Färber
> > - * Copyright (c) 2018 Manivannan Sadhasivam
> > + * Copyright (c) 2018 Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> > + * License: GPL
> > */
> >
> > #include <linux/clk.h>
> > #include <linux/console.h>
> > #include <linux/delay.h>
> > #include <linux/io.h>
> > -#include <linux/module.h>
> > +#include <linux/init.h>
> > #include <linux/of.h>
> > #include <linux/platform_device.h>
> > #include <linux/serial.h>
> > @@ -712,7 +713,6 @@ static const struct of_device_id rda_uart_dt_matches[] = {
> > { .compatible = "rda,8810pl-uart" },
> > { }
> > };
> > -MODULE_DEVICE_TABLE(of, rda_uart_dt_matches);
> >
> > static int rda_uart_probe(struct platform_device *pdev)
> > {
> > @@ -783,21 +783,11 @@ static int rda_uart_probe(struct platform_device *pdev)
> > return ret;
> > }
> >
> > -static int rda_uart_remove(struct platform_device *pdev)
> > -{
> > - struct rda_uart_port *rda_port = platform_get_drvdata(pdev);
> > -
> > - uart_remove_one_port(&rda_uart_driver, &rda_port->port);
> > - rda_uart_ports[pdev->id] = NULL;
> > -
> > - return 0;
> > -}
> > -
> > static struct platform_driver rda_uart_platform_driver = {
> > .probe = rda_uart_probe,
> > - .remove = rda_uart_remove,
> > .driver = {
> > .name = "rda-uart",
> > + .suppress_bind_attrs = true,
> > .of_match_table = rda_uart_dt_matches,
> > },
> > };
> > @@ -816,16 +806,4 @@ static int __init rda_uart_init(void)
> >
> > return ret;
> > }
> > -
> > -static void __init rda_uart_exit(void)
> > -{
> > - platform_driver_unregister(&rda_uart_platform_driver);
> > - uart_unregister_driver(&rda_uart_driver);
> > -}
> > -
> > -module_init(rda_uart_init);
> > -module_exit(rda_uart_exit);
> > -
> > -MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>");
> > -MODULE_DESCRIPTION("RDA8810PL serial device driver");
> > -MODULE_LICENSE("GPL");
> > +device_initcall(rda_uart_init);
> > --
> > 2.7.4
> >
