Re: [PATCH 3/4] staging: rtl8723bs: remove overwriting of current IMR settings

[Nikolay Kulikov <nikolayof23@gmail.com>]

On Mon, May 11, 2026 at 12:18:58PM +0300, Dan Carpenter wrote:
> On Mon, May 11, 2026 at 10:55:30AM +0300, Nikolay Kulikov wrote:
> > The code reads a 32-bit value from REG_HSIMR and writes the result back
> > to the same address. This operation does not change the register's
> > value.
> > 
> > This code does nothing and simply performs unnecessary SDIO operations.
> > Furthermore, if rtw_read32() returns an error or returns an
> > uninitialized value, writing it back will have unknown consequences.
> > 
> 
> This commit message isn't very useful.
> 
> There can be times where reading from a register and then writing the
> exact same thing back to it is how the hardware is designed.  So it's
> not obvious at all that making this change is safe.
> 
> What you should say is that:
> 
> Originally, this code use to Step 1: read from the register,
> Step 2: add some bits to the value and then Step 3: write the result
> back to the register.
> 
> However the problem was that the bits in step 2 were always zero so
> I have removed that code.  Now this function just reads a value and
> writes the exact same thing back.  It is unnecessary and can be
> removed.
> 
> It's not 100% safe to make this change without testing, but it
> probably is safe enough once we see the explanation *why* the code
> *used* to exist.

Indeed, it's much clearer this way, I'll fix it in v2.


Thanks,
Nikolay