All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
@ 2015-10-20  6:38 ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-20  6:38 UTC (permalink / raw)
  To: mika.westerberg, baruch; +Cc: SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

DW I2C driver tries to register a clk from id->driver_data as an
alternative way besides intel lpss. But code doesn't register the
clk to clkdev. So, devm_clk_get will fail during probe.

The patch can fix this issue.

Signed-off-by: Ken Xue <Ken.Xue@amd.com>
Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3dd2de3..9ee1fc6 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -27,6 +27,7 @@
 #include <linux/i2c.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/clkdev.h>
 #include <linux/errno.h>
 #include <linux/sched.h>
 #include <linux/err.h>
@@ -78,6 +79,7 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
 	const struct acpi_device_id *id;
+	struct clk *clk;
 
 	dev->adapter.nr = -1;
 	dev->tx_fifo_depth = 32;
@@ -97,9 +99,11 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 	 * id->driver_data.
 	 */
 	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
-	if (id && id->driver_data)
-		clk_register_fixed_rate(&pdev->dev, dev_name(&pdev->dev), NULL,
-					CLK_IS_ROOT, id->driver_data);
+	if (id && id->driver_data) {
+		clk = clk_register_fixed_rate(&pdev->dev, dev_name(&pdev->dev),
+					NULL, CLK_IS_ROOT, id->driver_data);
+		clk_register_clkdev(clk, NULL, dev_name(&pdev->dev));
+	}
 
 	return 0;
 }
-- 
1.9.1




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

* [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
@ 2015-10-20  6:38 ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-20  6:38 UTC (permalink / raw)
  To: mika.westerberg, baruch; +Cc: SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

DW I2C driver tries to register a clk from id->driver_data as an
alternative way besides intel lpss. But code doesn't register the
clk to clkdev. So, devm_clk_get will fail during probe.

The patch can fix this issue.

Signed-off-by: Ken Xue <Ken.Xue@amd.com>
Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3dd2de3..9ee1fc6 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -27,6 +27,7 @@
 #include <linux/i2c.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/clkdev.h>
 #include <linux/errno.h>
 #include <linux/sched.h>
 #include <linux/err.h>
@@ -78,6 +79,7 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
 	const struct acpi_device_id *id;
+	struct clk *clk;
 
 	dev->adapter.nr = -1;
 	dev->tx_fifo_depth = 32;
@@ -97,9 +99,11 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 	 * id->driver_data.
 	 */
 	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
-	if (id && id->driver_data)
-		clk_register_fixed_rate(&pdev->dev, dev_name(&pdev->dev), NULL,
-					CLK_IS_ROOT, id->driver_data);
+	if (id && id->driver_data) {
+		clk = clk_register_fixed_rate(&pdev->dev, dev_name(&pdev->dev),
+					NULL, CLK_IS_ROOT, id->driver_data);
+		clk_register_clkdev(clk, NULL, dev_name(&pdev->dev));
+	}
 
 	return 0;
 }
-- 
1.9.1

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-20  6:38 ` Ken Xue
  (?)
@ 2015-10-20 11:17 ` Mika Westerberg
  2015-10-21  1:11     ` Ken Xue
  -1 siblings, 1 reply; 18+ messages in thread
From: Mika Westerberg @ 2015-10-20 11:17 UTC (permalink / raw)
  To: Ken Xue; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> DW I2C driver tries to register a clk from id->driver_data as an
> alternative way besides intel lpss. But code doesn't register the
> clk to clkdev. So, devm_clk_get will fail during probe.
> 
> The patch can fix this issue.

Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
create the clock there just like we do for Intel stuff?

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-20 11:17 ` Mika Westerberg
@ 2015-10-21  1:11     ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-21  1:11 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > DW I2C driver tries to register a clk from id->driver_data as an
> > alternative way besides intel lpss. But code doesn't register the
> > clk to clkdev. So, devm_clk_get will fail during probe.
> > 
> > The patch can fix this issue.
> 
> Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> create the clock there just like we do for Intel stuff?
Sure. APD already creates the clock for AMD0010 as you expected. And the
next patch([PATCH 2/2] i2c: designware: remove freq definition for
"AMD0010" in acpi_device_id) is dropping the old way for getting freq.


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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
@ 2015-10-21  1:11     ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-21  1:11 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > DW I2C driver tries to register a clk from id->driver_data as an
> > alternative way besides intel lpss. But code doesn't register the
> > clk to clkdev. So, devm_clk_get will fail during probe.
> > 
> > The patch can fix this issue.
> 
> Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> create the clock there just like we do for Intel stuff?
Sure. APD already creates the clock for AMD0010 as you expected. And the
next patch([PATCH 2/2] i2c: designware: remove freq definition for
"AMD0010" in acpi_device_id) is dropping the old way for getting freq.

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21  1:11     ` Ken Xue
  (?)
@ 2015-10-21  7:28     ` Mika Westerberg
  2015-10-21  8:42         ` Ken Xue
  -1 siblings, 1 reply; 18+ messages in thread
From: Mika Westerberg @ 2015-10-21  7:28 UTC (permalink / raw)
  To: Ken Xue; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > DW I2C driver tries to register a clk from id->driver_data as an
> > > alternative way besides intel lpss. But code doesn't register the
> > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > 
> > > The patch can fix this issue.
> > 
> > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > create the clock there just like we do for Intel stuff?
> Sure. APD already creates the clock for AMD0010 as you expected. And the
> next patch([PATCH 2/2] i2c: designware: remove freq definition for
> "AMD0010" in acpi_device_id) is dropping the old way for getting freq.

So this patch is not necessary, right?

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21  7:28     ` Mika Westerberg
@ 2015-10-21  8:42         ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-21  8:42 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > alternative way besides intel lpss. But code doesn't register the
> > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > 
> > > > The patch can fix this issue.
> > > 
> > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > create the clock there just like we do for Intel stuff?
> > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> 
> So this patch is not necessary, right?
Even though there is no use case that getting freq from id->driver_data,
But if we want to keep this design, then we should use current patch for
fixing the potential issue. So, the patch is nice to have.

Otherwise, we have to revert whole old design(a445900c).


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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
@ 2015-10-21  8:42         ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-21  8:42 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > alternative way besides intel lpss. But code doesn't register the
> > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > 
> > > > The patch can fix this issue.
> > > 
> > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > create the clock there just like we do for Intel stuff?
> > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> 
> So this patch is not necessary, right?
Even though there is no use case that getting freq from id->driver_data,
But if we want to keep this design, then we should use current patch for
fixing the potential issue. So, the patch is nice to have.

Otherwise, we have to revert whole old design(a445900c).

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21  8:42         ` Ken Xue
  (?)
@ 2015-10-21  9:25         ` Mika Westerberg
  2015-10-21  9:37             ` Ken Xue
  -1 siblings, 1 reply; 18+ messages in thread
From: Mika Westerberg @ 2015-10-21  9:25 UTC (permalink / raw)
  To: Ken Xue; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > 
> > > > > The patch can fix this issue.
> > > > 
> > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > create the clock there just like we do for Intel stuff?
> > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > 
> > So this patch is not necessary, right?
> Even though there is no use case that getting freq from id->driver_data,
> But if we want to keep this design, then we should use current patch for
> fixing the potential issue. So, the patch is nice to have.

What potential issue?

If you pass clock from drivers/acpi/acpi_apd.c and drop the hard coded
freq for AMD0010 in the I2C designware driver, the driver still works
just fine.

> Otherwise, we have to revert whole old design(a445900c).

Yes please :-)

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21  9:25         ` Mika Westerberg
@ 2015-10-21  9:37             ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-21  9:37 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > 
> > > > > > The patch can fix this issue.
> > > > > 
> > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > > create the clock there just like we do for Intel stuff?
> > > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > > 
> > > So this patch is not necessary, right?
> > Even though there is no use case that getting freq from id->driver_data,
> > But if we want to keep this design, then we should use current patch for
> > fixing the potential issue. So, the patch is nice to have.
> 
> What potential issue?
devm_clk_get will fail during probe for AMD0010 without current patch.

