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@xxxxxxx> >> --- >> 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
