Re: [PATCH v1 1/6] pwm: tegra: Check for match_data being NULL

["Uwe Kleine-König" <ukleinek@kernel.org>]

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

Hello,

On Wed, Jul 15, 2026 at 01:11:00PM +0900, Mikko Perttunen wrote:
> On Tuesday, July 14, 2026 9:02 PM 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 <ukleinek@kernel.org>
> > ---
> >  drivers/pwm/pwm-tegra.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> > index 5cdbe120ba2d..53743f83869a 100644
> > --- a/drivers/pwm/pwm-tegra.c
> > +++ b/drivers/pwm/pwm-tegra.c
> > @@ -322,6 +322,13 @@ static int tegra_pwm_probe(struct platform_device *pdev)
> >  	int ret;
> >  
> >  	soc = of_device_get_match_data(&pdev->dev);
> > +	if (!soc)
> 
> Very subjective, but my preference is to have curly braces whenever the
> if block is more than one line, for clarity.

If you read Documentation/process/coding-style.rst by the letter, this
case shouldn't have braces, but I agree that adding braces here is
clearer (and that coding-style.rst shouldn't be read by the letter).

> > +		/*
> > +		 * This can only happen if pdev was matched via pdev->name
> > +		 * (which should not happen today) or in combination with a
> > +		 * driver override.
> > +		 */
> 
> I feel like driver_override falls in the realm of 'root can mess with
> the system as they feel like but if they don't know what they're doing
> they get to keep the pieces'.

IMHO even root should not be able to trigger a NULL pointer exception.
Not sure there is a general agreed on policy about that though.

> So adding a check in every driver, or
> in practice having a random mix of drivers with and without the check,
> doesn't seem necessary to me.
> 
> If we actually want to check for this condition, could it be done
> centrally instead? I.e. don't call probe if there's no match data and
> the driver's match table implies it requires it.

So you'd want to check the device-id table before calling .probe() and
if all entries have a non-zero .driver_data don't honor the override?
Hmm, maybe something to discuss at the "Driver Core" microconference (at
LPC 2026 in Prague), but my spontanous reaction is that this is a
heuristic only and might prevent valid use-cases.

> > +		return dev_err_probe(dev, -ENODEV, "Unsupported device\n");
> 
> 'dev' is not defined (yet).

Ooops, that leaked in as I reordered the patches to have the fixes
first. Thanks for noticing.

Best regards
Uwe

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