linux-kernel - Re: [Device-drivers-devel] [PATCH] Add driver for Analog Devices ADAU1701 SigmaDSP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 7 Mar 2011 07:29:12 -0500
From:	Mike Frysinger <vapier.adi@...il.com>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	cliff.cai@...log.com, device-drivers-devel@...ckfin.uclinux.org,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	alsa-devel@...a-project.org, lrg@...mlogic.co.uk
Subject: Re: [Device-drivers-devel] [PATCH] Add driver for Analog Devices
 ADAU1701 SigmaDSP

On Mon, Mar 7, 2011 at 07:15, Mark Brown wrote:
> On Mon, Mar 07, 2011 at 06:50:15AM -0500, Mike Frysinger wrote:
>> On Mon, Mar 7, 2011 at 06:41, Mark Brown wrote:
>> > I can't apply this driver until it's merged into ASoC via some means
>>
>> and andrew has been holding off on merging the firmware driver until
>> something in mainline needs it.  once a driver gets to the point where
>> you're fine with merging it, we can bug akpm to push the sigma
>> firmware loader to linus.
>
> It'd seem easier to just merge the two patches together rather than
> trying to deal with cross-tree issues.

that sounds like a terrible idea.  they're logically different
changesets and we shouldnt be squashing them together simply to work
around problems with process workflows.

>> > More generally this doesn't seem like a great interface - apparently the
>> > device requries that firmware be loaded but the whole firmware load
>> > process isn't at all joined up with the driver code meaning that the
>> > application layer has to figure out when firmware needs to be reloaded,
>> > there's no understanding on the part of the driver of what the firmware
>> > on the device is doing or how it's glued into the audio subsystem.
>
>> the API should allow for userspace to specify the firmware name, but
>> that's about it.  it is up to the userspace application to arbitrarily
>> load different firmware files on the fly into the codec without the
>> codec caring what's going into it.  often times the firmware is DSP
>> code to do different post processing on the audio stream (cleanup or
>> whatever).
>
> Right, and this isn't a particularly unusual requirement especially if
> the driver isn't going to have any ability to interact with the DSP (at
> which point it's just coefficient data, the fact that it's a DSP program
> is uninteresting).  The problem is that this isn't a great interface for
> doing that.

i dont see suggestions of a better interface anywhere ...

> At a really basic level the code is not at all integrated with either
> power management or stream handling in ASoC meaning that the user could
> try to download firmware in the middle of an active audio stream (which
> isn't going to be ideal).  Obviously the driver doesn't really support
> any kind of power management at the minute but if it were improved in
> this area you'd also have issues with trying to download firmware while
> the device is off which probably isn't going to work either.

wrt pm, that's a trivial programming issue that would be resolved in
the driver.  userspace need not care.

wrt stream flows, all the customers we've talked to are fine with this
-- having stream control be an application issue.  their application
is driving the codec directly and knows when it needs to change the
firmware, so it pauses its alsa flow, loads the new firmware, and
moves on.

that said, i dont see anything in asoc today to address this, so if
we're simply missing it, please highlight it for us.

> It's also not an interface which offers any kind of discoverability or
> enumerability, meaning that it's not suitable for integration into
> application layer code such as the use case manager.  Applications need
> to be hard coded to know that a particular magic sysfs file needs to be
> poked.  This would be a big step backwards in terms of the ability to
> run off the shelf application software.

i dont see the issue here.  the firmware is *optional* and does not
impair basic audio output.  further, the firmware is fully
written/compiled/maintained by the end customer, just like the
application.  which means there is no "magic" here -- the end customer
is the wizard.

there is no "stock" firmware that ADI or anyone provides for any of
these SigmaDSP audio codecs.
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/