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

From: Andy Shevchenko
Date: Thu Oct 15 2015 - 05:40:34 EST


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.

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?

--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/