Re: [PATCH v7 4/7] PCI: rcar-gen4: Add rcar_gen4_pcie_drvdata

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

 



Hi Mani,

On Sat, May 11, 2024 at 9:37 AM Manivannan Sadhasivam <mani@xxxxxxxxxx> wrote:
> On Mon, Apr 15, 2024 at 05:11:32PM +0900, Yoshihiro Shimoda wrote:
> > In other to support future SoCs such as r8a779g0 and r8a779h0 that
> > require different initialization settings, let's introduce SoC
> > specific driver data with the initial member being the device mode.
> > No functional change.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
>
> One nitpick below. With that addressed,
>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>

> > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c

> > @@ -437,9 +441,9 @@ static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> >  /* Common */
> >  static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar)
> >  {
> > -     rcar->mode = (uintptr_t)of_device_get_match_data(&rcar->pdev->dev);
> > +     rcar->drvdata = of_device_get_match_data(&rcar->pdev->dev);
>
> Even though rcar->drvdata won't be NULL, the lack of NULL check will cause
> folks to send fixup patch later. So please add a NULL check here itself.

I tend to disagree: this can never return NULL.
Less than half of the callers of of_device_get_match_data() check for
a NULL pointer, and many of them do so because they are used both
with and without DT.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux