Re: [PATCH 15/16] ASoC: SOF: Intel: add initial SKL/KBL hardware support

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2022-04-11 10:17 PM, Pierre-Louis Bossart wrote:
> In preparation of the IPCv4 IPC support, this patch adds
> support for SkyLake and KabyLake boot and code loader and descriptors
> used when probing the PCI driver.
> 
> The work was initially contributed in 2018 by Liam Girdwood and Zhu
> Yingjiang, and abandoned due to firmware signature issues. With the
> upcoming support of IPC v4, and hence the Intel closed-source
> firmware, it's time to re-add this capability.
> 
> The SKL ops are left empty at this time since the driver cannot be
> tested with the SOF firmware. The ops will be populated when the IPC4
> is added during the next kernel cycles.
> 
> Tested with the IPC4 and closed-source firmware on Dell XPS 9350
> and KBL NUC with HDaudio codecs. The SSP and DMIC interfaces are not
> supported at this time.
> 
> Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
>   sound/soc/sof/intel/Kconfig          |  24 ++
>   sound/soc/sof/intel/Makefile         |   4 +
>   sound/soc/sof/intel/hda-dsp.c        |   4 +-
>   sound/soc/sof/intel/hda-ipc.c        |   8 +
>   sound/soc/sof/intel/hda-loader-skl.c | 583 +++++++++++++++++++++++++++
>   sound/soc/sof/intel/hda.c            |   2 +
>   sound/soc/sof/intel/hda.h            |  12 +
>   sound/soc/sof/intel/pci-skl.c        |  89 ++++
>   sound/soc/sof/intel/skl.c            |  69 ++++
>   9 files changed, 793 insertions(+), 2 deletions(-)
>   create mode 100644 sound/soc/sof/intel/hda-loader-skl.c
>   create mode 100644 sound/soc/sof/intel/pci-skl.c
>   create mode 100644 sound/soc/sof/intel/skl.c


Regardless if this piece should be or not, let's not hide the 
code-loading implementation behind "initial hardware support". It would 
have been better to split this change into two - one for CL, one for 
"initial hardware support".


Regards,
Czarek