From 2758d0d1ee49b9d2fd78c6b3f96bdbcf98503fb4 Mon Sep 17 00:00:00 2001 From: Shaun Ruffell Date: Thu, 12 Nov 2009 19:22:06 +0000 Subject: voicebus: Fix race when enabling/disabling hardware echocan. This closes a race condition where it was possible for the driver to believe it has enabled the VPMADT032 when in fact, it really has not. This fixes a regression introduced in dahdi-linux 2.2.0. (issue #15724) git-svn-id: http://svn.asterisk.org/svn/dahdi/linux/trunk@7565 a0bf4364-ded3-4de4-8d8a-66a801d63aff --- drivers/dahdi/voicebus/GpakCust.c | 257 ++++++++++++++++++++++++++------------ drivers/dahdi/voicebus/GpakCust.h | 4 +- drivers/dahdi/wctdm24xxp/base.c | 4 +- drivers/dahdi/wcte12xp/base.c | 1 - 4 files changed, 182 insertions(+), 84 deletions(-) (limited to 'drivers') diff --git a/drivers/dahdi/voicebus/GpakCust.c b/drivers/dahdi/voicebus/GpakCust.c index 50f11b2..db2122f 100644 --- a/drivers/dahdi/voicebus/GpakCust.c +++ b/drivers/dahdi/voicebus/GpakCust.c @@ -212,15 +212,31 @@ static int vpmadt032_setreg(struct vpmadt032 *vpm, unsigned int addr, u16 data) return res; } -static int vpmadt032_enable_ec(struct vpmadt032 *vpm, int channel) +struct change_order { + struct list_head node; + unsigned int channel; + struct adt_lec_params params; +}; + +static struct change_order *alloc_change_order(void) +{ + return kzalloc(sizeof(struct change_order), GFP_KERNEL); +} + +static void free_change_order(struct change_order *order) +{ + kfree(order); +} + +static int vpmadt032_enable_ec(struct vpmadt032 *vpm, int channel, + enum adt_companding companding) { int res; GPAK_AlgControlStat_t pstatus; GpakAlgCtrl_t control; - control = (ADT_COMP_ALAW == vpm->desiredecstate[channel].companding) ? - EnableALawSwCompanding : - EnableMuLawSwCompanding; + control = (ADT_COMP_ALAW == companding) ? EnableALawSwCompanding : + EnableMuLawSwCompanding; if (vpm->options.debug & DEBUG_ECHOCAN) { const char *law; @@ -255,6 +271,86 @@ static int vpmadt032_disable_ec(struct vpmadt032 *vpm, int channel) return res; } +static struct change_order *get_next_order(struct vpmadt032 *vpm) +{ + struct change_order *order; + + spin_lock(&vpm->change_list_lock); + if (!list_empty(&vpm->change_list)) { + order = list_entry(vpm->change_list.next, + struct change_order, node); + list_del(&order->node); + } else { + order = NULL; + } + spin_unlock(&vpm->change_list_lock); + + return order; +} + +static int nlp_settings_changed(const struct adt_lec_params *a, + const struct adt_lec_params *b) +{ + return ((a->nlp_type != b->nlp_type) || + (a->nlp_threshold != b->nlp_threshold) || + (a->nlp_max_suppress != b->nlp_max_suppress)); +} + +static int echocan_on(const struct adt_lec_params *new, + const struct adt_lec_params *old) +{ + return ((new->tap_length != old->tap_length) && + (new->tap_length > 0)); +} + +static int echocan_off(const struct adt_lec_params *new, + const struct adt_lec_params *old) +{ + return ((new->tap_length != old->tap_length) && + (0 == new->tap_length)); +} + +static void update_channel_config(struct vpmadt032 *vpm, unsigned int channel, + struct adt_lec_params *desired) +{ + int res; + GPAK_AlgControlStat_t pstatus; + GPAK_ChannelConfigStat_t cstatus; + GPAK_TearDownChanStat_t tstatus; + GpakChannelConfig_t chanconfig; + + if (vpm->options.debug & DEBUG_ECHOCAN) { + printk(KERN_DEBUG "Reconfiguring chan %d for nlp %d, " + "nlp_thresh %d, and max_supp %d\n", channel + 1, + desired->nlp_type, desired->nlp_threshold, + desired->nlp_max_suppress); + } + + vpm->setchanconfig_from_state(vpm, channel, &chanconfig); + + res = gpakTearDownChannel(vpm->dspid, channel, &tstatus); + if (res) + return; + + res = gpakConfigureChannel(vpm->dspid, channel, tdmToTdm, + &chanconfig, &cstatus); + if (res) + return; + + if (!desired->tap_length) { + res = gpakAlgControl(vpm->dspid, channel, + BypassSwCompanding, &pstatus); + if (res) { + printk("Unable to disable sw companding on echo " + "cancellation channel %d (reason %d)\n", + channel, res); + } + gpakAlgControl(vpm->dspid, channel, BypassEcanA, &pstatus); + } + + return; +} + /** * vpmadt032_bh - Changes the echocan parameters on the vpmadt032 module. * @@ -272,87 +368,67 @@ static void vpmadt032_bh(struct work_struct *data) { struct vpmadt032 *vpm = container_of(data, struct vpmadt032, work); #endif - struct adt_lec_params *curstate, *desiredstate; - int channel; - - /* Sweep through all the echo can channels on the VPMADT032 module, - * looking for ones where the desired state does not match the current - * state. - */ - for (channel = 0; channel < vpm->options.channels; channel++) { - GPAK_AlgControlStat_t pstatus; - int res = 1; - curstate = &vpm->curecstate[channel]; - desiredstate = &vpm->desiredecstate[channel]; - - if ((desiredstate->nlp_type != curstate->nlp_type) || - (desiredstate->nlp_threshold != curstate->nlp_threshold) || - (desiredstate->nlp_max_suppress != curstate->nlp_max_suppress)) { - - GPAK_ChannelConfigStat_t cstatus; - GPAK_TearDownChanStat_t tstatus; - GpakChannelConfig_t chanconfig; - - if (vpm->options.debug & DEBUG_ECHOCAN) - printk(KERN_DEBUG "Reconfiguring chan %d for nlp %d, nlp_thresh %d, and max_supp %d\n", channel + 1, vpm->desiredecstate[channel].nlp_type, - desiredstate->nlp_threshold, desiredstate->nlp_max_suppress); - - vpm->setchanconfig_from_state(vpm, channel, &chanconfig); - - res = gpakTearDownChannel(vpm->dspid, channel, &tstatus); - if (res) - goto vpm_bh_out; - - res = gpakConfigureChannel(vpm->dspid, channel, tdmToTdm, &chanconfig, &cstatus); - if (res) - goto vpm_bh_out; - - if (!desiredstate->tap_length) { - res = gpakAlgControl(vpm->dspid, channel, BypassSwCompanding, &pstatus); - if (res) - printk("Unable to disable sw companding on echo cancellation channel %d (reason %d)\n", channel, res); - res = gpakAlgControl(vpm->dspid, channel, BypassEcanA, &pstatus); - } + struct change_order *order; - } else if (desiredstate->tap_length != curstate->tap_length) { - if (desiredstate->tap_length) - res = vpmadt032_enable_ec(vpm, channel); - else - res = vpmadt032_disable_ec(vpm, channel); - } -vpm_bh_out: - if (!res) - *curstate = *desiredstate; + while ((order = get_next_order(vpm))) { + + struct adt_lec_params *old; + struct adt_lec_params *new; + unsigned int channel; + + channel = order->channel; + BUG_ON(channel >= MAX_CHANNELS_PER_SPAN); + old = &vpm->curecstate[channel]; + new = &order->params; + + if (nlp_settings_changed(new, old)) + update_channel_config(vpm, channel, new); + else if (echocan_on(new, old)) + vpmadt032_enable_ec(vpm, channel, new->companding); + else if (echocan_off(new, old)) + vpmadt032_disable_ec(vpm, channel); + + *old = order->params; + free_change_order(order); } - return; } + #include "adt_lec.c" -static void vpmadt032_check_and_schedule_update(struct vpmadt032 *vpm, int channo) -{ - int update; - /* Only update the parameters if the new state of the echo canceller - * is different than the current state. */ - update = memcmp(&vpm->curecstate[channo], - &vpm->desiredecstate[channo], - sizeof(vpm->curecstate[channo])); - if (update && test_bit(VPM150M_ACTIVE, &vpm->control)) { - /* Since updating the parameters can take a bit of time while - * the driver sends messages to the VPMADT032 and waits for - * their responses, we'll push the work of updating the - * parameters to a work queue so the caller can continue to - * proceed with setting up the call. - */ - queue_work(vpm->wq, &vpm->work); +static void vpmadt032_check_and_schedule_update(struct vpmadt032 *vpm, + struct change_order *order) +{ + struct change_order *cur; + struct change_order *n; + + INIT_LIST_HEAD(&order->node); + spin_lock(&vpm->change_list_lock); + list_for_each_entry_safe(cur, n, &vpm->change_list, node) { + if (cur->channel == order->channel) { + list_replace(&cur->node, &order->node); + free_change_order(cur); + break; + } } + if (list_empty(&order->node)) + list_add_tail(&order->node, &vpm->change_list); + spin_unlock(&vpm->change_list_lock); + + queue_work(vpm->wq, &vpm->work); } int vpmadt032_echocan_create(struct vpmadt032 *vpm, int channo, struct dahdi_echocanparams *ecp, struct dahdi_echocanparam *p) { unsigned int ret; + struct change_order *order = alloc_change_order(); + if (!order) + return -ENOMEM; - ret = adt_lec_parse_params(&vpm->desiredecstate[channo], ecp, p); - if (ret) + memcpy(&order->params, &vpm->curecstate[channo], sizeof(order->params)); + ret = adt_lec_parse_params(&order->params, ecp, p); + if (ret) { + free_change_order(order); return ret; + } if (vpm->options.debug & DEBUG_ECHOCAN) printk(KERN_DEBUG "echocan: Channel is %d length %d\n", channo, ecp->tap_length); @@ -360,9 +436,10 @@ int vpmadt032_echocan_create(struct vpmadt032 *vpm, int channo, /* The driver cannot control the number of taps on the VPMADT032 * module. Instead, it uses tap_length to enable or disable the echo * cancellation. */ - vpm->desiredecstate[channo].tap_length = (ecp->tap_length) ? 1 : 0; + order->params.tap_length = (ecp->tap_length) ? 1 : 0; + order->channel = channo; - vpmadt032_check_and_schedule_update(vpm, channo); + vpmadt032_check_and_schedule_update(vpm, order); return 0; } EXPORT_SYMBOL(vpmadt032_echocan_create); @@ -370,15 +447,22 @@ EXPORT_SYMBOL(vpmadt032_echocan_create); void vpmadt032_echocan_free(struct vpmadt032 *vpm, int channo, struct dahdi_echocan_state *ec) { - adt_lec_init_defaults(&vpm->desiredecstate[channo], 0); - vpm->desiredecstate[channo].nlp_type = vpm->options.vpmnlptype; - vpm->desiredecstate[channo].nlp_threshold = vpm->options.vpmnlpthresh; - vpm->desiredecstate[channo].nlp_max_suppress = vpm->options.vpmnlpmaxsupp; + struct change_order *order; + order = alloc_change_order(); + WARN_ON(!order); + if (!order) + return; + + adt_lec_init_defaults(&order->params, 0); + order->params.nlp_type = vpm->options.vpmnlptype; + order->params.nlp_threshold = vpm->options.vpmnlpthresh; + order->params.nlp_max_suppress = vpm->options.vpmnlpmaxsupp; + order->channel = channo; if (vpm->options.debug & DEBUG_ECHOCAN) printk(KERN_DEBUG "echocan: Channel is %d length 0\n", channo); - vpmadt032_check_and_schedule_update(vpm, channo); + vpmadt032_check_and_schedule_update(vpm, order); } EXPORT_SYMBOL(vpmadt032_echocan_free); @@ -410,6 +494,8 @@ vpmadt032_alloc(struct vpmadt032_options *options, const char *board_name) /* Init our vpmadt032 struct */ memcpy(&vpm->options, options, sizeof(*options)); spin_lock_init(&vpm->list_lock); + spin_lock_init(&vpm->change_list_lock); + INIT_LIST_HEAD(&vpm->change_list); INIT_LIST_HEAD(&vpm->free_cmds); INIT_LIST_HEAD(&vpm->pending_cmds); INIT_LIST_HEAD(&vpm->active_cmds); @@ -580,6 +666,7 @@ void vpmadt032_free(struct vpmadt032 *vpm) { unsigned long flags; struct vpmadt032_cmd *cmd; + struct change_order *order; LIST_HEAD(local_list); BUG_ON(!vpm); @@ -600,6 +687,16 @@ void vpmadt032_free(struct vpmadt032 *vpm) kfree(cmd); } + spin_lock(&vpm->change_list_lock); + list_splice(&vpm->change_list, &local_list); + spin_unlock(&vpm->change_list_lock); + + while (!list_empty(&local_list)) { + order = list_entry(local_list.next, struct change_order, node); + list_del(&order->node); + kfree(order); + } + BUG_ON(ifaces[vpm->dspid] != vpm); write_lock(&ifacelock); ifaces[vpm->dspid] = NULL; diff --git a/drivers/dahdi/voicebus/GpakCust.h b/drivers/dahdi/voicebus/GpakCust.h index 9c84df7..1055cb6 100644 --- a/drivers/dahdi/voicebus/GpakCust.h +++ b/drivers/dahdi/voicebus/GpakCust.h @@ -115,8 +115,10 @@ struct vpmadt032 { unsigned long control; unsigned char curpage; unsigned short version; + enum adt_companding companding; struct adt_lec_params curecstate[MAX_CHANNELS_PER_SPAN]; - struct adt_lec_params desiredecstate[MAX_CHANNELS_PER_SPAN]; + spinlock_t change_list_lock; + struct list_head change_list; spinlock_t list_lock; /* Commands that are ready to be used. */ struct list_head free_cmds; diff --git a/drivers/dahdi/wctdm24xxp/base.c b/drivers/dahdi/wctdm24xxp/base.c index 0c555f6..bbfaefc 100644 --- a/drivers/dahdi/wctdm24xxp/base.c +++ b/drivers/dahdi/wctdm24xxp/base.c @@ -312,7 +312,8 @@ void setchanconfig_from_state(struct vpmadt032 *vpm, int channel, GpakChannelCon chanconfig->MuteToneB = Disabled; chanconfig->FaxCngDetB = Disabled; - chanconfig->SoftwareCompand = (vpm->desiredecstate[channel].companding == ADT_COMP_ALAW) ? cmpPCMA : cmpPCMU; + chanconfig->SoftwareCompand = (ADT_COMP_ALAW == vpm->companding) ? + cmpPCMA : cmpPCMU; chanconfig->FrameRate = rate2ms; p = &chanconfig->EcanParametersA; @@ -422,7 +423,6 @@ static int config_vpmadt032(struct vpmadt032 *vpm, struct wctdm *wc) vpm->curecstate[i].nlp_threshold = vpm->options.vpmnlpthresh; vpm->curecstate[i].nlp_max_suppress = vpm->options.vpmnlpmaxsupp; vpm->curecstate[i].companding = (wc->span.deflaw == DAHDI_LAW_MULAW) ? ADT_COMP_ULAW : ADT_COMP_ALAW; - memcpy(&vpm->desiredecstate[i], &vpm->curecstate[i], sizeof(vpm->curecstate[i])); /* set_vpmadt032_chanconfig_from_state(&vpm->curecstate[i], &vpm->options, i, &chanconfig); !!! */ vpm->setchanconfig_from_state(vpm, i, &chanconfig); diff --git a/drivers/dahdi/wcte12xp/base.c b/drivers/dahdi/wcte12xp/base.c index 4da240d..a5f8814 100644 --- a/drivers/dahdi/wcte12xp/base.c +++ b/drivers/dahdi/wcte12xp/base.c @@ -355,7 +355,6 @@ static int config_vpmadt032(struct vpmadt032 *vpm, struct t1 *wc) vpm->curecstate[channel].nlp_threshold = vpm->options.vpmnlpthresh; vpm->curecstate[channel].nlp_max_suppress = vpm->options.vpmnlpmaxsupp; vpm->curecstate[channel].companding = (wc->spantype == TYPE_T1) ? ADT_COMP_ULAW : ADT_COMP_ALAW; - memcpy(&vpm->desiredecstate[channel], &vpm->curecstate[channel], sizeof(vpm->curecstate[channel])); vpm->setchanconfig_from_state(vpm, channel, &chanconfig); if ((res = gpakConfigureChannel(vpm->dspid, channel, tdmToTdm, &chanconfig, &cstatus))) { -- cgit v1.2.3