Hi Evan, One minor comment from me below. With that
Reviewed-by: Chris Packham <[email protected]> On 1/09/20 12:57 pm, Evan Nimmo wrote: > If something goes wrong (such as the SCL being stuck low) then we need > to reset the pca chip. The issue with this is that on reset we lose all > config settings and the chip ends up in a disabled state which results > in a lock up/high cpu usage. We need to re-apply any configuration that > had previously been set and re-enable the chip. > > Signed-off-by: Evan Nimmo <[email protected]> > --- > drivers/i2c/algos/i2c-algo-pca.c | 36 +++++++++++++++++++++----------- > include/linux/i2c-algo-pca.h | 15 +++++++++++++ > 2 files changed, 39 insertions(+), 12 deletions(-) > > diff --git a/drivers/i2c/algos/i2c-algo-pca.c > b/drivers/i2c/algos/i2c-algo-pca.c > index 710fbef9a9c2..2e4e27073f40 100644 > --- a/drivers/i2c/algos/i2c-algo-pca.c > +++ b/drivers/i2c/algos/i2c-algo-pca.c > @@ -41,8 +41,22 @@ static void pca_reset(struct i2c_algo_pca_data *adap) > pca_outw(adap, I2C_PCA_INDPTR, I2C_PCA_IPRESET); > pca_outw(adap, I2C_PCA_IND, 0xA5); > pca_outw(adap, I2C_PCA_IND, 0x5A); > + > + /* We need to apply any configuration settings that > + * were calculated in the pca_init function. The reset > + * results in these changes being set back to defaults. > + */ "these changes" doesn't read well. How about /* * After a reset we need to re-apply any configuration (calculated in pca_init) * to get the bus in a working state. */ > + pca_outw(adap, I2C_PCA_INDPTR, I2C_PCA_IMODE); > + pca_outw(adap, I2C_PCA_IND, adap->bus_settings.mode); > + pca_outw(adap, I2C_PCA_INDPTR, I2C_PCA_ISCLL); > + pca_outw(adap, I2C_PCA_IND, adap->bus_settings.tlow); > + pca_outw(adap, I2C_PCA_INDPTR, I2C_PCA_ISCLH); > + pca_outw(adap, I2C_PCA_IND, adap->bus_settings.thi); > + > + pca_set_con(adap, I2C_PCA_CON_ENSIO); > } else { > adap->reset_chip(adap->data); > + pca_set_con(adap, I2C_PCA_CON_ENSIO | > adap->bus_settings.clock_freq); > } > } > > @@ -423,13 +437,15 @@ static int pca_init(struct i2c_adapter *adap) > " Use the nominal frequency.\n", adap->name); > } > > + clock = pca_clock(pca_data); > + > + /* Store settings as these will be needed when the pca chip is > reset */ > + pca_data->bus_settings.clock_freq = clock; > + > pca_reset(pca_data); > > - clock = pca_clock(pca_data); > printk(KERN_INFO "%s: Clock frequency is %dkHz\n", > adap->name, freqs[clock]); > - > - pca_set_con(pca_data, I2C_PCA_CON_ENSIO | clock); > } else { > int clock; > int mode; > @@ -496,19 +512,15 @@ static int pca_init(struct i2c_adapter *adap) > thi = tlow * min_thi / min_tlow; > } > > + /* Store settings as these will be needed when the pca chip is > reset */ > + pca_data->bus_settings.mode = mode; > + pca_data->bus_settings.tlow = tlow; > + pca_data->bus_settings.thi = thi; > + > pca_reset(pca_data); > > printk(KERN_INFO > "%s: Clock frequency is %dHz\n", adap->name, clock * 100); > - > - pca_outw(pca_data, I2C_PCA_INDPTR, I2C_PCA_IMODE); > - pca_outw(pca_data, I2C_PCA_IND, mode); > - pca_outw(pca_data, I2C_PCA_INDPTR, I2C_PCA_ISCLL); > - pca_outw(pca_data, I2C_PCA_IND, tlow); > - pca_outw(pca_data, I2C_PCA_INDPTR, I2C_PCA_ISCLH); > - pca_outw(pca_data, I2C_PCA_IND, thi); > - > - pca_set_con(pca_data, I2C_PCA_CON_ENSIO); > } > udelay(500); /* 500 us for oscillator to stabilise */ > > diff --git a/include/linux/i2c-algo-pca.h b/include/linux/i2c-algo-pca.h > index d03071732db4..ebeadb80c797 100644 > --- a/include/linux/i2c-algo-pca.h > +++ b/include/linux/i2c-algo-pca.h > @@ -53,6 +53,20 @@ > #define I2C_PCA_CON_SI 0x08 /* Serial Interrupt */ > #define I2C_PCA_CON_CR 0x07 /* Clock Rate (MASK) */ > > +/** > + * struct i2c_bus_settings - The configured i2c bus settings > + * @mode: Configured i2c bus mode (PCA9665) > + * @tlow: Configured SCL LOW period (PCA9665) > + * @thi: Configured SCL HIGH period (PCA9665) > + * @clock_freq: The configured clock frequency (PCA9564) > + */ > +struct i2c_bus_settings { > + int mode; > + int tlow; > + int thi; > + int clock_freq; > +}; > + > struct i2c_algo_pca_data { > void *data; /* private low level data */ > void (*write_byte) (void *data, int reg, int val); > @@ -64,6 +78,7 @@ struct i2c_algo_pca_data { > * For PCA9665, use the frequency you want here. */ > unsigned int i2c_clock; > unsigned int chip; > + struct i2c_bus_settings bus_settings; > }; > > int i2c_pca_add_bus(struct i2c_adapter *);
