diff mbox

[1/2,v3] i2c: exynos5: add support for HSI2C on Exynos5260 SoC

Message ID 1391688408-8077-1-git-send-email-ch.naveen@samsung.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Naveen Krishna Chatradhi Feb. 6, 2014, 12:06 p.m. UTC 
This patch implements a variant struct to handle the differences
(like fifo_depths) in the HSI2C modules across SoCs.

Adds a new compatible to support HSI2C module on Exynos5260.
Also resets the module as an init sequence (Needed by Exynos5260).

Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
---
Changes since v2:
1. Used variant struct as suggested by Tomasz Figa.
2. Change compatible strings from samsung,exynos5-hsi2c to
   samsung,exynos5250-hsi2c based on the first SoC to see the feature.
3. Using reset as init sequences.
4. Merged the 2 patches into one.

 .../devicetree/bindings/i2c/i2c-exynos5.txt        |    8 ++-
 drivers/i2c/busses/i2c-exynos5.c                   |   64 ++++++++++++++++----
 2 files changed, 58 insertions(+), 14 deletions(-)

Comments

Tomasz Figa Feb. 6, 2014, 1:31 p.m. UTC | #1 
Hi Naveen,

On 06.02.2014 13:06, Naveen Krishna Chatradhi wrote:
> This patch implements a variant struct to handle the differences
> (like fifo_depths) in the HSI2C modules across SoCs.
>
> Adds a new compatible to support HSI2C module on Exynos5260.
> Also resets the module as an init sequence (Needed by Exynos5260).
>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> ---
> Changes since v2:
> 1. Used variant struct as suggested by Tomasz Figa.
> 2. Change compatible strings from samsung,exynos5-hsi2c to
>     samsung,exynos5250-hsi2c based on the first SoC to see the feature.
> 3. Using reset as init sequences.
> 4. Merged the 2 patches into one.
>
>   .../devicetree/bindings/i2c/i2c-exynos5.txt        |    8 ++-
>   drivers/i2c/busses/i2c-exynos5.c                   |   64 ++++++++++++++++----
>   2 files changed, 58 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> index 056732c..5bc4998 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> @@ -5,7 +5,11 @@ at various speeds ranging from 100khz to 3.4Mhz.
>
>   Required properties:
>     - compatible: value should be.
> -      -> "samsung,exynos5-hsi2c", for i2c compatible with exynos5 hsi2c.

Device tree bindings need to be backwards compatible, so you need to 
keep this compatible string supported, just marked as (DEPRECATED).

Driver-wise, it will use the same driver data / variant struct as 
"samsung,exynos5250-hsi2c", just one more entry in OF match table is needed.

> +	-> "samsung,exynos5250-hsi2c", for i2c compatible with HSI2C available
> +				on Exynos5250 and Exynos5420 SoCs.
> +	-> "samsung,exynos5260-hsi2c", for i2c compatible with HSI2C available
> +				on Exynos5260 SoCs.
> +
>     - reg: physical base address of the controller and length of memory mapped
>       region.
>     - interrupts: interrupt number to the cpu.
> @@ -26,7 +30,7 @@ Optional properties:
>   Example:
>
>   hsi2c@12ca0000 {
> -	compatible = "samsung,exynos5-hsi2c";
> +	compatible = "samsung,exynos5250-hsi2c";
>   	reg = <0x12ca0000 0x100>;
>   	interrupts = <56>;
>   	clock-frequency = <100000>;

[snip]

> @@ -483,6 +514,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);
> @@ -493,13 +525,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->variant->fifo_depth) ?
> +			(i2c->variant->fifo_depth * 3/4) : i2c->msg->len;
> +		fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(trig_lvl);
> +

This is a rather serious semantic change, that doesn't look to belong to 
this patch. If this is needed, it should be done in a separate patch.

>   		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->variant->fifo_depth) ?
> +			(i2c->variant->fifo_depth * 1/4) : i2c->msg->len;
> +		fifo_ctl |= HSI2C_TXFIFO_TRIGGER_LEVEL(trig_lvl);
> +

Ditto.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa Feb. 6, 2014, 1:50 p.m. UTC | #2 
Also, please use correct addresses of DT ML and Wolfram's e-mail (fixed 
in this message).

