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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aLgacK0MMojkaKuW@smile.fi.intel.com>
Date: Wed, 3 Sep 2025 13:37:36 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Akshay Jindal <akshayaj.lkd@...il.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 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.

> 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.

-- 
With Best Regards,
Andy Shevchenko