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@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.
>
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
--
மணிவண்ணன் சதாசிவம்
