[0/4] ASoC: Intel: Haswell: Adjust machine device private

Message

Cezary Rojewski Aug. 22, 2019, 11:36 a.m. UTC

Apart from Haswell machines, all other devices have their private data
set to snd_soc_acpi_mach instance.

Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/

added support for dai_link platform_name adjustments within card probe
routines. These take for granted private_data points to
snd_soc_acpi_mach whereas for Haswell, it's sst_pdata instead. Change
private context of platform_device - representing machine board - to
address this.

Caught by recent cleanups where content of sst_pdata was moved.
Currently, despite the incorrect cast, dereferenced field points happily
to NULL (uninitialized field), so no panics were observed.

Cezary Rojewski (4):
  ASoC: Intel: Haswell: Adjust machine device private context
  ASoC: Intel: haswell: Simplify device probe
  ASoC: Intel: bdw-rt5677: Simplify device probe
  ASoC: Intel: broadwell: Simplify device probe

 sound/soc/intel/boards/bdw-rt5677.c | 6 +-----
 sound/soc/intel/boards/broadwell.c  | 6 +-----
 sound/soc/intel/boards/haswell.c    | 6 +-----
 sound/soc/intel/common/sst-acpi.c   | 3 ++-
 4 files changed, 5 insertions(+), 16 deletions(-)

Comments

Pierre-Louis Bossart Aug. 29, 2019, 10:45 p.m. UTC | #1

On 8/22/19 6:36 AM, Cezary Rojewski wrote:
> Apart from Haswell machines, all other devices have their private data
> set to snd_soc_acpi_mach instance.
> 
> Changes for HSW/ BDW boards introduced with series:
> https://patchwork.kernel.org/cover/10782035/
> 
> added support for dai_link platform_name adjustments within card probe
> routines. These take for granted private_data points to
> snd_soc_acpi_mach whereas for Haswell, it's sst_pdata instead. Change
> private context of platform_device - representing machine board - to
> address this.
> 
> Caught by recent cleanups where content of sst_pdata was moved.
> Currently, despite the incorrect cast, dereferenced field points happily
> to NULL (uninitialized field), so no panics were observed.

No issues seen on Broadwell w/ rt5677 (Samus) so

Tested-by: Pierre-Louis Bossart <[e-mail]>

I think the two broadwell/bdw-rt5677 cases are the only ones used in 
products, I am not aware of anyone using the haswell machine driver.

Sorry for being thick with the review, I couldn't figure out from the 
patch itself how the code worked and what this DRV_NAME represented. 
It's definitively legit when looking at the entire tree, but I 
discovered that the 'DRV_NAME' is defined 3 times, see below. It's just 
lunacy to use the same define with different strings in different Intel 
drivers, we should clean this up with a prefix to avoid ambiguities.

atom/sst-mfld-platform.h:#define DRV_NAME "sst"
baytrail/sst-baytrail-pcm.c:#define DRV_NAME "byt-dai"
haswell/sst-haswell-ipc.h:#define DRV_NAME "haswell-dai"

> 
> Cezary Rojewski (4):
>    ASoC: Intel: Haswell: Adjust machine device private context
>    ASoC: Intel: haswell: Simplify device probe
>    ASoC: Intel: bdw-rt5677: Simplify device probe
>    ASoC: Intel: broadwell: Simplify device probe
> 
>   sound/soc/intel/boards/bdw-rt5677.c | 6 +-----
>   sound/soc/intel/boards/broadwell.c  | 6 +-----
>   sound/soc/intel/boards/haswell.c    | 6 +-----
>   sound/soc/intel/common/sst-acpi.c   | 3 ++-
>   4 files changed, 5 insertions(+), 16 deletions(-)
>