linux-kernel - Re: [PATCH v11 5/5] drivers/tty/serial: add LiteUART driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdVzUJwV=E-AssEHHH1GQuXcHHBUGAJZ-+GwNyPFN_KL+A@mail.gmail.com>
Date:   Fri, 25 Sep 2020 15:40:59 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Mateusz Holenko <mholenko@...micro.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        Stafford Horne <shorne@...il.com>,
        Karol Gugala <kgugala@...micro.com>,
        Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        Filip Kokosinski <fkokosinski@...micro.com>,
        Pawel Czarnecki <pczarnecki@...ernships.antmicro.com>,
        Joel Stanley <joel@....id.au>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Maxime Ripard <mripard@...nel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Heiko Stuebner <heiko@...ech.de>,
        Sam Ravnborg <sam@...nborg.org>,
        Icenowy Zheng <icenowy@...c.io>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Gabriel L. Somlo" <gsomlo@...il.com>
Subject: Re: [PATCH v11 5/5] drivers/tty/serial: add LiteUART driver

Hi Mateusz,

On Wed, Sep 23, 2020 at 12:12 PM Mateusz Holenko <mholenko@...micro.com> wrote:
> From: Filip Kokosinski <fkokosinski@...micro.com>
>
> This commit adds driver for the FPGA-based LiteUART serial controller
> from LiteX SoC builder.
>
> The current implementation supports LiteUART configured
> for 32 bit data width and 8 bit CSR bus width.
>
> It does not support IRQ.
>
> Signed-off-by: Filip Kokosinski <fkokosinski@...micro.com>
> Signed-off-by: Mateusz Holenko <mholenko@...micro.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/tty/serial/liteuart.c

> +static int liteuart_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct liteuart_port *uart;
> +       struct uart_port *port;
> +       struct xa_limit limit;
> +       int dev_id, ret;
> +
> +       /* no device tree */
> +       if (!np)
> +               return -ENODEV;
> +
> +       /* look for aliases; auto-enumerate for free index if not found */
> +       dev_id = of_alias_get_id(np, "serial");
> +       if (dev_id < 0)
> +               limit = XA_LIMIT(0, CONFIG_SERIAL_LITEUART_MAX_PORTS);
> +       else
> +               limit = XA_LIMIT(dev_id, dev_id);
> +
> +       uart = kzalloc(sizeof(struct liteuart_port), GFP_KERNEL);

Who frees this memory? Use devm_kzalloc()?

> +       if (!uart)
> +               return -ENOMEM;
> +
> +       ret = xa_alloc(&liteuart_array, &dev_id, uart, limit, GFP_KERNEL);

Who frees this entry?

> +       if (ret)
> +               return ret;
> +
> +       port = &uart->port;
> +
> +       /* get membase */
> +       port->membase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> +       if (!port->membase)
> +               return -ENXIO;
> +
> +       /* values not from device tree */
> +       port->dev = &pdev->dev;
> +       port->iotype = UPIO_MEM;
> +       port->flags = UPF_BOOT_AUTOCONF;
> +       port->ops = &liteuart_ops;
> +       port->regshift = 2;
> +       port->fifosize = 16;
> +       port->iobase = 1;
> +       port->type = PORT_UNKNOWN;
> +       port->line = dev_id;
> +       spin_lock_init(&port->lock);
> +
> +       return uart_add_one_port(&liteuart_driver, &uart->port);
> +}

> +static int __init liteuart_init(void)
> +{
> +       int res;
> +
> +       res = uart_register_driver(&liteuart_driver);
> +       if (res)
> +               return res;
> +
> +       res = platform_driver_register(&liteuart_platform_driver);
> +       if (res) {
> +               uart_unregister_driver(&liteuart_driver);
> +               return res;
> +       }
> +
> +       return 0;
> +}
> +
> +static void __exit liteuart_exit(void)
> +{
> +       platform_driver_unregister(&liteuart_platform_driver);
> +       uart_unregister_driver(&liteuart_driver);
> +}
> +
> +module_init(liteuart_init);
> +module_exit(liteuart_exit);

Several drivers call uart_{,un}register_driver() from their .probe()
resp. .remove() callbacks, so they can use module_platform_driver()
instead of the above boilerplate.  Greg, what's your stance on that?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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