[alsa-devel] [Sound-open-firmware] [GIT PULL] SOF v1.3 firmware binaries

[Pierre-Louis Bossart]

>>> ----------------------------------------------------------------
>>> Liam Girdwood (1):
>>>         sof: Add Intel SOF V1.3 release firmware binaries.
>>>
>>>    LICENCE.sof                                  | 1090
>>> ++++++++++++++++++++++++++
>>
>> Humm, that LICENSE file needs to be double-checked. Is there any
>> reason
>> why the text of this LICENSE.sof is different the usual text, e.g.
>> from
>> the LICENSE.adsp_sst?
> 
> LICENCE.adsp_sst is for the closed source firmware and LICENCE.sof is
> for SOF. The key difference is the removal of Intel binary FW licence
> and addition of BSD 3c, MIT, ISC and BSD 2c from SOF LICENCE file. Both
> files are the same wrt newlib.

I would kindly ask that you run this by legal.

The BSD license is provided for the source files, it's a stretch to say 
that the license extends to binaries without a statement that says that 
the binary firmware is only made of those source files, unmodified and 
without proprietary extensions. And to the best of my knowledge the 
newlib dependencies do not apply when compiling with XCC.

>> *    No reverse engineering, decompilation, or disassembly of this
>> software is
>>        permitted.
> 
> I'm not following why we need the reverse engineering conditions for
> opensource binaries.

me neither, that's why I stated that the model has to be different from SST.


>> and the DISCLAIMER part, both of which seem pretty important to me.
> 
> Disclaimer is in BSD 3 clause and MIT licence - exact same as the
> sources.

Maybe I am splitting hair, but I'd like a professional opinion on that 
license file. Better safe than sorry. I had GKH tell me on Friday to fix 
a (c) 2017-19 and use (c) 2017-2019...

>>   
>> IANAL, but seeing only a patent clause looks odd. There should be a
>> mention of redistribution and a clear disclaimer (not sure about the
>> reverse engineering parts since the code is available it makes no
>> sense).
> 
> Patent clause is exactly the same as SST FW.
> 
>>
>>>    WHENCE                                       |   33 +
>>>    intel/sof/apl/intel/sof-apl-v1.3.ri          |  Bin 0 -> 270336
>>> bytes
>>>    intel/sof/bdw/sof-bdw-v1.3.ri                |  Bin 0 -> 100144
>>> bytes
>>>    intel/sof/byt/sof-byt-v1.3.ri                |  Bin 0 -> 89668
>>> bytes
>>>    intel/sof/cht/sof-cht-v1.3.ri                |  Bin 0 -> 90484
>>> bytes
>>>    intel/sof/cnl/intel/sof-cnl-v1.3-6cc8da10.ri |  Bin 0 -> 274432
>>> bytes
>>>    intel/sof/icl/intel/sof-icl-v1.3.ri          |  Bin 0 -> 278528
>>> bytes
>>
>> There are two types of platforms, the ones which require the Intel
>> firmware to be signed with a private production key and the ones
>> that
>> are signed with the SOF community key.
>>
>> if we have a single directory, then how do we deal with the two
>> cases?
> 
> I've not yet upstreamed the community signed versions yet so everything
> is in the intel/sof/platform/key/ directory structure.
> 
>> It's not even clear to me which of the two cases are handled here.
>>
> 
> Intel signed binaries, since they are in intel/sof/platform/intel
> directory. Community signed will go in intel/sof/platform/community/
> dir.

I don't understand what you're suggesting nor how it would work with the 
way the kernel deals with directories. I tried to explain that we need 
an intel/sof-production and intel/sof directories, each with generic 
names that are symlinked to another location. This helps if you want to 
build quirks that select one or the other capabilities by just changing 
the firmware directory prefix. Pointers below.

https://github.com/thesofproject/linux/blob/9d7da69a1e85db2cdbbaae78dd7eda4eeaa1eb1c/sound/soc/sof/sof-pci-dev.c#L24

https://github.com/thesofproject/linux/blob/9d7da69a1e85db2cdbbaae78dd7eda4eeaa1eb1c/sound/soc/sof/loader.c#L269

https://github.com/thesofproject/linux/blob/9d7da69a1e85db2cdbbaae78dd7eda4eeaa1eb1c/sound/soc/sof/sof-pci-dev.c#L149

I guess we need to talk off-line since we are evidently not on the same 
page or something is missing for people to use this pull request.

> 
>>>    intel/sof/sof-apl.ri                         |    1 +
>>>    intel/sof/sof-bdw.ri                         |    1 +
>>>    intel/sof/sof-byt.ri                         |    1 +
>>>    intel/sof/sof-cht.ri                         |    1 +
>>>    intel/sof/sof-cml.ri                         |    1 +
>>>    intel/sof/sof-cnl.ri                         |    1 +
>>>    intel/sof/sof-glk.ri                         |    1 +
>>>    intel/sof/sof-icl.ri                         |    1 +
>>>    intel/sof/sof-whl.ri                         |    1 +
>>
>> unless I am missing something, we don't have any tables in the Linux
>> kernel for the WHL and CML configurations, and IIRC we only generate
>> sof-cnl.ri. Is there actually a user for sof-whl.ri and sof-cml.ri?
>>
> 
> There are glk users hence the addition of whl and cml.

glk has nothing to do with whl and cml. Not sure if there is a typo or 
something I missed in your reply?