Re: [PATCH v2 2/3] pwm: tegra: Check for match_data being NULL

[Thierry Reding <thierry.reding@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3500 bytes --]

On Mon, Sep 21, 2026 at 04:34:09PM +0200, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Mon, Sep 21, 2026 at 11:47:17AM +0200, Thierry Reding wrote:
> > On Fri, Sep 18, 2026 at 04:33:46PM +0200, Uwe Kleine-König wrote:
> > > It's unlikely but not impossible that of_device_get_match_data() returns
> > > NULL. Handle this case instead of triggering a NULL pointer exception.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> > > ---
> > >  drivers/pwm/pwm-tegra.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> > > index efb7ab60f602..b461d3877f43 100644
> > > --- a/drivers/pwm/pwm-tegra.c
> > > +++ b/drivers/pwm/pwm-tegra.c
> > > @@ -323,6 +323,14 @@ static int tegra_pwm_probe(struct platform_device *pdev)
> > >  	int ret;
> > >  
> > >  	soc = of_device_get_match_data(dev);
> > > +	if (!soc) {
> > > +		/*
> > > +		 * This can only happen if pdev was matched via pdev->name
> > > +		 * (which should not happen today) or in combination with a
> > > +		 * driver override.
> > > +		 */
> > > +		return dev_err_probe(dev, -ENODEV, "Unsupported device\n");
> > > +	}
> > 
> > We don't usually do this. Matching via anything other than OF device ID
> > tables (or ACPI, I suppose) is a programming error and you deserve the
> > crash which forces you to fix things rather than continue with an error
> > that is easy to miss.
> 
> I don't agree to "you deserve the crash". IMHO even root should be
> unable to make the kernel crash. I don't understand what you think
> should be fixed if I hit that crash. My userspace interactions in /sys?
> Which error is easy to miss?

Oh, root can easily make the kernel crash in any number of ways. That's
really kind of baked into the concept.

To me this is in the same category as force-unloading a module. You can
do it, but you should know that it's potentially dangerous and most of
the time doesn't make sense either. It's called forcing because there
are guardrails in place to prevent you from trying to do it.

If a device cannot operate without device data, it doesn't make sense to
bind to it with a driver override because then you just don't get that
data.

I'll grant you that purposefully crashing the system is maybe a bit of
an exaggeration if there's a knob specifically designed to let you do
this, hence why I volunteered to look into opting out of driver_override
where it doesn't make sense.

> > Driver overrides aren't going to work with these devices anyway, so I'm
> > beginning to think it might be worth looking into opting out of the
> > override behaviour for select drivers.
> 
> I think there are much more drivers that don't expect to be forced on a
> device, so opting in for override would be a more sensible result. Of
> course the path to there is more painful ...
> 
> Best regards
> Uwe

Yeah, driver_override has been baked into the driver core for quite a
long time (platform devices seem to have had this functionality for more
than a decade).

You're probably not wrong about opt-in being the more natural choice,
but looking at commit 3d713e0e382e ("driver core: platform: add device
binding path 'driver_override'"), the intended use-cases are very
generic, so it would probably lead to a continuous stream of patches
needing to be added whenever a new device wants to be supported with
vfio or something.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]