On Thu, 03 Sep 2020 16:59:30 +0100
Marc Zyngier <maz@xxxxxxxxxx> wrote:
> On 2020-09-03 15:35, Rob Herring wrote:
> > On Thu, Sep 3, 2020 at 3:19 AM Lorenzo Pieralisi
> > <lorenzo.pieralisi@xxxxxxx> wrote:
> >>
> >> Ok I think I understand what the problem is.
> >>
> >> Can you give this patch a shot please ? I think we are
> >> dereferencing a NULL pointer if bus is the root bus and dev == 0,
> >> we can rewrite the check if this patch fixes the issue.
> >
> > Indeed. I checked all the other cases of
> > pci_is_root_bus(bus->parent) and they should be fine because they
> > are only reached if !root_bus.
> >
> > I would restructure the check like this instead:
> >
> > diff --git a/drivers/pci/controller/pcie-rockchip-host.c
> > b/drivers/pci/controller/pcie-rockchip-host.c
> > index 0bb2fb3e8a0b..9b485bea8b92 100644
> > --- a/drivers/pci/controller/pcie-rockchip-host.c
> > +++ b/drivers/pci/controller/pcie-rockchip-host.c
> > @@ -72,14 +72,14 @@ static int rockchip_pcie_valid_device(struct
> > rockchip_pcie *rockchip,
> > struct pci_bus *bus, int dev)
> > {
> > /* access only one slot on each root port */
> > - if (pci_is_root_bus(bus) && dev > 0)
> > - return 0;
> > -
> > - /*
> > - * do not read more than one device on the bus directly
> > attached
> > - * to RC's downstream side.
> > - */
> > - if (pci_is_root_bus(bus->parent) && dev > 0)
> > + if (pci_is_root_bus(bus))
> > + if (dev > 0)
> > + return 0;
> > + else if (pci_is_root_bus(bus->parent) && dev > 0)
>
> Careful here, this else is relative to the *closest* if,
> and not what the indentation suggests...
>
> > + /*
> > + * do not read more than one device on the bus
> > directly attached
> > + * to RC's downstream side.
> > + */
> > return 0;
> >
> > return 1;
>
>
> M.
Both patches work fine. They both allow the device to boot and Wi-Fi to
work as expected.
I will echo Marc's concerns over the indentation. I am only casually
acquainted with C and this indentation would bring to wrong conclusions
if it hadn't been pointed out to me.
Cc me on an eventual patch, FWIW I can give it a final test just in
case.
Thanks again,
--
Samuel Dionne-Riel
