From 0dec4d2f2636b9e54d9d29f17afc7687c5407f78 Mon Sep 17 00:00:00 2001 From: Shreeya Patel Date: Fri, 26 Aug 2022 17:53:52 +0530 Subject: [PATCH 01/21] iio: light: tsl2583: Fix module unloading tsl2583 probe() uses devm_iio_device_register() and calling iio_device_unregister() causes the unregister to occur twice. s Switch to iio_device_register() instead of devm_iio_device_register() in probe to avoid the device managed cleanup. Fixes: 371894f5d1a0 ("iio: tsl2583: add runtime power management support") Signed-off-by: Shreeya Patel Link: https://lore.kernel.org/r/20220826122352.288438-1-shreeya.patel@collabora.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/light/tsl2583.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/light/tsl2583.c b/drivers/iio/light/tsl2583.c index 0a2ca1a8146d..7bcb5c718922 100644 --- a/drivers/iio/light/tsl2583.c +++ b/drivers/iio/light/tsl2583.c @@ -858,7 +858,7 @@ static int tsl2583_probe(struct i2c_client *clientp, TSL2583_POWER_OFF_DELAY_MS); pm_runtime_use_autosuspend(&clientp->dev); - ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev); + ret = iio_device_register(indio_dev); if (ret) { dev_err(&clientp->dev, "%s: iio registration failed\n", __func__); From 7578847b5949db3a75163908bd99c46d27e8b19f Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 26 Sep 2022 12:10:37 +0300 Subject: [PATCH 02/21] iio: adc: mcp3911: fix sizeof() vs ARRAY_SIZE() bug This code uses sizeof() instead of ARRAY_SIZE() so it reads beyond the end of the mcp3911_osr_table[] array. Fixes: 6d965885f4ea ("iio: adc: mcp3911: add support for oversampling ratio") Signed-off-by: Dan Carpenter Reviewed-by: Marcus Folkesson Link: https://lore.kernel.org/r/YzFsjY3xLHUQMjVr@kili Signed-off-by: Jonathan Cameron --- drivers/iio/adc/mcp3911.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c index b35fd2c9c3c0..015a9ffdb26a 100644 --- a/drivers/iio/adc/mcp3911.c +++ b/drivers/iio/adc/mcp3911.c @@ -248,7 +248,7 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev, break; case IIO_CHAN_INFO_OVERSAMPLING_RATIO: - for (int i = 0; i < sizeof(mcp3911_osr_table); i++) { + for (int i = 0; i < ARRAY_SIZE(mcp3911_osr_table); i++) { if (val == mcp3911_osr_table[i]) { val = FIELD_PREP(MCP3911_CONFIG_OSR, i); ret = mcp3911_update(adc, MCP3911_REG_CONFIG, MCP3911_CONFIG_OSR, From a83695a666eb3541873c3c9734ec4e1d10ca2d7f Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Tue, 27 Sep 2022 11:25:37 +0200 Subject: [PATCH 03/21] iio: adc: mcp3911: return proper error code on failure to allocate trigger smatch warnings: drivers/iio/adc/mcp3911.c:441 mcp3911_probe() warn: passing zero to 'PTR_ERR' Fixes: 08a65f61db69 ("iio: adc: mcp3911: add support for interrupts") Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: Marcus Folkesson Link: https://lore.kernel.org/r/20220927092537.94663-1-marcus.folkesson@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/adc/mcp3911.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c index 015a9ffdb26a..7f0b37994fe5 100644 --- a/drivers/iio/adc/mcp3911.c +++ b/drivers/iio/adc/mcp3911.c @@ -496,7 +496,7 @@ static int mcp3911_probe(struct spi_device *spi) indio_dev->name, iio_device_id(indio_dev)); if (!adc->trig) - return PTR_ERR(adc->trig); + return -ENOMEM; adc->trig->ops = &mcp3911_trigger_ops; iio_trigger_set_drvdata(adc->trig, adc); From 815f1647a603a822d66630bbe22cab4bc097c8c3 Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Mon, 10 Oct 2022 21:46:41 +0200 Subject: [PATCH 04/21] iio: adc: mcp3911: use correct id bits The device ID should be shifted 6 bits to left according to datasheet. Fixes: 3a89b289df5d ("iio: adc: add support for mcp3911") Signed-off-by: Marcus Folkesson Link: https://lore.kernel.org/r/20221010194641.676484-1-marcus.folkesson@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/adc/mcp3911.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c index 7f0b37994fe5..f57f5eb23d49 100644 --- a/drivers/iio/adc/mcp3911.c +++ b/drivers/iio/adc/mcp3911.c @@ -55,8 +55,8 @@ /* Internal voltage reference in mV */ #define MCP3911_INT_VREF_MV 1200 -#define MCP3911_REG_READ(reg, id) ((((reg) << 1) | ((id) << 5) | (1 << 0)) & 0xff) -#define MCP3911_REG_WRITE(reg, id) ((((reg) << 1) | ((id) << 5) | (0 << 0)) & 0xff) +#define MCP3911_REG_READ(reg, id) ((((reg) << 1) | ((id) << 6) | (1 << 0)) & 0xff) +#define MCP3911_REG_WRITE(reg, id) ((((reg) << 1) | ((id) << 6) | (0 << 0)) & 0xff) #define MCP3911_NUM_CHANNELS 2 From aa6c77d05eb1c57ee5b95a7b83a39384c37df4d9 Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Mon, 10 Oct 2022 21:46:54 +0200 Subject: [PATCH 05/21] iio: adc: mcp3911: mask out device ID in debug prints The Device ID should not be included when printing register. Signed-off-by: Marcus Folkesson Link: https://lore.kernel.org/r/20221010194654.676525-1-marcus.folkesson@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/adc/mcp3911.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c index f57f5eb23d49..76b334f5ac61 100644 --- a/drivers/iio/adc/mcp3911.c +++ b/drivers/iio/adc/mcp3911.c @@ -57,6 +57,7 @@ #define MCP3911_REG_READ(reg, id) ((((reg) << 1) | ((id) << 6) | (1 << 0)) & 0xff) #define MCP3911_REG_WRITE(reg, id) ((((reg) << 1) | ((id) << 6) | (0 << 0)) & 0xff) +#define MCP3911_REG_MASK GENMASK(4, 1) #define MCP3911_NUM_CHANNELS 2 @@ -89,8 +90,8 @@ static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len) be32_to_cpus(val); *val >>= ((4 - len) * 8); - dev_dbg(&adc->spi->dev, "reading 0x%x from register 0x%x\n", *val, - reg >> 1); + dev_dbg(&adc->spi->dev, "reading 0x%x from register 0x%lx\n", *val, + FIELD_GET(MCP3911_REG_MASK, reg)); return ret; } From 174dac5dc800e4e2e4552baf6340846a344d01a3 Mon Sep 17 00:00:00 2001 From: Olivier Moysan Date: Wed, 12 Oct 2022 16:21:58 +0200 Subject: [PATCH 06/21] iio: adc: stm32-adc: fix channel sampling time init Fix channel init for ADC generic channel bindings. In generic channel initialization, stm32_adc_smpr_init() is called to initialize channel sampling time. The "st,min-sample-time-ns" property is an optional property. If it is not defined, stm32_adc_smpr_init() is currently skipped. However stm32_adc_smpr_init() must always be called, to force a minimum sampling time for the internal channels, as the minimum sampling time is known. Make stm32_adc_smpr_init() call unconditional. Fixes: 796e5d0b1e9b ("iio: adc: stm32-adc: use generic binding for sample-time") Signed-off-by: Olivier Moysan Reviewed-by: Andy Shevchenko Reviewed-by: Fabrice Gasnier Link: https://lore.kernel.org/r/20221012142205.13041-2-olivier.moysan@foss.st.com Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/stm32-adc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c index 6256977eb7f7..3cda529f081d 100644 --- a/drivers/iio/adc/stm32-adc.c +++ b/drivers/iio/adc/stm32-adc.c @@ -2086,18 +2086,19 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev, stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val, vin[1], scan_index, differential); + val = 0; ret = fwnode_property_read_u32(child, "st,min-sample-time-ns", &val); /* st,min-sample-time-ns is optional */ - if (!ret) { - stm32_adc_smpr_init(adc, channels[scan_index].channel, val); - if (differential) - stm32_adc_smpr_init(adc, vin[1], val); - } else if (ret != -EINVAL) { + if (ret && ret != -EINVAL) { dev_err(&indio_dev->dev, "Invalid st,min-sample-time-ns property %d\n", ret); goto err; } + stm32_adc_smpr_init(adc, channels[scan_index].channel, val); + if (differential) + stm32_adc_smpr_init(adc, vin[1], val); + scan_index++; } From 72b2aa38191bcba28389b0e20bf6b4f15017ff2b Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Thu, 13 Oct 2022 15:04:04 +0300 Subject: [PATCH 07/21] tools: iio: iio_utils: fix digit calculation The iio_utils uses a digit calculation in order to know length of the file name containing a buffer number. The digit calculation does not work for number 0. This leads to allocation of one character too small buffer for the file-name when file name contains value '0'. (Eg. buffer0). Fix digit calculation by returning one digit to be present for number '0'. Fixes: 096f9b862e60 ("tools:iio:iio_utils: implement digit calculation") Signed-off-by: Matti Vaittinen Link: https://lore.kernel.org/r/Y0f+tKCz+ZAIoroQ@dc75zzyyyyyyyyyyyyycy-3.rev.dnainternet.fi Cc: Signed-off-by: Jonathan Cameron --- tools/iio/iio_utils.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c index aadee6d34c74..8d35893b2fa8 100644 --- a/tools/iio/iio_utils.c +++ b/tools/iio/iio_utils.c @@ -547,6 +547,10 @@ static int calc_digits(int num) { int count = 0; + /* It takes a digit to represent zero */ + if (!num) + return 1; + while (num != 0) { num /= 10; count++; From 4132f19173211856d35180958d2754f5c56d520a Mon Sep 17 00:00:00 2001 From: Cosmin Tanislav Date: Fri, 14 Oct 2022 15:37:22 +0300 Subject: [PATCH 08/21] iio: temperature: ltc2983: allocate iio channels once Currently, every time the device wakes up from sleep, the iio_chan array is reallocated, leaking the previous one until the device is removed (basically never). Move the allocation to the probe function to avoid this. Signed-off-by: Cosmin Tanislav Fixes: f110f3188e563 ("iio: temperature: Add support for LTC2983") Cc: Link: https://lore.kernel.org/r/20221014123724.1401011-2-demonsingur@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/temperature/ltc2983.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c index b652d2b39bcf..a60ccf183687 100644 --- a/drivers/iio/temperature/ltc2983.c +++ b/drivers/iio/temperature/ltc2983.c @@ -1385,13 +1385,6 @@ static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio) return ret; } - st->iio_chan = devm_kzalloc(&st->spi->dev, - st->iio_channels * sizeof(*st->iio_chan), - GFP_KERNEL); - - if (!st->iio_chan) - return -ENOMEM; - ret = regmap_update_bits(st->regmap, LTC2983_GLOBAL_CONFIG_REG, LTC2983_NOTCH_FREQ_MASK, LTC2983_NOTCH_FREQ(st->filter_notch_freq)); @@ -1514,6 +1507,12 @@ static int ltc2983_probe(struct spi_device *spi) gpiod_set_value_cansleep(gpio, 0); } + st->iio_chan = devm_kzalloc(&spi->dev, + st->iio_channels * sizeof(*st->iio_chan), + GFP_KERNEL); + if (!st->iio_chan) + return -ENOMEM; + ret = ltc2983_setup(st, true); if (ret) return ret; From 54246b9034da08087ceb2083478c0d13403e12b4 Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Mon, 3 Oct 2022 11:11:33 +0300 Subject: [PATCH 09/21] iio: at91-sama5d2_adc: Fix unsafe buffer attributes The iio_triggered_buffer_setup_ext() was changed by commit 15097c7a1adc ("iio: buffer: wrap all buffer attributes into iio_dev_attr") to silently expect that all attributes given in buffer_attrs array are device-attributes. This expectation was not forced by the API - and some drivers did register attributes created by IIO_CONST_ATTR(). The added attribute "wrapping" does not copy the pointer to stored string constant and when the sysfs file is read the kernel will access to invalid location. Change the IIO_CONST_ATTRs from the driver to IIO_DEVICE_ATTR in order to prevent the invalid memory access. Signed-off-by: Matti Vaittinen Fixes: 15097c7a1adc ("iio: buffer: wrap all buffer attributes into iio_dev_attr") Tested-by: Claudiu Beznea Link: https://lore.kernel.org/r/be69775aa302159f088b8b91894e6ec449bca65b.1664782676.git.mazziesaccount@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/adc/at91-sama5d2_adc.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c index 4294d6539cdb..33e251552214 100644 --- a/drivers/iio/adc/at91-sama5d2_adc.c +++ b/drivers/iio/adc/at91-sama5d2_adc.c @@ -2193,17 +2193,30 @@ static ssize_t at91_adc_get_watermark(struct device *dev, return scnprintf(buf, PAGE_SIZE, "%d\n", st->dma_st.watermark); } +static ssize_t hwfifo_watermark_min_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", "2"); +} + +static ssize_t hwfifo_watermark_max_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", AT91_HWFIFO_MAX_SIZE_STR); +} + static IIO_DEVICE_ATTR(hwfifo_enabled, 0444, at91_adc_get_fifo_state, NULL, 0); static IIO_DEVICE_ATTR(hwfifo_watermark, 0444, at91_adc_get_watermark, NULL, 0); - -static IIO_CONST_ATTR(hwfifo_watermark_min, "2"); -static IIO_CONST_ATTR(hwfifo_watermark_max, AT91_HWFIFO_MAX_SIZE_STR); +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_min, 0); +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0); static const struct attribute *at91_adc_fifo_attributes[] = { - &iio_const_attr_hwfifo_watermark_min.dev_attr.attr, - &iio_const_attr_hwfifo_watermark_max.dev_attr.attr, + &iio_dev_attr_hwfifo_watermark_min.dev_attr.attr, + &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr, &iio_dev_attr_hwfifo_watermark.dev_attr.attr, &iio_dev_attr_hwfifo_enabled.dev_attr.attr, NULL, From ab0ee36e90f611f32c3a53afe9dc743de48138e2 Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Mon, 3 Oct 2022 11:10:51 +0300 Subject: [PATCH 10/21] iio: adxl372: Fix unsafe buffer attributes The iio_triggered_buffer_setup_ext() was changed by commit 15097c7a1adc ("iio: buffer: wrap all buffer attributes into iio_dev_attr") to silently expect that all attributes given in buffer_attrs array are device-attributes. This expectation was not forced by the API - and some drivers did register attributes created by IIO_CONST_ATTR(). The added attribute "wrapping" does not copy the pointer to stored string constant and when the sysfs file is read the kernel will access to invalid location. Change the IIO_CONST_ATTRs from the driver to IIO_DEVICE_ATTR in order to prevent the invalid memory access. Signed-off-by: Matti Vaittinen Fixes: 15097c7a1adc ("iio: buffer: wrap all buffer attributes into iio_dev_attr") Cc: Link: https://lore.kernel.org/r/19158499623cdf7f9c5efae1f13c9f1a918ff75f.1664782676.git.mazziesaccount@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/accel/adxl372.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c index e3ecbaee61f7..bc53af809d5d 100644 --- a/drivers/iio/accel/adxl372.c +++ b/drivers/iio/accel/adxl372.c @@ -998,17 +998,30 @@ static ssize_t adxl372_get_fifo_watermark(struct device *dev, return sprintf(buf, "%d\n", st->watermark); } -static IIO_CONST_ATTR(hwfifo_watermark_min, "1"); -static IIO_CONST_ATTR(hwfifo_watermark_max, - __stringify(ADXL372_FIFO_SIZE)); +static ssize_t hwfifo_watermark_min_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", "1"); +} + +static ssize_t hwfifo_watermark_max_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", __stringify(ADXL372_FIFO_SIZE)); +} + +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_min, 0); +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0); static IIO_DEVICE_ATTR(hwfifo_watermark, 0444, adxl372_get_fifo_watermark, NULL, 0); static IIO_DEVICE_ATTR(hwfifo_enabled, 0444, adxl372_get_fifo_enabled, NULL, 0); static const struct attribute *adxl372_fifo_attributes[] = { - &iio_const_attr_hwfifo_watermark_min.dev_attr.attr, - &iio_const_attr_hwfifo_watermark_max.dev_attr.attr, + &iio_dev_attr_hwfifo_watermark_min.dev_attr.attr, + &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr, &iio_dev_attr_hwfifo_watermark.dev_attr.attr, &iio_dev_attr_hwfifo_enabled.dev_attr.attr, NULL, From 5e23b33d1e84f04c80da6f1d89cbb3d3a3f81e01 Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Mon, 3 Oct 2022 11:10:29 +0300 Subject: [PATCH 11/21] iio: adxl367: Fix unsafe buffer attributes The devm_iio_kfifo_buffer_setup_ext() was changed by commit 15097c7a1adc ("iio: buffer: wrap all buffer attributes into iio_dev_attr") to silently expect that all attributes given in buffer_attrs array are device-attributes. This expectation was not forced by the API - and some drivers did register attributes created by IIO_CONST_ATTR(). The added attribute "wrapping" does not copy the pointer to stored string constant and when the sysfs file is read the kernel will access to invalid location. Change the IIO_CONST_ATTRs from the driver to IIO_DEVICE_ATTR in order to prevent the invalid memory access. Signed-off-by: Matti Vaittinen Fixes: 15097c7a1adc ("iio: buffer: wrap all buffer attributes into iio_dev_attr") Cc: Link: https://lore.kernel.org/r/2e2d9ec34fb1df8ab8e2749199822db8cc91d302.1664782676.git.mazziesaccount@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/accel/adxl367.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c index 47feb375b70b..7c7d78040793 100644 --- a/drivers/iio/accel/adxl367.c +++ b/drivers/iio/accel/adxl367.c @@ -1185,17 +1185,30 @@ static ssize_t adxl367_get_fifo_watermark(struct device *dev, return sysfs_emit(buf, "%d\n", fifo_watermark); } -static IIO_CONST_ATTR(hwfifo_watermark_min, "1"); -static IIO_CONST_ATTR(hwfifo_watermark_max, - __stringify(ADXL367_FIFO_MAX_WATERMARK)); +static ssize_t hwfifo_watermark_min_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", "1"); +} + +static ssize_t hwfifo_watermark_max_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", __stringify(ADXL367_FIFO_MAX_WATERMARK)); +} + +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_min, 0); +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0); static IIO_DEVICE_ATTR(hwfifo_watermark, 0444, adxl367_get_fifo_watermark, NULL, 0); static IIO_DEVICE_ATTR(hwfifo_enabled, 0444, adxl367_get_fifo_enabled, NULL, 0); static const struct attribute *adxl367_fifo_attributes[] = { - &iio_const_attr_hwfifo_watermark_min.dev_attr.attr, - &iio_const_attr_hwfifo_watermark_max.dev_attr.attr, + &iio_dev_attr_hwfifo_watermark_min.dev_attr.attr, + &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr, &iio_dev_attr_hwfifo_watermark.dev_attr.attr, &iio_dev_attr_hwfifo_enabled.dev_attr.attr, NULL, From a10a0f385ab8af08ddb762ac3eca11e1b6d1fe69 Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Mon, 3 Oct 2022 11:11:12 +0300 Subject: [PATCH 12/21] iio: bmc150-accel-core: Fix unsafe buffer attributes The iio_triggered_buffer_setup_ext() was changed by commit 15097c7a1adc ("iio: buffer: wrap all buffer attributes into iio_dev_attr") to silently expect that all attributes given in buffer_attrs array are device-attributes. This expectation was not forced by the API - and some drivers did register attributes created by IIO_CONST_ATTR(). The added attribute "wrapping" does not copy the pointer to stored string constant and when the sysfs file is read the kernel will access to invalid location. Change the IIO_CONST_ATTRs from the driver to IIO_DEVICE_ATTR in order to prevent the invalid memory access. Signed-off-by: Matti Vaittinen Fixes: 15097c7a1adc ("iio: buffer: wrap all buffer attributes into iio_dev_attr") Link: https://lore.kernel.org/r/cf8a56658fc38db8bed64f456d898f5ad5a2814f.1664782676.git.mazziesaccount@gmail.com Signed-off-by: Jonathan Cameron --- drivers/iio/accel/bmc150-accel-core.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c index 57e8a8350cd1..92f8b139acce 100644 --- a/drivers/iio/accel/bmc150-accel-core.c +++ b/drivers/iio/accel/bmc150-accel-core.c @@ -925,17 +925,30 @@ static const struct iio_chan_spec_ext_info bmc150_accel_ext_info[] = { { } }; -static IIO_CONST_ATTR(hwfifo_watermark_min, "1"); -static IIO_CONST_ATTR(hwfifo_watermark_max, - __stringify(BMC150_ACCEL_FIFO_LENGTH)); +static ssize_t hwfifo_watermark_min_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", "1"); +} + +static ssize_t hwfifo_watermark_max_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", __stringify(BMC150_ACCEL_FIFO_LENGTH)); +} + +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_min, 0); +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0); static IIO_DEVICE_ATTR(hwfifo_enabled, S_IRUGO, bmc150_accel_get_fifo_state, NULL, 0); static IIO_DEVICE_ATTR(hwfifo_watermark, S_IRUGO, bmc150_accel_get_fifo_watermark, NULL, 0); static const struct attribute *bmc150_accel_fifo_attributes[] = { - &iio_const_attr_hwfifo_watermark_min.dev_attr.attr, - &iio_const_attr_hwfifo_watermark_max.dev_attr.attr, + &iio_dev_attr_hwfifo_watermark_min.dev_attr.attr, + &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr, &iio_dev_attr_hwfifo_watermark.dev_attr.attr, &iio_dev_attr_hwfifo_enabled.dev_attr.attr, NULL, From 472a1482325b3a285e0bcf82c0b0edc689b7e8cd Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 2 Oct 2022 08:04:19 -0400 Subject: [PATCH 13/21] counter: Reduce DEFINE_COUNTER_ARRAY_POLARITY() to defining counter_array A spare warning was reported for drivers/counter/ti-ecap-capture.c:: sparse warnings: (new ones prefixed by >>) >> drivers/counter/ti-ecap-capture.c:380:8: sparse: sparse: symbol 'ecap_cnt_pol_array' was not declared. Should it be static? vim +/ecap_cnt_pol_array +380 drivers/counter/ti-ecap-capture.c 379 > 380 static DEFINE_COUNTER_ARRAY_POLARITY(ecap_cnt_pol_array, ecap_cnt_pol_avail, ECAP_NB_CEVT); 381 The first argument to the DEFINE_COUNTER_ARRAY_POLARITY() macro is a token serving as the symbol name in the definition of a new struct counter_array structure. However, this macro actually expands to two statements:: #define DEFINE_COUNTER_ARRAY_POLARITY(_name, _enums, _length) \ DEFINE_COUNTER_AVAILABLE(_name##_available, _enums); \ struct counter_array _name = { \ .type = COUNTER_COMP_SIGNAL_POLARITY, \ .avail = &(_name##_available), \ .length = (_length), \ } Because of this, the "static" on line 380 only applies to the first statement. This patch splits out the DEFINE_COUNTER_AVAILABLE() line and leaves DEFINE_COUNTER_ARRAY_POLARITY() as a simple structure definition to avoid issues like this. Reported-by: kernel test robot Link: https://lore.kernel.org/all/202210020619.NQbyomII-lkp@intel.com/ Cc: Julien Panis Signed-off-by: William Breathitt Gray --- drivers/counter/ti-ecap-capture.c | 3 ++- include/linux/counter.h | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/counter/ti-ecap-capture.c b/drivers/counter/ti-ecap-capture.c index af10de30aba5..b8dbf0212a8e 100644 --- a/drivers/counter/ti-ecap-capture.c +++ b/drivers/counter/ti-ecap-capture.c @@ -377,7 +377,8 @@ static const enum counter_signal_polarity ecap_cnt_pol_avail[] = { COUNTER_SIGNAL_POLARITY_NEGATIVE, }; -static DEFINE_COUNTER_ARRAY_POLARITY(ecap_cnt_pol_array, ecap_cnt_pol_avail, ECAP_NB_CEVT); +static DEFINE_COUNTER_AVAILABLE(ecap_cnt_pol_available, ecap_cnt_pol_avail); +static DEFINE_COUNTER_ARRAY_POLARITY(ecap_cnt_pol_array, ecap_cnt_pol_available, ECAP_NB_CEVT); static struct counter_comp ecap_cnt_signal_ext[] = { COUNTER_COMP_ARRAY_POLARITY(ecap_cnt_pol_read, ecap_cnt_pol_write, ecap_cnt_pol_array), diff --git a/include/linux/counter.h b/include/linux/counter.h index c41fa602ed28..b63746637de2 100644 --- a/include/linux/counter.h +++ b/include/linux/counter.h @@ -542,11 +542,10 @@ struct counter_array { #define DEFINE_COUNTER_ARRAY_CAPTURE(_name, _length) \ DEFINE_COUNTER_ARRAY_U64(_name, _length) -#define DEFINE_COUNTER_ARRAY_POLARITY(_name, _enums, _length) \ - DEFINE_COUNTER_AVAILABLE(_name##_available, _enums); \ +#define DEFINE_COUNTER_ARRAY_POLARITY(_name, _available, _length) \ struct counter_array _name = { \ .type = COUNTER_COMP_SIGNAL_POLARITY, \ - .avail = &(_name##_available), \ + .avail = &(_available), \ .length = (_length), \ } From ec0286dce78c3bb0e6a665c0baade2f2db56ce00 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 12 Oct 2022 17:51:25 +0300 Subject: [PATCH 14/21] counter: ti-ecap-capture: fix IS_ERR() vs NULL check The devm_counter_alloc() function returns NULL on error. It doesn't return error pointers. Fixes: 4e2f42aa00b6 ("counter: ti-ecap-capture: capture driver support for ECAP") Signed-off-by: Dan Carpenter Reviewed-by: Julien Panis Acked-by: Vignesh Raghavendra Link: https://lore.kernel.org/r/Y0bUbZvfDJHBG9C6@kili/ Signed-off-by: William Breathitt Gray --- drivers/counter/ti-ecap-capture.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/counter/ti-ecap-capture.c b/drivers/counter/ti-ecap-capture.c index b8dbf0212a8e..fb1cb1774674 100644 --- a/drivers/counter/ti-ecap-capture.c +++ b/drivers/counter/ti-ecap-capture.c @@ -480,8 +480,8 @@ static int ecap_cnt_probe(struct platform_device *pdev) int ret; counter_dev = devm_counter_alloc(dev, sizeof(*ecap_dev)); - if (IS_ERR(counter_dev)) - return PTR_ERR(counter_dev); + if (!counter_dev) + return -ENOMEM; counter_dev->name = ECAP_DRV_NAME; counter_dev->parent = dev; From 23722fb46725da42b80bc55a91a9bac69e35188a Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Thu, 21 Jul 2022 14:03:29 +0100 Subject: [PATCH 15/21] coresight: Fix possible deadlock with lock dependency With lockdeps enabled, we get the following warning: ====================================================== WARNING: possible circular locking dependency detected ------------------------------------------------------ kworker/u12:1/53 is trying to acquire lock: ffff80000adce220 (coresight_mutex){+.+.}-{4:4}, at: coresight_set_assoc_ectdev_mutex+0x3c/0x5c but task is already holding lock: ffff80000add1f60 (ect_mutex){+.+.}-{4:4}, at: cti_probe+0x318/0x394 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (ect_mutex){+.+.}-{4:4}: __mutex_lock_common+0xd8/0xe60 mutex_lock_nested+0x44/0x50 cti_add_assoc_to_csdev+0x4c/0x184 coresight_register+0x2f0/0x314 tmc_probe+0x33c/0x414 -> #0 (coresight_mutex){+.+.}-{4:4}: __lock_acquire+0x1a20/0x32d0 lock_acquire+0x160/0x308 __mutex_lock_common+0xd8/0xe60 mutex_lock_nested+0x44/0x50 coresight_set_assoc_ectdev_mutex+0x3c/0x5c cti_update_conn_xrefs+0x6c/0xf8 cti_probe+0x33c/0x394 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(ect_mutex); lock(coresight_mutex); lock(ect_mutex); lock(coresight_mutex); *** DEADLOCK *** 4 locks held by kworker/u12:1/53: #0: ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x1fc/0x63c #1: (deferred_probe_work){+.+.}-{0:0}, at: process_one_work+0x228/0x63c #2: (&dev->mutex){....}-{4:4}, at: __device_attach+0x48/0x1a8 #3: (ect_mutex){+.+.}-{4:4}, at: cti_probe+0x318/0x394 To fix the same, call cti_add_assoc_to_csdev without the holding coresight_mutex and confine the locking while setting the associated ect / cti device using coresight_set_assoc_ectdev_mutex(). Fixes: 177af8285b59 ("coresight: cti: Enable CTI associated with devices") Cc: Mathieu Poirier Cc: Suzuki K Poulose Cc: Mike Leach Cc: Leo Yan Signed-off-by: Sudeep Holla Reviewed-by: Mike Leach Signed-off-by: Suzuki K Poulose Link: https://lore.kernel.org/r/20220721130329.3787211-1-sudeep.holla@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 7 ++++--- drivers/hwtracing/coresight/coresight-cti-core.c | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index d5dbc67bacb4..f3068175ca9d 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1687,14 +1687,15 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) ret = coresight_fixup_device_conns(csdev); if (!ret) ret = coresight_fixup_orphan_conns(csdev); - if (!ret && cti_assoc_ops && cti_assoc_ops->add) - cti_assoc_ops->add(csdev); out_unlock: mutex_unlock(&coresight_mutex); /* Success */ - if (!ret) + if (!ret) { + if (cti_assoc_ops && cti_assoc_ops->add) + cti_assoc_ops->add(csdev); return csdev; + } /* Unregister the device if needed */ if (registered) { diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index 8988b2ed2ea6..1be92342b5b9 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -541,7 +541,7 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name, /* * Search the cti list to add an associated CTI into the supplied CS device * This will set the association if CTI declared before the CS device. - * (called from coresight_register() with coresight_mutex locked). + * (called from coresight_register() without coresight_mutex locked). */ static void cti_add_assoc_to_csdev(struct coresight_device *csdev) { @@ -569,7 +569,8 @@ static void cti_add_assoc_to_csdev(struct coresight_device *csdev) * if we found a matching csdev then update the ECT * association pointer for the device with this CTI. */ - csdev->ect_dev = ect_item->csdev; + coresight_set_assoc_ectdev_mutex(csdev->ect_dev, + ect_item->csdev); break; } } From 665c157e0204176023860b51a46528ba0ba62c33 Mon Sep 17 00:00:00 2001 From: James Clark Date: Wed, 5 Oct 2022 14:14:52 +0100 Subject: [PATCH 16/21] coresight: cti: Fix hang in cti_disable_hw() cti_enable_hw() and cti_disable_hw() are called from an atomic context so shouldn't use runtime PM because it can result in a sleep when communicating with firmware. Since commit 3c6656337852 ("Revert "firmware: arm_scmi: Add clock management to the SCMI power domain""), this causes a hang on Juno when running the Perf Coresight tests or running this command: perf record -e cs_etm//u -- ls This was also missed until the revert commit because pm_runtime_put() was called with the wrong device until commit 692c9a499b28 ("coresight: cti: Correct the parameter for pm_runtime_put") With lock and scheduler debugging enabled the following is output: coresight cti_sys0: cti_enable_hw -- dev:cti_sys0 parent: 20020000.cti BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1151 in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 330, name: perf-exec preempt_count: 2, expected: 0 RCU nest depth: 0, expected: 0 INFO: lockdep is turned off. irq event stamp: 0 hardirqs last enabled at (0): [<0000000000000000>] 0x0 hardirqs last disabled at (0): [] copy_process+0xa0c/0x1948 softirqs last enabled at (0): [] copy_process+0xa0c/0x1948 softirqs last disabled at (0): [<0000000000000000>] 0x0 CPU: 3 PID: 330 Comm: perf-exec Not tainted 6.0.0-00053-g042116d99298 #7 Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Sep 13 2022 Call trace: dump_backtrace+0x134/0x140 show_stack+0x20/0x58 dump_stack_lvl+0x8c/0xb8 dump_stack+0x18/0x34 __might_resched+0x180/0x228 __might_sleep+0x50/0x88 __pm_runtime_resume+0xac/0xb0 cti_enable+0x44/0x120 coresight_control_assoc_ectdev+0xc0/0x150 coresight_enable_path+0xb4/0x288 etm_event_start+0x138/0x170 etm_event_add+0x48/0x70 event_sched_in.isra.122+0xb4/0x280 merge_sched_in+0x1fc/0x3d0 visit_groups_merge.constprop.137+0x16c/0x4b0 ctx_sched_in+0x114/0x1f0 perf_event_sched_in+0x60/0x90 ctx_resched+0x68/0xb0 perf_event_exec+0x138/0x508 begin_new_exec+0x52c/0xd40 load_elf_binary+0x6b8/0x17d0 bprm_execve+0x360/0x7f8 do_execveat_common.isra.47+0x218/0x238 __arm64_sys_execve+0x48/0x60 invoke_syscall+0x4c/0x110 el0_svc_common.constprop.4+0xfc/0x120 do_el0_svc+0x34/0xc0 el0_svc+0x40/0x98 el0t_64_sync_handler+0x98/0xc0 el0t_64_sync+0x170/0x174 Fix the issue by removing the runtime PM calls completely. They are not needed here because it must have already been done when building the path for a trace. Fixes: 835d722ba10a ("coresight: cti: Initial CoreSight CTI Driver") Reported-by: Aishwarya TCV Reported-by: Cristian Marussi Suggested-by: Suzuki Poulose Signed-off-by: James Clark Reviewed-by: Mike Leach Tested-by: Mike Leach Signed-off-by: Suzuki K Poulose Link: https://lore.kernel.org/r/20221005131452.1506328-1-james.clark@arm.com --- drivers/hwtracing/coresight/coresight-cti-core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index 1be92342b5b9..4a02ae23d3a0 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -94,7 +94,6 @@ static int cti_enable_hw(struct cti_drvdata *drvdata) unsigned long flags; int rc = 0; - pm_runtime_get_sync(dev->parent); spin_lock_irqsave(&drvdata->spinlock, flags); /* no need to do anything if enabled or unpowered*/ @@ -119,7 +118,6 @@ static int cti_enable_hw(struct cti_drvdata *drvdata) /* cannot enable due to error */ cti_err_not_enabled: spin_unlock_irqrestore(&drvdata->spinlock, flags); - pm_runtime_put(dev->parent); return rc; } @@ -175,7 +173,6 @@ static int cti_disable_hw(struct cti_drvdata *drvdata) coresight_disclaim_device_unlocked(csdev); CS_LOCK(drvdata->base); spin_unlock(&drvdata->spinlock); - pm_runtime_put(dev->parent); return 0; /* not disabled this call */ From d917a62af81b133f35f627e7936e193c842a7947 Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Tue, 18 Oct 2022 08:10:14 -0400 Subject: [PATCH 17/21] counter: microchip-tcb-capture: Handle Signal1 read and Synapse The signal_read(), action_read(), and action_write() callbacks have been assuming Signal0 is requested without checking. This results in requests for Signal1 returning data for Signal0. This patch fixes these oversights by properly checking for the Signal's id in the respective callbacks and handling accordingly based on the particular Signal requested. The trig_inverted member of the mchp_tc_data is removed as superfluous. Fixes: 106b104137fd ("counter: Add microchip TCB capture counter") Cc: stable@vger.kernel.org Reviewed-by: Kamel Bouhara Link: https://lore.kernel.org/r/20221018121014.7368-1-william.gray@linaro.org/ Signed-off-by: William Breathitt Gray --- drivers/counter/microchip-tcb-capture.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/counter/microchip-tcb-capture.c b/drivers/counter/microchip-tcb-capture.c index f9dee15d9777..e2d1dc6ca668 100644 --- a/drivers/counter/microchip-tcb-capture.c +++ b/drivers/counter/microchip-tcb-capture.c @@ -28,7 +28,6 @@ struct mchp_tc_data { int qdec_mode; int num_channels; int channel[2]; - bool trig_inverted; }; static const enum counter_function mchp_tc_count_functions[] = { @@ -153,7 +152,7 @@ static int mchp_tc_count_signal_read(struct counter_device *counter, regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], SR), &sr); - if (priv->trig_inverted) + if (signal->id == 1) sigstatus = (sr & ATMEL_TC_MTIOB); else sigstatus = (sr & ATMEL_TC_MTIOA); @@ -171,6 +170,17 @@ static int mchp_tc_count_action_read(struct counter_device *counter, struct mchp_tc_data *const priv = counter_priv(counter); u32 cmr; + if (priv->qdec_mode) { + *action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES; + return 0; + } + + /* Only TIOA signal is evaluated in non-QDEC mode */ + if (synapse->signal->id != 0) { + *action = COUNTER_SYNAPSE_ACTION_NONE; + return 0; + } + regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], CMR), &cmr); switch (cmr & ATMEL_TC_ETRGEDG) { @@ -199,8 +209,8 @@ static int mchp_tc_count_action_write(struct counter_device *counter, struct mchp_tc_data *const priv = counter_priv(counter); u32 edge = ATMEL_TC_ETRGEDG_NONE; - /* QDEC mode is rising edge only */ - if (priv->qdec_mode) + /* QDEC mode is rising edge only; only TIOA handled in non-QDEC mode */ + if (priv->qdec_mode || synapse->signal->id != 0) return -EINVAL; switch (action) { From d501d37841d3b7f18402d71a9ef057eb9dde127e Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Thu, 20 Oct 2022 10:11:21 -0400 Subject: [PATCH 18/21] counter: 104-quad-8: Fix race getting function mode and direction The quad8_action_read() function checks the Count function mode and Count direction without first acquiring a lock. This is a race condition because the function mode could change by the time the direction is checked. Because the quad8_function_read() already acquires a lock internally, the quad8_function_read() is refactored to spin out the no-lock code to a new quad8_function_get() function. To resolve the race condition in quad8_action_read(), a lock is acquired before calling quad8_function_get() and quad8_direction_read() in order to get both function mode and direction atomically. Fixes: f1d8a071d45b ("counter: 104-quad-8: Add Generic Counter interface support") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20221020141121.15434-1-william.gray@linaro.org/ Signed-off-by: William Breathitt Gray --- drivers/counter/104-quad-8.c | 64 +++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c index 77a863b7eefe..deed4afadb29 100644 --- a/drivers/counter/104-quad-8.c +++ b/drivers/counter/104-quad-8.c @@ -232,34 +232,45 @@ static const enum counter_function quad8_count_functions_list[] = { COUNTER_FUNCTION_QUADRATURE_X4, }; +static int quad8_function_get(const struct quad8 *const priv, const size_t id, + enum counter_function *const function) +{ + if (!priv->quadrature_mode[id]) { + *function = COUNTER_FUNCTION_PULSE_DIRECTION; + return 0; + } + + switch (priv->quadrature_scale[id]) { + case 0: + *function = COUNTER_FUNCTION_QUADRATURE_X1_A; + return 0; + case 1: + *function = COUNTER_FUNCTION_QUADRATURE_X2_A; + return 0; + case 2: + *function = COUNTER_FUNCTION_QUADRATURE_X4; + return 0; + default: + /* should never reach this path */ + return -EINVAL; + } +} + static int quad8_function_read(struct counter_device *counter, struct counter_count *count, enum counter_function *function) { struct quad8 *const priv = counter_priv(counter); - const int id = count->id; unsigned long irqflags; + int retval; spin_lock_irqsave(&priv->lock, irqflags); - if (priv->quadrature_mode[id]) - switch (priv->quadrature_scale[id]) { - case 0: - *function = COUNTER_FUNCTION_QUADRATURE_X1_A; - break; - case 1: - *function = COUNTER_FUNCTION_QUADRATURE_X2_A; - break; - case 2: - *function = COUNTER_FUNCTION_QUADRATURE_X4; - break; - } - else - *function = COUNTER_FUNCTION_PULSE_DIRECTION; + retval = quad8_function_get(priv, count->id, function); spin_unlock_irqrestore(&priv->lock, irqflags); - return 0; + return retval; } static int quad8_function_write(struct counter_device *counter, @@ -359,6 +370,7 @@ static int quad8_action_read(struct counter_device *counter, enum counter_synapse_action *action) { struct quad8 *const priv = counter_priv(counter); + unsigned long irqflags; int err; enum counter_function function; const size_t signal_a_id = count->synapses[0].signal->id; @@ -374,9 +386,21 @@ static int quad8_action_read(struct counter_device *counter, return 0; } - err = quad8_function_read(counter, count, &function); - if (err) + spin_lock_irqsave(&priv->lock, irqflags); + + /* Get Count function and direction atomically */ + err = quad8_function_get(priv, count->id, &function); + if (err) { + spin_unlock_irqrestore(&priv->lock, irqflags); return err; + } + err = quad8_direction_read(counter, count, &direction); + if (err) { + spin_unlock_irqrestore(&priv->lock, irqflags); + return err; + } + + spin_unlock_irqrestore(&priv->lock, irqflags); /* Default action mode */ *action = COUNTER_SYNAPSE_ACTION_NONE; @@ -389,10 +413,6 @@ static int quad8_action_read(struct counter_device *counter, return 0; case COUNTER_FUNCTION_QUADRATURE_X1_A: if (synapse->signal->id == signal_a_id) { - err = quad8_direction_read(counter, count, &direction); - if (err) - return err; - if (direction == COUNTER_COUNT_DIRECTION_FORWARD) *action = COUNTER_SYNAPSE_ACTION_RISING_EDGE; else From d76308f03ee1574b0deffde45604252a51c77f6d Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 24 Oct 2022 08:32:39 +0200 Subject: [PATCH 19/21] Revert "coresight: cti: Fix hang in cti_disable_hw()" This reverts commit 665c157e0204176023860b51a46528ba0ba62c33. It causes reported build warnings: drivers/hwtracing/coresight/coresight-cti-core.c: In functio n 'cti_enable_hw': drivers/hwtracing/coresight/coresight-cti-core.c:93:24: warning: unused variable 'dev' [-Wunused-variable] 93 | struct device *dev = &drvdata->csdev->dev; | ^~~ drivers/hwtracing/coresight/coresight-cti-core.c: In function 'cti_disable_hw': drivers/hwtracing/coresight/coresight-cti-core.c:154:24: warning: unused variable 'dev' [-Wunused-variable] 154 | struct device *dev = &drvdata->csdev->dev; | ^~~ Reported-by: Stephen Rothwell Cc: Aishwarya TCV Cc: Cristian Marussi Cc: Suzuki Poulose Cc: James Clark Cc: Mike Leach Cc: Mike Leach Cc: Suzuki K Poulose Fixes: 665c157e0204 ("coresight: cti: Fix hang in cti_disable_hw()") Link: https://lore.kernel.org/r/20221024135752.2b83af97@canb.auug.org.au Signed-off-by: Greg Kroah-Hartman --- drivers/hwtracing/coresight/coresight-cti-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index 4a02ae23d3a0..1be92342b5b9 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -94,6 +94,7 @@ static int cti_enable_hw(struct cti_drvdata *drvdata) unsigned long flags; int rc = 0; + pm_runtime_get_sync(dev->parent); spin_lock_irqsave(&drvdata->spinlock, flags); /* no need to do anything if enabled or unpowered*/ @@ -118,6 +119,7 @@ static int cti_enable_hw(struct cti_drvdata *drvdata) /* cannot enable due to error */ cti_err_not_enabled: spin_unlock_irqrestore(&drvdata->spinlock, flags); + pm_runtime_put(dev->parent); return rc; } @@ -173,6 +175,7 @@ static int cti_disable_hw(struct cti_drvdata *drvdata) coresight_disclaim_device_unlocked(csdev); CS_LOCK(drvdata->base); spin_unlock(&drvdata->spinlock); + pm_runtime_put(dev->parent); return 0; /* not disabled this call */ From 6746eae4bbaddcc16b40efb33dab79210828b3ce Mon Sep 17 00:00:00 2001 From: James Clark Date: Tue, 25 Oct 2022 14:10:32 +0100 Subject: [PATCH 20/21] coresight: cti: Fix hang in cti_disable_hw() cti_enable_hw() and cti_disable_hw() are called from an atomic context so shouldn't use runtime PM because it can result in a sleep when communicating with firmware. Since commit 3c6656337852 ("Revert "firmware: arm_scmi: Add clock management to the SCMI power domain""), this causes a hang on Juno when running the Perf Coresight tests or running this command: perf record -e cs_etm//u -- ls This was also missed until the revert commit because pm_runtime_put() was called with the wrong device until commit 692c9a499b28 ("coresight: cti: Correct the parameter for pm_runtime_put") With lock and scheduler debugging enabled the following is output: coresight cti_sys0: cti_enable_hw -- dev:cti_sys0 parent: 20020000.cti BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1151 in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 330, name: perf-exec preempt_count: 2, expected: 0 RCU nest depth: 0, expected: 0 INFO: lockdep is turned off. irq event stamp: 0 hardirqs last enabled at (0): [<0000000000000000>] 0x0 hardirqs last disabled at (0): [] copy_process+0xa0c/0x1948 softirqs last enabled at (0): [] copy_process+0xa0c/0x1948 softirqs last disabled at (0): [<0000000000000000>] 0x0 CPU: 3 PID: 330 Comm: perf-exec Not tainted 6.0.0-00053-g042116d99298 #7 Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Sep 13 2022 Call trace: dump_backtrace+0x134/0x140 show_stack+0x20/0x58 dump_stack_lvl+0x8c/0xb8 dump_stack+0x18/0x34 __might_resched+0x180/0x228 __might_sleep+0x50/0x88 __pm_runtime_resume+0xac/0xb0 cti_enable+0x44/0x120 coresight_control_assoc_ectdev+0xc0/0x150 coresight_enable_path+0xb4/0x288 etm_event_start+0x138/0x170 etm_event_add+0x48/0x70 event_sched_in.isra.122+0xb4/0x280 merge_sched_in+0x1fc/0x3d0 visit_groups_merge.constprop.137+0x16c/0x4b0 ctx_sched_in+0x114/0x1f0 perf_event_sched_in+0x60/0x90 ctx_resched+0x68/0xb0 perf_event_exec+0x138/0x508 begin_new_exec+0x52c/0xd40 load_elf_binary+0x6b8/0x17d0 bprm_execve+0x360/0x7f8 do_execveat_common.isra.47+0x218/0x238 __arm64_sys_execve+0x48/0x60 invoke_syscall+0x4c/0x110 el0_svc_common.constprop.4+0xfc/0x120 do_el0_svc+0x34/0xc0 el0_svc+0x40/0x98 el0t_64_sync_handler+0x98/0xc0 el0t_64_sync+0x170/0x174 Fix the issue by removing the runtime PM calls completely. They are not needed here because it must have already been done when building the path for a trace. Fixes: 835d722ba10a ("coresight: cti: Initial CoreSight CTI Driver") Cc: stable Reported-by: Aishwarya TCV Reported-by: Cristian Marussi Suggested-by: Suzuki K Poulose Signed-off-by: James Clark Reviewed-by: Mike Leach Tested-by: Mike Leach [ Fix build warnings ] Signed-off-by: Suzuki K Poulose Link: https://lore.kernel.org/r/20221025131032.1149459-1-suzuki.poulose@arm.com Signed-off-by: Greg Kroah-Hartman --- drivers/hwtracing/coresight/coresight-cti-core.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index 1be92342b5b9..c6e8c6542f24 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -90,11 +90,9 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata) static int cti_enable_hw(struct cti_drvdata *drvdata) { struct cti_config *config = &drvdata->config; - struct device *dev = &drvdata->csdev->dev; unsigned long flags; int rc = 0; - pm_runtime_get_sync(dev->parent); spin_lock_irqsave(&drvdata->spinlock, flags); /* no need to do anything if enabled or unpowered*/ @@ -119,7 +117,6 @@ static int cti_enable_hw(struct cti_drvdata *drvdata) /* cannot enable due to error */ cti_err_not_enabled: spin_unlock_irqrestore(&drvdata->spinlock, flags); - pm_runtime_put(dev->parent); return rc; } @@ -153,7 +150,6 @@ static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata) static int cti_disable_hw(struct cti_drvdata *drvdata) { struct cti_config *config = &drvdata->config; - struct device *dev = &drvdata->csdev->dev; struct coresight_device *csdev = drvdata->csdev; spin_lock(&drvdata->spinlock); @@ -175,7 +171,6 @@ static int cti_disable_hw(struct cti_drvdata *drvdata) coresight_disclaim_device_unlocked(csdev); CS_LOCK(drvdata->base); spin_unlock(&drvdata->spinlock); - pm_runtime_put(dev->parent); return 0; /* not disabled this call */ From 677047383296ea25fdfc001be3cdcdf5cc874be2 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Tue, 25 Oct 2022 04:52:23 +0200 Subject: [PATCH 21/21] misc: sgi-gru: use explicitly signed char With char becoming unsigned by default, and with `char` alone being ambiguous and based on architecture, signed chars need to be marked explicitly as such. This fixes warnings like: drivers/misc/sgi-gru/grumain.c:711 gru_check_chiplet_assignment() warn: 'gts->ts_user_chiplet_id' is unsigned Cc: Dimitri Sivanich Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Signed-off-by: Jason A. Donenfeld Link: https://lore.kernel.org/r/20221025025223.573543-1-Jason@zx2c4.com Signed-off-by: Greg Kroah-Hartman --- drivers/misc/sgi-gru/grumain.c | 6 +++--- drivers/misc/sgi-gru/grutables.h | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/misc/sgi-gru/grumain.c b/drivers/misc/sgi-gru/grumain.c index 9afda47efbf2..6706ef3c5977 100644 --- a/drivers/misc/sgi-gru/grumain.c +++ b/drivers/misc/sgi-gru/grumain.c @@ -152,7 +152,7 @@ static int gru_assign_asid(struct gru_state *gru) * Optionally, build an array of chars that contain the bit numbers allocated. */ static unsigned long reserve_resources(unsigned long *p, int n, int mmax, - char *idx) + signed char *idx) { unsigned long bits = 0; int i; @@ -170,14 +170,14 @@ static unsigned long reserve_resources(unsigned long *p, int n, int mmax, } unsigned long gru_reserve_cb_resources(struct gru_state *gru, int cbr_au_count, - char *cbmap) + signed char *cbmap) { return reserve_resources(&gru->gs_cbr_map, cbr_au_count, GRU_CBR_AU, cbmap); } unsigned long gru_reserve_ds_resources(struct gru_state *gru, int dsr_au_count, - char *dsmap) + signed char *dsmap) { return reserve_resources(&gru->gs_dsr_map, dsr_au_count, GRU_DSR_AU, dsmap); diff --git a/drivers/misc/sgi-gru/grutables.h b/drivers/misc/sgi-gru/grutables.h index 5efc869fe59a..8c52776db234 100644 --- a/drivers/misc/sgi-gru/grutables.h +++ b/drivers/misc/sgi-gru/grutables.h @@ -351,7 +351,7 @@ struct gru_thread_state { pid_t ts_tgid_owner; /* task that is using the context - for migration */ short ts_user_blade_id;/* user selected blade */ - char ts_user_chiplet_id;/* user selected chiplet */ + signed char ts_user_chiplet_id;/* user selected chiplet */ unsigned short ts_sizeavail; /* Pagesizes in use */ int ts_tsid; /* thread that owns the structure */ @@ -364,11 +364,11 @@ struct gru_thread_state { required for contest */ unsigned char ts_cbr_au_count;/* Number of CBR resources required for contest */ - char ts_cch_req_slice;/* CCH packet slice */ - char ts_blade; /* If >= 0, migrate context if + signed char ts_cch_req_slice;/* CCH packet slice */ + signed char ts_blade; /* If >= 0, migrate context if ref from different blade */ - char ts_force_cch_reload; - char ts_cbr_idx[GRU_CBR_AU];/* CBR numbers of each + signed char ts_force_cch_reload; + signed char ts_cbr_idx[GRU_CBR_AU];/* CBR numbers of each allocated CB */ int ts_data_valid; /* Indicates if ts_gdata has valid data */ @@ -643,9 +643,9 @@ extern struct gru_thread_state *gru_alloc_gts(struct vm_area_struct *vma, int cbr_au_count, int dsr_au_count, unsigned char tlb_preload_count, int options, int tsid); extern unsigned long gru_reserve_cb_resources(struct gru_state *gru, - int cbr_au_count, char *cbmap); + int cbr_au_count, signed char *cbmap); extern unsigned long gru_reserve_ds_resources(struct gru_state *gru, - int dsr_au_count, char *dsmap); + int dsr_au_count, signed char *dsmap); extern vm_fault_t gru_fault(struct vm_fault *vmf); extern struct gru_mm_struct *gru_register_mmu_notifier(void); extern void gru_drop_mmu_notifier(struct gru_mm_struct *gms);