All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rojewski, Cezary" <cezary.rojewski@intel.com>
To: "Pierre-Louis Bossart" <pierre-louis.bossart@linux.intel.com>,
	"Łukasz Majczak" <lma@semihalf.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"Guenter Roeck" <groeck@google.com>,
	"Radoslaw Biernacki" <rad@semihalf.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>,
	"Liam Girdwood" <liam.r.girdwood@linux.intel.com>,
	"Mateusz Gorski" <mateusz.gorski@linux.intel.com>,
	"Marcin Wojtas" <mw@semihalf.com>,
	"Alex Levin" <levinale@google.com>
Subject: RE: [PATCH v2] ASoC: Intel: Skylake: Check the kcontrol against NULL
Date: Wed, 20 Jan 2021 16:41:16 +0000	[thread overview]
Message-ID: <371e612ac59c458cad1bafb82ca09c9f@intel.com> (raw)
In-Reply-To: <8c22c415-1807-b673-20e3-bc8d7f4c05b7@linux.intel.com>

On 2021-01-20 5:33 PM, Pierre-Louis Bossart wrote:
> On 1/20/21 9:49 AM, Łukasz Majczak wrote:
>> Hi Pierre,
>>
>> Is there anything more to do to get the ACK for this patch?
> 
> Adding Cezary and Amadeusz for feedback, I can't pretend having any sort 
> of knowledge on the Skylake driver internals and topology usage.
> 

Thanks for the CC, Pierre.

...

