Patchwork [2/2] i2c: exynos5: configure fifo_depth based on HSI2C module version

login
register
mail settings
Submitter Naveen Krishna Ch
Date Nov. 22, 2013, 6:14 a.m.
Message ID <1385100851-32254-1-git-send-email-ch.naveen@samsung.com>
Download mbox | patch
Permalink /patch/293338/
State Superseded
Headers show

Comments

Naveen Krishna Ch - Nov. 22, 2013, 6:14 a.m.
fifo_depth of the HSI2C is not constant
Exynos5420 and Exynos5250 supports fifo_depth of 64bytes
Exynos5260 supports fifo_depth of 16bytes

This patch configures the fifo_depth based on HSI2C modules version.
Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
---
 drivers/i2c/busses/i2c-exynos5.c |   29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)
Tomasz Figa - Dec. 9, 2013, 4:31 p.m.
Hi Naveen,

On Friday 22 of November 2013 11:44:11 Naveen Krishna Chatradhi wrote:
> fifo_depth of the HSI2C is not constant
> Exynos5420 and Exynos5250 supports fifo_depth of 64bytes
> Exynos5260 supports fifo_depth of 16bytes
> 
> This patch configures the fifo_depth based on HSI2C modules version.
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> ---
>  drivers/i2c/busses/i2c-exynos5.c |   29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
> index cbb49e2..19277d8 100644
> --- a/drivers/i2c/busses/i2c-exynos5.c
> +++ b/drivers/i2c/busses/i2c-exynos5.c
> @@ -77,12 +77,6 @@
>  #define HSI2C_RXFIFO_TRIGGER_LEVEL(x)		((x) << 4)
>  #define HSI2C_TXFIFO_TRIGGER_LEVEL(x)		((x) << 16)
>  
> -/* As per user manual FIFO max depth is 64bytes */
> -#define HSI2C_FIFO_MAX				0x40
> -/* default trigger levels for Tx and Rx FIFOs */
> -#define HSI2C_DEF_TXFIFO_LVL			(HSI2C_FIFO_MAX - 0x30)
> -#define HSI2C_DEF_RXFIFO_LVL			(HSI2C_FIFO_MAX - 0x10)
> -
>  /* I2C_TRAILING_CTL Register bits */
>  #define HSI2C_TRAILING_COUNT			(0xf)
>  
> @@ -187,6 +181,9 @@ struct exynos5_i2c {
>  
>  	/* Version of HS-I2C Hardware */
>  	unsigned int		version;
> +
> +	/* FIFO depth */
> +	unsigned int		fifo_depth;
>  };
>  
>  enum hsi2c_version {
> @@ -437,7 +434,7 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
>  		fifo_status = readl(i2c->regs + HSI2C_FIFO_STATUS);
>  		fifo_level = HSI2C_TX_FIFO_LVL(fifo_status);
>  
> -		len = HSI2C_FIFO_MAX - fifo_level;
> +		len = i2c->fifo_depth - fifo_level;
>  		if (len > (i2c->msg->len - i2c->msg_ptr))
>  			len = i2c->msg->len - i2c->msg_ptr;
>  
> @@ -505,6 +502,7 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop)
>  	u32 i2c_auto_conf = 0;
>  	u32 fifo_ctl;
>  	unsigned long flags;
> +	unsigned short trig_lvl;
>  
>  	i2c_ctl = readl(i2c->regs + HSI2C_CTL);
>  	i2c_ctl &= ~(HSI2C_TXCHON | HSI2C_RXCHON);
> @@ -515,13 +513,19 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop)
>  
>  		i2c_auto_conf = HSI2C_READ_WRITE;
>  
> -		fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(HSI2C_DEF_TXFIFO_LVL);
> +		trig_lvl = (i2c->msg->len > i2c->fifo_depth) ?
> +			(i2c->fifo_depth * 3/4) : i2c->msg->len;

This patch changes the fifo trigger level calculation (it's dependent now
on message length), not just maximum fifo size, as the description says.
It should be split into two separate patches, explaining why both changes
are necessary.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
naveen krishna chatradhi - Dec. 10, 2013, 4:56 a.m.
Hello Tomasz,