Best regards,
Tomasz

On 06.02.2014 14:31, Tomasz Figa wrote:
> Hi Naveen,
>
> On 06.02.2014 13:06, Naveen Krishna Chatradhi wrote:
>> This patch implements a variant struct to handle the differences
>> (like fifo_depths) in the HSI2C modules across SoCs.
>>
>> Adds a new compatible to support HSI2C module on Exynos5260.
>> Also resets the module as an init sequence (Needed by Exynos5260).
>>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> ---
>> Changes since v2:
>> 1. Used variant struct as suggested by Tomasz Figa.
>> 2. Change compatible strings from samsung,exynos5-hsi2c to
>>     samsung,exynos5250-hsi2c based on the first SoC to see the feature.
>> 3. Using reset as init sequences.
>> 4. Merged the 2 patches into one.
>>
>>   .../devicetree/bindings/i2c/i2c-exynos5.txt        |    8 ++-
>>   drivers/i2c/busses/i2c-exynos5.c                   |   64
>> ++++++++++++++++----
>>   2 files changed, 58 insertions(+), 14 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
>> b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
>> index 056732c..5bc4998 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
>> @@ -5,7 +5,11 @@ at various speeds ranging from 100khz to 3.4Mhz.
>>
>>   Required properties:
>>     - compatible: value should be.
>> -      -> "samsung,exynos5-hsi2c", for i2c compatible with exynos5 hsi2c.
>
> Device tree bindings need to be backwards compatible, so you need to
> keep this compatible string supported, just marked as (DEPRECATED).
>
> Driver-wise, it will use the same driver data / variant struct as
> "samsung,exynos5250-hsi2c", just one more entry in OF match table is
> needed.
>
>> +    -> "samsung,exynos5250-hsi2c", for i2c compatible with HSI2C
>> available
>> +                on Exynos5250 and Exynos5420 SoCs.
>> +    -> "samsung,exynos5260-hsi2c", for i2c compatible with HSI2C
>> available
>> +                on Exynos5260 SoCs.
>> +
>>     - reg: physical base address of the controller and length of
>> memory mapped
>>       region.
>>     - interrupts: interrupt number to the cpu.
>> @@ -26,7 +30,7 @@ Optional properties:
>>   Example:
>>
>>   hsi2c@12ca0000 {
>> -    compatible = "samsung,exynos5-hsi2c";
>> +    compatible = "samsung,exynos5250-hsi2c";
>>       reg = <0x12ca0000 0x100>;
>>       interrupts = <56>;
>>       clock-frequency = <100000>;
>
> [snip]
>
>> @@ -483,6 +514,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);
>> @@ -493,13 +525,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->variant->fifo_depth) ?
>> +            (i2c->variant->fifo_depth * 3/4) : i2c->msg->len;
>> +        fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(trig_lvl);
>> +
>
> This is a rather serious semantic change, that doesn't look to belong to
> this patch. If this is needed, it should be done in a separate patch.
>
>>           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->variant->fifo_depth) ?
>> +            (i2c->variant->fifo_depth * 1/4) : i2c->msg->len;
>> +        fifo_ctl |= HSI2C_TXFIFO_TRIGGER_LEVEL(trig_lvl);
>> +
>
> Ditto.
>
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Feb. 7, 2014, 10:17 a.m. UTC | #3 
On Thu, Feb 06, 2014 at 02:50:51PM +0100, Tomasz Figa wrote:
> Also, please use correct addresses of DT ML and Wolfram's e-mail
> (fixed in this message).

And please don't use In-Reply-To when sending new versions of patches.
The message threading became hard to read here...
Tomasz Figa Feb. 7, 2014, 10:21 a.m. UTC | #4 
On 07.02.2014 11:17, Wolfram Sang wrote:
> On Thu, Feb 06, 2014 at 02:50:51PM +0100, Tomasz Figa wrote:
>> Also, please use correct addresses of DT ML and Wolfram's e-mail
>> (fixed in this message).
>
> And please don't use In-Reply-To when sending new versions of patches.
> The message threading became hard to read here...
>

+1

