From 20f291b88ecf23f674ee2ed980a4d93b7f16a06f Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 14 Feb 2023 18:47:30 +0300 Subject: iio: adc: imx93: fix a signedness bug in imx93_adc_read_raw() The problem is these lines: ret = vref_uv = regulator_get_voltage(adc->vref); if (ret < 0) The "ret" variable is type long and "vref_uv" is u32 so that means the condition can never be true on a 64bit system. A negative error code from regulator_get_voltage() would be cast to a high positive u32 value and then remain a high positive value when cast to a long. The "ret" variable only ever stores ints so it should be declared as an int. We can delete the "vref_uv" variable and use "ret" directly. Fixes: 7d02296ac8b8 ("iio: adc: add imx93 adc support") Signed-off-by: Dan Carpenter Reviewed-by: Haibo Chen Link: https://lore.kernel.org/r/Y+utEvjfjQRQo2QB@kili Signed-off-by: Jonathan Cameron --- drivers/iio/adc/imx93_adc.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c index a775d2e40567..dce9ec91e4a7 100644 --- a/drivers/iio/adc/imx93_adc.c +++ b/drivers/iio/adc/imx93_adc.c @@ -236,8 +236,7 @@ static int imx93_adc_read_raw(struct iio_dev *indio_dev, { struct imx93_adc *adc = iio_priv(indio_dev); struct device *dev = adc->dev; - long ret; - u32 vref_uv; + int ret; switch (mask) { case IIO_CHAN_INFO_RAW: @@ -253,10 +252,10 @@ static int imx93_adc_read_raw(struct iio_dev *indio_dev, return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: - ret = vref_uv = regulator_get_voltage(adc->vref); + ret = regulator_get_voltage(adc->vref); if (ret < 0) return ret; - *val = vref_uv / 1000; + *val = ret / 1000; *val2 = 12; return IIO_VAL_FRACTIONAL_LOG2; -- cgit v1.2.3 From 00ffdd6fa90298522d45ca0c348b23485584dcdc Mon Sep 17 00:00:00 2001 From: ChiaEn Wu Date: Mon, 10 Apr 2023 18:34:22 +0800 Subject: iio: adc: mt6370: Fix ibus and ibat scaling value of some specific vendor ID chips The scale value of ibus and ibat on the datasheet is incorrect due to the customer report after the experimentation with some specific vendor ID chips. Fixes: c1404d1b659f ("iio: adc: mt6370: Add MediaTek MT6370 support") Signed-off-by: ChiaEn Wu Reviewed-by: Alexandre Mergnat Link: https://lore.kernel.org/r/1681122862-1994-1-git-send-email-chiaen_wu@richtek.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/mt6370-adc.c | 53 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/drivers/iio/adc/mt6370-adc.c b/drivers/iio/adc/mt6370-adc.c index bc62e5a9d50d..0bc112135bca 100644 --- a/drivers/iio/adc/mt6370-adc.c +++ b/drivers/iio/adc/mt6370-adc.c @@ -19,6 +19,7 @@ #include +#define MT6370_REG_DEV_INFO 0x100 #define MT6370_REG_CHG_CTRL3 0x113 #define MT6370_REG_CHG_CTRL7 0x117 #define MT6370_REG_CHG_ADC 0x121 @@ -27,6 +28,7 @@ #define MT6370_ADC_START_MASK BIT(0) #define MT6370_ADC_IN_SEL_MASK GENMASK(7, 4) #define MT6370_AICR_ICHG_MASK GENMASK(7, 2) +#define MT6370_VENID_MASK GENMASK(7, 4) #define MT6370_AICR_100_mA 0x0 #define MT6370_AICR_150_mA 0x1 @@ -47,6 +49,10 @@ #define ADC_CONV_TIME_MS 35 #define ADC_CONV_POLLING_TIME_US 1000 +#define MT6370_VID_RT5081 0x8 +#define MT6370_VID_RT5081A 0xA +#define MT6370_VID_MT6370 0xE + struct mt6370_adc_data { struct device *dev; struct regmap *regmap; @@ -55,6 +61,7 @@ struct mt6370_adc_data { * from being read at the same time. */ struct mutex adc_lock; + unsigned int vid; }; static int mt6370_adc_read_channel(struct mt6370_adc_data *priv, int chan, @@ -98,6 +105,30 @@ adc_unlock: return ret; } +static int mt6370_adc_get_ibus_scale(struct mt6370_adc_data *priv) +{ + switch (priv->vid) { + case MT6370_VID_RT5081: + case MT6370_VID_RT5081A: + case MT6370_VID_MT6370: + return 3350; + default: + return 3875; + } +} + +static int mt6370_adc_get_ibat_scale(struct mt6370_adc_data *priv) +{ + switch (priv->vid) { + case MT6370_VID_RT5081: + case MT6370_VID_RT5081A: + case MT6370_VID_MT6370: + return 2680; + default: + return 3870; + } +} + static int mt6370_adc_read_scale(struct mt6370_adc_data *priv, int chan, int *val1, int *val2) { @@ -123,7 +154,7 @@ static int mt6370_adc_read_scale(struct mt6370_adc_data *priv, case MT6370_AICR_250_mA: case MT6370_AICR_300_mA: case MT6370_AICR_350_mA: - *val1 = 3350; + *val1 = mt6370_adc_get_ibus_scale(priv); break; default: *val1 = 5000; @@ -150,7 +181,7 @@ static int mt6370_adc_read_scale(struct mt6370_adc_data *priv, case MT6370_ICHG_600_mA: case MT6370_ICHG_700_mA: case MT6370_ICHG_800_mA: - *val1 = 2680; + *val1 = mt6370_adc_get_ibat_scale(priv); break; default: *val1 = 5000; @@ -251,6 +282,20 @@ static const struct iio_chan_spec mt6370_adc_channels[] = { MT6370_ADC_CHAN(TEMP_JC, IIO_TEMP, 12, BIT(IIO_CHAN_INFO_OFFSET)), }; +static int mt6370_get_vendor_info(struct mt6370_adc_data *priv) +{ + unsigned int dev_info; + int ret; + + ret = regmap_read(priv->regmap, MT6370_REG_DEV_INFO, &dev_info); + if (ret) + return ret; + + priv->vid = FIELD_GET(MT6370_VENID_MASK, dev_info); + + return 0; +} + static int mt6370_adc_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -272,6 +317,10 @@ static int mt6370_adc_probe(struct platform_device *pdev) priv->regmap = regmap; mutex_init(&priv->adc_lock); + ret = mt6370_get_vendor_info(priv); + if (ret) + return dev_err_probe(dev, ret, "Failed to get vid\n"); + ret = regmap_write(priv->regmap, MT6370_REG_CHG_ADC, 0); if (ret) return dev_err_probe(dev, ret, "Failed to reset ADC\n"); -- cgit v1.2.3 From 265c82ea8b172129cb6d4eff41af856c3aff6168 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Thu, 13 Apr 2023 18:37:52 -0700 Subject: iio: tmag5273: Fix runtime PM leak on measurement error The tmag5273 gets a runtime PM reference before reading a measurement and releases it when done. But if the measurement fails the tmag5273_read_raw() function exits before releasing the reference. Make sure that this error path also releases the runtime PM reference. Fixes: 866a1389174b ("iio: magnetometer: add ti tmag5273 driver") Signed-off-by: Lars-Peter Clausen Acked-by: Gerald Loacker Reviewed-by: Nuno Sa Link: https://lore.kernel.org/r/20230414013752.498767-1-lars@metafoo.de Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/magnetometer/tmag5273.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/iio/magnetometer/tmag5273.c b/drivers/iio/magnetometer/tmag5273.c index 28bb7efe8df8..e155a75b3cd2 100644 --- a/drivers/iio/magnetometer/tmag5273.c +++ b/drivers/iio/magnetometer/tmag5273.c @@ -296,12 +296,13 @@ static int tmag5273_read_raw(struct iio_dev *indio_dev, return ret; ret = tmag5273_get_measure(data, &t, &x, &y, &z, &angle, &magnitude); - if (ret) - return ret; pm_runtime_mark_last_busy(data->dev); pm_runtime_put_autosuspend(data->dev); + if (ret) + return ret; + switch (chan->address) { case TEMPERATURE: *val = t; -- cgit v1.2.3 From 28f73ded19d403697f87473c9b85a27eb8ed9cf2 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Fri, 14 Apr 2023 08:07:02 -0700 Subject: iio: ad4130: Make sure clock provider gets removed The ad4130 driver registers a clock provider, but never removes it. This leaves a stale clock provider behind that references freed clocks when the device is unbound. Register a managed action to remove the clock provider when the device is removed. Fixes: 62094060cf3a ("iio: adc: ad4130: add AD4130 driver") Signed-off-by: Lars-Peter Clausen Link: https://lore.kernel.org/r/20230414150702.518441-1-lars@metafoo.de Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ad4130.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/iio/adc/ad4130.c b/drivers/iio/adc/ad4130.c index 38394341fd6e..5a5dd5e87ffc 100644 --- a/drivers/iio/adc/ad4130.c +++ b/drivers/iio/adc/ad4130.c @@ -1817,6 +1817,11 @@ static const struct clk_ops ad4130_int_clk_ops = { .unprepare = ad4130_int_clk_unprepare, }; +static void ad4130_clk_del_provider(void *of_node) +{ + of_clk_del_provider(of_node); +} + static int ad4130_setup_int_clk(struct ad4130_state *st) { struct device *dev = &st->spi->dev; @@ -1824,6 +1829,7 @@ static int ad4130_setup_int_clk(struct ad4130_state *st) struct clk_init_data init; const char *clk_name; struct clk *clk; + int ret; if (st->int_pin_sel == AD4130_INT_PIN_CLK || st->mclk_sel != AD4130_MCLK_76_8KHZ) @@ -1843,7 +1849,11 @@ static int ad4130_setup_int_clk(struct ad4130_state *st) if (IS_ERR(clk)) return PTR_ERR(clk); - return of_clk_add_provider(of_node, of_clk_src_simple_get, clk); + ret = of_clk_add_provider(of_node, of_clk_src_simple_get, clk); + if (ret) + return ret; + + return devm_add_action_or_reset(dev, ad4130_clk_del_provider, of_node); } static int ad4130_setup(struct iio_dev *indio_dev) -- cgit v1.2.3 From 27b2ed5b6d53cd62fc61c3f259ae52f5cac23b66 Mon Sep 17 00:00:00 2001 From: Jiakai Luo Date: Sat, 22 Apr 2023 06:34:06 -0700 Subject: iio: adc: mxs-lradc: fix the order of two cleanup operations Smatch reports: drivers/iio/adc/mxs-lradc-adc.c:766 mxs_lradc_adc_probe() warn: missing unwind goto? the order of three init operation: 1.mxs_lradc_adc_trigger_init 2.iio_triggered_buffer_setup 3.mxs_lradc_adc_hw_init thus, the order of three cleanup operation should be: 1.mxs_lradc_adc_hw_stop 2.iio_triggered_buffer_cleanup 3.mxs_lradc_adc_trigger_remove we exchange the order of two cleanup operations, introducing the following differences: 1.if mxs_lradc_adc_trigger_init fails, returns directly; 2.if trigger_init succeeds but iio_triggered_buffer_setup fails, goto err_trig and remove the trigger. In addition, we also reorder the unwind that goes on in the remove() callback to match the new ordering. Fixes: 6dd112b9f85e ("iio: adc: mxs-lradc: Add support for ADC driver") Signed-off-by: Jiakai Luo Reviewed-by: Dongliang Mu Link: https://lore.kernel.org/r/20230422133407.72908-1-jkluo@hust.edu.cn Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/mxs-lradc-adc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c index bca79a93cbe4..a50f39143d3e 100644 --- a/drivers/iio/adc/mxs-lradc-adc.c +++ b/drivers/iio/adc/mxs-lradc-adc.c @@ -757,13 +757,13 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev) ret = mxs_lradc_adc_trigger_init(iio); if (ret) - goto err_trig; + return ret; ret = iio_triggered_buffer_setup(iio, &iio_pollfunc_store_time, &mxs_lradc_adc_trigger_handler, &mxs_lradc_adc_buffer_ops); if (ret) - return ret; + goto err_trig; adc->vref_mv = mxs_lradc_adc_vref_mv[lradc->soc]; @@ -801,9 +801,9 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev) err_dev: mxs_lradc_adc_hw_stop(adc); - mxs_lradc_adc_trigger_remove(iio); -err_trig: iio_triggered_buffer_cleanup(iio); +err_trig: + mxs_lradc_adc_trigger_remove(iio); return ret; } @@ -814,8 +814,8 @@ static int mxs_lradc_adc_remove(struct platform_device *pdev) iio_device_unregister(iio); mxs_lradc_adc_hw_stop(adc); - mxs_lradc_adc_trigger_remove(iio); iio_triggered_buffer_cleanup(iio); + mxs_lradc_adc_trigger_remove(iio); return 0; } -- cgit v1.2.3 From 279c3a2a5eb2c1054fbe38e5c33be08584229047 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 21 Apr 2023 13:41:56 +0300 Subject: iio: adc: palmas: fix off by one bugs Valid values for "adc_chan" are zero to (PALMAS_ADC_CH_MAX - 1). Smatch detects some buffer overflows caused by this: drivers/iio/adc/palmas_gpadc.c:721 palmas_gpadc_read_event_value() error: buffer overflow 'adc->thresholds' 16 <= 16 drivers/iio/adc/palmas_gpadc.c:758 palmas_gpadc_write_event_value() error: buffer overflow 'adc->thresholds' 16 <= 16 The effect of this bug in other functions is more complicated but obviously we should fix all of them. Fixes: a99544c6c883 ("iio: adc: palmas: add support for iio threshold events") Signed-off-by: Dan Carpenter Link: https://lore.kernel.org/r/14fee94a-7db7-4371-b7d6-e94d86b9561e@kili.mountain Signed-off-by: Jonathan Cameron --- drivers/iio/adc/palmas_gpadc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c index c1c439215aeb..7dfc9c927a23 100644 --- a/drivers/iio/adc/palmas_gpadc.c +++ b/drivers/iio/adc/palmas_gpadc.c @@ -547,7 +547,7 @@ static int palmas_gpadc_read_raw(struct iio_dev *indio_dev, int adc_chan = chan->channel; int ret = 0; - if (adc_chan > PALMAS_ADC_CH_MAX) + if (adc_chan >= PALMAS_ADC_CH_MAX) return -EINVAL; mutex_lock(&adc->lock); @@ -595,7 +595,7 @@ static int palmas_gpadc_read_event_config(struct iio_dev *indio_dev, int adc_chan = chan->channel; int ret = 0; - if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH) + if (adc_chan >= PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH) return -EINVAL; mutex_lock(&adc->lock); @@ -684,7 +684,7 @@ static int palmas_gpadc_write_event_config(struct iio_dev *indio_dev, int adc_chan = chan->channel; int ret; - if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH) + if (adc_chan >= PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH) return -EINVAL; mutex_lock(&adc->lock); @@ -710,7 +710,7 @@ static int palmas_gpadc_read_event_value(struct iio_dev *indio_dev, int adc_chan = chan->channel; int ret; - if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH) + if (adc_chan >= PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH) return -EINVAL; mutex_lock(&adc->lock); @@ -744,7 +744,7 @@ static int palmas_gpadc_write_event_value(struct iio_dev *indio_dev, int old; int ret; - if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH) + if (adc_chan >= PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH) return -EINVAL; mutex_lock(&adc->lock); -- cgit v1.2.3 From 672cde9ef80ffde9e76d38f7aa2b287c4a18de9a Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Fri, 21 Apr 2023 08:46:11 +0300 Subject: iio: fix doc for iio_gts_find_sel_by_int_time The kerneldoc for iio_gts_find_sel_by_int_time() has an error. Documentation states that function is searching a selector for a HW-gain while it is searching a selector for an integration time. Fix the documentation by saying the function is looking for a selector for an integration time. Fixes: 38416c28e168 ("iio: light: Add gain-time-scale helpers") Signed-off-by: Matti Vaittinen Link: https://lore.kernel.org/r/ZEIjI4YUzqPZk/9X@fedora Signed-off-by: Jonathan Cameron --- include/linux/iio/iio-gts-helper.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/iio/iio-gts-helper.h b/include/linux/iio/iio-gts-helper.h index dd64e544a3da..9cb6c80dea71 100644 --- a/include/linux/iio/iio-gts-helper.h +++ b/include/linux/iio/iio-gts-helper.h @@ -135,7 +135,7 @@ static inline int iio_gts_find_int_time_by_sel(struct iio_gts *gts, int sel) /** * iio_gts_find_sel_by_int_time - find selector matching integration time * @gts: Gain time scale descriptor - * @gain: HW-gain for which matching selector is searched for + * @time: Integration time for which matching selector is searched for * * Return: a selector matching given integration time or -EINVAL if * selector was not found. -- cgit v1.2.3 From b7b04f393ffa1530213f522115e7d77761eb5c4f Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Mon, 17 Apr 2023 12:19:57 +0300 Subject: iio: bu27034: Fix integration time The bu27034 uses micro seconds for integration time configuration. This is incorrect as the ABI mandates use of seconds. Change BU27034 driver to use seconds for integration time. Fixes: e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor") Signed-off-by: Matti Vaittinen Link: https://lore.kernel.org/r/a05647669af22ba919c7c87dccb43975e3235a87.1681722914.git.mazziesaccount@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/light/rohm-bu27034.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c index e486dcf35eba..25c9b79574a5 100644 --- a/drivers/iio/light/rohm-bu27034.c +++ b/drivers/iio/light/rohm-bu27034.c @@ -1167,11 +1167,12 @@ static int bu27034_read_raw(struct iio_dev *idev, switch (mask) { case IIO_CHAN_INFO_INT_TIME: - *val = bu27034_get_int_time(data); - if (*val < 0) - return *val; + *val = 0; + *val2 = bu27034_get_int_time(data); + if (*val2 < 0) + return *val2; - return IIO_VAL_INT; + return IIO_VAL_INT_PLUS_MICRO; case IIO_CHAN_INFO_SCALE: return bu27034_get_scale(data, chan->channel, val, val2); @@ -1229,7 +1230,10 @@ static int bu27034_write_raw(struct iio_dev *idev, ret = bu27034_set_scale(data, chan->channel, val, val2); break; case IIO_CHAN_INFO_INT_TIME: - ret = bu27034_try_set_int_time(data, val); + if (!val) + ret = bu27034_try_set_int_time(data, val2); + else + ret = -EINVAL; break; default: ret = -EINVAL; -- cgit v1.2.3 From e65065132fa6d9e1fd3b7418eae5215e3b7bf6c7 Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Mon, 17 Apr 2023 12:20:18 +0300 Subject: iio: gts-helpers: fix integration time units The IIO ABI mandates expressing integration times in seconds. The GTS helper errorneously uses micro seconds in integration_times_available. Fix this by converting the lists to IIO_VAL_INT_PLUS_MICRO. Fixes: 38416c28e168 ("iio: light: Add gain-time-scale helpers") Signed-off-by: Matti Vaittinen Link: https://lore.kernel.org/r/eeacd192c259e885850b5a2dd8b776bccfc44fa8.1681722914.git.mazziesaccount@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/industrialio-gts-helper.c | 42 ++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c index 8bb68975b259..7653261d2dc2 100644 --- a/drivers/iio/industrialio-gts-helper.c +++ b/drivers/iio/industrialio-gts-helper.c @@ -337,6 +337,17 @@ free_gains: return ret; } +static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times, + int num_times) +{ + int i; + + for (i = 0; i < num_times; i++) { + int_micro_times[i * 2] = time_us[i] / 1000000; + int_micro_times[i * 2 + 1] = time_us[i] % 1000000; + } +} + /** * iio_gts_build_avail_time_table - build table of available integration times * @gts: Gain time scale descriptor @@ -351,7 +362,7 @@ free_gains: */ static int iio_gts_build_avail_time_table(struct iio_gts *gts) { - int *times, i, j, idx = 0; + int *times, i, j, idx = 0, *int_micro_times; if (!gts->num_itime) return 0; @@ -378,13 +389,24 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts) } } } - gts->avail_time_tables = times; - /* - * This is just to survive a unlikely corner-case where times in the - * given time table were not unique. Else we could just trust the - * gts->num_itime. - */ - gts->num_avail_time_tables = idx; + + /* create a list of times formatted as list of IIO_VAL_INT_PLUS_MICRO */ + int_micro_times = kcalloc(idx, sizeof(int) * 2, GFP_KERNEL); + if (int_micro_times) { + /* + * This is just to survive a unlikely corner-case where times in + * the given time table were not unique. Else we could just + * trust the gts->num_itime. + */ + gts->num_avail_time_tables = idx; + iio_gts_us_to_int_micro(times, int_micro_times, idx); + } + + gts->avail_time_tables = int_micro_times; + kfree(times); + + if (!int_micro_times) + return -ENOMEM; return 0; } @@ -683,8 +705,8 @@ int iio_gts_avail_times(struct iio_gts *gts, const int **vals, int *type, return -EINVAL; *vals = gts->avail_time_tables; - *type = IIO_VAL_INT; - *length = gts->num_avail_time_tables; + *type = IIO_VAL_INT_PLUS_MICRO; + *length = gts->num_avail_time_tables * 2; return IIO_AVAIL_LIST; } -- cgit v1.2.3 From 79b8ded9d9c595db9bd5b2f62f5f738b36de1e22 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 16 Apr 2023 23:24:09 +0200 Subject: iio: accel: st_accel: Fix invalid mount_matrix on devices without ACPI _ONT method When apply_acpi_orientation() fails, st_accel_common_probe() will fall back to iio_read_mount_matrix(), which checks for a mount-matrix device property and if that is not set falls back to the identity matrix. But when a sensor has no ACPI companion fwnode, or when the ACPI fwnode does not have a "_ONT" method apply_acpi_orientation() was returning 0, causing iio_read_mount_matrix() to never get called resulting in an invalid mount_matrix: [root@fedora ~]# cat /sys/bus/iio/devices/iio\:device0/mount_matrix (null), (null), (null); (null), (null), (null); (null), (null), (null) Fix this by making apply_acpi_orientation() always return an error when it did not set the mount_matrix. Fixes: 3d8ad94bb175 ("iio: accel: st_sensors: Support generic mounting matrix") Signed-off-by: Hans de Goede Reviewed-by: Linus Walleij Tested-by: Marius Hoch Link: https://lore.kernel.org/r/20230416212409.310936-1-hdegoede@redhat.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/accel/st_accel_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c index 5f7d81b44b1d..282e539157f8 100644 --- a/drivers/iio/accel/st_accel_core.c +++ b/drivers/iio/accel/st_accel_core.c @@ -1291,12 +1291,12 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev) adev = ACPI_COMPANION(indio_dev->dev.parent); if (!adev) - return 0; + return -ENXIO; /* Read _ONT data, which should be a package of 6 integers. */ status = acpi_evaluate_object(adev->handle, "_ONT", NULL, &buffer); if (status == AE_NOT_FOUND) { - return 0; + return -ENXIO; } else if (ACPI_FAILURE(status)) { dev_warn(&indio_dev->dev, "failed to execute _ONT: %d\n", status); -- cgit v1.2.3 From e55245d115bb9054cb72cdd5dda5660f4484873a Mon Sep 17 00:00:00 2001 From: Paul Cercueil Date: Thu, 30 Mar 2023 12:21:00 +0200 Subject: iio: adc: ad7192: Change "shorted" channels to differential The AD7192 provides a specific channel configuration where both negative and positive inputs are connected to AIN2. This was represented in the ad7192 driver as a IIO channel with .channel = 2 and .extended_name set to "shorted". The problem with this approach, is that the driver provided two IIO channels with the identifier .channel = 2; one "shorted" and the other not. This goes against the IIO ABI, as a channel identifier should be unique. Address this issue by changing "shorted" channels to being differential instead, with channel 2 vs. itself, as we're actually measuring AIN2 vs. itself. Note that the fix tag is for the commit that moved the driver out of staging. The bug existed before that, but backporting would become very complex further down and unlikely to happen. Fixes: b581f748cce0 ("staging: iio: adc: ad7192: move out of staging") Signed-off-by: Paul Cercueil Co-developed-by: Alisa Roman Signed-off-by: Alisa Roman Reviewed-by: Nuno Sa Link: https://lore.kernel.org/r/20230330102100.17590-1-paul@crapouillou.net Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ad7192.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c index 55a6ab591016..99bb604b78c8 100644 --- a/drivers/iio/adc/ad7192.c +++ b/drivers/iio/adc/ad7192.c @@ -897,10 +897,6 @@ static const struct iio_info ad7195_info = { __AD719x_CHANNEL(_si, _channel1, -1, _address, NULL, IIO_VOLTAGE, \ BIT(IIO_CHAN_INFO_SCALE), ad7192_calibsys_ext_info) -#define AD719x_SHORTED_CHANNEL(_si, _channel1, _address) \ - __AD719x_CHANNEL(_si, _channel1, -1, _address, "shorted", IIO_VOLTAGE, \ - BIT(IIO_CHAN_INFO_SCALE), ad7192_calibsys_ext_info) - #define AD719x_TEMP_CHANNEL(_si, _address) \ __AD719x_CHANNEL(_si, 0, -1, _address, NULL, IIO_TEMP, 0, NULL) @@ -908,7 +904,7 @@ static const struct iio_chan_spec ad7192_channels[] = { AD719x_DIFF_CHANNEL(0, 1, 2, AD7192_CH_AIN1P_AIN2M), AD719x_DIFF_CHANNEL(1, 3, 4, AD7192_CH_AIN3P_AIN4M), AD719x_TEMP_CHANNEL(2, AD7192_CH_TEMP), - AD719x_SHORTED_CHANNEL(3, 2, AD7192_CH_AIN2P_AIN2M), + AD719x_DIFF_CHANNEL(3, 2, 2, AD7192_CH_AIN2P_AIN2M), AD719x_CHANNEL(4, 1, AD7192_CH_AIN1), AD719x_CHANNEL(5, 2, AD7192_CH_AIN2), AD719x_CHANNEL(6, 3, AD7192_CH_AIN3), @@ -922,7 +918,7 @@ static const struct iio_chan_spec ad7193_channels[] = { AD719x_DIFF_CHANNEL(2, 5, 6, AD7193_CH_AIN5P_AIN6M), AD719x_DIFF_CHANNEL(3, 7, 8, AD7193_CH_AIN7P_AIN8M), AD719x_TEMP_CHANNEL(4, AD7193_CH_TEMP), - AD719x_SHORTED_CHANNEL(5, 2, AD7193_CH_AIN2P_AIN2M), + AD719x_DIFF_CHANNEL(5, 2, 2, AD7193_CH_AIN2P_AIN2M), AD719x_CHANNEL(6, 1, AD7193_CH_AIN1), AD719x_CHANNEL(7, 2, AD7193_CH_AIN2), AD719x_CHANNEL(8, 3, AD7193_CH_AIN3), -- cgit v1.2.3 From 9c0d6ccd7d6bbd275e390b55a3390b4274291d95 Mon Sep 17 00:00:00 2001 From: Sean Nyekjaer Date: Wed, 3 May 2023 18:20:28 +0200 Subject: iio: adc: stm32-adc: skip adc-diff-channels setup if none is present If no adc differential channels are defined driver will fail with EINVAL: stm32-adc: probe of 48003000.adc:adc@0 failed with error -22 Fix this by skipping the initialization if no channels are defined. This applies only to the legacy way of initializing adc channels. Fixes: d7705f35448a ("iio: adc: stm32-adc: convert to device properties") Signed-off-by: Sean Nyekjaer Link: https://lore.kernel.org/r/20230503162029.3654093-1-sean@geanix.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/stm32-adc.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c index 1aadb2ad2cab..c105d82c3e45 100644 --- a/drivers/iio/adc/stm32-adc.c +++ b/drivers/iio/adc/stm32-adc.c @@ -2006,16 +2006,15 @@ static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm * to get the *real* number of channels. */ ret = device_property_count_u32(dev, "st,adc-diff-channels"); - if (ret < 0) - return ret; - - ret /= (int)(sizeof(struct stm32_adc_diff_channel) / sizeof(u32)); - if (ret > adc_info->max_channels) { - dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n"); - return -EINVAL; - } else if (ret > 0) { - adc->num_diff = ret; - num_channels += ret; + if (ret > 0) { + ret /= (int)(sizeof(struct stm32_adc_diff_channel) / sizeof(u32)); + if (ret > adc_info->max_channels) { + dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n"); + return -EINVAL; + } else if (ret > 0) { + adc->num_diff = ret; + num_channels += ret; + } } /* Optional sample time is provided either for each, or all channels */ -- cgit v1.2.3 From 3e27ef0ced49f8ae7883c25fadf76a2086e99025 Mon Sep 17 00:00:00 2001 From: Sean Nyekjaer Date: Wed, 3 May 2023 18:20:29 +0200 Subject: iio: adc: stm32-adc: skip adc-channels setup if none is present If only adc differential channels are defined driver will fail with stm32-adc: probe of 48003000.adc:adc@0 failed with error -22 Fix this by skipping the initialization if no channels are defined. This applies only to the legacy way of initializing adc channels. Fixes: d7705f35448a ("iio: adc: stm32-adc: convert to device properties") Signed-off-by: Sean Nyekjaer Reviewed-by: Olivier Moysan Link: https://lore.kernel.org/r/20230503162029.3654093-2-sean@geanix.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/stm32-adc.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c index c105d82c3e45..bd7e2408bf28 100644 --- a/drivers/iio/adc/stm32-adc.c +++ b/drivers/iio/adc/stm32-adc.c @@ -2036,6 +2036,7 @@ static int stm32_adc_legacy_chan_init(struct iio_dev *indio_dev, struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX]; struct device *dev = &indio_dev->dev; u32 num_diff = adc->num_diff; + int num_se = nchans - num_diff; int size = num_diff * sizeof(*diff) / sizeof(u32); int scan_index = 0, ret, i, c; u32 smp = 0, smps[STM32_ADC_CH_MAX], chans[STM32_ADC_CH_MAX]; @@ -2062,29 +2063,32 @@ static int stm32_adc_legacy_chan_init(struct iio_dev *indio_dev, scan_index++; } } - - ret = device_property_read_u32_array(dev, "st,adc-channels", chans, - nchans); - if (ret) - return ret; - - for (c = 0; c < nchans; c++) { - if (chans[c] >= adc_info->max_channels) { - dev_err(&indio_dev->dev, "Invalid channel %d\n", - chans[c]); - return -EINVAL; + if (num_se > 0) { + ret = device_property_read_u32_array(dev, "st,adc-channels", chans, num_se); + if (ret) { + dev_err(&indio_dev->dev, "Failed to get st,adc-channels %d\n", ret); + return ret; } - /* Channel can't be configured both as single-ended & diff */ - for (i = 0; i < num_diff; i++) { - if (chans[c] == diff[i].vinp) { - dev_err(&indio_dev->dev, "channel %d misconfigured\n", chans[c]); + for (c = 0; c < num_se; c++) { + if (chans[c] >= adc_info->max_channels) { + dev_err(&indio_dev->dev, "Invalid channel %d\n", + chans[c]); return -EINVAL; } + + /* Channel can't be configured both as single-ended & diff */ + for (i = 0; i < num_diff; i++) { + if (chans[c] == diff[i].vinp) { + dev_err(&indio_dev->dev, "channel %d misconfigured\n", + chans[c]); + return -EINVAL; + } + } + stm32_adc_chan_init_one(indio_dev, &channels[scan_index], + chans[c], 0, scan_index, false); + scan_index++; } - stm32_adc_chan_init_one(indio_dev, &channels[scan_index], - chans[c], 0, scan_index, false); - scan_index++; } if (adc->nsmps > 0) { @@ -2305,7 +2309,7 @@ static int stm32_adc_chan_fw_init(struct iio_dev *indio_dev, bool timestamping) if (legacy) ret = stm32_adc_legacy_chan_init(indio_dev, adc, channels, - num_channels); + timestamping ? num_channels - 1 : num_channels); else ret = stm32_adc_generic_chan_init(indio_dev, adc, channels); if (ret < 0) -- cgit v1.2.3 From 72c1d11074d6160f19760d57fc78b2a7f52bcb1a Mon Sep 17 00:00:00 2001 From: Alexander Stein Date: Mon, 24 Apr 2023 11:23:12 +0200 Subject: dt-bindings: iio: imx8qxp-adc: add missing vref-supply Although this property is used right now for IIO_CHAN_INFO_SCALE, this ADC has two internal reference voltages, which the driver currently doesn't make use of. Fixes: db73419d8c06 ("dt-bindings: iio: adc: Add binding documentation for NXP IMX8QXP ADC") Signed-off-by: Alexander Stein Reviewed-by: Haibo Chen Acked-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20230424092312.61746-1-alexander.stein@ew.tq-group.com Signed-off-by: Jonathan Cameron --- Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml index 63369ba388e4..0a192ca192c5 100644 --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml @@ -39,6 +39,12 @@ properties: power-domains: maxItems: 1 + vref-supply: + description: | + External ADC reference voltage supply on VREFH pad. If VERID[MVI] is + set, there are additional, internal reference voltages selectable. + VREFH1 is always from VREFH pad. + "#io-channel-cells": const: 1 @@ -72,6 +78,7 @@ examples: assigned-clocks = <&clk IMX_SC_R_ADC_0>; assigned-clock-rates = <24000000>; power-domains = <&pd IMX_SC_R_ADC_0>; + vref-supply = <®_1v8>; #io-channel-cells = <1>; }; }; -- cgit v1.2.3 From a551c26e8e568fad42120843521529241b9bceec Mon Sep 17 00:00:00 2001 From: Frank Li Date: Mon, 1 May 2023 10:36:04 -0400 Subject: iio: light: vcnl4035: fixed chip ID check VCNL4035 register(0xE) ID_L and ID_M define as: ID_L: 0x80 ID_H: 7:6 (0:0) 5:4 (0:0) slave address = 0x60 (7-bit) (0:1) slave address = 0x51 (7-bit) (1:0) slave address = 0x40 (7-bit) (1:0) slave address = 0x41 (7-bit) 3:0 Version code default (0:0:0:0) So just check ID_L. Fixes: 55707294c4eb ("iio: light: Add support for vishay vcnl4035") Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20230501143605.1615549-1-Frank.Li@nxp.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/light/vcnl4035.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c index 14e29330e972..94f5d611e98c 100644 --- a/drivers/iio/light/vcnl4035.c +++ b/drivers/iio/light/vcnl4035.c @@ -8,6 +8,7 @@ * TODO: Proximity */ #include +#include #include #include #include @@ -42,6 +43,7 @@ #define VCNL4035_ALS_PERS_MASK GENMASK(3, 2) #define VCNL4035_INT_ALS_IF_H_MASK BIT(12) #define VCNL4035_INT_ALS_IF_L_MASK BIT(13) +#define VCNL4035_DEV_ID_MASK GENMASK(7, 0) /* Default values */ #define VCNL4035_MODE_ALS_ENABLE BIT(0) @@ -413,6 +415,7 @@ static int vcnl4035_init(struct vcnl4035_data *data) return ret; } + id = FIELD_GET(VCNL4035_DEV_ID_MASK, id); if (id != VCNL4035_DEV_ID_VAL) { dev_err(&data->client->dev, "Wrong id, got %x, expected %x\n", id, VCNL4035_DEV_ID_VAL); -- cgit v1.2.3 From 24febc99ca725dcf42d57168a2f4e8a75a5ade92 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Wed, 3 May 2023 11:58:17 +0200 Subject: iio: addac: ad74413: fix resistance input processing On success, ad74413r_get_single_adc_result() returns IIO_VAL_INT aka 1. So currently, the IIO_CHAN_INFO_PROCESSED case is effectively equivalent to the IIO_CHAN_INFO_RAW case, and we never call ad74413r_adc_to_resistance_result() to convert the adc measurement to ohms. Check ret for being negative rather than non-zero. Fixes: fea251b6a5dbd (iio: addac: add AD74413R driver) Signed-off-by: Rasmus Villemoes Reviewed-by: Nuno Sa Link: https://lore.kernel.org/r/20230503095817.452551-1-linux@rasmusvillemoes.dk Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/addac/ad74413r.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c index 07e9f6ae16a8..e3366cf5eb31 100644 --- a/drivers/iio/addac/ad74413r.c +++ b/drivers/iio/addac/ad74413r.c @@ -1007,7 +1007,7 @@ static int ad74413r_read_raw(struct iio_dev *indio_dev, ret = ad74413r_get_single_adc_result(indio_dev, chan->channel, val); - if (ret) + if (ret < 0) return ret; ad74413r_adc_to_resistance_result(*val, val); -- cgit v1.2.3 From a146eccb68be161ae9eab5f3f68bb0ed7c0fbaa8 Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Mon, 8 May 2023 06:02:08 +0200 Subject: iio: dac: build ad5758 driver when AD5758 is selected Commit 28d1a7ac2a0d ("iio: dac: Add AD5758 support") adds the config AD5758 and the corresponding driver ad5758.c. In the Makefile, the ad5758 driver is however included when AD5755 is selected, not when AD5758 is selected. Probably, this was simply a mistake that happened by copy-and-paste and forgetting to adjust the actual line. Surprisingly, no one has ever noticed that this driver is actually only included when AD5755 is selected and that the config AD5758 has actually no effect on the build. Fixes: 28d1a7ac2a0d ("iio: dac: Add AD5758 support") Signed-off-by: Lukas Bulwahn Link: https://lore.kernel.org/r/20230508040208.12033-1-lukas.bulwahn@gmail.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/dac/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index 6c74fea21736..addd97a78838 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -17,7 +17,7 @@ obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o obj-$(CONFIG_AD5592R) += ad5592r.o obj-$(CONFIG_AD5593R) += ad5593r.o obj-$(CONFIG_AD5755) += ad5755.o -obj-$(CONFIG_AD5755) += ad5758.o +obj-$(CONFIG_AD5758) += ad5758.o obj-$(CONFIG_AD5761) += ad5761.o obj-$(CONFIG_AD5764) += ad5764.o obj-$(CONFIG_AD5766) += ad5766.o -- cgit v1.2.3 From d049151adde4ec17392592d145e94f0086f8ee80 Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Mon, 8 May 2023 14:01:25 +0300 Subject: iio: bu27034: Ensure reset is written The reset bit must be always written to the hardware no matter what value is in a cache or register. Ensure this by using regmap_write_bits() instead of the regmap_update_bits(). Furthermore, the SWRESET bit may be self-clearing, so mark the SYSTEM_CONTROL register volatile to guarantee we do also read the right state - should we ever need to read it. Finally, writing the SWRESET bit will restore the default register values. This can cause register cache to be outdated if there are any register values cached. Rebuild register cache after SWRESET and use regmap_update_bits() when performing the reset. Signed-off-by: Matti Vaittinen Fixes: e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor") Link: https://lore.kernel.org/r/ZFjWhbfuN5XcKty+@fedora Signed-off-by: Jonathan Cameron --- drivers/iio/light/rohm-bu27034.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c index 25c9b79574a5..f85194fda6b0 100644 --- a/drivers/iio/light/rohm-bu27034.c +++ b/drivers/iio/light/rohm-bu27034.c @@ -231,6 +231,9 @@ struct bu27034_result { static const struct regmap_range bu27034_volatile_ranges[] = { { + .range_min = BU27034_REG_SYSTEM_CONTROL, + .range_max = BU27034_REG_SYSTEM_CONTROL, + }, { .range_min = BU27034_REG_MODE_CONTROL4, .range_max = BU27034_REG_MODE_CONTROL4, }, { @@ -1272,12 +1275,19 @@ static int bu27034_chip_init(struct bu27034_data *data) int ret, sel; /* Reset */ - ret = regmap_update_bits(data->regmap, BU27034_REG_SYSTEM_CONTROL, + ret = regmap_write_bits(data->regmap, BU27034_REG_SYSTEM_CONTROL, BU27034_MASK_SW_RESET, BU27034_MASK_SW_RESET); if (ret) return dev_err_probe(data->dev, ret, "Sensor reset failed\n"); msleep(1); + + ret = regmap_reinit_cache(data->regmap, &bu27034_regmap); + if (ret) { + dev_err(data->dev, "Failed to reinit reg cache\n"); + return ret; + } + /* * Read integration time here to ensure it is in regmap cache. We do * this to speed-up the int-time acquisition in the start of the buffer -- cgit v1.2.3 From 56cd3d1c5c5b073a1a444eafdcf97d4d866d351a Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Fri, 12 May 2023 10:53:41 +0300 Subject: iio: accel: kx022a fix irq getting The fwnode_irq_get_byname() was returning 0 at device-tree mapping error. If this occurred, the KX022A driver did abort the probe but errorneously directly returned the return value from fwnode_irq_get_byname() from probe. In case of a device-tree mapping error this indicated success. The fwnode_irq_get_byname() has since been fixed to not return zero on error so the check for fwnode_irq_get_byname() can be relaxed to only treat negative values as errors. This will also do decent fix even when backported to branches where fwnode_irq_get_byname() can still return zero on error because KX022A probe should later fail at IRQ requesting and a prober error handling should follow. Relax the return value check for fwnode_irq_get_byname() to only treat negative values as errors. Reported-by: kernel test robot Reported-by: Dan Carpenter Closes: https://lore.kernel.org/r/202305110245.MFxC9bUj-lkp@intel.com/ Link: https://lore.kernel.org/r/202305110245.MFxC9bUj-lkp@intel.com/ Signed-off-by: Matti Vaittinen Fixes: 7c1d1677b322 ("iio: accel: Support Kionix/ROHM KX022A accelerometer") Link: https://lore.kernel.org/r/b45b4b638db109c6078d243252df3a7b0485f7d5.1683875389.git.mazziesaccount@gmail.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/accel/kionix-kx022a.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c index f98393d74666..b8636fa8eaeb 100644 --- a/drivers/iio/accel/kionix-kx022a.c +++ b/drivers/iio/accel/kionix-kx022a.c @@ -1048,7 +1048,7 @@ int kx022a_probe_internal(struct device *dev) data->ien_reg = KX022A_REG_INC4; } else { irq = fwnode_irq_get_byname(fwnode, "INT2"); - if (irq <= 0) + if (irq < 0) return dev_err_probe(dev, irq, "No suitable IRQ\n"); data->inc_reg = KX022A_REG_INC5; -- cgit v1.2.3 From 09d3bec7009186bdba77039df01e5834788b3f95 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Thu, 11 May 2023 02:43:30 +0200 Subject: iio: dac: mcp4725: Fix i2c_master_send() return value handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The i2c_master_send() returns number of sent bytes on success, or negative on error. The suspend/resume callbacks expect zero on success and non-zero on error. Adapt the return value of the i2c_master_send() to the expectation of the suspend and resume callbacks, including proper validation of the return value. Fixes: cf35ad61aca2 ("iio: add mcp4725 I2C DAC driver") Signed-off-by: Marek Vasut Reviewed-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230511004330.206942-1-marex@denx.de Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/dac/mcp4725.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c index 46bf758760f8..3f5661a3718f 100644 --- a/drivers/iio/dac/mcp4725.c +++ b/drivers/iio/dac/mcp4725.c @@ -47,12 +47,18 @@ static int mcp4725_suspend(struct device *dev) struct mcp4725_data *data = iio_priv(i2c_get_clientdata( to_i2c_client(dev))); u8 outbuf[2]; + int ret; outbuf[0] = (data->powerdown_mode + 1) << 4; outbuf[1] = 0; data->powerdown = true; - return i2c_master_send(data->client, outbuf, 2); + ret = i2c_master_send(data->client, outbuf, 2); + if (ret < 0) + return ret; + else if (ret != 2) + return -EIO; + return 0; } static int mcp4725_resume(struct device *dev) @@ -60,13 +66,19 @@ static int mcp4725_resume(struct device *dev) struct mcp4725_data *data = iio_priv(i2c_get_clientdata( to_i2c_client(dev))); u8 outbuf[2]; + int ret; /* restore previous DAC value */ outbuf[0] = (data->dac_value >> 8) & 0xf; outbuf[1] = data->dac_value & 0xff; data->powerdown = false; - return i2c_master_send(data->client, outbuf, 2); + ret = i2c_master_send(data->client, outbuf, 2); + if (ret < 0) + return ret; + else if (ret != 2) + return -EIO; + return 0; } static DEFINE_SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume); -- cgit v1.2.3 From 55720d242052e860b9fde445e302e0425722e7f1 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 9 May 2023 14:34:22 +0200 Subject: dt-bindings: iio: adc: renesas,rcar-gyroadc: Fix adi,ad7476 compatible value The conversion to json-schema accidentally dropped the "ad" part prefix from the compatible value. Fixes: 8c41245872e2 ("dt-bindings:iio:adc:renesas,rcar-gyroadc: txt to yaml conversion.") Signed-off-by: Geert Uytterhoeven Reviewed-by: Marek Vasut Reviewed-by: Krzysztof Kozlowski Reviewed-by: Wolfram Sang Link: https://lore.kernel.org/r/6b328a3f52657c20759f3a5bb2fe033d47644ba8.1683635404.git.geert+renesas@glider.be Cc: Signed-off-by: Jonathan Cameron --- Documentation/devicetree/bindings/iio/adc/renesas,rcar-gyroadc.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,rcar-gyroadc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,rcar-gyroadc.yaml index 1c7aee5ed3e0..36dff3250ea7 100644 --- a/Documentation/devicetree/bindings/iio/adc/renesas,rcar-gyroadc.yaml +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rcar-gyroadc.yaml @@ -90,7 +90,7 @@ patternProperties: of the MAX chips to the GyroADC, while MISO line of each Maxim ADC connects to a shared input pin of the GyroADC. enum: - - adi,7476 + - adi,ad7476 - fujitsu,mb88101a - maxim,max1162 - maxim,max11100 -- cgit v1.2.3 From 626d312028bec44209d0ecd5beaa9b1aa8945f7d Mon Sep 17 00:00:00 2001 From: Masahiro Honda Date: Thu, 18 May 2023 20:08:16 +0900 Subject: iio: adc: ad_sigma_delta: Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag The Sigma-Delta ADCs supported by this driver can use SDO as an interrupt line to indicate the completion of a conversion. However, some devices cannot properly detect the completion of a conversion by an interrupt. This is for the reason mentioned in the following commit. commit e9849777d0e2 ("genirq: Add flag to force mask in disable_irq[_nosync]()") A read operation is performed by an extra interrupt before the completion of a conversion. At this time, the value read from the ADC data register is the same as the previous conversion result. This patch fixes the issue by setting IRQ_DISABLE_UNLAZY flag. Fixes: 0c6ef985a1fd ("iio: adc: ad7791: fix IRQ flags") Fixes: 1a913270e57a ("iio: adc: ad7793: Fix IRQ flag") Fixes: e081102f3077 ("iio: adc: ad7780: Fix IRQ flag") Fixes: 89a86da5cb8e ("iio: adc: ad7192: Add IRQ flag") Fixes: 79ef91493f54 ("iio: adc: ad7124: Set IRQ type to falling") Signed-off-by: Masahiro Honda Link: https://lore.kernel.org/r/20230518110816.248-1-honda@mechatrax.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ad_sigma_delta.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c index d8570f620785..7e2192870743 100644 --- a/drivers/iio/adc/ad_sigma_delta.c +++ b/drivers/iio/adc/ad_sigma_delta.c @@ -584,6 +584,10 @@ static int devm_ad_sd_probe_trigger(struct device *dev, struct iio_dev *indio_de init_completion(&sigma_delta->completion); sigma_delta->irq_dis = true; + + /* the IRQ core clears IRQ_DISABLE_UNLAZY flag when freeing an IRQ */ + irq_set_status_flags(sigma_delta->spi->irq, IRQ_DISABLE_UNLAZY); + ret = devm_request_irq(dev, sigma_delta->spi->irq, ad_sd_data_rdy_trig_poll, sigma_delta->info->irq_flags | IRQF_NO_AUTOEN, -- cgit v1.2.3 From bbaae0c79ebd49f61ad942a8bf9e12bfc7f821bb Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Maneyrol Date: Tue, 9 May 2023 15:22:02 +0000 Subject: iio: imu: inv_icm42600: fix timestamp reset Timestamp reset is not done in the correct place. It must be done before enabling buffer. The reason is that interrupt timestamping is always happening when the chip is on, even if the corresponding sensor is off. When the sensor restarts, timestamp is wrong if you don't do a reset first. Fixes: ec74ae9fd37c ("iio: imu: inv_icm42600: add accurate timestamping") Signed-off-by: Jean-Baptiste Maneyrol Cc: Link: https://lore.kernel.org/r/20230509152202.245444-1-inv.git-commit@tdk.com Signed-off-by: Jonathan Cameron --- drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c index 99576b2c171f..32d7f8364230 100644 --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c @@ -275,9 +275,14 @@ static int inv_icm42600_buffer_preenable(struct iio_dev *indio_dev) { struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); struct device *dev = regmap_get_device(st->map); + struct inv_icm42600_timestamp *ts = iio_priv(indio_dev); pm_runtime_get_sync(dev); + mutex_lock(&st->lock); + inv_icm42600_timestamp_reset(ts); + mutex_unlock(&st->lock); + return 0; } @@ -375,7 +380,6 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev) struct device *dev = regmap_get_device(st->map); unsigned int sensor; unsigned int *watermark; - struct inv_icm42600_timestamp *ts; struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT; unsigned int sleep_temp = 0; unsigned int sleep_sensor = 0; @@ -385,11 +389,9 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev) if (indio_dev == st->indio_gyro) { sensor = INV_ICM42600_SENSOR_GYRO; watermark = &st->fifo.watermark.gyro; - ts = iio_priv(st->indio_gyro); } else if (indio_dev == st->indio_accel) { sensor = INV_ICM42600_SENSOR_ACCEL; watermark = &st->fifo.watermark.accel; - ts = iio_priv(st->indio_accel); } else { return -EINVAL; } @@ -417,8 +419,6 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev) if (!st->fifo.on) ret = inv_icm42600_set_temp_conf(st, false, &sleep_temp); - inv_icm42600_timestamp_reset(ts); - out_unlock: mutex_unlock(&st->lock); -- cgit v1.2.3 From a6e766dea0a22918735176e4af862d535962f11e Mon Sep 17 00:00:00 2001 From: Ekansh Gupta Date: Tue, 23 May 2023 16:25:47 +0100 Subject: misc: fastrpc: Pass proper scm arguments for secure map request If a map request is made with securemap attribute, the memory ownership needs to be reassigned to new VMID to allow access from protection domain. Currently only DSP VMID is passed to the reassign call which is incorrect as only a combination of HLOS and DSP VMID is allowed for memory ownership reassignment and passing only DSP VMID will cause assign call failure. Also pass proper restoring permissions to HLOS as the source permission will now carry both HLOS and DSP VMID permission. Change is also made to get valid physical address from scatter/gather for this allocation request. Fixes: e90d91190619 ("misc: fastrpc: Add support to secure memory map") Cc: stable Tested-by: Ekansh Gupta Signed-off-by: Ekansh Gupta Signed-off-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20230523152550.438363-2-srinivas.kandagatla@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/misc/fastrpc.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index f48466960f1b..32a5415624bf 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -316,12 +316,14 @@ static void fastrpc_free_map(struct kref *ref) if (map->table) { if (map->attr & FASTRPC_ATTR_SECUREMAP) { struct qcom_scm_vmperm perm; + int vmid = map->fl->cctx->vmperms[0].vmid; + u64 src_perms = BIT(QCOM_SCM_VMID_HLOS) | BIT(vmid); int err = 0; perm.vmid = QCOM_SCM_VMID_HLOS; perm.perm = QCOM_SCM_PERM_RWX; err = qcom_scm_assign_mem(map->phys, map->size, - &map->fl->cctx->perms, &perm, 1); + &src_perms, &perm, 1); if (err) { dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d", map->phys, map->size, err); @@ -787,8 +789,12 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd, goto map_err; } - map->phys = sg_dma_address(map->table->sgl); - map->phys += ((u64)fl->sctx->sid << 32); + if (attr & FASTRPC_ATTR_SECUREMAP) { + map->phys = sg_phys(map->table->sgl); + } else { + map->phys = sg_dma_address(map->table->sgl); + map->phys += ((u64)fl->sctx->sid << 32); + } map->size = len; map->va = sg_virt(map->table->sgl); map->len = len; @@ -798,9 +804,15 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd, * If subsystem VMIDs are defined in DTSI, then do * hyp_assign from HLOS to those VM(s) */ + u64 src_perms = BIT(QCOM_SCM_VMID_HLOS); + struct qcom_scm_vmperm dst_perms[2] = {0}; + + dst_perms[0].vmid = QCOM_SCM_VMID_HLOS; + dst_perms[0].perm = QCOM_SCM_PERM_RW; + dst_perms[1].vmid = fl->cctx->vmperms[0].vmid; + dst_perms[1].perm = QCOM_SCM_PERM_RWX; map->attr = attr; - err = qcom_scm_assign_mem(map->phys, (u64)map->size, &fl->cctx->perms, - fl->cctx->vmperms, fl->cctx->vmcount); + err = qcom_scm_assign_mem(map->phys, (u64)map->size, &src_perms, dst_perms, 2); if (err) { dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d", map->phys, map->size, err); -- cgit v1.2.3 From 3c7d0079a1831118ef232bd9c2f34d058a1f31c2 Mon Sep 17 00:00:00 2001 From: Ekansh Gupta Date: Tue, 23 May 2023 16:25:48 +0100 Subject: misc: fastrpc: Reassign memory ownership only for remote heap The userspace map request for remote heap allocates CMA memory. The ownership of this memory needs to be reassigned to proper owners to allow access from the protection domain running on DSP. This reassigning of ownership is not correct if done for any other supported flags. When any other flag is requested from userspace, fastrpc is trying to reassign the ownership of memory and this reassignment is getting skipped for remote heap request which is incorrect. Add proper flag check to reassign the memory only if remote heap is requested. Fixes: 532ad70c6d44 ("misc: fastrpc: Add mmap request assigning for static PD pool") Cc: stable Tested-by: Ekansh Gupta Signed-off-by: Ekansh Gupta Reviewed-by: Srinivas Kandagatla Signed-off-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20230523152550.438363-3-srinivas.kandagatla@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/misc/fastrpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 32a5415624bf..a654dc416480 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -1904,7 +1904,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) req.vaddrout = rsp_msg.vaddr; /* Add memory to static PD pool, protection thru hypervisor */ - if (req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) { + if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) { struct qcom_scm_vmperm perm; perm.vmid = QCOM_SCM_VMID_HLOS; -- cgit v1.2.3 From b6a062853ddf6b4f653af2d8b75ba45bb9a036ad Mon Sep 17 00:00:00 2001 From: Richard Acayan Date: Tue, 23 May 2023 16:25:49 +0100 Subject: misc: fastrpc: return -EPIPE to invocations on device removal The return value is initialized as -1, or -EPERM. The completion of an invocation implies that the return value is set appropriately, but "Permission denied" does not accurately describe the outcome of the invocation. Set the invocation's return value to a more appropriate "Broken pipe", as the cleanup breaks the driver's connection with rpmsg. Fixes: c68cfb718c8f ("misc: fastrpc: Add support for context Invoke method") Cc: stable Signed-off-by: Richard Acayan Reviewed-by: Srinivas Kandagatla Signed-off-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20230523152550.438363-4-srinivas.kandagatla@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/misc/fastrpc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index a654dc416480..964f67dad2f9 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -2349,8 +2349,10 @@ static void fastrpc_notify_users(struct fastrpc_user *user) struct fastrpc_invoke_ctx *ctx; spin_lock(&user->lock); - list_for_each_entry(ctx, &user->pending, node) + list_for_each_entry(ctx, &user->pending, node) { + ctx->retval = -EPIPE; complete(&ctx->work); + } spin_unlock(&user->lock); } -- cgit v1.2.3 From 46248400d81e2aa0b65cd659d6f40188192a58b6 Mon Sep 17 00:00:00 2001 From: Richard Acayan Date: Tue, 23 May 2023 16:25:50 +0100 Subject: misc: fastrpc: reject new invocations during device removal The channel's rpmsg object allows new invocations to be made. After old invocations are already interrupted, the driver shouldn't try to invoke anymore. Invalidating the rpmsg at the end of the driver removal function makes it easy to cause a race condition in userspace. Even closing a file descriptor before the driver finishes its cleanup can cause an invocation via fastrpc_release_current_dsp_process() and subsequent timeout. Invalidate the channel before the invocations are interrupted to make sure that no invocations can be created to hang after the device closes. Fixes: c68cfb718c8f ("misc: fastrpc: Add support for context Invoke method") Cc: stable Signed-off-by: Richard Acayan Signed-off-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20230523152550.438363-5-srinivas.kandagatla@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/misc/fastrpc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 964f67dad2f9..30d4d0476248 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -2363,7 +2363,9 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev) struct fastrpc_user *user; unsigned long flags; + /* No invocations past this point */ spin_lock_irqsave(&cctx->lock, flags); + cctx->rpdev = NULL; list_for_each_entry(user, &cctx->users, user) fastrpc_notify_users(user); spin_unlock_irqrestore(&cctx->lock, flags); @@ -2382,7 +2384,6 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev) of_platform_depopulate(&rpdev->dev); - cctx->rpdev = NULL; fastrpc_channel_ctx_put(cctx); } -- cgit v1.2.3 From 929f4e7c06ca40c8e58b418c94708d880518d9b4 Mon Sep 17 00:00:00 2001 From: Ivan Bornyakov Date: Tue, 30 May 2023 21:49:35 +0800 Subject: MAINTAINERS: update Microchip MPF FPGA reviewers As I'm leaving Metrotek, hand over reviewing duty of Microchip MPF FPGA driver to Vladimir. Signed-off-by: Ivan Bornyakov Acked-by: Conor Dooley Acked-by: Vladimir Georgiev Acked-by: Xu Yilun Link: https://lore.kernel.org/r/20230429104838.5064-2-i.bornyakov@metrotek.ru Signed-off-by: Xu Yilun Link: https://lore.kernel.org/r/20230530134936.634370-2-yilun.xu@intel.com Signed-off-by: Greg Kroah-Hartman --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 250518fc70ff..205b545d0a4a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13836,7 +13836,7 @@ F: drivers/tty/serial/8250/8250_pci1xxxx.c MICROCHIP POLARFIRE FPGA DRIVERS M: Conor Dooley -R: Ivan Bornyakov +R: Vladimir Georgiev L: linux-fpga@vger.kernel.org S: Supported F: Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml -- cgit v1.2.3 From 9da6225bc7377941acff4476493d515b9fdfea48 Mon Sep 17 00:00:00 2001 From: Ivan Bornyakov Date: Tue, 30 May 2023 21:49:36 +0800 Subject: dt-bindings: fpga: replace Ivan Bornyakov maintainership As I'm leaving Metrotek, hand over Lattice Slave SPI sysCONFIG FPGA manager and Microchip Polarfire FPGA manager maintainership duties to Vladimir. Signed-off-by: Ivan Bornyakov Acked-by: Conor Dooley Acked-by: Vladimir Georgiev Acked-by: Rob Herring Link: https://lore.kernel.org/r/20230429104838.5064-3-i.bornyakov@metrotek.ru Signed-off-by: Xu Yilun Link: https://lore.kernel.org/r/20230530134936.634370-3-yilun.xu@intel.com Signed-off-by: Greg Kroah-Hartman --- Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml | 2 +- Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml b/Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml index 4fb05eb84e2a..164331eb6275 100644 --- a/Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml +++ b/Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Lattice Slave SPI sysCONFIG FPGA manager maintainers: - - Ivan Bornyakov + - Vladimir Georgiev description: | Lattice sysCONFIG port, which is used for FPGA configuration, among others, diff --git a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml index 527532f039ce..a157eecfb5fc 100644 --- a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml +++ b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Microchip Polarfire FPGA manager. maintainers: - - Ivan Bornyakov + - Vladimir Georgiev description: Device Tree Bindings for Microchip Polarfire FPGA Manager using slave SPI to -- cgit v1.2.3 From 71698da37ce70c457751baea507a122851a2d481 Mon Sep 17 00:00:00 2001 From: Samuel Iglesias Gonsálvez Date: Tue, 30 May 2023 10:35:46 +0200 Subject: MAINTAINERS: Vaibhav Gupta is the new ipack maintainer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I have no longer access to the HW, nor time to properly maintain it. Adding Vaibhav as maintainer as he currently has access to the HW, he is working at CERN (user of these drivers) and he is maintaining them internally there. Signed-off-by: Samuel Iglesias Gonsálvez Acked-by: Vaibhav Gupta Link: https://lore.kernel.org/r/20230530083546.4831-1-vaibhavgupta40@gmail.com Signed-off-by: Greg Kroah-Hartman --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 205b545d0a4a..4d623e78ac2f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10114,7 +10114,7 @@ S: Maintained F: Documentation/process/kernel-docs.rst INDUSTRY PACK SUBSYSTEM (IPACK) -M: Samuel Iglesias Gonsalvez +M: Vaibhav Gupta M: Jens Taprogge M: Greg Kroah-Hartman L: industrypack-devel@lists.sourceforge.net -- cgit v1.2.3 From ffa28312e2837ae788855840e126fa6f8a1ac35d Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 16 May 2023 11:12:51 +0300 Subject: firmware_loader: Fix a NULL vs IS_ERR() check The crypto_alloc_shash() function doesn't return NULL, it returns error pointers. Update the check accordingly. Fixes: 02fe26f25325 ("firmware_loader: Add debug message with checksum for FW file") Signed-off-by: Dan Carpenter Reviewed-by: Cezary Rojewski Acked-by: Luis Chamberlain Link: https://lore.kernel.org/r/36ef6042-ce74-4e8e-9e2c-5b5c28940610@kili.mountain Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 9d79d5ad9102..b58c42f1b1ce 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -812,7 +812,7 @@ static void fw_log_firmware_info(const struct firmware *fw, const char *name, st char *outbuf; alg = crypto_alloc_shash("sha256", 0, 0); - if (!alg) + if (IS_ERR(alg)) return; sha256buf = kmalloc(SHA256_DIGEST_SIZE, GFP_KERNEL); -- cgit v1.2.3 From 4acfe3dfde685a5a9eaec5555351918e2d7266a1 Mon Sep 17 00:00:00 2001 From: Mirsad Goran Todorovac Date: Tue, 9 May 2023 10:47:45 +0200 Subject: test_firmware: prevent race conditions by a correct implementation of locking Dan Carpenter spotted a race condition in a couple of situations like these in the test_firmware driver: static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) { u8 val; int ret; ret = kstrtou8(buf, 10, &val); if (ret) return ret; mutex_lock(&test_fw_mutex); *(u8 *)cfg = val; mutex_unlock(&test_fw_mutex); /* Always return full write size even if we didn't consume all */ return size; } static ssize_t config_num_requests_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { int rc; mutex_lock(&test_fw_mutex); if (test_fw_config->reqs) { pr_err("Must call release_all_firmware prior to changing config\n"); rc = -EINVAL; mutex_unlock(&test_fw_mutex); goto out; } mutex_unlock(&test_fw_mutex); rc = test_dev_config_update_u8(buf, count, &test_fw_config->num_requests); out: return rc; } static ssize_t config_read_fw_idx_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { return test_dev_config_update_u8(buf, count, &test_fw_config->read_fw_idx); } The function test_dev_config_update_u8() is called from both the locked and the unlocked context, function config_num_requests_store() and config_read_fw_idx_store() which can both be called asynchronously as they are driver's methods, while test_dev_config_update_u8() and siblings change their argument pointed to by u8 *cfg or similar pointer. To avoid deadlock on test_fw_mutex, the lock is dropped before calling test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8() itself, but alas this creates a race condition. Having two locks wouldn't assure a race-proof mutual exclusion. This situation is best avoided by the introduction of a new, unlocked function __test_dev_config_update_u8() which can be called from the locked context and reducing test_dev_config_update_u8() to: static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) { int ret; mutex_lock(&test_fw_mutex); ret = __test_dev_config_update_u8(buf, size, cfg); mutex_unlock(&test_fw_mutex); return ret; } doing the locking and calling the unlocked primitive, which enables both locked and unlocked versions without duplication of code. The similar approach was applied to all functions called from the locked and the unlocked context, which safely mitigates both deadlocks and race conditions in the driver. __test_dev_config_update_bool(), __test_dev_config_update_u8() and __test_dev_config_update_size_t() unlocked versions of the functions were introduced to be called from the locked contexts as a workaround without releasing the main driver's lock and thereof causing a race condition. The test_dev_config_update_bool(), test_dev_config_update_u8() and test_dev_config_update_size_t() locked versions of the functions are being called from driver methods without the unnecessary multiplying of the locking and unlocking code for each method, and complicating the code with saving of the return value across lock. Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf") Cc: Luis Chamberlain Cc: Greg Kroah-Hartman Cc: Russ Weight Cc: Takashi Iwai Cc: Tianfei Zhang Cc: Shuah Khan Cc: Colin Ian King Cc: Randy Dunlap Cc: linux-kselftest@vger.kernel.org Cc: stable@vger.kernel.org # v5.4 Suggested-by: Dan Carpenter Signed-off-by: Mirsad Goran Todorovac Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr Signed-off-by: Greg Kroah-Hartman --- lib/test_firmware.c | 52 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 05ed84c2fc4c..35417e0af3f4 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -353,16 +353,26 @@ static ssize_t config_test_show_str(char *dst, return len; } -static int test_dev_config_update_bool(const char *buf, size_t size, +static inline int __test_dev_config_update_bool(const char *buf, size_t size, bool *cfg) { int ret; - mutex_lock(&test_fw_mutex); if (kstrtobool(buf, cfg) < 0) ret = -EINVAL; else ret = size; + + return ret; +} + +static int test_dev_config_update_bool(const char *buf, size_t size, + bool *cfg) +{ + int ret; + + mutex_lock(&test_fw_mutex); + ret = __test_dev_config_update_bool(buf, size, cfg); mutex_unlock(&test_fw_mutex); return ret; @@ -373,7 +383,8 @@ static ssize_t test_dev_config_show_bool(char *buf, bool val) return snprintf(buf, PAGE_SIZE, "%d\n", val); } -static int test_dev_config_update_size_t(const char *buf, +static int __test_dev_config_update_size_t( + const char *buf, size_t size, size_t *cfg) { @@ -384,9 +395,7 @@ static int test_dev_config_update_size_t(const char *buf, if (ret) return ret; - mutex_lock(&test_fw_mutex); *(size_t *)cfg = new; - mutex_unlock(&test_fw_mutex); /* Always return full write size even if we didn't consume all */ return size; @@ -402,7 +411,7 @@ static ssize_t test_dev_config_show_int(char *buf, int val) return snprintf(buf, PAGE_SIZE, "%d\n", val); } -static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) +static int __test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) { u8 val; int ret; @@ -411,14 +420,23 @@ static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) if (ret) return ret; - mutex_lock(&test_fw_mutex); *(u8 *)cfg = val; - mutex_unlock(&test_fw_mutex); /* Always return full write size even if we didn't consume all */ return size; } +static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) +{ + int ret; + + mutex_lock(&test_fw_mutex); + ret = __test_dev_config_update_u8(buf, size, cfg); + mutex_unlock(&test_fw_mutex); + + return ret; +} + static ssize_t test_dev_config_show_u8(char *buf, u8 val) { return snprintf(buf, PAGE_SIZE, "%u\n", val); @@ -471,10 +489,10 @@ static ssize_t config_num_requests_store(struct device *dev, mutex_unlock(&test_fw_mutex); goto out; } - mutex_unlock(&test_fw_mutex); - rc = test_dev_config_update_u8(buf, count, - &test_fw_config->num_requests); + rc = __test_dev_config_update_u8(buf, count, + &test_fw_config->num_requests); + mutex_unlock(&test_fw_mutex); out: return rc; @@ -518,10 +536,10 @@ static ssize_t config_buf_size_store(struct device *dev, mutex_unlock(&test_fw_mutex); goto out; } - mutex_unlock(&test_fw_mutex); - rc = test_dev_config_update_size_t(buf, count, - &test_fw_config->buf_size); + rc = __test_dev_config_update_size_t(buf, count, + &test_fw_config->buf_size); + mutex_unlock(&test_fw_mutex); out: return rc; @@ -548,10 +566,10 @@ static ssize_t config_file_offset_store(struct device *dev, mutex_unlock(&test_fw_mutex); goto out; } - mutex_unlock(&test_fw_mutex); - rc = test_dev_config_update_size_t(buf, count, - &test_fw_config->file_offset); + rc = __test_dev_config_update_size_t(buf, count, + &test_fw_config->file_offset); + mutex_unlock(&test_fw_mutex); out: return rc; -- cgit v1.2.3 From be37bed754ed90b2655382f93f9724b3c1aae847 Mon Sep 17 00:00:00 2001 From: Mirsad Goran Todorovac Date: Tue, 9 May 2023 10:47:47 +0200 Subject: test_firmware: fix a memory leak with reqs buffer Dan Carpenter spotted that test_fw_config->reqs will be leaked if trigger_batched_requests_store() is called two or more times. The same appears with trigger_batched_requests_async_store(). This bug wasn't trigger by the tests, but observed by Dan's visual inspection of the code. The recommended workaround was to return -EBUSY if test_fw_config->reqs is already allocated. Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf") Cc: Luis Chamberlain Cc: Greg Kroah-Hartman Cc: Russ Weight Cc: Tianfei Zhang Cc: Shuah Khan Cc: Colin Ian King Cc: Randy Dunlap Cc: linux-kselftest@vger.kernel.org Cc: stable@vger.kernel.org # v5.4 Suggested-by: Dan Carpenter Suggested-by: Takashi Iwai Signed-off-by: Mirsad Goran Todorovac Reviewed-by: Dan Carpenter Acked-by: Luis Chamberlain Link: https://lore.kernel.org/r/20230509084746.48259-2-mirsad.todorovac@alu.unizg.hr Signed-off-by: Greg Kroah-Hartman --- lib/test_firmware.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 35417e0af3f4..91b232ed3161 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -913,6 +913,11 @@ static ssize_t trigger_batched_requests_store(struct device *dev, mutex_lock(&test_fw_mutex); + if (test_fw_config->reqs) { + rc = -EBUSY; + goto out_bail; + } + test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2)); @@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, mutex_lock(&test_fw_mutex); + if (test_fw_config->reqs) { + rc = -EBUSY; + goto out_bail; + } + test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2)); -- cgit v1.2.3 From 48e156023059e57a8fc68b498439832f7600ffff Mon Sep 17 00:00:00 2001 From: Mirsad Goran Todorovac Date: Tue, 9 May 2023 10:47:49 +0200 Subject: test_firmware: fix the memory leak of the allocated firmware buffer The following kernel memory leak was noticed after running tools/testing/selftests/firmware/fw_run_tests.sh: [root@pc-mtodorov firmware]# cat /sys/kernel/debug/kmemleak . . . unreferenced object 0xffff955389bc3400 (size 1024): comm "test_firmware-0", pid 5451, jiffies 4294944822 (age 65.652s) hex dump (first 32 bytes): 47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00 GH4567.......... 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [] slab_post_alloc_hook+0x8c/0x3c0 [] __kmem_cache_alloc_node+0x184/0x240 [] kmalloc_trace+0x2e/0xc0 [] test_fw_run_batch_request+0x9d/0x180 [] kthread+0x10b/0x140 [] ret_from_fork+0x29/0x50 unreferenced object 0xffff9553c334b400 (size 1024): comm "test_firmware-1", pid 5452, jiffies 4294944822 (age 65.652s) hex dump (first 32 bytes): 47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00 GH4567.......... 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [] slab_post_alloc_hook+0x8c/0x3c0 [] __kmem_cache_alloc_node+0x184/0x240 [] kmalloc_trace+0x2e/0xc0 [] test_fw_run_batch_request+0x9d/0x180 [] kthread+0x10b/0x140 [] ret_from_fork+0x29/0x50 unreferenced object 0xffff9553c334f000 (size 1024): comm "test_firmware-2", pid 5453, jiffies 4294944822 (age 65.652s) hex dump (first 32 bytes): 47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00 GH4567.......... 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [] slab_post_alloc_hook+0x8c/0x3c0 [] __kmem_cache_alloc_node+0x184/0x240 [] kmalloc_trace+0x2e/0xc0 [] test_fw_run_batch_request+0x9d/0x180 [] kthread+0x10b/0x140 [] ret_from_fork+0x29/0x50 unreferenced object 0xffff9553c3348400 (size 1024): comm "test_firmware-3", pid 5454, jiffies 4294944822 (age 65.652s) hex dump (first 32 bytes): 47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00 GH4567.......... 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [] slab_post_alloc_hook+0x8c/0x3c0 [] __kmem_cache_alloc_node+0x184/0x240 [] kmalloc_trace+0x2e/0xc0 [] test_fw_run_batch_request+0x9d/0x180 [] kthread+0x10b/0x140 [] ret_from_fork+0x29/0x50 [root@pc-mtodorov firmware]# Note that the size 1024 corresponds to the size of the test firmware buffer. The actual number of the buffers leaked is around 70-110, depending on the test run. The cause of the leak is the following: request_partial_firmware_into_buf() and request_firmware_into_buf() provided firmware buffer isn't released on release_firmware(), we have allocated it and we are responsible for deallocating it manually. This is introduced in a number of context where previously only release_firmware() was called, which was insufficient. Reported-by: Mirsad Goran Todorovac Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf") Cc: Greg Kroah-Hartman Cc: Dan Carpenter Cc: Takashi Iwai Cc: Luis Chamberlain Cc: Russ Weight Cc: Tianfei zhang Cc: Christophe JAILLET Cc: Zhengchao Shao Cc: Colin Ian King Cc: linux-kernel@vger.kernel.org Cc: Kees Cook Cc: Scott Branden Cc: Luis R. Rodriguez Cc: linux-kselftest@vger.kernel.org Cc: stable@vger.kernel.org # v5.4 Signed-off-by: Mirsad Goran Todorovac Link: https://lore.kernel.org/r/20230509084746.48259-3-mirsad.todorovac@alu.unizg.hr Signed-off-by: Greg Kroah-Hartman --- lib/test_firmware.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 91b232ed3161..1d7d480b8eeb 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -45,6 +45,7 @@ struct test_batched_req { bool sent; const struct firmware *fw; const char *name; + const char *fw_buf; struct completion completion; struct task_struct *task; struct device *dev; @@ -175,8 +176,14 @@ static void __test_release_all_firmware(void) for (i = 0; i < test_fw_config->num_requests; i++) { req = &test_fw_config->reqs[i]; - if (req->fw) + if (req->fw) { + if (req->fw_buf) { + kfree_const(req->fw_buf); + req->fw_buf = NULL; + } release_firmware(req->fw); + req->fw = NULL; + } } vfree(test_fw_config->reqs); @@ -670,6 +677,8 @@ static ssize_t trigger_request_store(struct device *dev, mutex_lock(&test_fw_mutex); release_firmware(test_firmware); + if (test_fw_config->reqs) + __test_release_all_firmware(); test_firmware = NULL; rc = request_firmware(&test_firmware, name, dev); if (rc) { @@ -770,6 +779,8 @@ static ssize_t trigger_async_request_store(struct device *dev, mutex_lock(&test_fw_mutex); release_firmware(test_firmware); test_firmware = NULL; + if (test_fw_config->reqs) + __test_release_all_firmware(); rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL, NULL, trigger_async_request_cb); if (rc) { @@ -812,6 +823,8 @@ static ssize_t trigger_custom_fallback_store(struct device *dev, mutex_lock(&test_fw_mutex); release_firmware(test_firmware); + if (test_fw_config->reqs) + __test_release_all_firmware(); test_firmware = NULL; rc = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOUEVENT, name, dev, GFP_KERNEL, NULL, @@ -874,6 +887,8 @@ static int test_fw_run_batch_request(void *data) test_fw_config->buf_size); if (!req->fw) kfree(test_buf); + else + req->fw_buf = test_buf; } else { req->rc = test_fw_config->req_firmware(&req->fw, req->name, @@ -934,6 +949,7 @@ static ssize_t trigger_batched_requests_store(struct device *dev, req->fw = NULL; req->idx = i; req->name = test_fw_config->name; + req->fw_buf = NULL; req->dev = dev; init_completion(&req->completion); req->task = kthread_run(test_fw_run_batch_request, req, @@ -1038,6 +1054,7 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, for (i = 0; i < test_fw_config->num_requests; i++) { req = &test_fw_config->reqs[i]; req->name = test_fw_config->name; + req->fw_buf = NULL; req->fw = NULL; req->idx = i; init_completion(&req->completion); -- cgit v1.2.3