Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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 :-) -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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). -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration
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. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
