On Tue, Jan 17, 2023 at 04:01:21PM +0530, Shyam Sundar S K wrote: > On 1/16/2023 3:08 PM, Jarkko Nikula wrote: > > 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. ... > > 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. Maybe you should elaborate this in the commit message and/or the code. -- With Best Regards, Andy Shevchenko
