Re: drivers/iio/accel/kionix-kx022a.c:1053 kx022a_probe_internal() warn: passing zero to 'dev_err_probe'

[Matti Vaittinen]

On 5/11/23 13:48, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   ad2fd53a7870a395b8564697bef6c329d017c6c9
> commit: 7c1d1677b3227c6b18ac999f2b84778baa280b8f iio: accel: Support Kionix/ROHM KX022A accelerometer
> config: openrisc-randconfig-m041-20230509 (https://download.01.org/0day-ci/archive/20230511/202305110245.MFxC9bUj-lkp@xxxxxxxxx/config)
> compiler: or1k-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
> | Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> | Link: https://lore.kernel.org/r/202305110245.MFxC9bUj-lkp@xxxxxxxxx/
>
> smatch warnings:
> drivers/iio/accel/kionix-kx022a.c:1053 kx022a_probe_internal() warn: passing zero to 'dev_err_probe'
>
> vim +/dev_err_probe +1053 drivers/iio/accel/kionix-kx022a.c
>
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1000  int kx022a_probe_internal(struct device *dev)
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1001  {
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1002  	static const char * const regulator_names[] = {"io-vdd", "vdd"};
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1003  	struct iio_trigger *indio_trig;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1004  	struct fwnode_handle *fwnode;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1005  	struct kx022a_data *data;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1006  	struct regmap *regmap;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1007  	unsigned int chip_id;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1008  	struct iio_dev *idev;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1009  	int ret, irq;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1010  	char *name;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1011
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1012  	regmap = dev_get_regmap(dev, NULL);
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1013  	if (!regmap) {
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1014  		dev_err(dev, "no regmap\n");
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1015  		return -EINVAL;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1016  	}
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1017
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1018  	fwnode = dev_fwnode(dev);
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1019  	if (!fwnode)
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1020  		return -ENODEV;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1021
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1022  	idev = devm_iio_device_alloc(dev, sizeof(*data));
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1023  	if (!idev)
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1024  		return -ENOMEM;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1025
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1026  	data = iio_priv(idev);
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1027
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1028  	/*
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1029  	 * VDD is the analog and digital domain voltage supply and
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1030  	 * IO_VDD is the digital I/O voltage supply.
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1031  	 */
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1032  	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names),
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1033  					     regulator_names);
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1034  	if (ret && ret != -ENODEV)
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1035  		return dev_err_probe(dev, ret, "failed to enable regulator\n");
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1036
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1037  	ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1038  	if (ret)
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1039  		return dev_err_probe(dev, ret, "Failed to access sensor\n");
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1040
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1041  	if (chip_id != KX022A_ID) {
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1042  		dev_err(dev, "unsupported device 0x%x\n", chip_id);
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1043  		return -EINVAL;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1044  	}
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1045
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1046  	irq = fwnode_irq_get_byname(fwnode, "INT1");
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1047  	if (irq > 0) {
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1048  		data->inc_reg = KX022A_REG_INC1;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1049  		data->ien_reg = KX022A_REG_INC4;
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1050  	} else {
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1051  		irq = fwnode_irq_get_byname(fwnode, "INT2");
> 7c1d1677b3227c Matti Vaittinen 2022-10-24  1052  		if (irq <= 0)
>                                                                      ^^^^^^^^
> This code assumes fwnode_irq_get_byname() can return zero.
>
> 7c1d1677b3227c Matti Vaittinen 2022-10-24 @1053  			return dev_err_probe(dev, irq, "No suitable IRQ\n");
>
> But fortunately, it can't otherwise this would return success.

Ouch. Actually, the fwnode_irq_get_byname() can return zero on 
device-tree mapping error. I did actually send a patch to address this 
some time ago:
https://lore.kernel.org/lkml/cover.1666710197.git.mazziesaccount@xxxxxxxxx/

but got stuck on not knowing "the right way" of handling an smbus 
use-case. Later I forgot this.

It seems I did describe this very same error in the cover-letter - and 
right after that I fell on this very trap I described... :/

Thanks for the heads-up Dan!

Yours,
	-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~