Messages in this thread | | | Subject | Re: [PATCH v11 3/4] phy: Add Sparx5 ethernet serdes PHY driver | From | Steen Hegelund <> | Date | Mon, 4 Jan 2021 14:16:38 +0100 |
| |
Hi Leon,
On Mon, 2021-01-04 at 14:15 +0200, Leon Romanovsky wrote: > > <...> > > > +struct sparx5_sd10g28_args { > > + bool skip_cmu_cfg; /* Enable/disable CMU cfg > > */ > > + bool no_pwrcycle; /* Omit initial power- > > cycle */ > > + bool txinvert; /* Enable inversion of > > output data */ > > + bool rxinvert; /* Enable inversion of > > input data */ > > + bool txmargin; /* Set output level to > > half/full */ > > + u16 txswing; /* Set output level */ > > + bool mute; /* Mute Output Buffer */ > > + bool is_6g; > > + bool reg_rst; > > +}; > > All those bools in structs can be squeezed into one u8, just use > bitfields, e.g. "u8 a:1;".
Got you.
> > Also I strongly advise do not do vertical alignment, it will cause to > too many churn later when this code will be updated.
So just a single space between the type and the name and the comment is preferable?
> > > + > > <...> > > > +static inline void __iomem *sdx5_addr(void __iomem *base[], > > + int id, int tinst, int tcnt, > > + int gbase, int ginst, > > + int gcnt, int gwidth, > > + int raddr, int rinst, > > + int rcnt, int rwidth) > > +{ > > +#if defined(CONFIG_DEBUG_KERNEL) > > + WARN_ON((tinst) >= tcnt); > > + WARN_ON((ginst) >= gcnt); > > + WARN_ON((rinst) >= rcnt); > > +#endif > > Please don't put "#if defined(CONFIG_DEBUG_KERNEL)", print WARN_ON().
OK, I will drop the #if and keep the WARN_ON...
> > Thanks
Thank you for your comments.
BR Steen
| |