On 2017-05-21 10:09, Ard Biesheuvel wrote: > On 19 May 2017 at 09:56, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: >> Commit bd698d24b1b57 ("i2c: designware: Get selected speed mode >> sda-hold-time via ACPI") updated the logic that reads the timing >> parameters for various I2C bus rates from the DSDT, to only read >> the timing parameters for the currently selected mode. >> >> This causes a WARN_ON() splat on platforms that legally omit the clock >> frequency from the ACPI description, because in the new situation, the >> core I2C designware driver still accesses the fields in the driver >> struct that we no longer populate, and proceeds to calculate them from >> the clock frequency. Since the clock frequency is unspecified, the >> driver complains loudly using a WARN_ON(). >> >> So revert back to the old situation, where the struct fields for all >> timings are populated, but retain the new logic which chooses the SDA >> hold time from the timing mode that is currently in use. >> >> Fixes: bd698d24b1b57 ("i2c: designware: Get selected speed mode ...") >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> >> --- >> drivers/i2c/busses/i2c-designware-platdrv.c | 18 ++++++++++-------- >> 1 file changed, 10 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c >> index f2acd4b6bf01..6283b99d2b17 100644 >> --- a/drivers/i2c/busses/i2c-designware-platdrv.c >> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c >> @@ -96,6 +96,7 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) >> struct dw_i2c_dev *dev = platform_get_drvdata(pdev); >> acpi_handle handle = ACPI_HANDLE(&pdev->dev); >> const struct acpi_device_id *id; >> + u32 ss_ht, fp_ht, hs_ht, fs_ht; > > Should these be initialized to zero? I realized that > dw_i2c_acpi_params() could fail, resulting in bogus hold time values > being returnted, but it is unclear to me how that affects the logic in > the core driver. I can tell you what happens: this breaks e.g. the Galileo and the IOT2000 boards. Will send a patch to initialize the vars to 0, restoring the previous behavior in the absence to the ACPI parameters. The core is prepared for sda_hold_time == 0. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux -- 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