On 9 December 2013 22:01, Tomasz Figa <t.figa@samsung.com> wrote:
>
> Hi Naveen,
>
> On Friday 22 of November 2013 11:44:11 Naveen Krishna Chatradhi wrote:
> > fifo_depth of the HSI2C is not constant
> > Exynos5420 and Exynos5250 supports fifo_depth of 64bytes
> > Exynos5260 supports fifo_depth of 16bytes
> >
> > This patch configures the fifo_depth based on HSI2C modules version.
> > Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> > ---
> >  drivers/i2c/busses/i2c-exynos5.c |   29 ++++++++++++++++++-----------
> >  1 file changed, 18 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
> > index cbb49e2..19277d8 100644
> > --- a/drivers/i2c/busses/i2c-exynos5.c
> > +++ b/drivers/i2c/busses/i2c-exynos5.c
> > @@ -77,12 +77,6 @@
> >  #define HSI2C_RXFIFO_TRIGGER_LEVEL(x)                ((x) << 4)
> >  #define HSI2C_TXFIFO_TRIGGER_LEVEL(x)                ((x) << 16)
> >
> > -/* As per user manual FIFO max depth is 64bytes */
> > -#define HSI2C_FIFO_MAX                               0x40
> > -/* default trigger levels for Tx and Rx FIFOs */
> > -#define HSI2C_DEF_TXFIFO_LVL                 (HSI2C_FIFO_MAX - 0x30)
> > -#define HSI2C_DEF_RXFIFO_LVL                 (HSI2C_FIFO_MAX - 0x10)
> > -
> >  /* I2C_TRAILING_CTL Register bits */
> >  #define HSI2C_TRAILING_COUNT                 (0xf)
> >
> > @@ -187,6 +181,9 @@ struct exynos5_i2c {
> >
> >       /* Version of HS-I2C Hardware */
> >       unsigned int            version;
> > +
> > +     /* FIFO depth */
> > +     unsigned int            fifo_depth;
> >  };
> >
> >  enum hsi2c_version {
> > @@ -437,7 +434,7 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
> >               fifo_status = readl(i2c->regs + HSI2C_FIFO_STATUS);
> >               fifo_level = HSI2C_TX_FIFO_LVL(fifo_status);
> >
> > -             len = HSI2C_FIFO_MAX - fifo_level;
> > +             len = i2c->fifo_depth - fifo_level;
> >               if (len > (i2c->msg->len - i2c->msg_ptr))
> >                       len = i2c->msg->len - i2c->msg_ptr;
> >
> > @@ -505,6 +502,7 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop)
> >       u32 i2c_auto_conf = 0;
> >       u32 fifo_ctl;
> >       unsigned long flags;
> > +     unsigned short trig_lvl;
> >
> >       i2c_ctl = readl(i2c->regs + HSI2C_CTL);
> >       i2c_ctl &= ~(HSI2C_TXCHON | HSI2C_RXCHON);
> > @@ -515,13 +513,19 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop)
> >
> >               i2c_auto_conf = HSI2C_READ_WRITE;
> >
> > -             fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(HSI2C_DEF_TXFIFO_LVL);
> > +             trig_lvl = (i2c->msg->len > i2c->fifo_depth) ?
> > +                     (i2c->fifo_depth * 3/4) : i2c->msg->len;
>
> This patch changes the fifo trigger level calculation (it's dependent now
> on message length), not just maximum fifo size, as the description says.
Actually, message->length need not be involved in this calculation.
Involving msg->len
will raise another interrupt for every small transactions. It better be avoided.
> It should be split into two separate patches, explaining why both changes
> are necessary.
I will split the fifo_depth configuration code along with comments addressed on
https://lkml.org/lkml/2013/11/22/31

I can think of 3 ways to address the h/w version changes
1. Compatible string as i implemented
2. Varient struct
3. Passing the information via device tree
How about passing fifo_depth from device tree information.

Currently, HSI2C Module on Exynso5260 is not another H/W version.
It only defer in fifo_depth and init sequence needs a reset.

