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

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 1/16/2023 3:08 PM, Jarkko Nikula wrote:
> Hi
> 
> On 1/16/23 06:35, Shyam Sundar S K wrote:
>> On some AMD platforms, based on the new designware datasheet,
>> BIOS sets the BIT(11) within the IC_CON register to advertise
>> the "bus clear feature capability".
>>
>> Since the current driver implementation completely ignores what
>> is advertised by BIOS, we just build the master_cfg and
>> overwrite the entire thing into IC_CON during
>> i2c_dw_configure_master().
>>
>> Since, the bus clear feature is not enabled, sometimes there is
>> no way to reset if the BIT(11) is not set.
>>
>> AMD/Designware datasheet says:
>>
>> Bit(11) BUS_CLEAR_FEATURE_CTRL. Read-write,Volatile. Reset: 0.
>> Description: In Master mode:
>> - 1'b1: Bus Clear Feature is enabled.
>> - 1'b0: Bus Clear Feature is Disabled.
>> In Slave mode, this register bit is not applicable.
>>
>> Hence add a check in i2c_dw_configure_master() that if the BIOS
>> advertises the bus clear feature, let driver not ignore it and
>> adapt accordingly.
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>   drivers/i2c/busses/i2c-designware-core.h   | 1 +
>>   drivers/i2c/busses/i2c-designware-master.c | 5 +++++
>>   2 files changed, 6 insertions(+)
>>
> Is this change alone enough? I understood from the specification that
> the SCL/SDA stuck low timeout registers should be set and a bus recovery
> procedure (additional code) is required.

Double checked with our HW and FW teams, and understand that (atleast in
AMD platform designs):

1. BIOS actually programs the BUS_CLEAR_FEATURE_CTRL and also enables
the detection of SCL/SDA stuck low.
2. Whenever the stuck low is detected, the SMU FW shall do the bus
recovery procedure.

Currently, the way in which the "master_cfg" is built in the driver it
overrides the BUS_CLEAR_FEATURE_CTRL advertised by BIOS and the SMU FW
cannot initiate the bus recovery if the stuck low is detected.

Hence this proposed check should be sufficient enough.

Thanks,
Shyam