[PATCH] i2c: samsung: add support for Exynos7 HS-I2C

[Heiko Schocher]

Hello Kaustabh,

On 17.10.25 17:23, Kaustabh Chakraborty wrote:
> Exynos7 (and later) HS-I2C blocks have special interrupts regarding
> various data transfer states (see HSI2C_INT_I2C_TRANS_EN). Add support
> for enabling and handling these interrupt bits.
> 
> Add the corresponding compatible, 'samsung,exynos7-hsi2c'. In order to
> differentiate between the multiple device variants, an enum is
> introduced which is used where difference in implementations exist.
> 
> Signed-off-by: Kaustabh Chakraborty <kauschluss at disroot.org>
> ---
>   drivers/i2c/exynos_hs_i2c.c | 105 ++++++++++++++++++++++++++++++++++++--------
>   drivers/i2c/s3c24x0_i2c.h   |   7 +++
>   2 files changed, 94 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/i2c/exynos_hs_i2c.c b/drivers/i2c/exynos_hs_i2c.c
> index fa0d1c8f64ab593942a4ebe52f7b028366ce489c..67b497b228b151a2b23befc7fa62b34bbd07f388 100644
> --- a/drivers/i2c/exynos_hs_i2c.c
> +++ b/drivers/i2c/exynos_hs_i2c.c
> @@ -54,6 +54,17 @@ DECLARE_GLOBAL_DATA_PTR;
>   				 HSI2C_RX_OVERRUN_EN  |\
>   				 HSI2C_INT_TRAILING_EN)
>   
> +#define HSI2C_INT_TRANS_DONE		(1u << 7)
> +#define HSI2C_INT_TRANS_ABORT		(1u << 8)
> +#define HSI2C_INT_NO_DEV_ACK		(1u << 9)
> +#define HSI2C_INT_NO_DEV		(1u << 10)
> +#define HSI2C_INT_TIMEOUT_AUTO		(1u << 11)
> +#define HSI2C_INT_I2C_TRANS_EN		(HSI2C_INT_TRANS_DONE  |\
> +					 HSI2C_INT_TRANS_ABORT |\
> +					 HSI2C_INT_NO_DEV_ACK  |\
> +					 HSI2C_INT_NO_DEV      |\
> +					 HSI2C_INT_TIMEOUT_AUTO)
> +
>   /* I2C_CONF Register bits */
>   #define HSI2C_AUTO_MODE			(1u << 31)
>   #define HSI2C_10BIT_ADDR_MODE		(1u << 30)
> @@ -99,24 +110,32 @@ DECLARE_GLOBAL_DATA_PTR;
>    * bit to be set, which indicates copletion of a transaction.
>    *
>    * @param i2c: pointer to the appropriate register bank
> + * @param variant: hardware variant of the i2c device
>    *
>    * @return: I2C_OK in case of successful completion, I2C_NOK_TIMEOUT in case
>    *          the status bits do not get set in time, or an approrpiate error
>    *          value in case of transfer errors.
>    */
> -static int hsi2c_wait_for_trx(struct exynos5_hsi2c *i2c)
> +static int hsi2c_wait_for_trx(struct exynos5_hsi2c *i2c,
> +			      enum s3c24x0_i2c_variant variant)
>   {
>   	int i = HSI2C_TIMEOUT_US;
>   
>   	while (i-- > 0) {
>   		u32 int_status = readl(&i2c->usi_int_stat);
> +		u32 trans_status;
>   
> -		if (int_status & HSI2C_INT_I2C_EN) {
> -			u32 trans_status = readl(&i2c->usi_trans_status);
> +		switch (variant) {
> +		case VARIANT_HSI2C_EXYNOS5:
> +			trans_status = readl(&i2c->usi_trans_status);
>   
>   			/* Deassert pending interrupt. */
>   			writel(int_status, &i2c->usi_int_stat);
>   
> +			if (!(int_status & HSI2C_INT_I2C_EN)) {
> +				udelay(1);
> +				continue;
> +			}
>   			if (trans_status & HSI2C_NO_DEV_ACK) {
>   				debug("%s: no ACK from device\n", __func__);
>   				return I2C_NACK;
> @@ -134,8 +153,32 @@ static int hsi2c_wait_for_trx(struct exynos5_hsi2c *i2c)
>   				return I2C_NOK_TOUT;
>   			}
>   			return I2C_OK;
> +		case VARIANT_HSI2C_EXYNOS7:
> +			if (int_status & HSI2C_INT_TRANS_DONE)
> +				return I2C_OK;
> +
> +			if (int_status & HSI2C_INT_TRANS_ABORT) {
> +				debug("%s: arbitration lost\n", __func__);
> +				return I2C_NOK_LA;
> +			}
> +			if (int_status & HSI2C_NO_DEV_ACK) {
> +				debug("%s: no ACK from device\n", __func__);
> +				return I2C_NACK;
> +			}
> +			if (int_status & HSI2C_NO_DEV) {
> +				debug("%s: no device\n", __func__);
> +				return I2C_NOK;
> +			}
> +			if (int_status & HSI2C_TIMEOUT_AUTO) {
> +				debug("%s: device timed out\n", __func__);
> +				return I2C_NOK_TOUT;
> +			}
> +			udelay(1);
> +			continue;
> +		default:
> +			debug("%s: unknown variant\n", __func__);
> +			return I2C_NOK;
>   		}
> -		udelay(1);
>   	}
>   	debug("%s: transaction timeout!\n", __func__);
>   	return I2C_NOK_TOUT;

May I oversee something .. but is for the new variant the i2c->usi_int_stat
register ever cleared? I do not see a write for it in this case... or
is it there cleared on read?

In linux driver:/drivers/i2c/busses/i2c-exynos5.c

  501 static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
[...]
  512         int_status = readl(i2c->regs + HSI2C_INT_STATUS);
  513         writel(int_status, i2c->regs + HSI2C_INT_STATUS);

independent from the variant... may it is valid to do this in our
driver too?

Some Testing on VARIANT_HSI2C_EXYNOS5 would be nice ... do you have
such a hardware?

Beside of this, change looks good to me.

Reviewed-by: Heiko Schocher <hs at nabladev.com>

bye,
Heiko

-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office at nabladev.com
Geschäftsführer : Stefano Babic