Re: [PATCH] PCI: rcar: Check for OF device match early

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 31, 2017 at 10:43:39PM +0100, Geert Uytterhoeven wrote:
> Hi Bjorn,
> 
> On Tue, Jan 31, 2017 at 7:09 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> > On Tue, Jan 31, 2017 at 04:33:15PM +0100, Geert Uytterhoeven wrote:
> >> On Tue, Jan 31, 2017 at 4:10 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
> >> > A match in the rcar_pcie_of_match[] table is required, so check that first,
> >> > before we start setting up things that need to be undone if it fails.  No
> >> > functional change intended.
> >> >
> >> > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> >> > ---
> >> >  drivers/pci/host/pcie-rcar.c |   10 +++++-----
> >> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> >> > index 0d9b96c3c49d..c91ff0b91be8 100644
> >> > --- a/drivers/pci/host/pcie-rcar.c
> >> > +++ b/drivers/pci/host/pcie-rcar.c
> >> > @@ -1129,6 +1129,10 @@ static int rcar_pcie_probe(struct platform_device *pdev)
> >> >         int err;
> >> >         int (*hw_init_fn)(struct rcar_pcie *);
> >> >
> >> > +       of_id = of_match_device(rcar_pcie_of_match, dev);
> >> > +       if (!of_id || !of_id->data)
> >> > +               return -EINVAL;
> >> > +
> >>
> >> As this driver is DT-only, none of the above can fail, and you could just do
> >>
> >>        hw_init_fn = of_device_get_match_data(dev);
> >>
> >> instead, getting rid of of_id completely.
> >
> > Oh, I really like that, thanks for pointing that out!
> >
> > I was about to say that I personally would not check of_id->data for NULL,
> > because it is only NULL if somebody adds an entry to rcar_pcie_of_match
> > without a .data member.  In that case, I'd rather take the NULL pointer
> > dereference than return -EINVAL because it's too easy to ignore the
> > -EINVAL.
> >
> > What do you think about the following?
> 
> Thanks, looks fine!
> 
> > commit 25bd3aa972ee32f04590aa68b2b785dce36b036a
> > Author: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > Date:   Tue Jan 31 08:45:49 2017 -0600
> >
> >     PCI: rcar: Use of_device_get_match_data() to simplify probe
> >
> >     This is a DT-only driver, so the only way to call rcar_pcie_probe() is to
> >     match an entry in rcar_pcie_of_match[], so of_id cannot be NULL.
> >
> >     Furthermore, of_id->data can only be NULL if an rcar_pcie_of_match[] entry
> >     has a NULL .data member.  That's a driver defect, and we don't want to
> >     return -EINVAL, which is easy to ignore.  We'd rather take the NULL pointer
> >     dereference so we notice the problem and fix it.
> >
> >     Use of_device_get_match_data() to retrieve the hw_init_fn pointer.  No
> >     functional change intended.
> >
> >     Suggested-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> >     Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Acked-by: Simon Horman <horms+renesas@xxxxxxxxxxxx>




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux