Re: [PATCH v2] i2c: designware: add a new bit check for IC_CON control

[Jarkko Nikula <jarkko.nikula@linux.intel.com>]

On 1/19/23 05:44, Shyam Sundar S K wrote:
> 
> 
> On 1/18/2023 8:04 PM, Andy Shevchenko wrote:
>> On Wed, Jan 18, 2023 at 07:27:06PM +0530, Shyam Sundar S K wrote:
>>> On 1/17/2023 8:38 PM, Andy Shevchenko wrote:
>>>> On Tue, Jan 17, 2023 at 05:58:01PM +0530, Shyam Sundar S K wrote:
>>> In order to use regmap_read() instead of ioread32() in this case, we
>>> have to defer calling i2c_dw_configure()
>>
Comment to change in your previous mail: Moving i2c_dw_configure() after 
i2c_dw_probe() will cause regression to high speed mode since 
DW_IC_CON_SPEED_HIGH is not set when i2c_dw_set_timings_master() is called.

>> I think we need to try to be consistent with IO accessors across the driver
>> which means to try hard to have regmap being initialised beforehand or other
>> functions being moved accordingly. However, it seems a bit non-trivial
>> ordering case and I leave this to you, I²C maintainers and this driver
>> maintainer to decide how to proceed.
>>
Yeah, we don't want to potentially cause a regression on those machines 
that use swapped or word accessors by reading and writing a wrong bit in 
the DW_IC_CON.

> 
> Jarkko, How would you like me to proceed? Would you be OK to pull this
> change without regmap_read() or do you like me to submit a patch for
> reordering the _configure_* call ?
> 
I think safe place to do this is between i2c_dw_init_regmap() and 
dev->init() calls in i2c_dw_probe_master(). Then regmap is initialized 
and the DW_IC_CON is not yet written.

Not sure would reading DW_IC_CON require 
i2c_dw_acquire_lock()/i2c_dw_release_lock() but I would play safe.