>>>>
>>>> diff --git a/sound/soc/intel/skylake/skl-topology.c 
>>>> b/sound/soc/intel/skylake/skl-topology.c
>>>> index ae466cd592922..8f0bfda7096a9 100644
>>>> --- a/sound/soc/intel/skylake/skl-topology.c
>>>> +++ b/sound/soc/intel/skylake/skl-topology.c
>>>> @@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct 
>>>> snd_soc_component *component)
>>>>          int i;
>>>>
>>>>          list_for_each_entry(dobj, &component->dobj_list, list) {
>>>> -               struct snd_kcontrol *kcontrol = dobj->control.kcontrol;
>>>> -               struct soc_enum *se =
>>>> -                       (struct soc_enum *)kcontrol->private_value;
>>>> -               char **texts = dobj->control.dtexts;
>>>> +               struct snd_kcontrol *kcontrol;
>>>> +               struct soc_enum *se;
>>>> +               char **texts;
>>>>                  char chan_text[4];
>>>>
>>>> +               kcontrol = dobj->control.kcontrol;
>>>> +               if (!kcontrol)
>>>> +                       continue;
>>>> +
>>>> +               se = (struct soc_enum *)kcontrol->private_value;
>>>> +               texts = dobj->control.dtexts;
>>>> +
>>>>                  if (dobj->type != SND_SOC_DOBJ_ENUM ||
>>>>                      dobj->control.kcontrol->put !=
>>>>                      skl_tplg_multi_config_set_dmic)

Just checked the history behind this. And must say, I liked Ricardo's
version better. Except for the "= {};" bit which Mark already pointed
out - it should be a separate fix - it's simply more optional

e.g.: 'kcontrol' gets assigned yet 'if' above is not updated accordingly:
s/dobj->control.kcontrol->put/kcontrol->put

Czarek


WARNING: multiple messages have this Message-ID (diff)
From: "Rojewski, Cezary" <cezary.rojewski@intel.com>
To: "Pierre-Louis Bossart" <pierre-louis.bossart@linux.intel.com>,
	"Łukasz Majczak" <lma@semihalf.com>
Cc: "Marcin Wojtas" <mw@semihalf.com>,
	"Liam Girdwood" <liam.r.girdwood@linux.intel.com>,
	"Mateusz Gorski" <mateusz.gorski@linux.intel.com>,
	"Radoslaw Biernacki" <rad@semihalf.com>,
	"Alex Levin" <levinale@google.com>,
	"Guenter Roeck" <groeck@google.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>
Subject: RE: [PATCH v2] ASoC: Intel: Skylake: Check the kcontrol against NULL
Date: Wed, 20 Jan 2021 16:41:16 +0000	[thread overview]
Message-ID: <371e612ac59c458cad1bafb82ca09c9f@intel.com> (raw)
In-Reply-To: <8c22c415-1807-b673-20e3-bc8d7f4c05b7@linux.intel.com>

On 2021-01-20 5:33 PM, Pierre-Louis Bossart wrote:
> On 1/20/21 9:49 AM, Łukasz Majczak wrote:
>> Hi Pierre,
>>
>> Is there anything more to do to get the ACK for this patch?
> 
> Adding Cezary and Amadeusz for feedback, I can't pretend having any sort 
> of knowledge on the Skylake driver internals and topology usage.
> 

Thanks for the CC, Pierre.

...

>>>>
>>>> diff --git a/sound/soc/intel/skylake/skl-topology.c 
>>>> b/sound/soc/intel/skylake/skl-topology.c
>>>> index ae466cd592922..8f0bfda7096a9 100644
>>>> --- a/sound/soc/intel/skylake/skl-topology.c
>>>> +++ b/sound/soc/intel/skylake/skl-topology.c
>>>> @@ -3618,12 +3618,18 @@ static void skl_tplg_complete(struct 
>>>> snd_soc_component *component)
>>>>          int i;
>>>>
>>>>          list_for_each_entry(dobj, &component->dobj_list, list) {
>>>> -               struct snd_kcontrol *kcontrol = dobj->control.kcontrol;
>>>> -               struct soc_enum *se =
>>>> -                       (struct soc_enum *)kcontrol->private_value;
>>>> -               char **texts = dobj->control.dtexts;
>>>> +               struct snd_kcontrol *kcontrol;
>>>> +               struct soc_enum *se;
>>>> +               char **texts;
>>>>                  char chan_text[4];
>>>>
>>>> +               kcontrol = dobj->control.kcontrol;
>>>> +               if (!kcontrol)
>>>> +                       continue;
>>>> +
>>>> +               se = (struct soc_enum *)kcontrol->private_value;
>>>> +               texts = dobj->control.dtexts;
>>>> +
>>>>                  if (dobj->type != SND_SOC_DOBJ_ENUM ||
>>>>                      dobj->control.kcontrol->put !=
>>>>                      skl_tplg_multi_config_set_dmic)

Just checked the history behind this. And must say, I liked Ricardo's
version better. Except for the "= {};" bit which Mark already pointed
out - it should be a separate fix - it's simply more optional

e.g.: 'kcontrol' gets assigned yet 'if' above is not updated accordingly:
s/dobj->control.kcontrol->put/kcontrol->put

Czarek


  reply	other threads:[~2021-01-20 16:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-10 12:14 [PATCH] ASoC: Intel: Skylake: Check the kcontrol against NULL Lukasz Majczak
2020-12-10 12:14 ` Lukasz Majczak
2020-12-10 15:55 ` Gorski, Mateusz
2020-12-10 15:55   ` Gorski, Mateusz
2020-12-10 16:01   ` Guenter Roeck
2020-12-11 15:42 ` Amadeusz Sławiński
2020-12-17 13:04 ` [PATCH v2] " Lukasz Majczak
2020-12-17 13:04   ` Lukasz Majczak
2021-01-12 11:34   ` Łukasz Majczak
2021-01-12 11:34     ` Łukasz Majczak
2021-01-20 15:49     ` Łukasz Majczak
2021-01-20 15:49       ` Łukasz Majczak
2021-01-20 16:33       ` Pierre-Louis Bossart
2021-01-20 16:33         ` Pierre-Louis Bossart
2021-01-20 16:41         ` Rojewski, Cezary [this message]
2021-01-20 16:41           ` Rojewski, Cezary
2021-01-20 16:45           ` Rojewski, Cezary
2021-01-20 16:45             ` Rojewski, Cezary

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=371e612ac59c458cad1bafb82ca09c9f@intel.com \
    --to=cezary.rojewski@intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=amadeuszx.slawinski@linux.intel.com \
    --cc=groeck@google.com \
    --cc=levinale@google.com \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lma@semihalf.com \
    --cc=mateusz.gorski@linux.intel.com \
    --cc=mw@semihalf.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=rad@semihalf.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.