lkml.org 
[lkml]   [2018]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 06/10] ASoC: wcd9335: add basic controls
    On Fri, Jul 27, 2018 at 01:18:02PM +0100, Srinivas Kandagatla wrote:

    > +static const char * const wcd9335_ear_pa_gain_text[] = {
    > + "G_6_DB", "G_4P5_DB", "G_3_DB", "G_1P5_DB",
    > + "G_0_DB", "G_M2P5_DB", "UNDEFINED", "G_M12_DB"
    > +};

    This is fairly clearly a volume control so shouldn't be an enum. I
    don't think we have a standard control that enables you to skip values
    in a regular volume control (we do for enums which you should've used
    rather than having the undefined entry) so you'd have to have custom get
    and put operators but that's better than abusing an enum.

    > +static const char * const rx_hph_mode_mux_text[] = {
    > + "CLS_H_INVALID", "CLS_H_HIFI", "CLS_H_LP", "CLS_AB", "CLS_H_LOHIFI"
    > +};

    Use a SOC_VALUE_ENUM to hide the invalid option from users.

    > + wcd->comp_enabled[comp] = value;
    > + sel = value ? WCD9335_HPH_GAIN_SRC_SEL_COMPANDER :
    > + WCD9335_HPH_GAIN_SRC_SEL_REGISTER;

    Just write normal if statements unless there's a reason not to, it's
    easier to read.
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2018-07-31 19:18    [W:3.890 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site