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

[Manivannan Sadhasivam <mani@kernel.org>]

On Mon, May 13, 2024 at 11:20:56AM +0200, Geert Uytterhoeven wrote:
> Hi Mani,
> 
> On Sat, May 11, 2024 at 9:37 AM Manivannan Sadhasivam <mani@kernel.org> 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@renesas.com>
> >
> > One nitpick below. With that addressed,
> >
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> > > --- 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.
> 

As I said, there is no way it can be NULL. But folks tend to send the 'fix'
patches as the automated tools report these kind of non-existent issues. And I
agree that we can decide not to accept those patches, but I just wanted to avoid
that noise. As a maintainer, I try to avoid seeing those kind of patches if
there is a way we can avoid it.

But no strong preference here.

- Mani

-- 
மணிவண்ணன் சதாசிவம்