Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 18 Mar 2022 11:53:48 +0100
From:   Wolfram Sang <wsa@kernel.org>
To:     Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
Cc:     Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>,
        Jamie Iles <quic_jiles@quicinc.com>,
        Graeme Gregory <quic_ggregory@quicinc.com>,
        linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
Subject: Re: [PATCH v2] i2c: add tracepoints for I2C slave events
Message-ID: <YjRkvPc9uahbozbQ@shikoro>
Mail-Followup-To: Wolfram Sang <wsa@kernel.org>,
        Jae Hyun Yoo <quic_jaehyoo@quicinc.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>, Jamie Iles <quic_jiles@quicinc.com>,
        Graeme Gregory <quic_ggregory@quicinc.com>,
        linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
References: <20220308163333.3985974-1-quic_jaehyoo@quicinc.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="rqHRRJko+wIyNI/C"
Content-Disposition: inline
In-Reply-To: <20220308163333.3985974-1-quic_jaehyoo@quicinc.com>
Precedence: bulk


--rqHRRJko+wIyNI/C
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

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


--rqHRRJko+wIyNI/C
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEOZGx6rniZ1Gk92RdFA3kzBSgKbYFAmI0ZLwACgkQFA3kzBSg
KbaU0xAAiJLhpuUxibhFyVl14QQR4Qh1XvhM6rcLT6jWqrp0cGOWeIg0aTXm+NPV
FY20MyoM8Upmgg1gr1R4wh0M2ShDau2IOHhMEQtAvXwvUtkmAUxdtvIOI6difKse
m8JbV4AfiWHQTvkcwr0zlbJg58tqGUmPiBzvXsTARthrCida24i9uLFQWFHoqBGX
E9463fdGmdzbuJojfRlQ601LPNTqwNpFNiZpg97J/CY+L8iVKQqvMfYWQ78d3Cze
g57NqWIMZAyCQSxhgdjBBsW87bbD/Qo5eQAmD/M6E96sClo+ZcYEtFqPiE0cPkMm
C+x3e4jW9nxS9YHcYbfGb0bXOjJ6VhQMNDd64f3uEvghI12P9NAqjXnQ5VgoXw8Z
99GD2NlJkoHPK4qvbj+aaf4QEwYctldE9NGtpMFWzgowgUu3vIZyWfcWDUIAXyk7
4+F9xmjcOqpUAR46o2w8POd5AfhI3WU5DsR/7nSRKqrkapYD2oCYq8k0Lvj1ZskT
1CwJSNZ1jvS2ipsFLVsvRDFJUrPSN/iu2Xs903v+bOSnCLj3HBMqbkNWY3cXPXpq
WrnKdVBXFb7/uWb3pysgYutrAIRUpdZgAmzE3SyzbnBudpNzj8QnZkJuh/ozDGhM
Vq5s+ELhxFx/G1HWeDwkRnucRJJIHuXP7T5XCKVkAggywYbXORk=
=PJ2T
-----END PGP SIGNATURE-----

--rqHRRJko+wIyNI/C--