[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <461ed34baaf49acda8a3287635f0b4670240cd35.camel@mediatek.com>
Date: Fri, 8 Oct 2021 15:13:21 +0800
From: Kewei Xu <kewei.xu@...iatek.com>
To: Wolfram Sang <wsa@...-dreams.de>
CC: <matthias.bgg@...il.com>, <robh+dt@...nel.org>,
<linux-i2c@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>,
<linux-mediatek@...ts.infradead.org>,
<srv_heupstream@...iatek.com>, <leilk.liu@...iatek.com>,
<qii.wang@...iatek.com>, <liguo.zhang@...iatek.com>,
<caiyu.chen@...iatek.com>, <ot_daolong.zhu@...iatek.com>,
<yuhan.wei@...iatek.com>
Subject: Re: [PATCH v7 3/7] i2c: mediatek: Dump i2c/dma register when a
timeout occurs
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~
Powered by blists - more mailing lists