Re: [PATCH v4] i2c: designware: Do not require clock when SSCN and FFCN are provided
Hi, On 01/06/2016 06:31 PM, Loc Ho wrote: Hi All, The current driver uses input clock source frequency to calculate values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we do not currently have a good way to provide the frequency information. Instead, we can leverage the SSCN and FFCN ACPI methods, which can be used to directly provide these values. So, the clock information should no longer be required during probing. However, since clk can be invalid, additional checks must be done where we are making use of it. Signed-off-by: Mika Westerberg <mika.westerb...@linux.intel.com> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com> Tested-by: Loc Ho <l...@apm.com> --- Note: This has been tested on AMD Seattle RevB for both DT and ACPI. Changes from V3 (https://lkml.org/lkml/2015/12/22/596): * Add i2c_dw_plat_prepare_clk() per Andy's suggestion * Add tested-by Loc Ho. The changes from V3 are big enough that I'd appreciate a new Tested-by tag. I had tested this via this mixes of test cases: a. NO APD driver + this patch ==> HCNT/LCNT as expected b. with APD driver + this patch ==> HCNT/LCNT as expected c. with APD driver + this patch + double the frequency APD driver ==> HCNT/LCNT as expected d. with APD driver + this patch + double the frequency APD driver + comment out the ACPI parameter retrieve ==> HCNT/LCNT changed as expected Therefore, you can add my - Tested-by: Loc Ho <l...@apm.com> -Loc Thanks Loc, Suravee -- 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 v3] i2c: designware: Do not require clock when SSCN and FFCN are provided
Hi Andy,
On 12/23/2015 12:27 PM, Andy Shevchenko wrote:
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>b/drivers/i2c/busses/i2c-designware-platdrv.c
>index 8ffc36b..04edd09 100644
>--- a/drivers/i2c/busses/i2c-designware-platdrv.c
>+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
Can we introduce
static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool
prepare)
{
if (IS_ERR(i_dev->clk))
return PTR_ERR(i_dev->clk);
if (prepare)
/* REMOVEME: Yes, you have to check return value and this is one
benefit of this change */
return clk_prepare_enable(i_dev->clk);
clk_disable_unprepare(i_dev->clk);
return 0;
}
and…
Yes, I think this change. I'll put this in the V4, and send out soon.
Suravee
--
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
[PATCH v4] i2c: designware: Do not require clock when SSCN and FFCN are provided
The current driver uses input clock source frequency to calculate
values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we do not
currently have a good way to provide the frequency information.
Instead, we can leverage the SSCN and FFCN ACPI methods, which can be used
to directly provide these values. So, the clock information should
no longer be required during probing.
However, since clk can be invalid, additional checks must be done where
we are making use of it.
Signed-off-by: Mika Westerberg <mika.westerb...@linux.intel.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
Tested-by: Loc Ho <l...@apm.com>
---
Note: This has been tested on AMD Seattle RevB for both DT and ACPI.
Changes from V3 (https://lkml.org/lkml/2015/12/22/596):
* Add i2c_dw_plat_prepare_clk() per Andy's suggestion
* Add tested-by Loc Ho.
Changes from V2 (https://lkml.org/lkml/2015/12/22/584):
* Add the i2c_dw_clk_rate from Mika.
* Add check if get_clk_rate_khz is set before setting sda_hold_time
Changes from V1 (https://lkml.org/lkml/2015/12/15/792):
In v1, I disregarded the clock if SSCN and FMCN are provided,
assuming that it was not needed. That was incorrect assumption,
and is now fixed in v2.
drivers/i2c/busses/i2c-designware-core.c| 22 +---
drivers/i2c/busses/i2c-designware-platdrv.c | 31 +++--
2 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 8c48b27..25dccd8 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -271,6 +271,17 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool
enable)
enable ? "en" : "dis");
}
+static unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
+{
+ /*
+* Clock is not necessary if we got LCNT/HCNT values directly from
+* the platform code.
+*/
+ if (WARN_ON_ONCE(!dev->get_clk_rate_khz))
+ return 0;
+ return dev->get_clk_rate_khz(dev);
+}
+
/**
* i2c_dw_init() - initialize the designware i2c master hardware
* @dev: device private data
@@ -281,7 +292,6 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool
enable)
*/
int i2c_dw_init(struct dw_i2c_dev *dev)
{
- u32 input_clock_khz;
u32 hcnt, lcnt;
u32 reg;
u32 sda_falling_time, scl_falling_time;
@@ -295,8 +305,6 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
}
}
- input_clock_khz = dev->get_clk_rate_khz(dev);
-
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
/* Configure register endianess access */
@@ -325,12 +333,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
hcnt = dev->ss_hcnt;
lcnt = dev->ss_lcnt;
} else {
- hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+ hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
4000, /* tHD;STA = tHIGH = 4.0 us */
sda_falling_time,
0, /* 0: DW default, 1: Ideal */
0); /* No offset */
- lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+ lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
4700, /* tLOW = 4.7 us */
scl_falling_time,
0); /* No offset */
@@ -344,12 +352,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
hcnt = dev->fs_hcnt;
lcnt = dev->fs_lcnt;
} else {
- hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+ hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
600,/* tHD;STA = tHIGH = 0.6 us */
sda_falling_time,
0, /* 0: DW default, 1: Ideal */
0); /* No offset */
- lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+ lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
1300, /* tLOW = 1.3 us */
scl_falling_time,
0); /* No offset */
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 8ffc36b..965512c 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -128,6 +128,18 @@ static inline int dw_i2c_acpi_configure(struct
platform_device *pdev)
}
#endif
+static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
+{
+
Re: [PATCH] i2c: designware: Add support for AMD Seattle I2C
Hi Wolfram,
On 01/03/2016 12:45 PM, Wolfram Sang wrote:
On Wed, Dec 16, 2015 at 06:49:59PM -0600, Suravee Suthikulanit wrote:
Mika,
On 12/16/2015 8:54 AM, Mika Westerberg wrote:
On Wed, Dec 16, 2015 at 08:29:38AM -0600, Suravee Suthikulpanit wrote:
On 12/16/2015 03:16 AM, Mika Westerberg wrote:
On Tue, Dec 15, 2015 at 08:14:34PM -0600, Suravee Suthikulpanit wrote:
Hi Mika,
On 12/15/15 15:55, Suravee Suthikulpanit wrote:
Add device HID AMDI0510 to match the I2C controlers on AMD Seattle platform
Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpa...@amd.com>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 57f623b..a027154 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -117,6 +117,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "80860F41", 0 },
{ "808622C1", 0 },
{ "AMD0010", 0 },
+ { "AMDI0510", 0 },
{ }
Since this driver seems to be used by several SOCs, and we have been adding
the HID from various SOC vendors. Do you think it would be better to assign
a CID so that each SOC vendor can specify in their ACPI DSDT and we can
match them here?
Sure _CID would work here.
Do you know if Synopsys has already provided a CID that we can use for this?
No.
If not, who do you think should provide this?
Why can't you make _CID for AMD part only? For Intel we are going to get
new IDs for every major SoC release no matter what.
Actually, after discussed with the team. We have decided to go with the
AMDI0510 at this point, and we will reuse this as CID in future SOC if it
contains compatible I2C controller.
So, can I take the patch as is?
Yes, please pull this as is.
Thank you,
Suravee
--
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
[PATCH v3] i2c: designware: Do not require clock when SSCN and FFCN are provided
The current driver uses input clock source frequency to calculate
values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we do not
currently have a good way to provide the frequency information.
Instead, we can leverage the SSCN and FFCN ACPI methods, which can be used
to directly provide these values. So, the clock information should
no longer be required during probing.
However, since clk can be invalid, additional checks must be done where
we are making use of it.
Signed-off-by: Mika Westerberg <mika.westerb...@linux.intel.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
Note: This has been tested on AMD Seattle RevB for both DT and ACPI.
Changes from V2 (https://lkml.org/lkml/2015/12/22/584):
* Add the i2c_dw_clk_rate from Mika.
* Add check if get_clk_rate_khz is set before setting sda_hold_time
Changes from V1 (https://lkml.org/lkml/2015/12/15/792):
In v1, I disregarded the clock if SSCN and FMCN are provided,
assuming that it was not needed. That was incorrect assumption,
and is now fixed in v2.
drivers/i2c/busses/i2c-designware-core.c| 22 +++---
drivers/i2c/busses/i2c-designware-platdrv.c | 24
2 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 8c48b27..25dccd8 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -271,6 +271,17 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool
enable)
enable ? "en" : "dis");
}
+static unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
+{
+ /*
+* Clock is not necessary if we got LCNT/HCNT values directly from
+* the platform code.
+*/
+ if (WARN_ON_ONCE(!dev->get_clk_rate_khz))
+ return 0;
+ return dev->get_clk_rate_khz(dev);
+}
+
/**
* i2c_dw_init() - initialize the designware i2c master hardware
* @dev: device private data
@@ -281,7 +292,6 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool
enable)
*/
int i2c_dw_init(struct dw_i2c_dev *dev)
{
- u32 input_clock_khz;
u32 hcnt, lcnt;
u32 reg;
u32 sda_falling_time, scl_falling_time;
@@ -295,8 +305,6 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
}
}
- input_clock_khz = dev->get_clk_rate_khz(dev);
-
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
/* Configure register endianess access */
@@ -325,12 +333,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
hcnt = dev->ss_hcnt;
lcnt = dev->ss_lcnt;
} else {
- hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+ hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
4000, /* tHD;STA = tHIGH = 4.0 us */
sda_falling_time,
0, /* 0: DW default, 1: Ideal */
0); /* No offset */
- lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+ lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
4700, /* tLOW = 4.7 us */
scl_falling_time,
0); /* No offset */
@@ -344,12 +352,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
hcnt = dev->fs_hcnt;
lcnt = dev->fs_lcnt;
} else {
- hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+ hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
600,/* tHD;STA = tHIGH = 0.6 us */
sda_falling_time,
0, /* 0: DW default, 1: Ideal */
0); /* No offset */
- lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+ lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
1300, /* tLOW = 1.3 us */
scl_falling_time,
0); /* No offset */
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 8ffc36b..04edd09 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -205,16 +205,14 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
dev->clk = devm_clk_get(>dev, NULL);
- dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
- if (IS_ERR(dev->clk))
- return PTR_ERR(dev->clk);
- clk_prepare_enable(dev-&
[PATCH v2] i2c: designware: Do not require clock when SSCN and FFCN are provided
The current driver uses input clock source frequency to calculate
values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we do not
currently have a good way to provide the frequency information.
Instead, we can leverage the SSCN and FFCN ACPI methods, which can be used
to directly provide these values. So, the clock information should
no longer be required during probing.
However, since clk can be invalid, additional checks must be done where
we are making use of it.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
Note: This has been tested on AMD Seattle RevB for both DT and ACPI.
Changes in V2:
In v1, I disregarded the clock if SSCN and FMCN are provided,
assuming that it was not needed. That was incorrect assumption,
and is now fixed in v2.
drivers/i2c/busses/i2c-designware-platdrv.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 8ffc36b..4615fe3 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -44,6 +44,9 @@
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
{
+ if (IS_ERR(dev->clk))
+ return 0;
+
return clk_get_rate(dev->clk)/1000;
}
@@ -206,9 +209,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
dev->clk = devm_clk_get(>dev, NULL);
dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
- if (IS_ERR(dev->clk))
- return PTR_ERR(dev->clk);
- clk_prepare_enable(dev->clk);
+ if (!IS_ERR(dev->clk))
+ clk_prepare_enable(dev->clk);
if (!dev->sda_hold_time && ht) {
u32 ic_clk = dev->get_clk_rate_khz(dev);
@@ -297,7 +299,8 @@ static int dw_i2c_plat_suspend(struct device *dev)
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
i2c_dw_disable(i_dev);
- clk_disable_unprepare(i_dev->clk);
+ if (!IS_ERR(i_dev->clk))
+ clk_disable_unprepare(i_dev->clk);
return 0;
}
@@ -307,7 +310,8 @@ static int dw_i2c_plat_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
- clk_prepare_enable(i_dev->clk);
+ if (!IS_ERR(i_dev->clk))
+ clk_prepare_enable(i_dev->clk);
if (!i_dev->pm_runtime_disabled)
i2c_dw_init(i_dev);
--
2.5.0
--
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] i2c: designware: Do not require clock when SSCN and FFCN are provided
Hi Mika, On 12/16/2015 03:42 AM, Mika Westerberg wrote: +Jarkko and Andy On Tue, Dec 15, 2015 at 04:38:58PM -0600, Suravee Suthikulpanit wrote: The current driver uses input clock source frequency to calculate values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we do not currently have a good way to provide the frequency information. Instead, we can leverage the SSCN and FFCN ACPI methods, which can be used to directly provide these values. So, this patch removes the clock requirement when SSCN and FFCN are provided. Actually I think the only thing you need to change is i2c_dw_init() so that it does not call dev->get_clk_rate_khz(dev) if *CNT values are already provided. Please see below. The clk framework should work fine if the returned clock is NULL (which I think is your case). The driver gates clocks when the device is suspended and on Intel LPSS there actually is a clock that gets gated. [..] @@ -203,13 +223,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE | DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST; - dev->clk = devm_clk_get(>dev, NULL); - dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz; - if (IS_ERR(dev->clk)) - return PTR_ERR(dev->clk); Actually, if we don't provide the clock (which is the case for ACPI), this would also return and cause the probing to fail. Thanks, Suravee -- 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] i2c: designware: Add support for AMD Seattle I2C
On 12/16/2015 03:16 AM, Mika Westerberg wrote:
On Tue, Dec 15, 2015 at 08:14:34PM -0600, Suravee Suthikulpanit wrote:
Hi Mika,
On 12/15/15 15:55, Suravee Suthikulpanit wrote:
Add device HID AMDI0510 to match the I2C controlers on AMD Seattle platform
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 57f623b..a027154 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -117,6 +117,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "80860F41", 0 },
{ "808622C1", 0 },
{ "AMD0010", 0 },
+ { "AMDI0510", 0 },
{ }
Since this driver seems to be used by several SOCs, and we have been adding
the HID from various SOC vendors. Do you think it would be better to assign
a CID so that each SOC vendor can specify in their ACPI DSDT and we can
match them here?
Sure _CID would work here.
Do you know if Synopsys has already provided a CID that we can use for
this? If not, who do you think should provide this? Also, do you think
the FMCN and SSCN should be documented somewhere in the spec so that FW
and OSes can agree upon going forward?
Then, we can also associate the FMCN and SSCN along with the CID, and
guarantee compatibility.
Well, the driver checks those everytime it finds that the device has
ACPI companion regardless of _HID/_CID.
Not sure what you mean by "device has ACPI companion". Do you mean the
driver check those for every matched devices here?
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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] i2c: designware: Do not require clock when SSCN and FFCN are provided
On 12/16/2015 08:28 AM, Mika Westerberg wrote: On Wed, Dec 16, 2015 at 08:11:12AM -0600, Suravee Suthikulpanit wrote: The clk framework should work fine if the returned clock is NULL (which I think is your case). The driver gates clocks when the device is suspended and on Intel LPSS there actually is a clock that gets gated. [..] @@ -203,13 +223,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE | DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST; - dev->clk = devm_clk_get(>dev, NULL); - dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz; - if (IS_ERR(dev->clk)) - return PTR_ERR(dev->clk); Actually, if we don't provide the clock (which is the case for ACPI), this would also return and cause the probing to fail. Indeed it seems that when you have CONFIG_COMMON_CLK selected the clock framework starts returning errors if the clock is not found. Since we need the clock for Intel LPSS I2C host controllers (and they may have *CNT methods), I think you just need to provide the clock for AMD I2C host controller in similar way than we do in drivers/acpi/acpi_lpss.c. I am trying to avoid having to hard-coded clock frequency value in the driver. Would it be alright to not return w/ error, and just do the following? dev->clk = devm_clk_get(>dev, NULL); if (!IS_ERR(dev->clk)) dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz; This should work for the Intel case when clock is also provided. Thanks, Suravee -- 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
[PATCH] i2c: designware: Do not require clock when SSCN and FFCN are provided
The current driver uses input clock source frequency to calculate
values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we do not
currently have a good way to provide the frequency information.
Instead, we can leverage the SSCN and FFCN ACPI methods, which can be used
to directly provide these values.
So, this patch removes the clock requirement when SSCN and FFCN
are provided.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
Note: This has been tested on AMD Seattle RevB for both DT and ACPI.
drivers/i2c/busses/i2c-designware-core.c| 5 +++--
drivers/i2c/busses/i2c-designware-platdrv.c | 30 +++--
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 8c48b27..ec458ec 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -281,7 +281,7 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool
enable)
*/
int i2c_dw_init(struct dw_i2c_dev *dev)
{
- u32 input_clock_khz;
+ u32 input_clock_khz = 0;
u32 hcnt, lcnt;
u32 reg;
u32 sda_falling_time, scl_falling_time;
@@ -295,7 +295,8 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
}
}
- input_clock_khz = dev->get_clk_rate_khz(dev);
+ if (dev->get_clk_rate_khz)
+ input_clock_khz = dev->get_clk_rate_khz(dev);
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579e..57f623b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -127,6 +127,26 @@ static inline int dw_i2c_acpi_configure(struct
platform_device *pdev)
}
#endif
+static int dw_i2c_configure_clk(struct platform_device *pdev,
+ struct dw_i2c_dev *dev)
+{
+ /* For ACPI, if SSCN and FMCN is provided, we should not
+* need the clock value.
+*/
+ if (has_acpi_companion(>dev) &&
+ dev->ss_hcnt && dev->ss_lcnt &&
+ dev->fs_hcnt && dev->fs_lcnt)
+ return 0;
+
+ dev->clk = devm_clk_get(>dev, NULL);
+ dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
+ if (IS_ERR(dev->clk))
+ return PTR_ERR(dev->clk);
+ clk_prepare_enable(dev->clk);
+
+ return 0;
+}
+
static int dw_i2c_plat_probe(struct platform_device *pdev)
{
struct dw_i2c_dev *dev;
@@ -203,13 +223,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
- dev->clk = devm_clk_get(>dev, NULL);
- dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
- if (IS_ERR(dev->clk))
- return PTR_ERR(dev->clk);
- clk_prepare_enable(dev->clk);
+ r = dw_i2c_configure_clk(pdev, dev);
+ if (r)
+ return r;
- if (!dev->sda_hold_time && ht) {
+ if (!dev->sda_hold_time && ht && dev->get_clk_rate_khz) {
u32 ic_clk = dev->get_clk_rate_khz(dev);
dev->sda_hold_time = div_u64((u64)ic_clk * ht + 50,
--
2.5.0
--
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
[PATCH] i2c: designware: Add support for AMD Seattle I2C
Add device HID AMDI0510 to match the I2C controlers on AMD Seattle platform
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 57f623b..a027154 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -117,6 +117,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "80860F41", 0 },
{ "808622C1", 0 },
{ "AMD0010", 0 },
+ { "AMDI0510", 0 },
{ }
};
MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
--
2.5.0
--
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] i2c: designware: Add support for AMD Seattle I2C
Hi Mika,
On 12/15/15 15:55, Suravee Suthikulpanit wrote:
Add device HID AMDI0510 to match the I2C controlers on AMD Seattle platform
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 57f623b..a027154 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -117,6 +117,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "80860F41", 0 },
{ "808622C1", 0 },
{ "AMD0010", 0 },
+ { "AMDI0510", 0 },
{ }
Since this driver seems to be used by several SOCs, and we have been
adding the HID from various SOC vendors. Do you think it would be better
to assign a CID so that each SOC vendor can specify in their ACPI DSDT
and we can match them here?
Then, we can also associate the FMCN and SSCN along with the CID, and
guarantee compatibility.
Thanks,
Suravee
};
MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
--
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 v2 0/2] i2c:dw: Add APM X-Gene ACPI I2C device support
On 12/15/2015 07:27 AM, Mika Westerberg wrote: On Mon, Dec 14, 2015 at 06:53:25PM -0600, Suravee Suthikulanit wrote: >I'm not sure if this has been discussed earlier. But after looking at the >the acpi_apd driver, all we need is just the platform-specific input clock >frequency value used by the drivers/i2c/busses/i2c-designware-core.c: >i2c_dw_init() to calculate the values to program into the DW_IC_SS_SCL_HCNT >and DW_IC_SS_SCL_LCNT registers. There is a way to pass *CNT values already from ACPI to the driver -- It looks for method called FMCN (or SSCN) and retrieves the values from there if found. Right, I also noticed this afterward. By the way, are FMCN and SSCN documented anywhere in the ACPI spec? I am trying to figure out how to update the ACPI table to add this information for the AMD Seattle (ARM64) platform, and I will also submit a patch to add the new HID for this driver. The driver could be modified not to require clock if it already knows *CNT values. Sounds good. Thanks, Suravee -- 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
