linux-kernel - Re: [PATCH 3/3] serial: sprd: cleanup the sprd_port when return -EPROBE

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] [day] [month] [year] [list]

Message-ID: <CAAfSe-vGjJY_+M7gSe3s4B9BnTxKq+D6TKxU5Q7e731a4Kw+pg@mail.gmail.com>
Date:   Tue, 17 Mar 2020 15:18:02 +0800
From:   Chunyan Zhang <zhang.lyra@...il.com>
To:     Baolin Wang <baolin.wang7@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Orson Zhai <orsonzhai@...il.com>,
        Chunyan Zhang <chunyan.zhang@...soc.com>
Subject: Re: [PATCH 3/3] serial: sprd: cleanup the sprd_port when return -EPROBE_DEFER

On Tue, 17 Mar 2020 at 14:22, Baolin Wang <baolin.wang7@...il.com> wrote:
>
> On Mon, Mar 16, 2020 at 6:19 PM Chunyan Zhang <zhang.lyra@...il.com> wrote:
> >
> > From: Chunyan Zhang <chunyan.zhang@...soc.com>
> >
> > cleanup the sprd_port for the device when its .probe() would be called
> > later, and then alloc memory for its sprd_port again.
> >
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@...soc.com>
> > ---
> >  drivers/tty/serial/sprd_serial.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> > index 914862844790..9917d7240172 100644
> > --- a/drivers/tty/serial/sprd_serial.c
> > +++ b/drivers/tty/serial/sprd_serial.c
> > @@ -1230,8 +1230,13 @@ static int sprd_probe(struct platform_device *pdev)
> >         up->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SPRD_CONSOLE);
> >
> >         ret = sprd_clk_init(up);
> > -       if (ret)
> > +       if (ret) {
> > +               if (ret == -EPROBE_DEFER) {
>
> I think we can remove the condition and devm_kfree() here, just
> simplify the code as below?
> if (ret) {
>        sprd_port[index] = NULL;
>        return ret;
> }

Humm.. I admit that here's a little confused.
Let assume we don't have aliases for serial nodes,  and we set both
earlycon and console via bootargs, what will happen?
We can simplify the code like above and we have to ensure all
platforms have serial aliases in their devicetree, then we also can
simplify the process of getting port index with of_alias_get_id()
only.
We can discuss further offline, and also save community resource :)

Thanks,
Chunyan

>
> > +                       devm_kfree(&pdev->dev, sprd_port[index]);
> > +                       sprd_port[index] = NULL;
> > +               }
> >                 return ret;
> > +       }
> >
> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >         up->membase = devm_ioremap_resource(&pdev->dev, res);
> > --
> > 2.20.1
> >
>
>
> --
> Baolin Wang