linux-kernel - Re: [PATCH v1 08/12] i2c: isch: Use read_poll

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: <zuijrzur6htcg4dhccfydl4gb4rj62jq5co4ege6fkggaceesg@3kkujnlrgbye>
Date: Thu, 12 Sep 2024 17:55:13 +0200
From: Andi Shyti <andi.shyti@...nel.org>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
	linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org, Jean Delvare <jdelvare@...e.com>
Subject: Re: [PATCH v1 08/12] i2c: isch: Use read_poll_timeout()

On Thu, Sep 12, 2024 at 06:35:46PM GMT, Andy Shevchenko wrote:
> Thu, Sep 12, 2024 at 09:29:38AM +0200, Andi Shyti kirjoitti:
> 
> ...
> 
> > > -		sch_io_wr8(priv, SMBHSTSTS, temp);
> > > +		sch_io_wr8(priv, SMBHSTSTS, temp & 0x0f);
> > 
> > there is still a dev_dbg() using temp. To be on the safe side, do
> > we want to do a "temp &= 0x0f" after the read_poll_timeout?
> 
> Isn't it even better that we have more information in the debug output?

I think not, because:

 1. It's information that we don't need, and we intentionally
    discard in every check. If we print a value we ignore, we
    risk providing incorrect information.

 2. It’s more future-proof. In future development, cleanups,
    refactorings, or copy-paste, temp can be used as is
    without needing to continuously & it with 0xf. This
    avoids unnecessary operations being repeated later on.

 3. It maintains the original design.

In any case, these are small details, and the patch is already
good as it is.

Thanks,
Andi