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~
