linux-kernel - Re: [PATCH 2/3] i2c: thunderx: Add support for High speed mode

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: <20230610124252.o7qmwgavqyrkfhq6@intel.intel>
Date:   Sat, 10 Jun 2023 14:42:52 +0200
From:   Andi Shyti <andi.shyti@...nel.org>
To:     Piyush Malgujar <pmalgujar@...vell.com>
Cc:     linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
        wsa@...nel.org, rric@...nel.org, jannadurai@...vell.com,
        cchavva@...vell.com, Suneel Garapati <sgarapati@...vell.com>
Subject: Re: [PATCH 2/3] i2c: thunderx: Add support for High speed mode

Hi Suneel and Piyush,

[...]

> @@ -61,10 +61,19 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
>  		return octeon_i2c_test_iflg(i2c) ? 0 : -ETIMEDOUT;
>  	}
>  
> -	i2c->int_enable(i2c);
> -	time_left = wait_event_timeout(i2c->queue, octeon_i2c_test_iflg(i2c),
> -				       i2c->adap.timeout);
> -	i2c->int_disable(i2c);
> +	if (IS_LS_FREQ(i2c->twsi_freq)) {
> +		i2c->int_enable(i2c);
> +		time_left = wait_event_timeout(i2c->queue,
> +					       octeon_i2c_test_iflg(i2c),
> +					       i2c->adap.timeout);
> +		i2c->int_disable(i2c);
> +	} else {
> +		time_left = 1000; /* 1ms */
> +		do {
> +			if (time_left--)
> +				__udelay(1);

Are you sure you want to wait 1ms with __udelay(). This is a bit
dsruptive, can we use a more relaxed waiting method?

> +		} while (!octeon_i2c_test_iflg(i2c) && time_left);
> +	}

[...]

>  	 * Find divisors to produce target frequency, start with large delta
>  	 * to cover wider range of divisors, note thp = TCLK half period.
>  	 */
> -	int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
> +	int ds = 10, thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;

unsigned?

[...]

> +	if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) {
> +		u64 mode;
> +
> +		mode = __raw_readq(i2c->twsi_base + MODE(i2c));
> +		/* Set REFCLK_SRC and HS_MODE in TWSX_MODE register */
> +		if (!IS_LS_FREQ(i2c->twsi_freq))
> +			mode |= BIT(4) | BIT(0);
> +		else
> +			mode &= ~(BIT(4) | BIT(0));

would be nice to have this defined and with some meaning as
comment.

> +		octeon_i2c_writeq_flush(mode, i2c->twsi_base + MODE(i2c));
> +	}

Robert, any comment here?

Thanks,
Andi