lkml.org 
[lkml]   [2021]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [Patch v2 net-next 2/7] octeontx2-af: Add new CGX_CMD to get PHY FEC statistics
On Sat, Jan 30, 2021 at 4:53 AM Hariprasad Kelam <hkelam@marvell.com> wrote:
>
> Hi Willem,
>
> > -----Original Message-----
> > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Sent: Thursday, January 28, 2021 1:50 AM
> > To: Hariprasad Kelam <hkelam@marvell.com>
> > Cc: Network Development <netdev@vger.kernel.org>; LKML <linux-
> > kernel@vger.kernel.org>; David Miller <davem@davemloft.net>; Jakub
> > Kicinski <kuba@kernel.org>; Sunil Kovvuri Goutham
> > <sgoutham@marvell.com>; Linu Cherian <lcherian@marvell.com>;
> > Geethasowjanya Akula <gakula@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; Subbaraya Sundeep Bhatta <sbhatta@marvell.com>;
> > Felix Manlunas <fmanlunas@marvell.com>; Christina Jacob
> > <cjacob@marvell.com>; Sunil Kovvuri Goutham
> > <Sunil.Goutham@cavium.com>
> > Subject: [EXT] Re: [Patch v2 net-next 2/7] octeontx2-af: Add new CGX_CMD
> > to get PHY FEC statistics
> >
> > On Wed, Jan 27, 2021 at 4:04 AM Hariprasad Kelam <hkelam@marvell.com>
> > wrote:
> > >
> > > From: Felix Manlunas <fmanlunas@marvell.com>
> > >
> > > This patch adds support to fetch fec stats from PHY. The stats are put
> > > in the shared data struct fwdata. A PHY driver indicates that it has
> > > FEC stats by setting the flag fwdata.phy.misc.has_fec_stats
> > >
> > > Besides CGX_CMD_GET_PHY_FEC_STATS, also add CGX_CMD_PRBS and
> > > CGX_CMD_DISPLAY_EYE to enum cgx_cmd_id so that Linux's enum list is in
> > > sync with firmware's enum list.
> > >
> > > Signed-off-by: Felix Manlunas <fmanlunas@marvell.com>
> > > Signed-off-by: Christina Jacob <cjacob@marvell.com>
> > > Signed-off-by: Sunil Kovvuri Goutham <Sunil.Goutham@cavium.com>
> > > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
> >
> >
> > > +struct phy_s {
> > > + struct {
> > > + u64 can_change_mod_type : 1;
> > > + u64 mod_type : 1;
> > > + u64 has_fec_stats : 1;
> >
> > this style is not customary
>
> These structures are shared with firmware and stored in a shared memory. Any change in size of structures will break compatibility. To avoid frequent compatible issues with new vs old firmware we have put spaces where ever we see that there could be more fields added in future.
> So changing this to u8 can have an impact in future.

My comment was intended much simpler: don't add whitespace between the
bit-field variable name and its size expression.

u64 mod_type:1;

not

u64 mod_type : 1;

At least, I have not seen that style anywhere else in the kernel.

\
 
 \ /
  Last update: 2021-01-30 17:26    [W:0.023 / U:3.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site