Re: [PATCH] sc16is7xx: null ptr check

[Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

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

On Mon, Oct 05, 2015 at 08:31:28AM +0200, Uwe Kleine-König wrote:

> So in the broken case the device in question was bound to the
> sc16is7xx driver because the compatible name was matched by the
> id_table and not the of_match_table. My first thought was that in this
> case the spi_device's dev->of_node should not be set, but after having
> thought a bit more I think this is right.

Yes, it's intended behaviour - the DT is buggy here.

> But the .probe routine could be a tad more clever here:

> 	devtype = of_device_get_match_data(&spi->dev);
> 	if (!devtype) {
> 		const struct spi_device_id *id_entry = spi_get_device_id(spi);
> 
> 		/* Do I need to check id_entry being != NULL here? */
> 		devtype = (struct sc16is7xx_devtype *)id_entry->driver_data;
> 		flags = IRQF_TRIGGER_FALLING;
> 	}

That's pretty much a requirement - the driver should work no matter
which binding method gets used, OF only drivers aren't great practice at
the best of times and for off-SoC devices like this one it's definitely
not appropriate.

> Apart from that using "flags = IRQF_TRIGGER_FALLING" iff the device was
> probed via spi_id_table is strange.

That seems like an obvious bug.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]