Re: [PATCH 2/2] i2c: designware: enable High-speed mode for pcidrv

[Xiang Wang <wangxfdu@gmail.com>]

2015-10-15 17:40 GMT+08:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> On Thu, 2015-10-15 at 11:32 +0300, Jarkko Nikula wrote:
>> On 10/15/2015 08:46 AM, Xiang Wang wrote:
>> >
>> > In conclusion, we have 2 solutions to set the i2c controller speed
>> > mode (pci driver):
>> > 1) use hardcode value in pci driver
>> > 2) use frequency setting of "i2c device" in ACPI table (more
>> > flexible,
>> > but looks a bit strange)
>> >
>> > Do you have any preference/suggestions for above solutions? Thanks
>>
>> I don't think we can hard code especially the high-speed mode because
>>
>> most typically buses are populated with slower devices.
>>
>> Things are a bit more clear when ACPI provides timing parameters for
>> the
>> bus (for standard and fast speed modes at the moment in
>> i2c-designware-platdrv.c: dw_i2c_acpi_configure()) but still I think
>> the
>> ACPI namespace walk may be needed against potential BIOS
>> misconfigurations. For instance if it provides timing parameters for
>> all
>> speeds but there are devices with lower speed on the same bus.
>>
>> I'd take these timing parameters as configuration data for bus
>> features
>> but actual speed (speed bits in IC_CON register) is defined
>> separately.
>> To me it looks only way to achieve that is to pick slowest device
>> from
>> I2cSerialBus resource descriptors.
>
> Should it (ACPI walk) be done in PCI case as well? If so, then it needs
> to be done up to i2c-core. There you may adjust the bus speed whenever
> slave device is enumerated.
>
I think the "ACPI walk for bus speed" also works for PCI case. It'll
be good to do this in i2c-core.
By doing this we can get a minimum device speed for this bus. I2C bus
drivers can use this speed to set corresponding mode into i2c
controller.
Waiting for comments from others.

> For PCI case you have still to have hardcoded values and it should be
> maximum supported by the bus I think. When you have implemented above
> algorithm you may do this safely. Am I missing something?
Agree. It'll be safer to set a hardcoded maximum supported speed of
the bus for PCI case.
I2C bus driver can use MIN(speed_get_by_ACPI_walk,
hardcoded_max_supported_speed) for bus speed.

>
> --
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy



-- 
Regards,
Xiang