Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a1ad0e10-23d2-a860-e9a1-2680313d643f@gmail.com>
Date:   Fri, 12 May 2023 08:16:02 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
To:     Dan Carpenter <dan.carpenter@linaro.org>,
        oe-kbuild@lists.linux.dev,
        Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Cc:     lkp@intel.com, oe-kbuild-all@lists.linux.dev,
        linux-kernel@vger.kernel.org,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
References: <46296e39-dbc1-4f27-b89c-af6f08d9f318@kili.mountain>
Content-Language: en-US, en-GB
From:   Matti Vaittinen <mazziesaccount@gmail.com>
Subject: Re: drivers/iio/accel/kionix-kx022a.c:1053 kx022a_probe_internal()
 warn: passing zero to 'dev_err_probe'
In-Reply-To: <46296e39-dbc1-4f27-b89c-af6f08d9f318@kili.mountain>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@intel.com/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@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202305110245.MFxC9bUj-lkp@intel.com/
> 
> 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@gmail.com/

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! ~~