On Mon, 2018-07-30 at 21:10 +0200, Alexandre Belloni wrote:
> > - dev_dbg(dev->dev, "Fast Mode%s HCNT:LCNT = %d:%d\n",
> > - fp_str, dev->fs_hcnt, dev->fs_lcnt);
> > + if (dev->fp_hcnt && dev->fp_lcnt) {
> > + dev_dbg(dev->dev, "Fast Mode Plus HCNT:LCNT =
> > %d:%d\n",
> > + dev->fs_hcnt, dev->fs_lcnt);
> > + } else {
> > + dev_dbg(dev->dev, "Fast Mode HCNT:LCNT = %d:%d\n",
> > + dev->fs_hcnt, dev->fs_lcnt);
> > + }
>
> As this duplicates strings, I'm pretty sure Wolfram doesn't like that:
> https://events.static.linuxfound.org/sites/events/files/slides/LCJ16-R
> efactor_Strings-WSang_0.pdf
The problem here is that Fast mode and Fast mode Plus are different
modes even they are sharing same fields in the structure to keep
timings. When I read the code I stumbled over this, since I need to
parse more code to get into.
If to be picky, duplication has been already there since in the same
function we print mode twice. So, it might be better to just keep them
like in separate array
enum modes_num {
...,
}
static const char * const modes_str[] = {
[...] = ...,
};
And use accordingly, but it would be better done on core level, I
suppose.
P.S. Anyway, I have no hard feelings if this patch would be dropped.
> > - mode_str = "Fast Mode";
> > + if (dev->fp_hcnt && dev->fp_lcnt)
> > + mode_str = "Fast Mode Plus";
> > + else
> > + mode_str = "Fast Mode";
> > + break;
> > }
> > - dev_dbg(dev->dev, "Bus speed: %s%s\n", mode_str, fp_str);
> > + dev_dbg(dev->dev, "Bus speed: %s\n", mode_str);
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
