On Mon, Apr 22, 2019 at 11:15:10AM -0400, Paul Gortmaker wrote:
> [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)?
>
Yes, I will. Thanks for spotting this inconsistency!
Regards,
Mani
> 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
> > >
