Re: [PATCH v6 2/4] i2c: thunderx: Support for High speed mode

[Andi Shyti]

Hi Piyush,

On Tue, Apr 02, 2024 at 06:40:12AM -0700, Piyush Malgujar wrote:
> From: Suneel Garapati <sgarapati@xxxxxxxxxxx>
> 
> 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@xxxxxxxxxxx>
> Signed-off-by: Piyush Malgujar <pmalgujar@xxxxxxxxxxx>

..

> -#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