Re: [PATCH v6 1/2] i2c: octeon: refactor hlc r/w operations

[Aryan Srivastava]

Hi Andi,
On Fri, 2024-06-14 at 00:55 +0100, Andi Shyti wrote:
> Hi Aryan,
> 
> > +/* Construct and send i2c transaction core cmd for read ops */
> > +static int octeon_i2c_hlc_read_cmd(struct octeon_i2c *i2c, struct
> > i2c_msg msg, u64 cmd)
> > +{
> > +       u64 ext = 0;
> > +
> > +       if (octeon_i2c_hlc_ext(i2c, msg, &cmd, &ext))
> > +               octeon_i2c_writeq_flush(ext, i2c->twsi_base +
This used to be called within this block previously for the read
function only:

	if (msgs[0].len == 2) {
		u64 ext = 0;

		cmd |= SW_TWSI_EIA;
		ext = (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
		cmd |= (u64)msgs[0].buf[1] << SW_TWSI_IA_SHIFT;
		octeon_i2c_writeq_flush(ext, i2c->twsi_base +
SW_TWSI_EXT(i2c));
	} else {
		cmd |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
	}

This is the write function equivelant:
	if (msgs[0].len == 2) {
		cmd |= SW_TWSI_EIA;
		ext |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
		set_ext = true;
		cmd |= (u64)msgs[0].buf[1] << SW_TWSI_IA_SHIFT;
	} else {
		cmd |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
	}
They are nearly identical blocks of code, bar the one write. 
So to use this block generically for both write and read, then I have
to pull out this line and run it only in the read case. Therefore this
is more of a reordering of operations, as the set_ext flag now gets set
where the line used to run, and is used to condition the running of the
line.
> > SW_TWSI_EXT(i2c));
> 
> I think this check here is the only logical change I see. Right?
> 
> If so, can you please describe in the log why you made this
> change?
> 
> Thanks,
> Andi
> 
> > +       return octeon_i2c_hlc_cmd_send(i2c, cmd);
> > +}
Thanks,
Aryan.