[PATCH 2/8] arm: orion5x: Add clk support for mv88f5181

Andrew Lunn
2016-08-26T14:24:32Z

On Fri, Aug 26, 2016 at 01:24:18PM +0100, Jamie Lentin wrote:
> On Fri, 26 Aug 2016, LABBE Corentin wrote:
>
> >Hello
> >
> >I have some minor comments below
> >
> >[...]
> >>+
> >>+static u32 __init mv88f5181_get_tclk_freq(void __iomem *sar)
> >>+{
> >>+ u32 opt = (readl(sar) >> SAR_MV88F5181_TCLK_FREQ) &
> >>+ SAR_MV88F5181_TCLK_FREQ_MASK;
> >
> >Checkpatch complain about a missing blank line after declaration.
> >And spliting the read and the & operation will prevent this line breaking
>
> It isn't here, although I'm not arguing the line is missing. Is
> there an extra switch I'm missing?

I agree it is a false positive.

Probably the best action is to report it to the checkpatch people.

> >>+ if (opt == 0)
> >>+ return 133333333;
> >>+ else if (opt == 1)
> >>+ return 150000000;
> >>+ else if (opt == 2)
> >>+ return 166666667;
> >>+ else
> >>+ return 0;
> >>+}
> >
> >Why not using a switch here ?
>
> If you look at this in context[0], this is one of several
> code-as-config declarations, none of which use a switch. IMO it's
> more useful to make the similarity as obvious as possible if this is
> ever refactored.

Agreed. Please leave it as is, and if anybody does care about this,
they can submit a followup patch refactoring all instances.

Andrew