Hi Willem, > -----Original Message----- > From: Willem de Bruijn <[email protected]> > Sent: Saturday, January 30, 2021 7:57 PM > To: Hariprasad Kelam <[email protected]> > Cc: Network Development <[email protected]>; LKML <linux- > [email protected]>; David Miller <[email protected]>; Jakub > Kicinski <[email protected]>; Sunil Kovvuri Goutham > <[email protected]>; Linu Cherian <[email protected]>; > Geethasowjanya Akula <[email protected]>; Jerin Jacob Kollanukkaran > <[email protected]>; Subbaraya Sundeep Bhatta <[email protected]>; > Felix Manlunas <[email protected]>; Christina Jacob > <[email protected]>; Sunil Kovvuri Goutham > <[email protected]> > Subject: [EXT] Re: [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 <[email protected]> > wrote: > > > > Hi Willem, > > > > > -----Original Message----- > > > From: Willem de Bruijn <[email protected]> > > > Sent: Thursday, January 28, 2021 1:50 AM > > > To: Hariprasad Kelam <[email protected]> > > > Cc: Network Development <[email protected]>; LKML <linux- > > > [email protected]>; David Miller <[email protected]>; Jakub > > > Kicinski <[email protected]>; Sunil Kovvuri Goutham > > > <[email protected]>; Linu Cherian <[email protected]>; > > > Geethasowjanya Akula <[email protected]>; Jerin Jacob Kollanukkaran > > > <[email protected]>; Subbaraya Sundeep Bhatta > > > <[email protected]>; Felix Manlunas <[email protected]>; > > > Christina Jacob <[email protected]>; Sunil Kovvuri Goutham > > > <[email protected]> > > > 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 > > > <[email protected]> > > > wrote: > > > > > > > > From: Felix Manlunas <[email protected]> > > > > > > > > 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 <[email protected]> > > > > Signed-off-by: Christina Jacob <[email protected]> > > > > Signed-off-by: Sunil Kovvuri Goutham <[email protected]> > > > > Signed-off-by: Hariprasad Kelam <[email protected]> > > > > > > > > > > +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. Got it . Will fix this in next version.
Thanks, Hariprasad k
