[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<FR3P281MB17577DF9326C6CC893711EC2CE45A@FR3P281MB1757.DEUP281.PROD.OUTLOOK.COM>
Date: Fri, 27 Jun 2025 15:32:04 +0000
From: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@....com>
To: Julia Lawall <julia.lawall@...ia.fr>,
Dan Carpenter
<dan.carpenter@...aro.org>
CC: Jonathan Cameron <jic23@...nel.org>,
David Lechner
<dlechner@...libre.com>,
Lars-Peter Clausen <lars@...afoo.de>,
Nuno Sá <nuno.sa@...log.com>,
Andy Shevchenko
<andy@...nel.org>,
"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 2/3] iio: imu: inv_icm42600: add WoM support
>
>
>________________________________________
>From: Julia Lawall <julia.lawall@...ia.fr>
>Sent: Thursday, June 26, 2025 22:41
>To: Dan Carpenter <dan.carpenter@...aro.org>
>Cc: Jonathan Cameron <jic23@...nel.org>; David Lechner <dlechner@...libre.com>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@....com>; Lars-Peter Clausen <lars@...afoo.de>; Nuno Sá <nuno.sa@...log.com>; Andy Shevchenko <andy@...nel.org>; linux-iio@...r.kernel.org <linux-iio@...r.kernel.org>; linux-kernel@...r.kernel.org <linux-kernel@...r.kernel.org>
>Subject: Re: [PATCH v5 2/3] iio: imu: inv_icm42600: add WoM support
>
>This Message Is From an External Sender
>This message came from outside your organization.
>
>On Thu, 26 Jun 2025, Dan Carpenter wrote:
>
>> On Thu, Jun 26, 2025 at 07:53:23PM +0100, Jonathan Cameron wrote:
>> > > > +static int inv_icm42600_accel_disable_wom(struct iio_dev *indio_dev)
>> > > > +{
>> > > > + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
>> > > > + struct device *pdev = regmap_get_device(st->map);
>> > > > + struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
>> > > > + unsigned int sleep_ms = 0;
>> > > > + int ret;
>> > > > +
>> > > > + scoped_guard(mutex, &st->lock) {
>> > > > + /*
>> > > > + * Consider that turning off WoM is always working to avoid
>> > > > + * blocking the chip in on mode and prevent going back to sleep.
>> > > > + * If there is an error, the chip will anyway go back to sleep
>> > > > + * and the feature will not work anymore.
>> > > > + */
>> > > > + st->apex.wom.enable = false;
>> > > > + st->apex.on--;
>> > > > + ret = inv_icm42600_disable_wom(st);
>> > > > + if (ret)
>> > > > + break;
>> > >
>> > > The fact that scoped_guard() uses a for loop is an implementation
>> > > detail so using break here makes this look like improper C code. I
>> > > think this would be better to split out the protected section to a
>> > > separate function and just use the regular guard() macro.
>> >
>> > Good catch. This feels like something we should have some static analysis
>> > around as we definitely don't want code assuming that implementation.
>> >
>> > +CC Dan / Julia to see if they agree.
>> >
>>
>> I feel like the scoped_guard() macro is so complicated because they
>> wanted break statements to work as expected... (As opposed to how I write
>> half my loop macros using nested for loops so that when I break it only
>> breaks from the inner loop and corrupts memory).
>
>How about a goto if making another function is not practical?
>
>julia
Hello David, Jonathan, Dan and Julia,
no problem for me for rewriting this code using functions and avoid any use
of break. It is quite easy.
Thanks,
JB
>
>>
>> regards,
>> dan carpenter
>>
>>
>
Powered by blists - more mailing lists