On Thu, Sep 12, 2024 at 6:55 PM Andi Shyti <andi.shyti@xxxxxxxxxx> wrote: > 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. Okay, but where do you see this debug message? I looked again into the code and do not see that _this_ value of temp is used in the messaging. What did I miss? > In any case, these are small details, and the patch is already > good as it is. Thank you for the review! -- With Best Regards, Andy Shevchenko
