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

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: <d577efcc4ea68715767c2e100d5b4262632d5b3b.camel@alliedtelesis.co.nz>
Date: Thu, 20 Jun 2024 00:56:54 +0000
From: Aryan Srivastava <Aryan.Srivastava@...iedtelesis.co.nz>
To: "andi.shyti@...nel.org" <andi.shyti@...nel.org>
CC: "Markus.Elfring@....de" <Markus.Elfring@....de>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"rric@...nel.org" <rric@...nel.org>
Subject: Re: [PATCH v6 1/2] i2c: octeon: refactor hlc r/w operations

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.