> 
> If you pass clock from drivers/acpi/acpi_apd.c and drop the hard coded
> freq for AMD0010 in the I2C designware driver, the driver still works
> just fine.
> 
> > Otherwise, we have to revert whole old design(a445900c).
> 
> Yes please :-)
Glad to do.



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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
@ 2015-10-21  9:37             ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-21  9:37 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > 
> > > > > > The patch can fix this issue.
> > > > > 
> > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > > create the clock there just like we do for Intel stuff?
> > > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > > 
> > > So this patch is not necessary, right?
> > Even though there is no use case that getting freq from id->driver_data,
> > But if we want to keep this design, then we should use current patch for
> > fixing the potential issue. So, the patch is nice to have.
> 
> What potential issue?
devm_clk_get will fail during probe for AMD0010 without current patch.

> 
> If you pass clock from drivers/acpi/acpi_apd.c and drop the hard coded
> freq for AMD0010 in the I2C designware driver, the driver still works
> just fine.
> 
> > Otherwise, we have to revert whole old design(a445900c).
> 
> Yes please :-)
Glad to do.

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21  9:37             ` Ken Xue
  (?)
@ 2015-10-21  9:49             ` Mika Westerberg
  2015-10-21  9:50                 ` Ken Xue
  -1 siblings, 1 reply; 18+ messages in thread
From: Mika Westerberg @ 2015-10-21  9:49 UTC (permalink / raw)
  To: Ken Xue; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, Oct 21, 2015 at 05:37:53PM +0800, Ken Xue wrote:
> On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> > On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > > 
> > > > > > > The patch can fix this issue.
> > > > > > 
> > > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > > > create the clock there just like we do for Intel stuff?
> > > > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > > > 
> > > > So this patch is not necessary, right?
> > > Even though there is no use case that getting freq from id->driver_data,
> > > But if we want to keep this design, then we should use current patch for
> > > fixing the potential issue. So, the patch is nice to have.
> > 
> > What potential issue?
> devm_clk_get will fail during probe for AMD0010 without current patch.

How can it fail if you provide the very clock from drivers/acpi/acpi_apd.c?

> > 
> > If you pass clock from drivers/acpi/acpi_apd.c and drop the hard coded
> > freq for AMD0010 in the I2C designware driver, the driver still works
> > just fine.
> > 
> > > Otherwise, we have to revert whole old design(a445900c).
> > 
> > Yes please :-)
> Glad to do.

Thanks.

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21  9:49             ` Mika Westerberg
@ 2015-10-21  9:50                 ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-21  9:50 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, 2015-10-21 at 12:49 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 05:37:53PM +0800, Ken Xue wrote:
> > On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> > > On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > > > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > > > 
> > > > > > > > The patch can fix this issue.
> > > > > > > 
> > > > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > > > > create the clock there just like we do for Intel stuff?
> > > > > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > > > > 
> > > > > So this patch is not necessary, right?
> > > > Even though there is no use case that getting freq from id->driver_data,
> > > > But if we want to keep this design, then we should use current patch for
> > > > fixing the potential issue. So, the patch is nice to have.
> > > 
> > > What potential issue?
> > devm_clk_get will fail during probe for AMD0010 without current patch.
> 
> How can it fail if you provide the very clock from drivers/acpi/acpi_apd.c?
After apd was accept in kernel V4.1, there is no issue. But between 3.18
and V4.1, there will be a problem.


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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
@ 2015-10-21  9:50                 ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-21  9:50 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, 2015-10-21 at 12:49 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 05:37:53PM +0800, Ken Xue wrote:
> > On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> > > On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > > > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > > > 
> > > > > > > > The patch can fix this issue.
> > > > > > > 
> > > > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > > > > create the clock there just like we do for Intel stuff?
> > > > > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > > > > 
> > > > > So this patch is not necessary, right?
> > > > Even though there is no use case that getting freq from id->driver_data,
> > > > But if we want to keep this design, then we should use current patch for
> > > > fixing the potential issue. So, the patch is nice to have.
> > > 
> > > What potential issue?
> > devm_clk_get will fail during probe for AMD0010 without current patch.
> 
> How can it fail if you provide the very clock from drivers/acpi/acpi_apd.c?
After apd was accept in kernel V4.1, there is no issue. But between 3.18
and V4.1, there will be a problem.

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21  9:50                 ` Ken Xue
  (?)
@ 2015-10-21 10:02                 ` Mika Westerberg
  -1 siblings, 0 replies; 18+ messages in thread
