On 18 May 2017 at 14:05, Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx> wrote: > On 05/18/2017 03:24 PM, Ard Biesheuvel wrote: >> >> On 10 May 2017 at 14:55, Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx> >> wrote: >>> >>> On 05/10/2017 12:24 PM, Lorenzo Pieralisi wrote: >>>> >>>> >>>> On Tue, May 09, 2017 at 07:20:48PM +0300, Andy Shevchenko wrote: >>>>>>> >>>>>>> >>>>>>> It means either ID is not added to drivers/acpi/acpi_apd.c or >>>>> >>>>> >>>>> >>>>> Just one question, did you look at all into above driver? >>>>> I suppose it missed ID and properties for the device. >>>> >>>> >>>> >>>> I looked into it yes. I have also looked at ACPI tables and they >>>> contain the SSCN and FMCN methods and AFAIK the set-up was working >>>> fine before the commit in $SUBJECT - AMD guys - please correct me >>>> if I am wrong, I found this thread which explains things a bit: >>>> >>>> >>>> >>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-December/395983.html >>>> >>>> If we have to patch drivers/acpi/acpi_apd.c to restore the previous >>>> behaviour that's fine by me but this is a regression nonetheless unless >>>> someone explains to me why the current firmware set-up (with SSCN and >>>> FMCN) is unreliable/deprecated, it is not clear to me at all and it >>>> may affect other platforms too. >>>> >>> I guess best would be to fix the i2c_dw_init(). Maybe by moving timing >>> parameters settings out to another function, do setting only for selected >>> speed and calculate only once in case parameters are not set. >>> >>> I don't think timing parameters for standard or fast speeds are not >>> needed >>> unless operating at those speeds but have to check that first. At least >>> that's what we are doing for high speed. >>> >> >> Commit bd698d24b1b57 is broken and should be reverted: the splat on >> AMD Seattle is the most visible symptom, but the logic change results >> in DW_IC_SS_SCL_HCNT/DW_IC_SS_SCL_LCNT possibly being set to different >> values than specified by the ACPI methods. The missing clock splat is >> only a symptom of this: the driver tries to calculate values from the >> clock frequency that should have been read from the DSDT. >> >> For reference, this is the description we have on the platform in >> question: >> >> Method (SSCN, 0, NotSerialized) >> { >> Return (Package (0x03) >> { >> 0x0430, >> 0x04E1, >> 0x00 >> }) >> } >> >> Method (FMCN, 0, NotSerialized) >> { >> Return (Package (0x03) >> { >> 0x00DE, >> 0x018F, >> 0x00 >> }) >> } >> >> Whether it hurts to program different standard mode values here is >> irrelevant. It is simply wrong. >> > I have a fix for this. Could you try does it fix the issue for you? > > http://www.spinics.net/lists/linux-i2c/msg29509.html > Thanks. But the question is really whether we want to change the driver so that the DW_IC_SS_SCL_HCNT/DW_IC_SS_SCL_LCNT registers are no longer programmed at all when running at a higher speed. Can you guarantee that this will not cause any change in behavior in all instances of this hardware that are supported currently, no matter how they are synthesized and described to the OS? The previous commit only affected the platdrv variety AFAIR -- 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
