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

From: Andrew Lunn <[email-masked]>
Date: Tue, 22 Nov 2016 16:19:32 +0100

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 <[email-masked]> 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