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/[email protected]/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 <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/
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.
7c1d1677b3227c Matti Vaittinen 2022-10-24 1054
7c1d1677b3227c Matti Vaittinen 2022-10-24 1055 data->inc_reg = KX022A_REG_INC5;
7c1d1677b3227c Matti Vaittinen 2022-10-24 1056 data->ien_reg = KX022A_REG_INC6;
7c1d1677b3227c Matti Vaittinen 2022-10-24 1057 }
7c1d1677b3227c Matti Vaittinen 2022-10-24 1058
7c1d1677b3227c Matti Vaittinen 2022-10-24 1059 data->regmap = regmap;
7c1d1677b3227c Matti Vaittinen 2022-10-24 1060 data->dev = dev;
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
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/[email protected]/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 <[email protected]>
> | Reported-by: Dan Carpenter <[email protected]>
> | Link: https://lore.kernel.org/r/[email protected]/
>
> 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/[email protected]/
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! ~~
On Fri, May 12, 2023 at 08:16:02AM +0300, Matti Vaittinen wrote:
> > 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/[email protected]/
>
Ah. I just went by the documentation instead of looking at the code.
Originally a bunch of irq functions return NO_IRQ on error which was a
design mistake... irq_of_parse_and_map() still returns zero on error
instead of negative error codes. I wrote a check for that yesterday.
drivers/gpu/drm/msm/dsi/dsi_host.c:1949 msm_dsi_host_init() warn: irq_of_parse_and_map() returns zero on failure
drivers/dma/ti/edma.c:2405 edma_probe() warn: irq_of_parse_and_map() returns zero on failure
drivers/dma/ti/edma.c:2421 edma_probe() warn: irq_of_parse_and_map() returns zero on failure
drivers/net/ethernet/xilinx/ll_temac_main.c:1570 temac_probe() warn: irq_of_parse_and_map() returns zero on failure
drivers/net/ethernet/xilinx/ll_temac_main.c:1573 temac_probe() warn: irq_of_parse_and_map() returns zero on failure
drivers/ata/sata_mv.c:4094 mv_platform_probe() warn: irq_of_parse_and_map() returns zero on failure
It would be good if we could apply your patch, otherwise I could create
an explicit check for fwnode_irq_get_byname() returns.
Ideally, everything would just return negative error codes on error.
It's harder to change irq_of_parse_and_map() because it doesn't return a
mix of errors and zero, but just zero. We would have to rename it to
something else, otherwise backports would get messed up.
regards,
dan carpenter
On 5/12/23 08:41, Dan Carpenter wrote:
> On Fri, May 12, 2023 at 08:16:02AM +0300, Matti Vaittinen wrote:
>>> 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/[email protected]/
>>
>
> Ah. I just went by the documentation instead of looking at the code.
>
> Originally a bunch of irq functions return NO_IRQ on error which was a
> design mistake... irq_of_parse_and_map() still returns zero on error
> instead of negative error codes. I wrote a check for that yesterday.
>
> drivers/gpu/drm/msm/dsi/dsi_host.c:1949 msm_dsi_host_init() warn: irq_of_parse_and_map() returns zero on failure
> drivers/dma/ti/edma.c:2405 edma_probe() warn: irq_of_parse_and_map() returns zero on failure
> drivers/dma/ti/edma.c:2421 edma_probe() warn: irq_of_parse_and_map() returns zero on failure
> drivers/net/ethernet/xilinx/ll_temac_main.c:1570 temac_probe() warn: irq_of_parse_and_map() returns zero on failure
> drivers/net/ethernet/xilinx/ll_temac_main.c:1573 temac_probe() warn: irq_of_parse_and_map() returns zero on failure
> drivers/ata/sata_mv.c:4094 mv_platform_probe() warn: irq_of_parse_and_map() returns zero on failure
>
> It would be good if we could apply your patch, otherwise I could create
> an explicit check for fwnode_irq_get_byname() returns.
I guess I can re-spin the series. Let's see if there is some good
suggestion(s) on how to fix the i2c-smbus.
> Ideally, everything would just return negative error codes on error.
I definitely agree.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~