>
>
> Best regards,
> Tomasz
>
Tomasz Figa - Dec. 18, 2013, 3:21 p.m.
On Tuesday 10 of December 2013 10:26:40 Naveen Krishna Ch wrote:
> Hello Tomasz,
> 
> 
> On 9 December 2013 22:01, Tomasz Figa <t.figa@samsung.com> wrote:
> >
> > Hi Naveen,
> >
> > On Friday 22 of November 2013 11:44:11 Naveen Krishna Chatradhi wrote:
> > > fifo_depth of the HSI2C is not constant
> > > Exynos5420 and Exynos5250 supports fifo_depth of 64bytes
> > > Exynos5260 supports fifo_depth of 16bytes
> > >
> > > This patch configures the fifo_depth based on HSI2C modules version.
> > > Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> > > ---
> > >  drivers/i2c/busses/i2c-exynos5.c |   29 ++++++++++++++++++-----------
> > >  1 file changed, 18 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
> > > index cbb49e2..19277d8 100644
> > > --- a/drivers/i2c/busses/i2c-exynos5.c
> > > +++ b/drivers/i2c/busses/i2c-exynos5.c
> > > @@ -77,12 +77,6 @@
> > >  #define HSI2C_RXFIFO_TRIGGER_LEVEL(x)                ((x) << 4)
> > >  #define HSI2C_TXFIFO_TRIGGER_LEVEL(x)                ((x) << 16)
> > >
> > > -/* As per user manual FIFO max depth is 64bytes */
> > > -#define HSI2C_FIFO_MAX                               0x40
> > > -/* default trigger levels for Tx and Rx FIFOs */
> > > -#define HSI2C_DEF_TXFIFO_LVL                 (HSI2C_FIFO_MAX - 0x30)
> > > -#define HSI2C_DEF_RXFIFO_LVL                 (HSI2C_FIFO_MAX - 0x10)
> > > -
> > >  /* I2C_TRAILING_CTL Register bits */
> > >  #define HSI2C_TRAILING_COUNT                 (0xf)
> > >
> > > @@ -187,6 +181,9 @@ struct exynos5_i2c {
> > >
> > >       /* Version of HS-I2C Hardware */
> > >       unsigned int            version;
> > > +
> > > +     /* FIFO depth */
> > > +     unsigned int            fifo_depth;
> > >  };
> > >
> > >  enum hsi2c_version {
> > > @@ -437,7 +434,7 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
> > >               fifo_status = readl(i2c->regs + HSI2C_FIFO_STATUS);
> > >               fifo_level = HSI2C_TX_FIFO_LVL(fifo_status);
> > >
> > > -             len = HSI2C_FIFO_MAX - fifo_level;
> > > +             len = i2c->fifo_depth - fifo_level;
> > >               if (len > (i2c->msg->len - i2c->msg_ptr))
> > >                       len = i2c->msg->len - i2c->msg_ptr;
> > >
> > > @@ -505,6 +502,7 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop)
> > >       u32 i2c_auto_conf = 0;
> > >       u32 fifo_ctl;
> > >       unsigned long flags;
> > > +     unsigned short trig_lvl;
> > >
> > >       i2c_ctl = readl(i2c->regs + HSI2C_CTL);
> > >       i2c_ctl &= ~(HSI2C_TXCHON | HSI2C_RXCHON);
> > > @@ -515,13 +513,19 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop)
> > >
> > >               i2c_auto_conf = HSI2C_READ_WRITE;
> > >
> > > -             fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(HSI2C_DEF_TXFIFO_LVL);
> > > +             trig_lvl = (i2c->msg->len > i2c->fifo_depth) ?
> > > +                     (i2c->fifo_depth * 3/4) : i2c->msg->len;
> >
> > This patch changes the fifo trigger level calculation (it's dependent now
> > on message length), not just maximum fifo size, as the description says.
> Actually, message->length need not be involved in this calculation.
> Involving msg->len
> will raise another interrupt for every small transactions. It better be avoided.
> > It should be split into two separate patches, explaining why both changes
> > are necessary.
> I will split the fifo_depth configuration code along with comments addressed on
> https://lkml.org/lkml/2013/11/22/31

OK.

> 
> I can think of 3 ways to address the h/w version changes
> 1. Compatible string as i implemented
> 2. Varient struct
> 3. Passing the information via device tree
> How about passing fifo_depth from device tree information.

