Re: [PATCH 1/2] x86: queensbay: Optimize disable_igd() a little

[Bin Meng <bmeng.cn@gmail.com>]

Hi Simon,

On Mon, Aug 2, 2021 at 3:19 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Sat, 31 Jul 2021 at 07:17, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Initialize igd and sdvo to NULL so that we don't need to test the
> > return value of dm_pci_bus_find_bdf() later.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  arch/x86/cpu/queensbay/tnc.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/cpu/queensbay/tnc.c b/arch/x86/cpu/queensbay/tnc.c
> > index 782ed863fe..e38c0198e4 100644
> > --- a/arch/x86/cpu/queensbay/tnc.c
> > +++ b/arch/x86/cpu/queensbay/tnc.c
> > @@ -18,18 +18,15 @@
> >
> >  static int __maybe_unused disable_igd(void)
> >  {
> > -       struct udevice *igd, *sdvo;
> > +       struct udevice *igd = NULL;
> > +       struct udevice *sdvo = NULL;
> >         int ret;
> >
> >         ret = dm_pci_bus_find_bdf(TNC_IGD, &igd);
> > -       if (ret)
> > -               return ret;
> >         if (!igd)
> >                 return 0;
>
> This is backwards. If ret returns 0 then we know igd is non-NULL. So
> check the return value. That is what it is for. You can drop the check
> of igd. Same below.
>

Ah, yes. I remembered why the original codes were written like this.
Actually the codes tried to be compatible that if the device is
already disabled it should not return error. I will reword the commit
message and add some comments here.

Regards,
Bin