Re: [PATCH v2 1/4] ASoC: DMIC: Adding the OMAP DMIC driver

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Fri, Jan 07, 2011 at 10:34:53AM -0600, Lambert, David wrote:
> On Thu, Jan 6, 2011 at 4:20 PM, Mark Brown

> > I suggested switch statements previously; you didn't comment on my
> > reply.

> Sorry... my personal standard on when to go with a switch statement is
> >2 choices,
> I'll change it over to a switch...

Think about the impression you're creating here: if someone had a review
comment on one version of a patch and you neither reply nor address it
in the patch they're very likely to have the same comment again.

> >> +     switch (rate) {
> >> +     case 192000:
> >> +             div = 5;
> >> +             break;
> >> +     default:
> >> +             div = 8;

> > Shouldn't the default case be a case 96000?

> The default case IS 96000 (only options for rate here are 96000 and
> 192000), isn't it?

Think about this from a robustness, legibility and maintainability point
of view - the above code doesn't clearly do the right thing, and if any
other sample rates are added it'll be buggy.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html