On 13.02.2017 10:41, Tan, Chin Yew wrote:
On Fri, 2017-02-10 at 19:28 +0800, chin.yew.tan@xxxxxxxxx wrote:
From: Tan Chin Yew <chin.yew.tan@xxxxxxxxx>
Sda-hold-time is an important parameter for tuning i2c to meet the
electrical specification especially for high speed. I2C with incorrect
sda-hold-time may cause lost arbitration error. Now, the driver is
able to get sda-hold-time for all the speed supported.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Couple of nitpicks below.
Signed-off-by: Tan Chin Yew <chin.yew.tan@xxxxxxxxx>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 27
++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 6ce4313..aa33088 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -101,15 +101,28 @@ static int dw_i2c_acpi_configure(struct
platform_device *pdev)
dev->rx_fifo_depth = 32;
/*
- * Try to get SDA hold time and *CNT values from an ACPI
method if
- * it exists for both supported speed modes.
+ * Try to get SDA hold time and *CNT values from an ACPI
method for
+ * selected speed modes.
*/
- dw_i2c_acpi_params(pdev, "SSCN", &dev->ss_hcnt, &dev-
ss_lcnt, NULL);
- dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev-
fs_lcnt,
+ switch (dev->clk_freq) {
+ case 100000:
+ dw_i2c_acpi_params(pdev, "SSCN", &dev->ss_hcnt, &dev-
ss_lcnt,
&dev->sda_hold_time);
This indentation should go in a way that & character in the same column as p
(in "p(s" context above).
I will realign the code according to suggestion.
- dw_i2c_acpi_params(pdev, "FPCN", &dev->fp_hcnt, &dev-
fp_lcnt, NULL);
- dw_i2c_acpi_params(pdev, "HSCN", &dev->hs_hcnt, &dev-
hs_lcnt, NULL);
-
+ break;
+ case 1000000:
+ dw_i2c_acpi_params(pdev, "FPCN", &dev->fp_hcnt, &dev-
fp_lcnt,
+ &dev->sda_hold_time);
+ break;
+ case 3400000:
+ dw_i2c_acpi_params(pdev, "HSCN", &dev->hs_hcnt, &dev-
hs_lcnt,
+ &dev->sda_hold_time);
+ break;
Can we prepend default with
case 400000:
here?
Yes, you are right, it is best not to load settings for speed mode that is
not supported.
Andy: I guess you were looking for adding "case 400000:" for readability
rather than removing the default case?
I think it's best to keep fall back to 400 kHz speed that has been the
default in this driver in case we get some not supported speed from ACPI.
--
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