From: Mika Westerberg @ 2015-10-21 10:02 UTC (permalink / raw)
  To: Ken Xue; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, Oct 21, 2015 at 05:50:02PM +0800, Ken Xue wrote:
> After apd was accept in kernel V4.1, there is no issue. But between 3.18
> and V4.1, there will be a problem.

We care about that because?

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21  9:50                 ` Ken Xue
  (?)
  (?)
@ 2015-10-21 10:46                 ` Mika Westerberg
  2015-10-23  5:29                     ` Ken Xue
  -1 siblings, 1 reply; 18+ messages in thread
From: Mika Westerberg @ 2015-10-21 10:46 UTC (permalink / raw)
  To: Ken Xue; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, Oct 21, 2015 at 05:50:02PM +0800, Ken Xue wrote:
> On Wed, 2015-10-21 at 12:49 +0300, Mika Westerberg wrote:
> > On Wed, Oct 21, 2015 at 05:37:53PM +0800, Ken Xue wrote:
> > > On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> > > > On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > > > > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > > > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > > > > 
> > > > > > > > > The patch can fix this issue.
> > > > > > > > 
> > > > > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > > > > > create the clock there just like we do for Intel stuff?
> > > > > > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > > > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > > > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > > > > > 
> > > > > > So this patch is not necessary, right?
> > > > > Even though there is no use case that getting freq from id->driver_data,
> > > > > But if we want to keep this design, then we should use current patch for
> > > > > fixing the potential issue. So, the patch is nice to have.
> > > > 
> > > > What potential issue?
> > > devm_clk_get will fail during probe for AMD0010 without current patch.
> > 
> > How can it fail if you provide the very clock from drivers/acpi/acpi_apd.c?
> After apd was accept in kernel V4.1, there is no issue. But between 3.18
> and V4.1, there will be a problem.

Ah, now I got it.

You are saying that the original commit a445900c906092 ("i2c:
designware: Add support for AMD I2C controller") actually never worked
because it failed to register the clock with clkdev? In that case it is
not even a regression ;-) Oh my...

In that case I don't think we need to fix that for 3.18+ because it
never worked in the first place.

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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
  2015-10-21 10:46                 ` Mika Westerberg
@ 2015-10-23  5:29                     ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-23  5:29 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, 2015-10-21 at 13:46 +0300, Mika Westerberg wrote:
> You are saying that the original commit a445900c906092 ("i2c:
> designware: Add support for AMD I2C controller") actually never worked
> because it failed to register the clock with clkdev? In that case it is
> not even a regression ;-) Oh my...

You are right :-). The new patch for reverting a445900c906092 is
submitted.
Please help to review. Thanks.


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

* Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
@ 2015-10-23  5:29                     ` Ken Xue
  0 siblings, 0 replies; 18+ messages in thread
From: Ken Xue @ 2015-10-23  5:29 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: baruch, SPG_Linux_Kernel, linux-i2c, linux-kernel, stable

On Wed, 2015-10-21 at 13:46 +0300, Mika Westerberg wrote:
> You are saying that the original commit a445900c906092 ("i2c:
> designware: Add support for AMD I2C controller") actually never worked
> because it failed to register the clock with clkdev? In that case it is
> not even a regression ;-) Oh my...

You are right :-). The new patch for reverting a445900c906092 is
submitted.
Please help to review. Thanks.

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

end of thread, other threads:[~2015-10-23  5:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-20  6:38 [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration Ken Xue
2015-10-20  6:38 ` Ken Xue
2015-10-20 11:17 ` Mika Westerberg
2015-10-21  1:11   ` Ken Xue
2015-10-21  1:11     ` Ken Xue
2015-10-21  7:28     ` Mika Westerberg
2015-10-21  8:42       ` Ken Xue
2015-10-21  8:42         ` Ken Xue
2015-10-21  9:25         ` Mika Westerberg
2015-10-21  9:37           ` Ken Xue
2015-10-21  9:37             ` Ken Xue
2015-10-21  9:49             ` Mika Westerberg
2015-10-21  9:50               ` Ken Xue
2015-10-21  9:50                 ` Ken Xue
2015-10-21 10:02                 ` Mika Westerberg
2015-10-21 10:46                 ` Mika Westerberg
2015-10-23  5:29                   ` Ken Xue
2015-10-23  5:29                     ` Ken Xue

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.