2020-02-11 21:43:26

by H. Nikolaus Schaller

[permalink] [raw]
Subject: [PATCH 01/14] i2c: jz4780: suppress txabrt reports for i2cdetect

This suppresses "simple" error reasons

ABRT_7B_ADDR_NOACK
ABRT_10ADDR1_NOACK
ABRT_10ADDR2_NOACK

from flooding the console log when running i2cdetect on
addresses without a responding slave.

Additionally, reading the JZ4780_I2C_TAR in this situation
seems to harm the controller state.

Signed-off-by: H. Nikolaus Schaller <[email protected]>
---
drivers/i2c/busses/i2c-jz4780.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
index 16a67a64284a..55b7518435f1 100644
--- a/drivers/i2c/busses/i2c-jz4780.c
+++ b/drivers/i2c/busses/i2c-jz4780.c
@@ -578,6 +578,9 @@ static void jz4780_i2c_txabrt(struct jz4780_i2c *i2c, int src)
{
int i;

+ if (!(src & ~7))
+ return;
+
dev_err(&i2c->adap.dev, "txabrt: 0x%08x\n", src);
dev_err(&i2c->adap.dev, "device addr=%x\n",
jz4780_i2c_readw(i2c, JZ4780_I2C_TAR));
--
2.23.0


2020-02-12 09:46:51

by Wolfram Sang

[permalink] [raw]
Subject: i2c: jz4780: silence log flood on txabrt


The printout for txabrt is way too talkative. Reduce it to the minimum,
the rest can be gained by I2C core debugging and datasheet information.
Also, make it a debug printout, it won't help the regular user.

Reported-by: H. Nikolaus Schaller <[email protected]>
Signed-off-by: Wolfram Sang <[email protected]>
---

Sorry, normally I don't do counter patches. Yet, this time I realized
that it would be faster to actually do what I envisioned than to
describe it in words. I hope you don't feel offended. This driver has
way too many dev_err anyhow, so this may be a start.

Obviously, I can't test, does it work for you?

drivers/i2c/busses/i2c-jz4780.c | 36 ++-------------------------------
1 file changed, 2 insertions(+), 34 deletions(-)

diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
index 16a67a64284a..b426fc956938 100644
--- a/drivers/i2c/busses/i2c-jz4780.c
+++ b/drivers/i2c/busses/i2c-jz4780.c
@@ -78,25 +78,6 @@

#define X1000_I2C_DC_STOP BIT(9)

-static const char * const jz4780_i2c_abrt_src[] = {
- "ABRT_7B_ADDR_NOACK",
- "ABRT_10ADDR1_NOACK",
- "ABRT_10ADDR2_NOACK",
- "ABRT_XDATA_NOACK",
- "ABRT_GCALL_NOACK",
- "ABRT_GCALL_READ",
- "ABRT_HS_ACKD",
- "SBYTE_ACKDET",
- "ABRT_HS_NORSTRT",
- "SBYTE_NORSTRT",
- "ABRT_10B_RD_NORSTRT",
- "ABRT_MASTER_DIS",
- "ARB_LOST",
- "SLVFLUSH_TXFIFO",
- "SLV_ARBLOST",
- "SLVRD_INTX",
-};
-
#define JZ4780_I2C_INTST_IGC BIT(11)
#define JZ4780_I2C_INTST_ISTT BIT(10)
#define JZ4780_I2C_INTST_ISTP BIT(9)
@@ -576,21 +557,8 @@ static irqreturn_t jz4780_i2c_irq(int irqno, void *dev_id)

static void jz4780_i2c_txabrt(struct jz4780_i2c *i2c, int src)
{
- int i;
-
- dev_err(&i2c->adap.dev, "txabrt: 0x%08x\n", src);
- dev_err(&i2c->adap.dev, "device addr=%x\n",
- jz4780_i2c_readw(i2c, JZ4780_I2C_TAR));
- dev_err(&i2c->adap.dev, "send cmd count:%d %d\n",
- i2c->cmd, i2c->cmd_buf[i2c->cmd]);
- dev_err(&i2c->adap.dev, "receive data count:%d %d\n",
- i2c->cmd, i2c->data_buf[i2c->cmd]);
-
- for (i = 0; i < 16; i++) {
- if (src & BIT(i))
- dev_dbg(&i2c->adap.dev, "I2C TXABRT[%d]=%s\n",
- i, jz4780_i2c_abrt_src[i]);
- }
+ dev_dbg(&i2c->adap.dev, "txabrt: 0x%08x, cmd: %d, send: %d, recv: %d\n",
+ src, i2c->cmd, i2c->cmd_buf[i2c->cmd], i2c->data_buf[i2c->cmd]);
}

static inline int jz4780_i2c_xfer_read(struct jz4780_i2c *i2c,
--
2.20.1


Attachments:
(No filename) (2.38 kB)
signature.asc (849.00 B)
Download all attachments

2020-02-12 14:47:35

by H. Nikolaus Schaller

[permalink] [raw]
Subject: Re: i2c: jz4780: silence log flood on txabrt

Hi,

> Am 12.02.2020 um 10:46 schrieb Wolfram Sang <[email protected]>:
>
>
> The printout for txabrt is way too talkative. Reduce it to the minimum,
> the rest can be gained by I2C core debugging and datasheet information.
> Also, make it a debug printout, it won't help the regular user.
>
> Reported-by: H. Nikolaus Schaller <[email protected]>
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Sorry, normally I don't do counter patches. Yet, this time I realized
> that it would be faster to actually do what I envisioned than to
> describe it in words. I hope you don't feel offended.

No problem. I had thought a little about that myself, but did not
dare to solve more than my problem...

> This driver has
> way too many dev_err anyhow, so this may be a start.
>
> Obviously, I can't test, does it work for you?

Yes,it works.

Do you want to push your patch yourself, or should I add it to my
patch series and resubmit in a v2?

BR and thanks,
Nikolaus

>
> drivers/i2c/busses/i2c-jz4780.c | 36 ++-------------------------------
> 1 file changed, 2 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
> index 16a67a64284a..b426fc956938 100644
> --- a/drivers/i2c/busses/i2c-jz4780.c
> +++ b/drivers/i2c/busses/i2c-jz4780.c
> @@ -78,25 +78,6 @@
>
> #define X1000_I2C_DC_STOP BIT(9)
>
> -static const char * const jz4780_i2c_abrt_src[] = {
> - "ABRT_7B_ADDR_NOACK",
> - "ABRT_10ADDR1_NOACK",
> - "ABRT_10ADDR2_NOACK",
> - "ABRT_XDATA_NOACK",
> - "ABRT_GCALL_NOACK",
> - "ABRT_GCALL_READ",
> - "ABRT_HS_ACKD",
> - "SBYTE_ACKDET",
> - "ABRT_HS_NORSTRT",
> - "SBYTE_NORSTRT",
> - "ABRT_10B_RD_NORSTRT",
> - "ABRT_MASTER_DIS",
> - "ARB_LOST",
> - "SLVFLUSH_TXFIFO",
> - "SLV_ARBLOST",
> - "SLVRD_INTX",
> -};
> -
> #define JZ4780_I2C_INTST_IGC BIT(11)
> #define JZ4780_I2C_INTST_ISTT BIT(10)
> #define JZ4780_I2C_INTST_ISTP BIT(9)
> @@ -576,21 +557,8 @@ static irqreturn_t jz4780_i2c_irq(int irqno, void *dev_id)
>
> static void jz4780_i2c_txabrt(struct jz4780_i2c *i2c, int src)
> {
> - int i;
> -
> - dev_err(&i2c->adap.dev, "txabrt: 0x%08x\n", src);
> - dev_err(&i2c->adap.dev, "device addr=%x\n",
> - jz4780_i2c_readw(i2c, JZ4780_I2C_TAR));
> - dev_err(&i2c->adap.dev, "send cmd count:%d %d\n",
> - i2c->cmd, i2c->cmd_buf[i2c->cmd]);
> - dev_err(&i2c->adap.dev, "receive data count:%d %d\n",
> - i2c->cmd, i2c->data_buf[i2c->cmd]);
> -
> - for (i = 0; i < 16; i++) {
> - if (src & BIT(i))
> - dev_dbg(&i2c->adap.dev, "I2C TXABRT[%d]=%s\n",
> - i, jz4780_i2c_abrt_src[i]);
> - }
> + dev_dbg(&i2c->adap.dev, "txabrt: 0x%08x, cmd: %d, send: %d, recv: %d\n",
> + src, i2c->cmd, i2c->cmd_buf[i2c->cmd], i2c->data_buf[i2c->cmd]);
> }
>
> static inline int jz4780_i2c_xfer_read(struct jz4780_i2c *i2c,
> --
> 2.20.1
>

2020-02-12 14:54:23

by Wolfram Sang

[permalink] [raw]
Subject: Re: i2c: jz4780: silence log flood on txabrt

Hi,

> > Sorry, normally I don't do counter patches. Yet, this time I realized
> > that it would be faster to actually do what I envisioned than to
> > describe it in words. I hope you don't feel offended.
>
> No problem. I had thought a little about that myself, but did not
> dare to solve more than my problem...

Glad you like it. Well, it still kinda solves your problem only, because
there are still too many dev_err in there, but I think this is good
enough for now.

> > Obviously, I can't test, does it work for you?
>
> Yes,it works.

Good!

> Do you want to push your patch yourself, or should I add it to my
> patch series and resubmit in a v2?

I'll apply the patch to my tree directly as a bugfix for 5.6. You can
drop the I2C list from V2 then.

Thanks and kind regards,

Wolfram


Attachments:
(No filename) (830.00 B)
signature.asc (849.00 B)
Download all attachments

2020-02-12 15:00:30

by H. Nikolaus Schaller

[permalink] [raw]
Subject: Re: i2c: jz4780: silence log flood on txabrt


> Am 12.02.2020 um 15:53 schrieb Wolfram Sang <[email protected]>:
>
> Hi,
>
>>> Sorry, normally I don't do counter patches. Yet, this time I realized
>>> that it would be faster to actually do what I envisioned than to
>>> describe it in words. I hope you don't feel offended.
>>
>> No problem. I had thought a little about that myself, but did not
>> dare to solve more than my problem...
>
> Glad you like it. Well, it still kinda solves your problem only, because
> there are still too many dev_err in there, but I think this is good
> enough for now.
>
>>> Obviously, I can't test, does it work for you?
>>
>> Yes,it works.
>
> Good!
>
>> Do you want to push your patch yourself, or should I add it to my
>> patch series and resubmit in a v2?
>
> I'll apply the patch to my tree directly as a bugfix for 5.6. You can
> drop the I2C list from V2 then.

Ok, fine.

BR and thanks,
Nikolaus

2020-02-13 09:11:22

by Wolfram Sang

[permalink] [raw]
Subject: Re: i2c: jz4780: silence log flood on txabrt

On Wed, Feb 12, 2020 at 10:46:28AM +0100, Wolfram Sang wrote:
>
> The printout for txabrt is way too talkative. Reduce it to the minimum,
> the rest can be gained by I2C core debugging and datasheet information.
> Also, make it a debug printout, it won't help the regular user.
>
> Reported-by: H. Nikolaus Schaller <[email protected]>
> Signed-off-by: Wolfram Sang <[email protected]>

Applied to for-current, thanks!


Attachments:
(No filename) (433.00 B)
signature.asc (849.00 B)
Download all attachments