Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
bmc150_accel_buffer_predisable(). This ensures the mutex is
released on all return paths and allows returning directly
without a goto label.
Aligned headers in alphabatical order.
Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
drivers/iio/accel/bmc150-accel-core.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 42ccf0316ce5..9ae325cdfdd3 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -4,23 +4,26 @@
* Copyright (c) 2014, Intel Corporation.
*/
-#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
-#include <linux/delay.h>
-#include <linux/slab.h>
-#include <linux/acpi.h>
+#include <linux/module.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
#include <linux/iio/buffer.h>
#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
#include <linux/iio/trigger.h>
#include <linux/iio/trigger_consumer.h>
#include <linux/iio/triggered_buffer.h>
-#include <linux/regmap.h>
+
#include <linux/regulator/consumer.h>
#include "bmc150-accel.h"
@@ -1485,7 +1488,7 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
if (iio_device_get_current_mode(indio_dev) == INDIO_BUFFER_TRIGGERED)
return 0;
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
if (!data->watermark)
goto out;
@@ -1517,19 +1520,16 @@ static int bmc150_accel_buffer_predisable(struct iio_dev *indio_dev)
if (iio_device_get_current_mode(indio_dev) == INDIO_BUFFER_TRIGGERED)
return 0;
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
if (!data->fifo_mode)
- goto out;
+ return 0;
bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_WATERMARK, false);
__bmc150_accel_fifo_flush(indio_dev, BMC150_ACCEL_FIFO_LENGTH, false);
data->fifo_mode = 0;
bmc150_accel_fifo_set_mode(data);
-out:
- mutex_unlock(&data->mutex);
-
return 0;
}
--
2.53.0
On Mon, Mar 09, 2026 at 19:53 +0530 Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote: > Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in > bmc150_accel_buffer_predisable(). This ensures the mutex is > released on all return paths and allows returning directly > without a goto label. > > Aligned headers in alphabatical order. Never mix unrelated changes. This should be separate patches. Why? Think about traceability (bisect etc.). This comment applies to all other patches in the series as well. I'll just state it here and wait for V2 before taking a deeper look. To be honest, I'm not even sure if sorting the headers is a "valid" patch. It's just cosmetics (and there seems to be no such code style guidelines)... [...]
On Mon, Mar 09, 2026 at 03:35:58PM +0100, Waqar Hameed wrote: > On Mon, Mar 09, 2026 at 19:53 +0530 Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote: [...] > > Aligned headers in alphabatical order. > > Never mix unrelated changes. This should be separate patches. Yep, I only mentioned to place cleanup.h in the better place. > Why? Think about traceability (bisect etc.). It's all about maintenance and development. Easy to see where need to put a new header if it's not present (also easier to find this at a glance). > This comment applies to all other patches in the series as well. I'll > just state it here and wait for V2 before taking a deeper look. > To be honest, I'm not even sure if sorting the headers is a "valid" > patch. It's just cosmetics (and there seems to be no such code style > guidelines)... It's valid when in a series (not as just for the sake of sorting). Here it would make sense to have it as a precursor to find the best place for a new header inclusion to place. Guidelines here are still missing maintainer preferences for IIO subsystem. -- With Best Regards, Andy Shevchenko
On Mon, Mar 09, 2026 at 18:26 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Mon, Mar 09, 2026 at 03:35:58PM +0100, Waqar Hameed wrote: [...] >> Why? Think about traceability (bisect etc.). > > It's all about maintenance and development. Easy to see where need to put > a new header if it's not present (also easier to find this at a glance). Totally agree (see below). > >> This comment applies to all other patches in the series as well. I'll >> just state it here and wait for V2 before taking a deeper look. > >> To be honest, I'm not even sure if sorting the headers is a "valid" >> patch. It's just cosmetics (and there seems to be no such code style >> guidelines)... > > It's valid when in a series (not as just for the sake of sorting). Here > it would make sense to have it as a precursor to find the best place for > a new header inclusion to place. > > Guidelines here are still missing maintainer preferences for IIO subsystem. Yeah, I guess that's kind of the issue here (or in general for kernel developing). Every sub-system have their preference. You just have to carefully navigate through all of this :) Moreover, I always tend to sort the include header when creating a _new_ file (for the reason you stated above). `clang-format` does that automatically for me, though you have to modify the `.clang-format`-file: diff --git a/.clang-format b/.clang-format index 1cc151e2adcc5..f6631d03bd3f3 100644 --- a/.clang-format +++ b/.clang-format @@ -784,7 +784,7 @@ PenaltyReturnTypeOnItsOwnLine: 60 PointerAlignment: Right ReflowComments: false -SortIncludes: false +SortIncludes: true SortUsingDeclarations: false SpaceAfterCStyleCast: false SpaceAfterTemplateKeyword: true The reason for the `false` value can be read in the last section "Extra features/option" in `Documentation/dev-tools/clang-format.rst`.
On Mon, Mar 9, 2026 at 10:18 PM Waqar Hameed <waqar.hameed@axis.com> wrote: > Yeah, I guess that's kind of the issue here (or in general for kernel > developing). Every sub-system have their preference. You just have to > carefully navigate through all of this :) Hello Waqar, yes In one patch in another sub-system, Maintainers told me to keep headers in alphabetical order. so i thought it would be a good idea to arrange the headers alphabetically here too. Thanks, Rajveer
On Mon, Mar 9, 2026 at 7:54 PM Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote: > @@ -1485,7 +1488,7 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev) > if (iio_device_get_current_mode(indio_dev) == INDIO_BUFFER_TRIGGERED) > return 0; > > - mutex_lock(&data->mutex); > + guard(mutex)(&data->mutex); > > if (!data->watermark) > goto out; I am sorry I accidentally forgot to clean up goto and mutex_unlock in bmc150_accel_buffer_postenable(). I will fix it in V2. Thankyou, Rajveer
© 2016 - 2026 Red Hat, Inc.