linux-kernel - Re: [PATCH v4] iio: light: ltr390: Implement runtime PM support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAE3SzaQCjoin+ToiD7iS9ev-v37H45gBkFmbXNcskET1m8rrgg@mail.gmail.com>
Date: Wed, 3 Sep 2025 16:57:47 +0530
From: Akshay Jindal <akshayaj.lkd@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: anshulusr@...il.com, jic23@...nel.org, dlechner@...libre.com, 
	nuno.sa@...log.com, andy@...nel.org, shuah@...nel.org, 
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] iio: light: ltr390: Implement runtime PM support

On Wed, Sep 3, 2025 at 4:07 PM Andy Shevchenko
<andriy.shevchenko@...el.com> wrote:
>
> On Wed, Sep 03, 2025 at 09:15:53AM +0530, Akshay Jindal wrote:
> > On Tue, Sep 2, 2025 at 6:27 PM Andy Shevchenko
> > <andriy.shevchenko@...el.com> wrote:
> > > On Tue, Sep 02, 2025 at 12:12:36AM +0530, Akshay Jindal wrote:
>
> ...
>
> > > >       /* Ensure that power off and interrupts are disabled */
> > > > -     if (regmap_clear_bits(data->regmap, LTR390_INT_CFG,
> > > > -                             LTR390_LS_INT_EN) < 0)
> > > > -             dev_err(&data->client->dev, "failed to disable interrupts\n");
> > > > +     if (data->irq_enabled) {
> > > > +             if (regmap_clear_bits(data->regmap, LTR390_INT_CFG,
> > > > +                                     LTR390_LS_INT_EN) < 0)
> > >
> > > Wrong indentation, hard to read line, either one line, or do better.
> > > Actually why not assign it to ret? The above not only simple style issue,
> > > but also makes readability much harder as the semantics of '0' is
> > > completely hidden. This style is discouraged.
> > Earlier did not use ret here, because powerdown function is of type void.
> > But if readability is the issue, I have used ret.
> >
> > Regarding clubbing into 1 line, I have my reservations there. I think we
> > should not violate the 80 char line limit.
>
> Shouldn't != mustn't, esp. when it's about readability.
Ok, made the changes.
>
> > Also since the line is already 1-level indented (begins at 9th column, due to
> > if(data->irq_enabled) check), the spillover will be too much. The readability
> > does not seem to be taking a substantial hit here. Let me know if this is
> > non-negotiable for you. Will happily make the changes.
>
>                 ret = regmap_clear_bits(data->regmap, LTR390_INT_CFG, LTR390_LS_INT_EN);
>
> only 88 characters. One can make it shorter, yes
>
>         struct regmap *map = data->regmap;
>
>                 ret = regmap_clear_bits(map, LTR390_INT_CFG, LTR390_LS_INT_EN);
>
>
> 79 characters.
>
Sent a v5. Kindly review.

Thanks,
Akshay