Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

From: Rob Herring <robh+dt@kernel.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	linux-clk <linux-clk@vger.kernel.org>,
	linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org,
	Ryder Lee <ryder.lee@mediatek.com>,
	Frank Rowand <frowand.list@gmail.com>
Subject: Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data
Date: Fri, 9 Nov 2018 10:30:11 -0600	[thread overview]
Message-ID: <CAL_Jsq+VuP6EkObw4wFeqOKoZEd9ktEErQaFfncz-2DfvNhp0A@mail.gmail.com> (raw)
In-Reply-To: <CAMuHMdWm2P8fa6d9vLhYtWHBe0uNPj9jfyL1rtmdLixPEi1rUg@mail.gmail.com>

On Fri, Nov 9, 2018 at 4:36 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Matthias,
>
> On Fri, Nov 9, 2018 at 11:29 AM Matthias Brugger <matthias.bgg@gmail.com> wrote:
> > On 08/11/2018 18:58, Stephen Boyd wrote:
> > > Quoting Matthias Brugger (2018-11-08 00:29:46)
> > >> On 06/11/2018 19:36, Stephen Boyd wrote:
> > >>> +int platform_driver_probe_by_of_match_data(struct platform_device *pdev)
> > >>> +{
> > >>> +     int (*probe_func)(struct platform_device *pdev);
> > >>> +
> > >>> +     probe_func = of_device_get_match_data(&pdev->dev);
> > >>
> > >> Shouldn't we check if probe_func is not NULL?
> > >
> > > Is the oops from the NULL pointer deref insufficient?
> >
> > Do you think we should crash the machine if someone uses the call wrongly? Or
> > should we provide the possibility to error out on the caller side?
>
> I believe that would be a bug in the driver, to be discovered ASAP.
> So yes, please do crash ;-)

Depends on the driver. If it's not needed to get a console out, then
it should just WARN. Otherwise, you've got to go enable earlycon to
see the crash. Of course, someone could just as easily stick data in
here instead of a function ptr and we'll be back to a crash.

Rob