linux-kernel - Re: [PATCH v2] i2c: add tracepoints for I2C slave events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YjRkvPc9uahbozbQ@shikoro>
Date:   Fri, 18 Mar 2022 11:53:48 +0100
From:   Wolfram Sang <wsa@...nel.org>
To:     Jae Hyun Yoo <quic_jaehyoo@...cinc.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        Jamie Iles <quic_jiles@...cinc.com>,
        Graeme Gregory <quic_ggregory@...cinc.com>,
        linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org
Subject: Re: [PATCH v2] i2c: add tracepoints for I2C slave events

Hi,

On Tue, Mar 08, 2022 at 08:33:33AM -0800, Jae Hyun Yoo wrote:
> I2C slave events tracepoints can be enabled by:
> 
> 	echo 1 > /sys/kernel/tracing/events/i2c_slave/enable
> 
> and logs in /sys/kernel/tracing/trace will look like:
> 
> 	... i2c_slave: i2c-0 a=010 WR_REQ []
> 	... i2c_slave: i2c-0 a=010 WR_RCV [02]
> 	... i2c_slave: i2c-0 a=010 WR_RCV [0c]
> 	... i2c_slave: i2c-0 a=010   STOP []
> 	... i2c_slave: i2c-0 a=010 RD_REQ [04]
> 	... i2c_slave: i2c-0 a=010 RD_PRO [b4]
> 	... i2c_slave: i2c-0 a=010   STOP []
> 
> formatted as:
> 
> 	i2c-<adapter-nr>
> 	a=<addr>
> 	<event>
> 	[<data>]
> 
> trace printings can be selected by adding a filter like:
> 
> 	echo adapter_nr==1 >/sys/kernel/tracing/events/i2c_slave/filter
> 
> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@...cinc.com>

Steven, are you happy with the tracepoint parts of this patch?


> +	if (trace_i2c_slave_enabled() && !ret)
> +		trace_i2c_slave(client, event, val);

Why '!ret'? I think we should always print 'ret' in the trace as well.
Backends are allowed to send errnos on WRITE_RECEIVED to NACK the
reception of a byte. This is useful information, too, or?

Rest looks good to me.

Thanks,

   Wolfram


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)