lkml.org 
[lkml]   [2023]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 2/3] ASoC: starfive: Add JH7110 PWM-DAC driver
    On Mon, Jul 31, 2023 at 11:28:28AM +0800, Hal Feng wrote:

    > +static const struct jh7110_ct_pwmdac pwmdac_ct_data_shift[] = {
    > + { .name = "left 0 bit", .vals = PWMDAC_DATA_LEFT_SHIFT_BIT_0 },
    > + { .name = "left 1 bit", .vals = PWMDAC_DATA_LEFT_SHIFT_BIT_1 },
    > + { .name = "left 2 bit", .vals = PWMDAC_DATA_LEFT_SHIFT_BIT_2 },
    > + { .name = "left 3 bit", .vals = PWMDAC_DATA_LEFT_SHIFT_BIT_3 },
    > + { .name = "left 4 bit", .vals = PWMDAC_DATA_LEFT_SHIFT_BIT_4 },
    > + { .name = "left 5 bit", .vals = PWMDAC_DATA_LEFT_SHIFT_BIT_5 },
    > + { .name = "left 6 bit", .vals = PWMDAC_DATA_LEFT_SHIFT_BIT_6 },
    > + { .name = "left 7 bit", .vals = PWMDAC_DATA_LEFT_SHIFT_BIT_7 }
    > +};

    It's not clear to me why this is user selectable - what does the control
    do?

    > +static int jh7110_pwmdac_put(struct snd_kcontrol *kcontrol,
    > + struct snd_ctl_elem_value *ucontrol,
    > + int pwmdac_ct)
    > +{
    > + struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
    > + struct jh7110_pwmdac_dev *dev = snd_soc_component_get_drvdata(component);
    > + int sel = ucontrol->value.enumerated.item[0];
    > + unsigned int items;

    > + if (pwmdac_ct == PWMDAC_CT_SHIFT)
    > + dev->shift = pwmdac_ct_shift[sel].vals;
    > + else if (pwmdac_ct == PWMDAC_CT_DUTY_CYCLE)
    > + dev->duty_cycle = pwmdac_ct_duty_cycle[sel].vals;
    > + else if (pwmdac_ct == PWMDAC_CT_DATA_CHANGE)
    > + dev->data_change = pwmdac_ct_data_change[sel].vals;
    > + else if (pwmdac_ct == PWMDAC_CT_DATA_MODE)
    > + dev->data_mode = pwmdac_ct_data_mode[sel].vals;
    > + else if (pwmdac_ct == PWMDAC_CT_DATA_SHIFT)
    > + dev->data_shift = pwmdac_ct_data_shift[sel].vals;
    > +
    > + return 0;
    > +}

    _put() operations should return 1 if the control value changes so event
    generation works - please test a card using this driver with the
    mixer-test selftest, it'll identify this and a bunch of other potential
    issues.

    > +static int jh7110_pwmdac_component_probe(struct snd_soc_component *component)
    > +{
    > + snd_soc_add_component_controls(component, jh7110_pwmdac_snd_controls,
    > + ARRAY_SIZE(jh7110_pwmdac_snd_controls));
    > + return 0;
    > +}

    The driver can just point to the controls from the _driver struct and
    skip having a probe() function.

    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2023-07-31 21:15    [W:5.789 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site