On Sat, Aug 7, 2010 at 18:36, Mark Brown wrote:
> On 7 Aug 2010, at 23:29, Mike Frysinger wrote:
>>>> +static ssize_t adau1371_dsp_load(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + const char *buf, size_t count)
>>>> +{
>>>> + struct snd_soc_device *socdev = dev_get_drvdata(dev);
>>>> + struct snd_soc_codec *codec = socdev->card->codec;
>>>> + int ret = 0;
>>>> +
>>>> + ret = adau1761_setprogram(codec);
>>>> + if (ret)
>>>> + return ret;
>>>> + else
>>>> + return count;
>>>> +}
>>>> +static DEVICE_ATTR(dsp, 0644, NULL, adau1371_dsp_load);
>>>
>>> This looks redundant - it doesn't offer any opportunity to configure the
>>> firmware to download and you're already (as one one would expect)
>>> automatically downloading the firmware. If this is being exposed at all it
>>> should be via debugfs.
>>
>> SigmaDSP parts have the ability to change firmwares on the fly. this
>> isnt a debug feature, it's somewhat core to these parts. idea being
>> that you can load up different mini signal processing engines based on
>> the current needs.
>
> Right, but see my point about there being no opportunity to configure the
> firmware - if you were allowing the user to select between multiple
> firmwares available then I expect that the value written into the sysfs file
> would be parsed as the name of the new firmware file to load or otherwise
> select a new firmware. With the current code the value written is totally
> ignored and the application layer has to overwrite the firmware in the
> filesystem (or have a custom helper program which does the equivalent
> thereof) so as far as the kernel is concerned it's going to reload the same
> firmware which isn't ideal.
yes, that is the current limitation that'll need to be fixed. not a
big hassle for us to replace the file when doing initial testing ;).
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
