All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] i2c-s3c2410: Updates for exynos4210 and DT-based systems
@ 2012-04-23 16:23 Karol Lewandowski
  2012-04-23 16:24 ` [PATCH 1/2] i2c-s3c2410: Rework device type handling Karol Lewandowski
  2012-04-23 16:24   ` Karol Lewandowski
  0 siblings, 2 replies; 12+ messages in thread
From: Karol Lewandowski @ 2012-04-23 16:23 UTC (permalink / raw)
  To: w.sang
  Cc: m.szyprowski, ben-linux, thomas.abraham, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	kyungmin.park, broonie, Karol Lewandowski

Changes since v3:
 - Replace DT-visible quirks with new controller type
   [Suggested by Wolfram Sang]
 - Dropped already merged of_match_ptr()-patch

Changes since v2:
 - Merge device type and flags into flat bitmask named quirks -
   Consequently, treat s3c24xx as baseline hardware platform and
   support all hw variations via quirks [Suggested by Mark Brown]

Changes since v1:
 - Move unrelated code fragment to separate patch (of_match_ptr())
   [Suggested by Thomas Abracham]
 - Move device-type handling to separate function and rework its
   internals a bit [likewise]

This patchset reworks i2c-s3c2410 driver a bit to better handle
device tree-enabled platforms and adds two quirks required by
exynos4210-specific I2C controller used by s5p-hdmi driver.

This patchset is based on "i2c-bjdooks/for-34/i2c/i2c-samsung" branch
taken from:

  git://git.fluff.org/bjdooks/linux.git


Karol Lewandowski (2):
  i2c-s3c2410: Rework device type handling
  i2c-s3c2410: Add HDMIPHY quirk for S3C2440

 .../devicetree/bindings/i2c/samsung-i2c.txt        |    8 +-
 drivers/i2c/busses/i2c-s3c2410.c                   |  105 ++++++++++++--------
 2 files changed, 70 insertions(+), 43 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] i2c-s3c2410: Rework device type handling
  2012-04-23 16:23 [PATCH v4 0/2] i2c-s3c2410: Updates for exynos4210 and DT-based systems Karol Lewandowski
@ 2012-04-23 16:24 ` Karol Lewandowski
  2012-04-23 18:20   ` Wolfram Sang
  2012-04-23 16:24   ` Karol Lewandowski
  1 sibling, 1 reply; 12+ messages in thread
From: Karol Lewandowski @ 2012-04-23 16:24 UTC (permalink / raw)
  To: w.sang
  Cc: m.szyprowski, ben-linux, thomas.abraham, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	kyungmin.park, broonie, Karol Lewandowski

Reorganize driver a bit to better handle device tree-based systems:

 - move machine type to driver's private structure instead of
   quering platform device variants in runtime

 - replace s3c24xx_i2c_type enum with unsigned int that holds
   bitmask with revision-specific quirks

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/i2c/busses/i2c-s3c2410.c |   75 +++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 41 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 85e3664..23736ff 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -44,8 +44,10 @@
 #include <plat/regs-iic.h>
 #include <plat/iic.h>
 
-/* i2c controller state */
+/* Treat S3C2410 as baseline hardware, anything else is supported via quirks */
+#define QUIRK_S3C2440		(1 << 0)
 
+/* i2c controller state */
 enum s3c24xx_i2c_state {
 	STATE_IDLE,
 	STATE_START,
@@ -54,14 +56,10 @@ enum s3c24xx_i2c_state {
 	STATE_STOP
 };
 
-enum s3c24xx_i2c_type {
-	TYPE_S3C2410,
-	TYPE_S3C2440,
-};
-
 struct s3c24xx_i2c {
 	spinlock_t		lock;
 	wait_queue_head_t	wait;
+	unsigned int            quirks;
 	unsigned int		suspended:1;
 
 	struct i2c_msg		*msg;
@@ -88,26 +86,40 @@ struct s3c24xx_i2c {
 #endif
 };
 
-/* default platform data removed, dev should always carry data. */
+static struct platform_device_id s3c24xx_driver_ids[] = {
+	{
+		.name		= "s3c2410-i2c",
+		.driver_data	= 0,
+	}, {
+		.name		= "s3c2440-i2c",
+		.driver_data	= QUIRK_S3C2440,
+	}, { },
+};
+MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id s3c24xx_i2c_match[] = {
+	{ .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
+	{ .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
+#endif
 
-/* s3c24xx_i2c_is2440()
+/* s3c24xx_get_device_quirks
  *
- * return true is this is an s3c2440
+ * Get controller type either from device tree or platform device variant.
 */
 
-static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
+static inline unsigned int s3c24xx_get_device_quirks(struct platform_device *pdev)
 {
-	struct platform_device *pdev = to_platform_device(i2c->dev);
-	enum s3c24xx_i2c_type type;
-
-#ifdef CONFIG_OF
-	if (i2c->dev->of_node)
-		return of_device_is_compatible(i2c->dev->of_node,
-				"samsung,s3c2440-i2c");
-#endif
+	if (pdev->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(&s3c24xx_i2c_match, pdev->dev.of_node);
+		return (unsigned int)match->data;
+	}
 
-	type = platform_get_device_id(pdev)->driver_data;
-	return type == TYPE_S3C2440;
+	return platform_get_device_id(pdev)->driver_data;
 }
 
 /* s3c24xx_i2c_master_complete
@@ -676,7 +688,7 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
 
 	writel(iiccon, i2c->regs + S3C2410_IICCON);
 
-	if (s3c24xx_i2c_is2440(i2c)) {
+	if (i2c->quirks & QUIRK_S3C2440) {
 		unsigned long sda_delay;
 
 		if (pdata->sda_delay) {
@@ -906,6 +918,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 		goto err_noclk;
 	}
 
+	i2c->quirks = s3c24xx_get_device_quirks(pdev);
 	if (pdata)
 		memcpy(i2c->pdata, pdata, sizeof(*pdata));
 	else
@@ -1110,26 +1123,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
 
 /* device driver for platform bus bits */
 
-static struct platform_device_id s3c24xx_driver_ids[] = {
-	{
-		.name		= "s3c2410-i2c",
-		.driver_data	= TYPE_S3C2410,
-	}, {
-		.name		= "s3c2440-i2c",
-		.driver_data	= TYPE_S3C2440,
-	}, { },
-};
-MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
-
-#ifdef CONFIG_OF
-static const struct of_device_id s3c24xx_i2c_match[] = {
-	{ .compatible = "samsung,s3c2410-i2c" },
-	{ .compatible = "samsung,s3c2440-i2c" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
-#endif
-
 static struct platform_driver s3c24xx_i2c_driver = {
 	.probe		= s3c24xx_i2c_probe,
 	.remove		= s3c24xx_i2c_remove,
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] i2c-s3c2410: Add HDMIPHY quirk for S3C2440
@ 2012-04-23 16:24   ` Karol Lewandowski
  0 siblings, 0 replies; 12+ messages in thread
