[v2] net/phy: add trace events for mdio accesses

Make it possible to generate trace events for mdio read and write accesses.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Changes since (implicit) v1:

 - make use of TRACE_EVENT_CONDITION

Alternatively to this patch the condition could be

+	TP_CONDITION(err == 0),

but then we'd need in the read callbacks:

+	trace_mdio_access(bus, 1, addr, regnum, retval, retval < 0 ? retval : 0);

or at least

+	trace_mdio_access(bus, 1, addr, regnum, retval, retval < 0);

which both looks more ugly IMHO.

Best regards
Uwe

 drivers/net/phy/mdio_bus.c  | 11 +++++++++++
 include/trace/events/mdio.h | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 include/trace/events/mdio.h

On Tue, 22 Nov 2016 11:01:27 +0100
Uwe Kleine-König <uwe@kleine-koenig.org> wrote:

> diff --git a/include/trace/events/mdio.h b/include/trace/events/mdio.h
> new file mode 100644
> index 000000000000..468e2d095d19
> --- /dev/null
> +++ b/include/trace/events/mdio.h
> @@ -0,0 +1,42 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM mdio
> +
> +#if !defined(_TRACE_MDIO_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_MDIO_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT_CONDITION(mdio_access,
> +
> +	TP_PROTO(struct mii_bus *bus, int read,
> +		 unsigned addr, unsigned regnum, u16 val, int err),
> +
> +	TP_ARGS(bus, read, addr, regnum, val, err),
> +
> +	TP_CONDITION(err >= 0),
> +
> +	TP_STRUCT__entry(
> +		__array(char, busid, MII_BUS_ID_SIZE)
> +		__field(int, read)

read is just a 0 or 1. What about making it a char? That way we can
pack this better. If I'm not mistaken, MII_BUS_ID_SIZE is (20 - 3) or
17. If read is just one byte, then it can fit in one of those three
bytes, and you save 4 extra bytes (assuming addr will be 4 byte
aligned).

-- Steve


> +		__field(unsigned, addr)
> +		__field(unsigned, regnum)
> +		__field(u16, val)
> +	),
> +
> +	TP_fast_assign(
> +		strncpy(__entry->busid, bus->id, MII_BUS_ID_SIZE);
> +		__entry->read = read;
> +		__entry->addr = addr;
> +		__entry->regnum = regnum;
> +		__entry->val = val;
> +	),
> +
> +	TP_printk("%s %-5s phy:0x%02x reg:0x%02x val:0x%04hx",
> +		  __entry->busid, __entry->read ? "read" : "write",
> +		  __entry->addr, __entry->regnum, __entry->val)
> +);
> +
> +#endif /* if !defined(_TRACE_MDIO_H) || defined(TRACE_HEADER_MULTI_READ) */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>

On Tue, Nov 22, 2016 at 09:55:21AM -0500, Steven Rostedt wrote:
> On Tue, 22 Nov 2016 11:01:27 +0100
> Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> 
> > diff --git a/include/trace/events/mdio.h b/include/trace/events/mdio.h
> > new file mode 100644
> > index 000000000000..468e2d095d19
> > --- /dev/null
> > +++ b/include/trace/events/mdio.h
> > @@ -0,0 +1,42 @@
> > +#undef TRACE_SYSTEM
> > +#define TRACE_SYSTEM mdio
> > +
> > +#if !defined(_TRACE_MDIO_H) || defined(TRACE_HEADER_MULTI_READ)
> > +#define _TRACE_MDIO_H
> > +
> > +#include <linux/tracepoint.h>
> > +
> > +TRACE_EVENT_CONDITION(mdio_access,
> > +
> > +	TP_PROTO(struct mii_bus *bus, int read,
> > +		 unsigned addr, unsigned regnum, u16 val, int err),
> > +
> > +	TP_ARGS(bus, read, addr, regnum, val, err),
> > +
> > +	TP_CONDITION(err >= 0),
> > +
> > +	TP_STRUCT__entry(
> > +		__array(char, busid, MII_BUS_ID_SIZE)
> > +		__field(int, read)
> 
> read is just a 0 or 1. What about making it a char? That way we can
> pack this better. If I'm not mistaken, MII_BUS_ID_SIZE is (20 - 3) or
> 17. If read is just one byte, then it can fit in one of those three
> bytes, and you save 4 extra bytes (assuming addr will be 4 byte
> aligned).

addr could also be cast into a u8. There are a maximum of 32
addresses on an MDIO bus. Because of clause 45 MDIO, regnum needs to
remain a u32.

       Andrew