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