From: Karol Lewandowski @ 2012-04-23 16:24 UTC (permalink / raw)
  To: w.sang
  Cc: m.szyprowski, ben-linux, thomas.abraham, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	kyungmin.park, broonie, Karol Lewandowski

This patch adds support for s3c2440 I2C bus controller dedicated HDMIPHY device on
Exynos4 platform. Some quirks are introduced due to differences between HDMIPHY
and other I2C controllers on Exynos4.  These differences are:
- no GPIOs, HDMIPHY is inside the SoC and the controller is connected
  internally
- due to unknown reason (probably HW bug in HDMIPHY and/or the controller) a
  transfer fails to finish. The controller hangs after sending the last byte,
  the workaround for this bug is resetting the controller after each transfer

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Tested-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../devicetree/bindings/i2c/samsung-i2c.txt        |    8 ++++--
 drivers/i2c/busses/i2c-s3c2410.c                   |   30 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
index 38832c7..b6cb5a1 100644
--- a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
@@ -6,14 +6,18 @@ Required properties:
   - compatible: value should be either of the following.
       (a) "samsung, s3c2410-i2c", for i2c compatible with s3c2410 i2c.
       (b) "samsung, s3c2440-i2c", for i2c compatible with s3c2440 i2c.
+      (c) "samsung, s3c2440-hdmiphy-i2c", for s3c2440-like i2c used
+          inside HDMIPHY block found on several samsung SoCs
   - reg: physical base address of the controller and length of memory mapped
     region.
   - interrupts: interrupt number to the cpu.
   - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
-  - gpios: The order of the gpios should be the following: <SDA, SCL>.
-    The gpio specifier depends on the gpio controller.
 
 Optional properties:
+  - gpios: The order of the gpios should be the following: <SDA, SCL>.
+    The gpio specifier depends on the gpio controller. Required in all
+    cases except for "samsung,s3c2440-hdmiphy-i2c" whose input/output
+    lines are permanently wired to the respective client
   - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
     specified, default value is 0.
   - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 23736ff..fa0b134 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -46,6 +46,8 @@
 
 /* Treat S3C2410 as baseline hardware, anything else is supported via quirks */
 #define QUIRK_S3C2440		(1 << 0)
+#define QUIRK_HDMIPHY		(1 << 1)
+#define QUIRK_NO_GPIO		(1 << 2)
 
 /* i2c controller state */
 enum s3c24xx_i2c_state {
@@ -93,6 +95,9 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
 	}, {
 		.name		= "s3c2440-i2c",
 		.driver_data	= QUIRK_S3C2440,
+	}, {
+		.name		= "s3c2440-hdmiphy-i2c",
+		.driver_data	= QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO,
 	}, { },
 };
 MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