I believe a combination of 1 and 2 is the recommended option, which is
a variant struct pointed by an entry in OF match table.

> 
> Currently, HSI2C Module on Exynso5260 is not another H/W version.
> It only defer in fifo_depth and init sequence needs a reset.

Well, this implies that it's another H/W version from kernel point of
view, as it needs different handling.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index cbb49e2..19277d8 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -77,12 +77,6 @@ 
 #define HSI2C_RXFIFO_TRIGGER_LEVEL(x)		((x) << 4)
 #define HSI2C_TXFIFO_TRIGGER_LEVEL(x)		((x) << 16)
 
-/* As per user manual FIFO max depth is 64bytes */
-#define HSI2C_FIFO_MAX				0x40
-/* default trigger levels for Tx and Rx FIFOs */
-#define HSI2C_DEF_TXFIFO_LVL			(HSI2C_FIFO_MAX - 0x30)
-#define HSI2C_DEF_RXFIFO_LVL			(HSI2C_FIFO_MAX - 0x10)
-
 /* I2C_TRAILING_CTL Register bits */
 #define HSI2C_TRAILING_COUNT			(0xf)
 
@@ -187,6 +181,9 @@  struct exynos5_i2c {
 
 	/* Version of HS-I2C Hardware */
 	unsigned int		version;
+
+	/* FIFO depth */
+	unsigned int		fifo_depth;
 };
 
 enum hsi2c_version {
@@ -437,7 +434,7 @@  static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
 		fifo_status = readl(i2c->regs + HSI2C_FIFO_STATUS);
 		fifo_level = HSI2C_TX_FIFO_LVL(fifo_status);
 
-		len = HSI2C_FIFO_MAX - fifo_level;
+		len = i2c->fifo_depth - fifo_level;
 		if (len > (i2c->msg->len - i2c->msg_ptr))
 			len = i2c->msg->len - i2c->msg_ptr;
 
@@ -505,6 +502,7 @@  static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop)
 	u32 i2c_auto_conf = 0;
 	u32 fifo_ctl;
 	unsigned long flags;
+	unsigned short trig_lvl;
 
 	i2c_ctl = readl(i2c->regs + HSI2C_CTL);
 	i2c_ctl &= ~(HSI2C_TXCHON | HSI2C_RXCHON);
@@ -515,13 +513,19 @@  static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop)
 
 		i2c_auto_conf = HSI2C_READ_WRITE;
 
-		fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(HSI2C_DEF_TXFIFO_LVL);
+		trig_lvl = (i2c->msg->len > i2c->fifo_depth) ?
+			(i2c->fifo_depth * 3/4) : i2c->msg->len;
+		fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(trig_lvl);
+
 		int_en |= (HSI2C_INT_RX_ALMOSTFULL_EN |
 			HSI2C_INT_TRAILING_EN);
 	} else {
 		i2c_ctl |= HSI2C_TXCHON;
 
-		fifo_ctl |= HSI2C_TXFIFO_TRIGGER_LEVEL(HSI2C_DEF_RXFIFO_LVL);
+		trig_lvl = (i2c->msg->len > i2c->fifo_depth) ?
+			(i2c->fifo_depth * 1/4) : i2c->msg->len;
+		fifo_ctl |= HSI2C_TXFIFO_TRIGGER_LEVEL(trig_lvl);
+
 		int_en |= HSI2C_INT_TX_ALMOSTEMPTY_EN;
 	}
 
@@ -716,10 +720,13 @@  static int exynos5_i2c_probe(struct platform_device *pdev)
 	i2c->version = exynos5_i2c_get_version(pdev);
 
 	/* The HS-I2C core on Exynos5260 needs a reset to start with */
-	if (i2c->version == EXYNOS_5260)
+	if (i2c->version == EXYNOS_5260) {
+		i2c->fifo_depth = 16;
 		exynos5_i2c_reset(i2c);
-	else
+	} else {
+		i2c->fifo_depth = 64;
 		exynos5_i2c_init(i2c);
+	}
 
 	ret = i2c_add_adapter(&i2c->adap);
 	if (ret < 0) {