On Thu, Aug 14, 2014 at 03:51:56PM +0530, Subhransu S. Prusty wrote:
On Wed, Aug 13, 2014 at 09:14:54PM +0100, Mark Brown wrote:
On Mon, Aug 04, 2014 at 03:16:01PM +0530, Subhransu S. Prusty wrote:
- pr_debug("%s: widget = %s\n", __func__, w->name);
- for (i = 0; i < w->num_kcontrols; i++) {
if (dapm_kcontrol_get_value(w->kcontrols[i])) {mc = (struct soc_mixer_control *)(w->kcontrols[i])->private_value;val |= 1 << mc->shift;}- }
All my concerns about this still stand - using something called _get_value() to do something other than read the control value (which this does - it uses it as a boolean then does something else to read the value) which doesn't seem good.
I think this can be optimized. This need a bit of rework in the code, but need to check. But this would need an assumption that the controls are created in order.
for (i = 0; i < w->num_kcontrols; i++) { if (dapm_kcontrol_get_value(w->kcontrols[i])) { val |= 1 << i; }
Is this ok?
No, that doesn't seem safe. Why not read the data from the control - if we were calling a function which returned the value that'd be much clearer.