Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Florinel Iordache <florinel.iordache@nxp.com>
To:     Andrew Lunn <andrew@lunn.ch>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "f.fainelli@gmail.com" <f.fainelli@gmail.com>,
        "hkallweit1@gmail.com" <hkallweit1@gmail.com>,
        "linux@armlinux.org.uk" <linux@armlinux.org.uk>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        Leo Li <leoyang.li@nxp.com>,
        "Madalin Bucur (OSS)" <madalin.bucur@oss.nxp.com>,
        Ioana Ciornei <ioana.ciornei@nxp.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Florinel Iordache <florinel.iordache@nxp.com>
Subject: RE: [EXT] Re: Re: [PATCH net-next v2 6/9] net: phy: add backplane kr
 driver support
Thread-Topic: [EXT] Re: Re: [PATCH net-next v2 6/9] net: phy: add backplane kr
 driver support
Thread-Index: AdYaRAr6OuhHlsgtRe2hojICsYdYXAABIF2AAAATOTA=
Date:   Fri, 24 Apr 2020 15:08:55 +0000
Message-ID: <AM0PR04MB54435DFD9870FA66C01E07CEFBD00@AM0PR04MB5443.eurprd04.prod.outlook.com>
References: <AM0PR04MB5443BCFEC71B6903BE6EFE02FBD00@AM0PR04MB5443.eurprd04.prod.outlook.com>
 <20200424145635.GB1088354@lunn.ch>
In-Reply-To: <20200424145635.GB1088354@lunn.ch>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 03aa4ed1-59e5-41c2-311a-08d7e8616882
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Apr 2020 15:08:55.7479
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: OOr4ckC2fh9cu/GZe408/dMh8ZezdzFuqu91XUh14rG/arlX61WzCpqA6rvDQJzpcTCStGV1carC9uoZ6DOOWPwv5+FZDET8ZdfG83jzzsg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB5619
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

 > Caution: EXT Email
>=20
> 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 =3D {                                   =
     \
> > > > +             .fmt =3D fmt,                                        =
     \
> > > > +     };                                                           =
   \
> > > > +     va_list args;                                                =
   \
> > > > +     va_start(args, fmt);                                         =
   \
> > > > +     vaf.va =3D &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 usefu=
l
> 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 =3D=3D NU=
LL.
> > 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 traine=
d, Tx
> equalization: C(-1)=3D0x0, C(0)=3D0x29, C(+1)=3D0x5
> > 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 traine=
d for
> interface: eth1 This information (the name of attached device: eth1) is n=
ot
> printed by phydev_ functions.
> > I'm sorry I have not explained all this earlier, the first time when yo=
u asked
> about it.
>=20
> So why not argue that the phydev_* functions should be extended to includ=
e this
> information? Is this extra information only valuable for link training, o=
r for
> anything a PHY does? If the core does not do something, fix the core, rat=
her
> than work around it in your driver.
>=20
>      Andrew

I think this is a very good idea: I think this extension would be useful fo=
r all PHYs not only for link training.=20
Its purpose is only more user friendly prints and I needed this feature for=
 backplane debugging.
But since I am not responsible for the PHY core, I made this workaround onl=
y in backplane driver.
If this small improvement could be done for all PHYs then I think this woul=
d be an added value from user friendliness perspective.=20
Thank you.
Florin.