@@ -101,6 +106,8 @@ MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
 static const struct of_device_id s3c24xx_i2c_match[] = {
 	{ .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
 	{ .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440 },
+	{ .compatible = "samsung,s3c2440-hdmiphy-i2c",
+	  .data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
 	{},
 };
 MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
@@ -483,6 +490,13 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
 	unsigned long iicstat;
 	int timeout = 400;
 
+	/* the timeout for HDMIPHY is reduced to 10 ms because
+	 * the hangup is expected to happen, so waiting 400 ms
+	 * causes only unnecessary system hangup
+	 */
+	if (i2c->quirks & QUIRK_HDMIPHY)
+		timeout = 10;
+
 	while (timeout-- > 0) {
 		iicstat = readl(i2c->regs + S3C2410_IICSTAT);
 
@@ -492,6 +506,15 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
 		msleep(1);
 	}
 
+	/* hang-up of bus dedicated for HDMIPHY occurred, resetting */
+	if (i2c->quirks & QUIRK_HDMIPHY) {
+		writel(0, i2c->regs + S3C2410_IICCON);
+		writel(0, i2c->regs + S3C2410_IICSTAT);
+		writel(0, i2c->regs + S3C2410_IICDS);
+
+		return 0;
+	}
+
 	return -ETIMEDOUT;
 }
 
@@ -773,6 +796,9 @@ static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
 {
 	int idx, gpio, ret;
 
+	if (i2c->quirks & QUIRK_NO_GPIO)
+		return 0;
+
 	for (idx = 0; idx < 2; idx++) {
 		gpio = of_get_gpio(i2c->dev->of_node, idx);
 		if (!gpio_is_valid(gpio)) {
@@ -797,6 +823,10 @@ free_gpio:
 static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
 {
 	unsigned int idx;
+
+	if (i2c->quirks & QUIRK_NO_GPIO)
+		return;
+
 	for (idx = 0; idx < 2; idx++)
 		gpio_free(i2c->gpios[idx]);
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] i2c-s3c2410: Add HDMIPHY quirk for S3C2440
@ 2012-04-23 16:24   ` Karol Lewandowski
  0 siblings, 0 replies; 12+ messages in thread
From: Karol Lewandowski @ 2012-04-23 16:24 UTC (permalink / raw)
  To: w.sang-bIcnvbaLZ9MEGnE8C9+IrQ
  Cc: m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg,
	thomas.abraham-QSEj5FYQhm4dnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
	broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
	Karol Lewandowski

This patch adds support for s3c2440 I2C bus controller dedicated HDMIPHY device on
Exynos4 platform. Some quirks are introduced due to differences between HDMIPHY
and other I2C controllers on Exynos4.  These differences are:
- no GPIOs, HDMIPHY is inside the SoC and the controller is connected
  internally
- due to unknown reason (probably HW bug in HDMIPHY and/or the controller) a
  transfer fails to finish. The controller hangs after sending the last byte,
  the workaround for this bug is resetting the controller after each transfer

Signed-off-by: Tomasz Stanislawski <t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Karol Lewandowski <k.lewandowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Tested-by: Tomasz Stanislawski <t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 .../devicetree/bindings/i2c/samsung-i2c.txt        |    8 ++++--
 drivers/i2c/busses/i2c-s3c2410.c                   |   30 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
index 38832c7..b6cb5a1 100644
--- a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
@@ -6,14 +6,18 @@ Required properties:
   - compatible: value should be either of the following.
       (a) "samsung, s3c2410-i2c", for i2c compatible with s3c2410 i2c.
       (b) "samsung, s3c2440-i2c", for i2c compatible with s3c2440 i2c.
+      (c) "samsung, s3c2440-hdmiphy-i2c", for s3c2440-like i2c used
+          inside HDMIPHY block found on several samsung SoCs
   - reg: physical base address of the controller and length of memory mapped
     region.
   - interrupts: interrupt number to the cpu.
   - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
-  - gpios: The order of the gpios should be the following: <SDA, SCL>.
-    The gpio specifier depends on the gpio controller.
 
 Optional properties:
+  - gpios: The order of the gpios should be the following: <SDA, SCL>.
+    The gpio specifier depends on the gpio controller. Required in all
+    cases except for "samsung,s3c2440-hdmiphy-i2c" whose input/output
+    lines are permanently wired to the respective client
   - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
     specified, default value is 0.
   - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 23736ff..fa0b134 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -46,6 +46,8 @@
 
 /* Treat S3C2410 as baseline hardware, anything else is supported via quirks */
 #define QUIRK_S3C2440		(1 << 0)
+#define QUIRK_HDMIPHY		(1 << 1)
+#define QUIRK_NO_GPIO		(1 << 2)
 
 /* i2c controller state */
 enum s3c24xx_i2c_state {
@@ -93,6 +95,9 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
 	}, {
 		.name		= "s3c2440-i2c",
 		.driver_data	= QUIRK_S3C2440,
+	}, {
+		.name		= "s3c2440-hdmiphy-i2c",
+		.driver_data	= QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO,
 	}, { },
 };
 MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
@@ -101,6 +106,8 @@ MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
 static const struct of_device_id s3c24xx_i2c_match[] = {
 	{ .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
 	{ .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440 },
+	{ .compatible = "samsung,s3c2440-hdmiphy-i2c",
+	  .data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
 	{},
 };
 MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
@@ -483,6 +490,13 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
 	unsigned long iicstat;
 	int timeout = 400;
 
+	/* the timeout for HDMIPHY is reduced to 10 ms because
+	 * the hangup is expected to happen, so waiting 400 ms
+	 * causes only unnecessary system hangup
+	 */
+	if (i2c->quirks & QUIRK_HDMIPHY)
+		timeout = 10;
+
 	while (timeout-- > 0) {
 		iicstat = readl(i2c->regs + S3C2410_IICSTAT);
 
@@ -492,6 +506,15 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
 		msleep(1);
 	}
 
+	/* hang-up of bus dedicated for HDMIPHY occurred, resetting */
+	if (i2c->quirks & QUIRK_HDMIPHY) {
+		writel(0, i2c->regs + S3C2410_IICCON);
+		writel(0, i2c->regs + S3C2410_IICSTAT);
+		writel(0, i2c->regs + S3C2410_IICDS);
+
+		return 0;
+	}
+
 	return -ETIMEDOUT;
 }
 
@@ -773,6 +796,9 @@ static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
 {
 	int idx, gpio, ret;
 
+	if (i2c->quirks & QUIRK_NO_GPIO)
+		return 0;
+
 	for (idx = 0; idx < 2; idx++) {
 		gpio = of_get_gpio(i2c->dev->of_node, idx);
 		if (!gpio_is_valid(gpio)) {
@@ -797,6 +823,10 @@ free_gpio:
 static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
 {
 	unsigned int idx;
+
+	if (i2c->quirks & QUIRK_NO_GPIO)
+		return;
+
 	for (idx = 0; idx < 2; idx++)
 		gpio_free(i2c->gpios[idx]);
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] i2c-s3c2410: Rework device type handling
  2012-04-23 16:24 ` [PATCH 1/2] i2c-s3c2410: Rework device type handling Karol Lewandowski
@ 2012-04-23 18:20   ` Wolfram Sang
  2012-04-24  8:40     ` Karol Lewandowski
  0 siblings, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2012-04-23 18:20 UTC (permalink / raw)
  To: Karol Lewandowski
  Cc: m.szyprowski, ben-linux, thomas.abraham, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	kyungmin.park, broonie

[-- Attachment #1: Type: text/plain, Size: 5017 bytes --]

On Mon, Apr 23, 2012 at 06:24:00PM +0200, Karol Lewandowski wrote:
> Reorganize driver a bit to better handle device tree-based systems:
> 
>  - move machine type to driver's private structure instead of
>    quering platform device variants in runtime
> 
>  - replace s3c24xx_i2c_type enum with unsigned int that holds
>    bitmask with revision-specific quirks
> 
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/i2c/busses/i2c-s3c2410.c |   75 +++++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 85e3664..23736ff 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -44,8 +44,10 @@
>  #include <plat/regs-iic.h>
>  #include <plat/iic.h>
>  
> -/* i2c controller state */
> +/* Treat S3C2410 as baseline hardware, anything else is supported via quirks */
> +#define QUIRK_S3C2440		(1 << 0)
>  
> +/* i2c controller state */
>  enum s3c24xx_i2c_state {
>  	STATE_IDLE,
>  	STATE_START,
> @@ -54,14 +56,10 @@ enum s3c24xx_i2c_state {
>  	STATE_STOP
>  };
>  
> -enum s3c24xx_i2c_type {
> -	TYPE_S3C2410,
> -	TYPE_S3C2440,
> -};
> -
>  struct s3c24xx_i2c {
>  	spinlock_t		lock;
>  	wait_queue_head_t	wait;
> +	unsigned int            quirks;
>  	unsigned int		suspended:1;
>  
>  	struct i2c_msg		*msg;
> @@ -88,26 +86,40 @@ struct s3c24xx_i2c {
>  #endif
>  };
>  
> -/* default platform data removed, dev should always carry data. */
> +static struct platform_device_id s3c24xx_driver_ids[] = {
> +	{
> +		.name		= "s3c2410-i2c",
> +		.driver_data	= 0,
> +	}, {
> +		.name		= "s3c2440-i2c",
> +		.driver_data	= QUIRK_S3C2440,
> +	}, { },
> +};
> +MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id s3c24xx_i2c_match[] = {
> +	{ .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
> +	{ .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
> +#endif
>  
> -/* s3c24xx_i2c_is2440()
> +/* s3c24xx_get_device_quirks
>   *
> - * return true is this is an s3c2440
> + * Get controller type either from device tree or platform device variant.
>  */
>  
> -static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
> +static inline unsigned int s3c24xx_get_device_quirks(struct platform_device *pdev)
>  {
> -	struct platform_device *pdev = to_platform_device(i2c->dev);
> -	enum s3c24xx_i2c_type type;
> -
> -#ifdef CONFIG_OF
> -	if (i2c->dev->of_node)
> -		return of_device_is_compatible(i2c->dev->of_node,
> -				"samsung,s3c2440-i2c");
> -#endif
> +	if (pdev->dev.of_node) {
> +		const struct of_device_id *match;
> +		match = of_match_node(&s3c24xx_i2c_match, pdev->dev.of_node);

I'd appreciate a comment explaining why match can't be NULL here (as to
my understanding, it can't). Or just check for it, but this way it looks
a bit fishy and people (hopefully ;)) will ask about it.

> +		return (unsigned int)match->data;
> +	}
>  
> -	type = platform_get_device_id(pdev)->driver_data;
> -	return type == TYPE_S3C2440;
> +	return platform_get_device_id(pdev)->driver_data;
>  }
>  
>  /* s3c24xx_i2c_master_complete
> @@ -676,7 +688,7 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
>  
>  	writel(iiccon, i2c->regs + S3C2410_IICCON);
>  
> -	if (s3c24xx_i2c_is2440(i2c)) {
> +	if (i2c->quirks & QUIRK_S3C2440) {
>  		unsigned long sda_delay;
>  
>  		if (pdata->sda_delay) {
> @@ -906,6 +918,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  		goto err_noclk;
>  	}
>  
> +	i2c->quirks = s3c24xx_get_device_quirks(pdev);
>  	if (pdata)
>  		memcpy(i2c->pdata, pdata, sizeof(*pdata));
>  	else
> @@ -1110,26 +1123,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
>  
>  /* device driver for platform bus bits */
>  
> -static struct platform_device_id s3c24xx_driver_ids[] = {
> -	{
> -		.name		= "s3c2410-i2c",
> -		.driver_data	= TYPE_S3C2410,
> -	}, {
> -		.name		= "s3c2440-i2c",
> -		.driver_data	= TYPE_S3C2440,
> -	}, { },
> -};
> -MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
> -
> -#ifdef CONFIG_OF
> -static const struct of_device_id s3c24xx_i2c_match[] = {
> -	{ .compatible = "samsung,s3c2410-i2c" },
> -	{ .compatible = "samsung,s3c2440-i2c" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
> -#endif
> -
>  static struct platform_driver s3c24xx_i2c_driver = {
>  	.probe		= s3c24xx_i2c_probe,
>  	.remove		= s3c24xx_i2c_remove,

Rest is looking good, second patch, too.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] i2c-s3c2410: Rework device type handling
  2012-04-23 18:20   ` Wolfram Sang
@ 2012-04-24  8:40     ` Karol Lewandowski
  2012-04-24 14:44       ` Wolfram Sang
  0 siblings, 1 reply; 12+ messages in thread
From: Karol Lewandowski @ 2012-04-24  8:40 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: m.szyprowski, ben-linux, thomas.abraham, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	kyungmin.park, broonie, grant.likely

On 23.04.2012 20:20, Wolfram Sang wrote:

Hi!

>> -static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
>> +static inline unsigned int s3c24xx_get_device_quirks(struct platform_device *pdev)
>>  {
>> -	struct platform_device *pdev = to_platform_device(i2c->dev);
>> -	enum s3c24xx_i2c_type type;
>> -
>> -#ifdef CONFIG_OF
>> -	if (i2c->dev->of_node)
>> -		return of_device_is_compatible(i2c->dev->of_node,
>> -				"samsung,s3c2440-i2c");
>> -#endif
>> +	if (pdev->dev.of_node) {
>> +		const struct of_device_id *match;
>> +		match = of_match_node(&s3c24xx_i2c_match, pdev->dev.of_node);
> 
> I'd appreciate a comment explaining why match can't be NULL here (as to
> my understanding, it can't). Or just check for it, but this way it looks
> a bit fishy and people (hopefully ;)) will ask about it.


My understanding is that it can't be null for exactly same reason why
platform_get_device_id(pdev) can't be null either (see below).

I.e. prerequisite for this code to be run at all (as it's called from
driver's .probe()) is to be already matched against very same match
table.

As far I can see the only possibility for it to fail is to have
dev.of_node pointing to device tree node and NOT being instantiated
from DT description...

>> +		return (unsigned int)match->data;
>> +	}
>>  
>> -	type = platform_get_device_id(pdev)->driver_data;
>> -	return type == TYPE_S3C2440;
>> +	return platform_get_device_id(pdev)->driver_data;
>>  }


Regards,

-- 

Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] i2c-s3c2410: Rework device type handling
  2012-04-24  8:40     ` Karol Lewandowski
@ 2012-04-24 14:44       ` Wolfram Sang
  2012-04-25 11:38           ` Karol Lewandowski
  0 siblings, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2012-04-24 14:44 UTC (permalink / raw)
  To: Karol Lewandowski
  Cc: m.szyprowski, ben-linux, thomas.abraham, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	kyungmin.park, broonie, grant.likely

[-- Attachment #1: Type: text/plain, Size: 1637 bytes --]

On Tue, Apr 24, 2012 at 10:40:49AM +0200, Karol Lewandowski wrote:
> On 23.04.2012 20:20, Wolfram Sang wrote:
> 
> Hi!
> 
> >> -static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
> >> +static inline unsigned int s3c24xx_get_device_quirks(struct platform_device *pdev)
> >>  {
> >> -	struct platform_device *pdev = to_platform_device(i2c->dev);
> >> -	enum s3c24xx_i2c_type type;
> >> -
> >> -#ifdef CONFIG_OF
> >> -	if (i2c->dev->of_node)
> >> -		return of_device_is_compatible(i2c->dev->of_node,
> >> -				"samsung,s3c2440-i2c");
> >> -#endif
> >> +	if (pdev->dev.of_node) {
> >> +		const struct of_device_id *match;
> >> +		match = of_match_node(&s3c24xx_i2c_match, pdev->dev.of_node);
> > 
> > I'd appreciate a comment explaining why match can't be NULL here (as to
> > my understanding, it can't). Or just check for it, but this way it looks
> > a bit fishy and people (hopefully ;)) will ask about it.
> 
> 
> My understanding is that it can't be null for exactly same reason why
> platform_get_device_id(pdev) can't be null either (see below).
> 
> I.e. prerequisite for this code to be run at all (as it's called from
> driver's .probe()) is to be already matched against very same match
> table.

Yes, I agree. Yet, this is not obvious and people might try to fix it
(especially since programs like smatch report it as potentially
dangerous). Ah, whatever, I could simply apply the fix then :) OK.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] i2c-s3c2410: Rework device type handling
@ 2012-04-25 11:38           ` Karol Lewandowski
  0 siblings, 0 replies; 12+ messages in thread
From: Karol Lewandowski @ 2012-04-25 11:38 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: m.szyprowski, ben-linux, thomas.abraham, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	kyungmin.park, broonie, grant.likely

On 24.04.2012 16:44, Wolfram Sang wrote:

> On Tue, Apr 24, 2012 at 10:40:49AM +0200, Karol Lewandowski wrote:
>> On 23.04.2012 20:20, Wolfram Sang wrote:


>>>> +	if (pdev->dev.of_node) {
>>>> +		const struct of_device_id *match;
>>>> +		match = of_match_node(&s3c24xx_i2c_match, pdev->dev.of_node);
>>>
>>> I'd appreciate a comment explaining why match can't be NULL here (as to
>>> my understanding, it can't). Or just check for it, but this way it looks
>>> a bit fishy and people (hopefully ;)) will ask about it.
>>
>>
>> My understanding is that it can't be null for exactly same reason why
>> platform_get_device_id(pdev) can't be null either (see below).
>>
>> I.e. prerequisite for this code to be run at all (as it's called from
>> driver's .probe()) is to be already matched against very same match
>> table.
> 
> Yes, I agree. Yet, this is not obvious and people might try to fix it
> (especially since programs like smatch report it as potentially
> dangerous). Ah, whatever, I could simply apply the fix then :) OK.


Great! I hope it won't cause any problems. :)

Regards,
-- 
Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] i2c-s3c2410: Rework device type handling
@ 2012-04-25 11:38           ` Karol Lewandowski
  0 siblings, 0 replies; 12+ messages in thread
From: Karol Lewandowski @ 2012-04-25 11:38 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg,
	thomas.abraham-QSEj5FYQhm4dnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
	broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
	grant.likely-s3s/WqlpOiPyB63q8FvJNQ

On 24.04.2012 16:44, Wolfram Sang wrote:

> On Tue, Apr 24, 2012 at 10:40:49AM +0200, Karol Lewandowski wrote:
>> On 23.04.2012 20:20, Wolfram Sang wrote:


>>>> +	if (pdev->dev.of_node) {
>>>> +		const struct of_device_id *match;
>>>> +		match = of_match_node(&s3c24xx_i2c_match, pdev->dev.of_node);
>>>
>>> I'd appreciate a comment explaining why match can't be NULL here (as to
>>> my understanding, it can't). Or just check for it, but this way it looks
>>> a bit fishy and people (hopefully ;)) will ask about it.
>>
>>
>> My understanding is that it can't be null for exactly same reason why
>> platform_get_device_id(pdev) can't be null either (see below).
>>
>> I.e. prerequisite for this code to be run at all (as it's called from
>> driver's .probe()) is to be already matched against very same match
>> table.
> 
> Yes, I agree. Yet, this is not obvious and people might try to fix it
> (especially since programs like smatch report it as potentially
> dangerous). Ah, whatever, I could simply apply the fix then :) OK.


Great! I hope it won't cause any problems. :)

Regards,
-- 
Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] i2c-s3c2410: Add HDMIPHY quirk for S3C2440
@ 2012-03-12  6:08     ` Thomas Abraham
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Abraham @ 2012-03-12  6:08 UTC (permalink / raw)
  To: Karol Lewandowski
  Cc: ben-linux, m.szyprowski, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	Kyungmin Park

On 9 March 2012 22:34, Karol Lewandowski <k.lewandowsk@samsung.com> wrote:
> From: Tomasz Stanislawski <t.stanislaws@samsung.com>
>
> This patch adds support for s3c2440 I2C bus controller dedicated HDMIPHY device on
> Exynos4 platform. Some quirks are introduced due to differences between HDMIPHY
> and other I2C controllers on Exynos4.  These differences are:
> - no GPIOs, HDMIPHY is inside the SoC and the controller is connected
>  internally
> - due to unknown reason (probably HW bug in HDMIPHY and/or the controller) a
>  transfer fails to finish. The controller hangs after sending the last byte,
>  the workaround for this bug is resetting the controller after each transfer
>
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Tested-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  .../devicetree/bindings/i2c/samsung-i2c.txt        |   10 +++++-
>  drivers/i2c/busses/i2c-s3c2410.c                   |   34 ++++++++++++++++++++
>  2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> index 38832c7..ac0917c 100644
> --- a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> @@ -10,14 +10,20 @@ Required properties:
>     region.
>   - interrupts: interrupt number to the cpu.
>   - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
> -  - gpios: The order of the gpios should be the following: <SDA, SCL>.
> -    The gpio specifier depends on the gpio controller.
>
>  Optional properties:
> +  - gpios: The order of the gpios should be the following: <SDA, SCL>.
> +    The gpio specifier depends on the gpio controller. Required in all cases
> +    except when "samsung,i2c-no-gpio" is also specified.
> +  - samsung,i2c-no-gpio: input/output lines of the controller are
> +    permanently wired to the respective client, there are no gpio
> +    lines that need to be configured to enable this controller
>   - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
>     specified, default value is 0.
>   - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
>     specified, the default value in Hz is 100000.
> +  - samsung,i2c-quirk-hdmiphy: Quirk for HDMI PHY block on S3C2440 -
> +    reduce timeout and reset controller when doing transefers
>
>  Example:
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 5b400c6..9f3be51 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -48,6 +48,8 @@
>  #define TYPE_BITS              0x000000ff
>  #define TYPE_S3C2410           0x00000001
>  #define TYPE_S3C2440           0x00000002
> +#define FLAG_HDMIPHY           0x00000100
> +#define FLAG_NO_GPIO           0x00000200
>
>  /* i2c controller state */
>  enum s3c24xx_i2c_state {
> @@ -449,6 +451,13 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
>        unsigned long iicstat;
>        int timeout = 400;
>
> +       /* the timeout for HDMIPHY is reduced to 10 ms because
> +        * the hangup is expected to happen, so waiting 400 ms
> +        * causes only unnecessary system hangup
> +        */
> +       if (i2c->type & FLAG_HDMIPHY)
> +               timeout = 10;
> +
>        while (timeout-- > 0) {
>                iicstat = readl(i2c->regs + S3C2410_IICSTAT);
>
> @@ -458,6 +467,15 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
>                msleep(1);
>        }
>
> +       /* hang-up of bus dedicated for HDMIPHY occurred, resetting */
> +       if (i2c->type & FLAG_HDMIPHY) {
> +               writel(0, i2c->regs + S3C2410_IICCON);
> +               writel(0, i2c->regs + S3C2410_IICSTAT);
> +               writel(0, i2c->regs + S3C2410_IICDS);
> +
> +               return 0;
> +       }
> +
>        return -ETIMEDOUT;
>  }
>
> @@ -739,6 +757,9 @@ static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
>  {
>        int idx, gpio, ret;
>
> +       if (i2c->type & FLAG_NO_GPIO)
> +               return 0;
> +
>        for (idx = 0; idx < 2; idx++) {
>                gpio = of_get_gpio(i2c->dev->of_node, idx);
>                if (!gpio_is_valid(gpio)) {
> @@ -763,6 +784,10 @@ free_gpio:
>  static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
>  {
>        unsigned int idx;
> +
> +       if (i2c->type & FLAG_NO_GPIO)
> +               return;
> +
>        for (idx = 0; idx < 2; idx++)
>                gpio_free(i2c->gpios[idx]);
>  }
> @@ -846,6 +871,12 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
>        match = of_match_node(&s3c24xx_i2c_match[0], np);
>        i2c->type = (unsigned int)match->data;
>
> +       if (i2c->type == TYPE_S3C2440 && of_get_property(np, "samsung,i2c-quirk-hdmiphy", NULL))
> +               i2c->type |= FLAG_HDMIPHY;
> +
> +       if (of_get_property(np, "samsung,i2c-no-gpio", NULL))
> +               i2c->type |= FLAG_NO_GPIO;
> +
>        of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata->sda_delay);
>        of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr);
>        of_property_read_u32(np, "samsung,i2c-max-bus-freq",
> @@ -1103,6 +1134,9 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
>        }, {
>                .name           = "s3c2440-i2c",
>                .driver_data    = TYPE_S3C2440,
> +       }, {
> +               .name           = "s3c2440-hdmiphy-i2c",
> +               .driver_data    = TYPE_S3C2440 | FLAG_HDMIPHY | FLAG_NO_GPIO,
>        }, { },
>  };
>  MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
> --
> 1.7.9
>

Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] i2c-s3c2410: Add HDMIPHY quirk for S3C2440
@ 2012-03-12  6:08     ` Thomas Abraham
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Abraham @ 2012-03-12  6:08 UTC (permalink / raw)
  To: Karol Lewandowski
  Cc: ben-linux-elnMNo+KYs3YtjvyW6yDsg,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ, Kyungmin Park

On 9 March 2012 22:34, Karol Lewandowski <k.lewandowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> From: Tomasz Stanislawski <t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
> This patch adds support for s3c2440 I2C bus controller dedicated HDMIPHY device on
> Exynos4 platform. Some quirks are introduced due to differences between HDMIPHY
> and other I2C controllers on Exynos4.  These differences are:
> - no GPIOs, HDMIPHY is inside the SoC and the controller is connected
>  internally
> - due to unknown reason (probably HW bug in HDMIPHY and/or the controller) a
>  transfer fails to finish. The controller hangs after sending the last byte,
>  the workaround for this bug is resetting the controller after each transfer
>
> Signed-off-by: Tomasz Stanislawski <t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Karol Lewandowski <k.lewandowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Tested-by: Tomasz Stanislawski <t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  .../devicetree/bindings/i2c/samsung-i2c.txt        |   10 +++++-
>  drivers/i2c/busses/i2c-s3c2410.c                   |   34 ++++++++++++++++++++
>  2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> index 38832c7..ac0917c 100644
> --- a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> @@ -10,14 +10,20 @@ Required properties:
>     region.
>   - interrupts: interrupt number to the cpu.
>   - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
> -  - gpios: The order of the gpios should be the following: <SDA, SCL>.
> -    The gpio specifier depends on the gpio controller.
>
>  Optional properties:
> +  - gpios: The order of the gpios should be the following: <SDA, SCL>.
> +    The gpio specifier depends on the gpio controller. Required in all cases
> +    except when "samsung,i2c-no-gpio" is also specified.
> +  - samsung,i2c-no-gpio: input/output lines of the controller are
> +    permanently wired to the respective client, there are no gpio
> +    lines that need to be configured to enable this controller
>   - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
>     specified, default value is 0.
>   - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
>     specified, the default value in Hz is 100000.
> +  - samsung,i2c-quirk-hdmiphy: Quirk for HDMI PHY block on S3C2440 -
> +    reduce timeout and reset controller when doing transefers
>
>  Example:
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 5b400c6..9f3be51 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -48,6 +48,8 @@
>  #define TYPE_BITS              0x000000ff
>  #define TYPE_S3C2410           0x00000001
>  #define TYPE_S3C2440           0x00000002
> +#define FLAG_HDMIPHY           0x00000100
> +#define FLAG_NO_GPIO           0x00000200
>
>  /* i2c controller state */
>  enum s3c24xx_i2c_state {
> @@ -449,6 +451,13 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
>        unsigned long iicstat;
>        int timeout = 400;
>
> +       /* the timeout for HDMIPHY is reduced to 10 ms because
> +        * the hangup is expected to happen, so waiting 400 ms
> +        * causes only unnecessary system hangup
> +        */
> +       if (i2c->type & FLAG_HDMIPHY)
> +               timeout = 10;
> +
>        while (timeout-- > 0) {
>                iicstat = readl(i2c->regs + S3C2410_IICSTAT);
>
> @@ -458,6 +467,15 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
>                msleep(1);
>        }
>
> +       /* hang-up of bus dedicated for HDMIPHY occurred, resetting */
> +       if (i2c->type & FLAG_HDMIPHY) {
> +               writel(0, i2c->regs + S3C2410_IICCON);
> +               writel(0, i2c->regs + S3C2410_IICSTAT);
> +               writel(0, i2c->regs + S3C2410_IICDS);
> +
> +               return 0;
> +       }
> +
>        return -ETIMEDOUT;
>  }
>
> @@ -739,6 +757,9 @@ static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
>  {
>        int idx, gpio, ret;
>
> +       if (i2c->type & FLAG_NO_GPIO)
> +               return 0;
> +
>        for (idx = 0; idx < 2; idx++) {
>                gpio = of_get_gpio(i2c->dev->of_node, idx);
>                if (!gpio_is_valid(gpio)) {
> @@ -763,6 +784,10 @@ free_gpio:
>  static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
>  {
>        unsigned int idx;
> +
> +       if (i2c->type & FLAG_NO_GPIO)
> +               return;
> +
>        for (idx = 0; idx < 2; idx++)
>                gpio_free(i2c->gpios[idx]);
>  }
> @@ -846,6 +871,12 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
>        match = of_match_node(&s3c24xx_i2c_match[0], np);
>        i2c->type = (unsigned int)match->data;
>
> +       if (i2c->type == TYPE_S3C2440 && of_get_property(np, "samsung,i2c-quirk-hdmiphy", NULL))
> +               i2c->type |= FLAG_HDMIPHY;
> +
> +       if (of_get_property(np, "samsung,i2c-no-gpio", NULL))
> +               i2c->type |= FLAG_NO_GPIO;
> +
>        of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata->sda_delay);
>        of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr);
>        of_property_read_u32(np, "samsung,i2c-max-bus-freq",
> @@ -1103,6 +1134,9 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
>        }, {
>                .name           = "s3c2440-i2c",
>                .driver_data    = TYPE_S3C2440,
> +       }, {
> +               .name           = "s3c2440-hdmiphy-i2c",
> +               .driver_data    = TYPE_S3C2440 | FLAG_HDMIPHY | FLAG_NO_GPIO,
>        }, { },
>  };
>  MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
> --
> 1.7.9
>

Reviewed-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] i2c-s3c2410: Add HDMIPHY quirk for S3C2440
  2012-03-09 17:04 [PATCH 0/2] i2c-s3c2410: Updates for exynos4210 and DT-based systems Karol Lewandowski
@ 2012-03-09 17:04 ` Karol Lewandowski
  2012-03-12  6:08     ` Thomas Abraham
  0 siblings, 1 reply; 12+ messages in thread
From: Karol Lewandowski @ 2012-03-09 17:04 UTC (permalink / raw)
  To: ben-linux
  Cc: thomas.abraham, m.szyprowski, linux-kernel, linux-i2c,
	devicetree-discuss, linux-samsung-soc, t.stanislaws,
	Karol Lewandowski, Kyungmin Park

From: Tomasz Stanislawski <t.stanislaws@samsung.com>

This patch adds support for s3c2440 I2C bus controller dedicated HDMIPHY device on
Exynos4 platform. Some quirks are introduced due to differences between HDMIPHY
and other I2C controllers on Exynos4.  These differences are:
- no GPIOs, HDMIPHY is inside the SoC and the controller is connected
  internally
- due to unknown reason (probably HW bug in HDMIPHY and/or the controller) a
  transfer fails to finish. The controller hangs after sending the last byte,
  the workaround for this bug is resetting the controller after each transfer

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Tested-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../devicetree/bindings/i2c/samsung-i2c.txt        |   10 +++++-
 drivers/i2c/busses/i2c-s3c2410.c                   |   34 ++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
index 38832c7..ac0917c 100644
--- a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
@@ -10,14 +10,20 @@ Required properties:
     region.
   - interrupts: interrupt number to the cpu.
   - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
-  - gpios: The order of the gpios should be the following: <SDA, SCL>.
-    The gpio specifier depends on the gpio controller.
 
 Optional properties:
+  - gpios: The order of the gpios should be the following: <SDA, SCL>.
+    The gpio specifier depends on the gpio controller. Required in all cases
+    except when "samsung,i2c-no-gpio" is also specified.
+  - samsung,i2c-no-gpio: input/output lines of the controller are
+    permanently wired to the respective client, there are no gpio
+    lines that need to be configured to enable this controller
   - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
     specified, default value is 0.
   - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
     specified, the default value in Hz is 100000.
+  - samsung,i2c-quirk-hdmiphy: Quirk for HDMI PHY block on S3C2440 -
+    reduce timeout and reset controller when doing transefers
 
 Example:
 
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 5b400c6..9f3be51 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -48,6 +48,8 @@
 #define TYPE_BITS		0x000000ff
 #define TYPE_S3C2410		0x00000001
 #define TYPE_S3C2440		0x00000002
+#define FLAG_HDMIPHY		0x00000100
+#define FLAG_NO_GPIO		0x00000200
 
 /* i2c controller state */
 enum s3c24xx_i2c_state {
@@ -449,6 +451,13 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
 	unsigned long iicstat;
 	int timeout = 400;
 
+	/* the timeout for HDMIPHY is reduced to 10 ms because
+	 * the hangup is expected to happen, so waiting 400 ms
+	 * causes only unnecessary system hangup
+	 */
+	if (i2c->type & FLAG_HDMIPHY)
+		timeout = 10;
+
 	while (timeout-- > 0) {
 		iicstat = readl(i2c->regs + S3C2410_IICSTAT);
 
@@ -458,6 +467,15 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
 		msleep(1);
 	}
 
+	/* hang-up of bus dedicated for HDMIPHY occurred, resetting */
+	if (i2c->type & FLAG_HDMIPHY) {
+		writel(0, i2c->regs + S3C2410_IICCON);
+		writel(0, i2c->regs + S3C2410_IICSTAT);
+		writel(0, i2c->regs + S3C2410_IICDS);
+
+		return 0;
+	}
+
 	return -ETIMEDOUT;
 }
 
@@ -739,6 +757,9 @@ static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
 {
 	int idx, gpio, ret;
 
+	if (i2c->type & FLAG_NO_GPIO)
+		return 0;
+
 	for (idx = 0; idx < 2; idx++) {
 		gpio = of_get_gpio(i2c->dev->of_node, idx);
 		if (!gpio_is_valid(gpio)) {
@@ -763,6 +784,10 @@ free_gpio:
 static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
 {
 	unsigned int idx;
+
+	if (i2c->type & FLAG_NO_GPIO)
+		return;
+
 	for (idx = 0; idx < 2; idx++)
 		gpio_free(i2c->gpios[idx]);
 }
@@ -846,6 +871,12 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
 	match = of_match_node(&s3c24xx_i2c_match[0], np);
 	i2c->type = (unsigned int)match->data;
 
+	if (i2c->type == TYPE_S3C2440 && of_get_property(np, "samsung,i2c-quirk-hdmiphy", NULL))
+		i2c->type |= FLAG_HDMIPHY;
+
+	if (of_get_property(np, "samsung,i2c-no-gpio", NULL))
+		i2c->type |= FLAG_NO_GPIO;
+
 	of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata->sda_delay);
 	of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr);
 	of_property_read_u32(np, "samsung,i2c-max-bus-freq",
@@ -1103,6 +1134,9 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
 	}, {
 		.name		= "s3c2440-i2c",
 		.driver_data	= TYPE_S3C2440,
+	}, {
+		.name		= "s3c2440-hdmiphy-i2c",
+		.driver_data	= TYPE_S3C2440 | FLAG_HDMIPHY | FLAG_NO_GPIO,
 	}, { },
 };
 MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
-- 
1.7.9


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-04-25 12:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-23 16:23 [PATCH v4 0/2] i2c-s3c2410: Updates for exynos4210 and DT-based systems Karol Lewandowski
2012-04-23 16:24 ` [PATCH 1/2] i2c-s3c2410: Rework device type handling Karol Lewandowski
2012-04-23 18:20   ` Wolfram Sang
2012-04-24  8:40     ` Karol Lewandowski
2012-04-24 14:44       ` Wolfram Sang
2012-04-25 11:38         ` Karol Lewandowski
2012-04-25 11:38           ` Karol Lewandowski
2012-04-23 16:24 ` [PATCH 2/2] i2c-s3c2410: Add HDMIPHY quirk for S3C2440 Karol Lewandowski
2012-04-23 16:24   ` Karol Lewandowski
  -- strict thread matches above, loose matches on Subject: below --
2012-03-09 17:04 [PATCH 0/2] i2c-s3c2410: Updates for exynos4210 and DT-based systems Karol Lewandowski
2012-03-09 17:04 ` [PATCH 2/2] i2c-s3c2410: Add HDMIPHY quirk for S3C2440 Karol Lewandowski
2012-03-12  6:08   ` Thomas Abraham
2012-03-12  6:08     ` Thomas Abraham

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.