It's hard to follow new versions of series that show up under the thread 
of old version. If you want to keep reference to old versions, you can 
add links to respective threads to change log entries in cover letter.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
index 056732c..5bc4998 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
@@ -5,7 +5,11 @@  at various speeds ranging from 100khz to 3.4Mhz.
 
 Required properties:
   - compatible: value should be.
-      -> "samsung,exynos5-hsi2c", for i2c compatible with exynos5 hsi2c.
+	-> "samsung,exynos5250-hsi2c", for i2c compatible with HSI2C available
+				on Exynos5250 and Exynos5420 SoCs.
+	-> "samsung,exynos5260-hsi2c", for i2c compatible with HSI2C available
+				on Exynos5260 SoCs.
+
   - reg: physical base address of the controller and length of memory mapped
     region.
   - interrupts: interrupt number to the cpu.
@@ -26,7 +30,7 @@  Optional properties:
 Example:
 
 hsi2c@12ca0000 {
-	compatible = "samsung,exynos5-hsi2c";
+	compatible = "samsung,exynos5250-hsi2c";
 	reg = <0x12ca0000 0x100>;
 	interrupts = <56>;
 	clock-frequency = <100000>;
diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index 9fd711c..5052e8f 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -76,12 +76,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)
 
@@ -183,14 +177,51 @@  struct exynos5_i2c {
 	 * 2. Fast speed upto 1Mbps
 	 */
 	int			speed_mode;
+
+	/* Version of HS-I2C Hardware */
+	struct exynos_hsi2c_variant	*variant;
+};
+
+/**
+ * struct exynos_hsi2c_variant - platform specific HSI2C driver data
+ * @fifo_depth: the fifo depth supported by the HSI2C module
+ *
+ * Specifies platform specific configuration of HSI2C module.
+ * Note: A structure for driver specific platform data is used for future
+ * expansion of its usage.
+ */
+struct exynos_hsi2c_variant {
+	unsigned int		    fifo_depth;
+};
+
+static const struct exynos_hsi2c_variant exynos5250_hsi2c_data = {
+	.fifo_depth	= 64,
+};
+
+static const struct exynos_hsi2c_variant exynos5260_hsi2c_data = {
+	.fifo_depth	= 16,
 };
 
 static const struct of_device_id exynos5_i2c_match[] = {
-	{ .compatible = "samsung,exynos5-hsi2c" },
-	{},
+	{
+		.compatible = "samsung,exynos5250-hsi2c",
+		.data = &exynos5250_hsi2c_data
+	}, {
+		.compatible = "samsung,exynos5260-hsi2c",
+		.data = &exynos5260_hsi2c_data
+	}, {},
 };
 MODULE_DEVICE_TABLE(of, exynos5_i2c_match);
 
+static inline struct exynos_hsi2c_variant *exynos5_i2c_get_variant
+					(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+
+	match = of_match_node(exynos5_i2c_match, pdev->dev.of_node);
+	return (struct exynos_hsi2c_variant *)match->data;
+}
+
 static void exynos5_i2c_clr_pend_irq(struct exynos5_i2c *i2c)
 {
 	writel(readl(i2c->regs + HSI2C_INT_STATUS),
@@ -415,7 +446,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->variant->fifo_depth - fifo_level;
 		if (len > (i2c->msg->len - i2c->msg_ptr))
 			len = i2c->msg->len - i2c->msg_ptr;
 
@@ -483,6 +514,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);
@@ -493,13 +525,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->variant->fifo_depth) ?
+			(i2c->variant->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->variant->fifo_depth) ?
+			(i2c->variant->fifo_depth * 1/4) : i2c->msg->len;
+		fifo_ctl |= HSI2C_TXFIFO_TRIGGER_LEVEL(trig_lvl);
+
 		int_en |= HSI2C_INT_TX_ALMOSTEMPTY_EN;
 	}
 
@@ -691,7 +729,9 @@  static int exynos5_i2c_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_clk;
 
-	exynos5_i2c_init(i2c);
+	i2c->variant = exynos5_i2c_get_variant(pdev);
+
+	exynos5_i2c_reset(i2c);
 
 	ret = i2c_add_adapter(&i2c->adap);
 	if (ret < 0) {