Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200316101930.9962-1-zhang.lyra@gmail.com> <20200316101930.9962-4-zhang.lyra@gmail.com>
 <CADBw62rjQMfNdxyDsNCbz-v7Ebz=MFu+CTMJEJORD3VXczVhtQ@mail.gmail.com>
In-Reply-To: <CADBw62rjQMfNdxyDsNCbz-v7Ebz=MFu+CTMJEJORD3VXczVhtQ@mail.gmail.com>
From:   Chunyan Zhang <zhang.lyra@gmail.com>
Date:   Tue, 17 Mar 2020 15:18:02 +0800
Message-ID: <CAAfSe-vGjJY_+M7gSe3s4B9BnTxKq+D6TKxU5Q7e731a4Kw+pg@mail.gmail.com>
Subject: Re: [PATCH 3/3] serial: sprd: cleanup the sprd_port when return -EPROBE_DEFER
To:     Baolin Wang <baolin.wang7@gmail.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jslaby@suse.com>,
        "open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Orson Zhai <orsonzhai@gmail.com>,
        Chunyan Zhang <chunyan.zhang@unisoc.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 17 Mar 2020 at 14:22, Baolin Wang <baolin.wang7@gmail.com> wrote:
>
> On Mon, Mar 16, 2020 at 6:19 PM Chunyan Zhang <zhang.lyra@gmail.com> wrote:
> >
> > From: Chunyan Zhang <chunyan.zhang@unisoc.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@unisoc.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