Hi, On Mon, Jul 04, 2022 at 11:24:20AM +0530, Shubhrajyoti Datta wrote: > From: Robert Hancock <robert.hancock@xxxxxxxxxx> > > Hook up the standard GPIO/pinctrl-based recovery support.. > In the discurssion > https://patchwork.ozlabs.org/project/linux-i2c/patch/20211129090116.16628-1-shubhrajyoti.datta@xxxxxxxxxx/ > > recovery should be done at the beginning of the transaction. > Here we are doing the recovery at the beginning on a timeout. Which is still wrong. > > Signed-off-by: Robert Hancock <robert.hancock@xxxxxxxxxx> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx> This is an AMD address, but the one you sent from is from Xilinx? > if (time_left == 0) { > + i2c_recover_bus(adap); This is the wrong part. > cdns_i2c_master_reset(adap); > dev_err(id->adap.dev.parent, > "timeout waiting on completion\n"); > @@ -852,8 +858,12 @@ static int cdns_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > #endif > > /* Check if the bus is free */ > - if (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) & CDNS_I2C_SR_BA) { > + ret = readl_poll_timeout(id->membase + CDNS_I2C_SR_OFFSET, reg, > + !(reg & CDNS_I2C_SR_BA), > + CDNS_I2C_POLL_US, CDNS_I2C_TIMEOUT_US); > + if (ret) { > ret = -EAGAIN; > + i2c_recover_bus(adap); > goto out; This is proper. > } > > @@ -1278,6 +1288,7 @@ static int cdns_i2c_probe(struct platform_device *pdev) > id->adap.retries = 3; /* Default retry value. */ > id->adap.algo_data = id; > id->adap.dev.parent = &pdev->dev; > + id->adap.bus_recovery_info = &id->rinfo; Since 'rinfo' is never populated with actual data, I am quite sure this patch has never been tested. Regards, Wolfram
Attachment:
signature.asc
Description: PGP signature