Received-SPF: pass (google.com: domain of linux-kernel+bounces-150663-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 18 Apr 2024 21:12:39 +0200
From: Andi Shyti <andi.shyti@kernel.org>
To: Piyush Malgujar <pmalgujar@marvell.com>
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, 
	sgarapati@marvell.com, cchavva@marvell.com, jannadurai@marvell.com
Subject: Re: [PATCH v6 2/4] i2c: thunderx: Support for High speed mode
Message-ID: <wdnzttdq5gqthmzztylpu42ps22hpbbwev6onevxl76d5zf7em@bjiryyhqcj6f>
References: <20240402134018.2686919-1-pmalgujar@marvell.com>
 <20240402134018.2686919-3-pmalgujar@marvell.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240402134018.2686919-3-pmalgujar@marvell.com>

Hi Piyush,

On Tue, Apr 02, 2024 at 06:40:12AM -0700, Piyush Malgujar wrote:
> From: Suneel Garapati <sgarapati@marvell.com>
> 
> To support bus operations for high speed bus frequencies greater than
> 400KHZ following control bits need to be setup accordingly
>  - hs_mode (bit 0) field in Mode register to switch controller
>    between low-speed and high-speed frequency operating mode.
>  - Setup clock divisors for desired TWSI bus frequency using
>    FOSCL output frequency divisor (D):
>    0 - sets the divisor to 10 for low speed mode
>    1 - sets the divisor to 15 for high speed mode.
> The TWSI bus output frequency, in master mode is based on:
> 		TCLK = 100MHz / (THP + 2)
> 		FOSCL = FSAMP / (M+1)?D = TCLK / (2 ^ N ? (M + 1) ? 15)
> 		FSAMP = TCLK / 2 ^ N
> where,
> 	N is <2:0> and M is <6:3> of TWSI Clock Control Register
> 	D is 10 for low speed or 15 for HS_MODE
> 
> With high speed mode support, HLC mode usage is limited to
> low speed frequency (<=400KHz) bus transfers in hardware.

Thanks! :-)

> Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
> Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>

..

> -#define SW_TWSI(x)	(x->roff.sw_twsi)
> -#define TWSI_INT(x)	(x->roff.twsi_int)
> -#define SW_TWSI_EXT(x)	(x->roff.sw_twsi_ext)
> +#define OCTEON_REG_SW_TWSI(x)		((x)->roff.sw_twsi)
> +#define OCTEON_REG_TWSI_INT(x)		((x)->roff.twsi_int)
> +#define OCTEON_REG_SW_TWSI_EXT(x)	((x)->roff.sw_twsi_ext)

This is a good cleanup, can we please put it in a different
patch? This way the patch gets smaller and it's easier to review
and bisect.

> +#define OCTEON_REG_MODE(x)		((x)->roff.mode)
> +
> +/* Set REFCLK_SRC and HS_MODE in TWSX_MODE register */
> +#define TWSX_MODE_REFCLK_SRC	BIT(4)
> +#define TWSX_MODE_HS_MODE	BIT(0)
> +#define TWSX_MODE_HS_MASK	(TWSX_MODE_REFCLK_SRC | TWSX_MODE_HS_MODE)

Thanks!

All the rest looks good.

Andi