On Fri, Apr 24, 2020 at 02:39:54PM +0000, Florinel Iordache wrote:
> > > +/* Backplane custom logging */
> > > +#define bpdev_fn(fn) \
> > > +void bpdev_##fn(struct phy_device *phydev, char *fmt, ...) \
> > > +{ \
> > > + struct va_format vaf = { \
> > > + .fmt = fmt, \
> > > + }; \
> > > + va_list args; \
> > > + va_start(args, fmt); \
> > > + vaf.va = &args; \
> > > + if (!phydev->attached_dev) \
> > > + dev_##fn(&phydev->mdio.dev, "%pV", &vaf); \
> > > + else \
> > > + dev_##fn(&phydev->mdio.dev, "%s: %pV", \
> > > + netdev_name(phydev->attached_dev), &vaf); \
> > > + va_end(args); \
> > > +}
> > > +
> > > +bpdev_fn(err)
> > > +EXPORT_SYMBOL(bpdev_err);
> > > +
> > > +bpdev_fn(warn)
> > > +EXPORT_SYMBOL(bpdev_warn);
> > > +
> > > +bpdev_fn(info)
> > > +EXPORT_SYMBOL(bpdev_info);
> > > +
> > > +bpdev_fn(dbg)
> > > +EXPORT_SYMBOL(bpdev_dbg);
> >
> > Didn't i say something about just using phydev_{err|warn|info|dbg}?
> >
> > Andrew
>
> Hi Andrew,
>
> I used this custom logging in order to be able to add any kind of useful information we might need to all prints (err/warn/info/dbg).
> For example all these bpdev_ functions are equivalent with phydev_ but only in the case when there is no attached device: phydev->attached_dev == NULL.
> Otherwise, if there is a device attached, then we also want to add its name to all these prints in order to know to which device the information refers to.
> For example in this case the print looks like this:
> [ 50.853515] backplane_qoriq 8c13000:00: eth1: 10gbase-kr link trained, Tx equalization: C(-1)=0x0, C(0)=0x29, C(+1)=0x5
> This is very useful because we can see very easy to which interface the information printed is related to: in this case the link was trained for interface: eth1
> This information (the name of attached device: eth1) is not printed by phydev_ functions.
> I'm sorry I have not explained all this earlier, the first time when you asked about it.
So why not argue that the phydev_* functions should be extended to
include this information? Is this extra information only valuable for
link training, or for anything a PHY does? If the core does not do
something, fix the core, rather than work around it in your driver.
Andrew
