Re: [PATCH v7 3/7] i2c: mediatek: Dump i2c/dma register when a timeout occurs

[Kewei Xu <kewei.xu@mediatek.com>]

On Sat, 2021-10-02 at 08:37 +0200, Wolfram Sang wrote:
> > @@ -837,6 +839,57 @@ static int mtk_i2c_set_speed(struct mtk_i2c
> > *i2c, unsigned int parent_clk)
> >  	return 0;
> >  }
> > +static void i2c_dump_register(struct mtk_i2c *i2c)
> > +{
> > +	dev_err(i2c->dev, "SLAVE_ADDR: 0x%x, INTR_MASK: 0x%x\n",
> > +		mtk_i2c_readw(i2c, OFFSET_SLAVE_ADDR),
> > +		mtk_i2c_readw(i2c, OFFSET_INTR_MASK));
> 
> I think this is too verbose and should be a debugging only patch not
> really suited for upstream. But if you like it this way, then keep
> the verbosity. However, dev_err is too strong, this really needs to
> be
> dev_dbg. Timeouts can happen on an I2C bus, think about an EEPROM in
> a
> long erase cycle while you want to read it. Perfectly normal.
> 
> 
> >  	if (ret == 0) {
> > -		dev_dbg(i2c->dev, "addr: %x, transfer timeout\n", msgs-
> > >addr);
> > +		dev_err(i2c->dev, "addr: %x, transfer timeout\n", msgs-
> > >addr);
> > +		i2c_dump_register(i2c);
> 
> Needs to stay dev_dbg as well.
> 
> Yes, It is used for debugging,but dump the value of value of the
> register is very important for debugging,so we think it is
> necessary. We will use dev_dbg to replace dev_err in V8. Thanks~