Re: [PATCH -next] staging: greybus: audio_gb.c: Change uint32_t to u32

[Vaibhav Agarwal]

On Sun, Jan 22, 2017 at 11:19 PM, Marcos Paulo de Souza
<marcos.souza@gmail.com> wrote:
> Change uint32_t to u32, solved the issue reported by checkpatch.pl:
>
> CHECK: Prefer kernel type 'u32' over 'uint32_t'
> +   uint32_t *format, uint32_t *rate, u8 *channels,
>
> CHECK: Prefer kernel type 'u32' over 'uint32_t'
> +   uint32_t format, uint32_t rate, u8 channels,
>
> Signed-off-by: Marcos Paulo de Souza <marcos.souza@gmail.com>
> ---

Acked-by: Vaibhav Agarwal <vaibhav...@gmail.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/4] staging: greybus: audio: Avoid less than zero check for le32 variable

[Vaibhav Agarwal]

mixer control->info call back function checks for -ve values to rebase
min and max values. However, le32 variable is used to fetch values from
GB module FW. Thus -ve value checking is not required. Fix this!!

Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_topology.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 3001a4971c06..a8b63a8b2bb0 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -361,8 +361,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct snd_kcontrol 
*kcontrol,
info = (struct gb_audio_ctl_elem_info *)data->info;
 
/* update uinfo */
-   platform_max = info->value.integer.max;
-   platform_min = info->value.integer.min;
+   platform_max = le32_to_cpu(info->value.integer.max);
+   platform_min = le32_to_cpu(info->value.integer.min);
 
if (platform_max == 1 &&
!strnstr(kcontrol->id.name, " Volume", NAME_SIZE))
@@ -371,12 +371,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct snd_kcontrol 
*kcontrol,
uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
 
uinfo->count = data->vcount;
-   uinfo->value.integer.min = 0;
-   if (info->value.integer.min < 0 &&
-   (uinfo->type == SNDRV_CTL_ELEM_TYPE_INTEGER))
-   uinfo->value.integer.max = platform_max - platform_min;
-   else
-   uinfo->value.integer.max = platform_max;
+   uinfo->value.integer.min = platform_min;
+   uinfo->value.integer.max = platform_max;
 
return 0;
 }
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 0/4] Cleanup greybus audio driver

[Vaibhav Agarwal]

This patch series include following cleanup changes in GB Audio driver.
- Avoid unnecessary checks for le32 variables
- Initialize sig_bits before configuring hw_params
- Remove junk codec register mapping
- Ensure proper byte ordering

Next task is to enable build for GB codec driver. However this requires
pushing some changes in ASoC framework. Possibly in another two weeks
(based on my freetime), I'll try to submit those changes to ASoC mailing
list. And once the same are accepted there, I'll share relevant patches
for GB Audio codec driver as well.

Changes from v1:
- Include review comments from Dan


Vaibhav Agarwal (4):
  staging: greybus: audio: Avoid less than zero check for le32 variable
  staging: greybus: audio: Initialize sig_bits before configuring
hwparams
  staging: greybus: audio: Cleanup junk codec registers
  staging: greybus: audio: Ensure proper byte order

 drivers/staging/greybus/audio_codec.c|  46 +++---
 drivers/staging/greybus/audio_module.c   |   2 +-
 drivers/staging/greybus/audio_topology.c | 100 ---
 3 files changed, 61 insertions(+), 87 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/4] staging: greybus: audio: Cleanup junk codec registers

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Dummy codec register were initially added while populating dummy codec
mixer controls until module topology parser was available. Now, these
dummy registers are nowhere used and thus can be safely removed.

Since ASoC framework requires a valid callback for both read & write
register APIS, currently empty placeholders are kept to avoid panic.

Later, register mapping logic can be defined:
1. Assuming fixed number of maximum modules connected and register bits
corresponds to basic info of each module OR
2. With a logic to dynamically grow register_cache_size based on codec
modules added/removed.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
---
 drivers/staging/greybus/audio_codec.c | 39 ++-
 1 file changed, 2 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index b9d66278ff87..30941f9e380d 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -1026,47 +1026,16 @@ static int gbcodec_remove(struct snd_soc_codec *codec)
return 0;
 }
 
-static u8 gbcodec_reg[GBCODEC_REG_COUNT] = {
-   [GBCODEC_CTL_REG] = GBCODEC_CTL_REG_DEFAULT,
-   [GBCODEC_MUTE_REG] = GBCODEC_MUTE_REG_DEFAULT,
-   [GBCODEC_PB_LVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
-   [GBCODEC_PB_RVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
-   [GBCODEC_CAP_LVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
-   [GBCODEC_CAP_RVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
-   [GBCODEC_APB1_MUX_REG] = GBCODEC_APB1_MUX_REG_DEFAULT,
-   [GBCODEC_APB2_MUX_REG] = GBCODEC_APB2_MUX_REG_DEFAULT,
-};
-
 static int gbcodec_write(struct snd_soc_codec *codec, unsigned int reg,
 unsigned int value)
 {
-   int ret = 0;
-
-   if (reg == SND_SOC_NOPM)
-   return 0;
-
-   BUG_ON(reg >= GBCODEC_REG_COUNT);
-
-   gbcodec_reg[reg] = value;
-   dev_dbg(codec->dev, "reg[%d] = 0x%x\n", reg, value);
-
-   return ret;
+   return 0;
 }
 
 static unsigned int gbcodec_read(struct snd_soc_codec *codec,
 unsigned int reg)
 {
-   unsigned int val = 0;
-
-   if (reg == SND_SOC_NOPM)
-   return 0;
-
-   BUG_ON(reg >= GBCODEC_REG_COUNT);
-
-   val = gbcodec_reg[reg];
-   dev_dbg(codec->dev, "reg[%d] = 0x%x\n", reg, val);
-
-   return val;
+   return 0;
 }
 
 static struct snd_soc_codec_driver soc_codec_dev_gbaudio = {
@@ -1076,10 +1045,6 @@ static struct snd_soc_codec_driver soc_codec_dev_gbaudio 
= {
.read = gbcodec_read,
.write = gbcodec_write,
 
-   .reg_cache_size = GBCODEC_REG_COUNT,
-   .reg_cache_default = gbcodec_reg_defaults,
-   .reg_word_size = 1,
-
.idle_bias_off = true,
.ignore_pmdown_time = 1,
 };
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/4] staging: greybus: audio: Initialize sig_bits before configuring hwparams

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Uninitialized variable sig_bits was used while configuring stream params
for codec module. These params are used to configure PCM settings for
APBridgeA.

Usually, this is dependent on codec capability and thus populated via
codec dai_driver definition. In our case, it is fixed to 16 based on the
data format, container supported.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
---
 drivers/staging/greybus/audio_codec.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index f8862c6d7102..b9d66278ff87 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -496,6 +496,11 @@ static int gbcodec_hw_params(struct snd_pcm_substream 
*substream,
 
gb_pm_runtime_put_noidle(bundle);
 
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+   sig_bits = dai->driver->playback.sig_bits;
+   else
+   sig_bits = dai->driver->capture.sig_bits;
+
params->state = GBAUDIO_CODEC_HWPARAMS;
params->format = format;
params->rate = rate;
@@ -689,6 +694,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
.rate_min = 48000,
.channels_min = 1,
.channels_max = 2,
+   .sig_bits = 16,
},
.capture = {
.stream_name = "I2S 0 Capture",
@@ -698,6 +704,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
.rate_min = 48000,
.channels_min = 1,
.channels_max = 2,
+   .sig_bits = 16,
},
.ops = _dai_ops,
},
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/4] staging: greybus: audio: Ensure proper byte order

[Vaibhav Agarwal]

On Mon, Jan 16, 2017 at 4:17 PM, Dan Carpenter <dan.carpen...@oracle.com> wrote:
> On Sat, Jan 14, 2017 at 11:17:07PM +0530, Vaibhav Agarwal wrote:
>> @@ -656,13 +660,13 @@ static int gbaudio_tplg_create_enum_kctl(struct 
>> gbaudio_module_info *gb,
>>   gb_enum = >info.value.enumerated;
>>
>>   /* since count=1, and reg is dummy */
>> - gbe->max = gb_enum->items;
>> + gbe->max = le32_to_cpu(gb_enum->items);
>>   gbe->texts = gb_generate_enum_strings(gb, gb_enum);
>>
>>   /* debug enum info */
>>   dev_dbg(gb->dev, "Max:%d, name_length:%d\n", gb_enum->items,
>
> This is printing little endian max.  Just use gbe->max here.

Yes, this makes more sense. I should have noticed this earlier :(
Surely, I'll include this one & other suggested changes in v2 series.
Thanks Dan :)

--
vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 4/4] staging: greybus: audio: Ensure proper byte order

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Proper byte order was completely disregarded for multi byte data shared
between AP and module (and APB1). Fix this.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
---
 drivers/staging/greybus/audio_module.c   |  2 +-
 drivers/staging/greybus/audio_topology.c | 88 +---
 2 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/greybus/audio_module.c 
b/drivers/staging/greybus/audio_module.c
index 17a9948b1ba1..094c3be79b33 100644
--- a/drivers/staging/greybus/audio_module.c
+++ b/drivers/staging/greybus/audio_module.c
@@ -134,7 +134,7 @@ static int gbaudio_request_stream(struct 
gbaudio_module_info *module,
  struct gb_audio_streaming_event_request *req)
 {
dev_warn(module->dev, "Audio Event received: cport: %u, event: %u\n",
-req->data_cport, req->event);
+le16_to_cpu(req->data_cport), req->event);
 
return 0;
 }
diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index a8b63a8b2bb0..4002a57977bd 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -141,13 +141,14 @@ static const char **gb_generate_enum_strings(struct 
gbaudio_module_info *gb,
 {
const char **strings;
int i;
+   unsigned int items;
__u8 *data;
 
-   strings = devm_kzalloc(gb->dev, sizeof(char *) * gbenum->items,
-  GFP_KERNEL);
+   items = le32_to_cpu(gbenum->items);
+   strings = devm_kzalloc(gb->dev, sizeof(char *) * items, GFP_KERNEL);
data = gbenum->names;
 
-   for (i = 0; i < gbenum->items; i++) {
+   for (i = 0; i < items; i++) {
strings[i] = (const char *)data;
while (*data != '\0')
data++;
@@ -185,11 +186,11 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
switch (info->type) {
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
-   uinfo->value.integer.min = info->value.integer.min;
-   uinfo->value.integer.max = info->value.integer.max;
+   uinfo->value.integer.min = le32_to_cpu(info->value.integer.min);
+   uinfo->value.integer.max = le32_to_cpu(info->value.integer.max);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
-   max = info->value.enumerated.items;
+   max = le32_to_cpu(info->value.enumerated.items);
uinfo->value.enumerated.items = max;
if (uinfo->value.enumerated.item > max - 1)
uinfo->value.enumerated.item = max - 1;
@@ -249,17 +250,17 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
ucontrol->value.integer.value[0] =
-   gbvalue.value.integer_value[0];
+   le32_to_cpu(gbvalue.value.integer_value[0]);
if (data->vcount == 2)
ucontrol->value.integer.value[1] =
-   gbvalue.value.integer_value[1];
+   le32_to_cpu(gbvalue.value.integer_value[1]);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
ucontrol->value.enumerated.item[0] =
-   gbvalue.value.enumerated_item[0];
+   le32_to_cpu(gbvalue.value.enumerated_item[0]);
if (data->vcount == 2)
ucontrol->value.enumerated.item[1] =
-   gbvalue.value.enumerated_item[1];
+   le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
@@ -296,17 +297,17 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
gbvalue.value.integer_value[0] =
-   ucontrol->value.integer.value[0];
+   cpu_to_le32(ucontrol->value.integer.value[0]);
if (data->vcount == 2)
gbvalue.value.integer_value[1] =
-   ucontrol->value.integer.value[1];
+   cpu_to_le32(ucontrol->value.integer.value[1]);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
gbvalue.value.enumerated_item[0] =
-   ucontrol->value.enumerated.item[0];
+   cpu_to_le32(ucontrol->value.e

Re: [PATCH v2 1/4] staging: greybus: audio: Avoid less than zero check for le32 variable

[Vaibhav Agarwal]

On Tue, Jan 17, 2017 at 10:56 PM, Mark Greer <mgr...@animalcreek.com> wrote:
> Hi Vaibhav.
>
> On Tue, Jan 17, 2017 at 08:19:27PM +0530, Vaibhav Agarwal wrote:
>> mixer control->info call back function checks for -ve values to rebase
>> min and max values. However, le32 variable is used to fetch values from
>> GB module FW. Thus -ve value checking is not required. Fix this!!
>
> nit: Please spell out "negative" so it is as easy as possible for the
>  casual observer to understand what you're saying.

Will modify in v3.

>
>> Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
>> ---
>>  drivers/staging/greybus/audio_topology.c | 12 
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/staging/greybus/audio_topology.c 
>> b/drivers/staging/greybus/audio_topology.c
>> index 3001a4971c06..a8b63a8b2bb0 100644
>> --- a/drivers/staging/greybus/audio_topology.c
>> +++ b/drivers/staging/greybus/audio_topology.c
>> @@ -361,8 +361,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct 
>> snd_kcontrol *kcontrol,
>>   info = (struct gb_audio_ctl_elem_info *)data->info;
>>
>>   /* update uinfo */
>> - platform_max = info->value.integer.max;
>> - platform_min = info->value.integer.min;
>> + platform_max = le32_to_cpu(info->value.integer.max);
>> + platform_min = le32_to_cpu(info->value.integer.min);
>
> Should this piece be in patch 4/4?  It seems out of place in this patch.

Yes. It should be part of 4/4 only. Will update in v3

--
vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 3/4] staging: greybus: audio: Cleanup junk codec registers

[Vaibhav Agarwal]

On Tue, Jan 17, 2017 at 11:04 PM, Mark Greer <mgr...@animalcreek.com> wrote:
> On Tue, Jan 17, 2017 at 08:19:29PM +0530, Vaibhav Agarwal wrote:
>> From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
>>
>> Dummy codec register were initially added while populating dummy codec
>> mixer controls until module topology parser was available. Now, these
>> dummy registers are nowhere used and thus can be safely removed.
>>
>> Since ASoC framework requires a valid callback for both read & write
>> register APIS, currently empty placeholders are kept to avoid panic.
>>
>> Later, register mapping logic can be defined:
>> 1. Assuming fixed number of maximum modules connected and register bits
>> corresponds to basic info of each module OR
>> 2. With a logic to dynamically grow register_cache_size based on codec
>> modules added/removed.
>>
>> Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
>> ---
>>  drivers/staging/greybus/audio_codec.c | 39 
>> ++-
>>  1 file changed, 2 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/staging/greybus/audio_codec.c 
>> b/drivers/staging/greybus/audio_codec.c
>> index b9d66278ff87..30941f9e380d 100644
>> --- a/drivers/staging/greybus/audio_codec.c
>> +++ b/drivers/staging/greybus/audio_codec.c
>> @@ -1026,47 +1026,16 @@ static int gbcodec_remove(struct snd_soc_codec 
>> *codec)
>>   return 0;
>>  }
>>
>> -static u8 gbcodec_reg[GBCODEC_REG_COUNT] = {
>> - [GBCODEC_CTL_REG] = GBCODEC_CTL_REG_DEFAULT,
>> - [GBCODEC_MUTE_REG] = GBCODEC_MUTE_REG_DEFAULT,
>> - [GBCODEC_PB_LVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
>> - [GBCODEC_PB_RVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
>> - [GBCODEC_CAP_LVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
>> - [GBCODEC_CAP_RVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
>> - [GBCODEC_APB1_MUX_REG] = GBCODEC_APB1_MUX_REG_DEFAULT,
>> - [GBCODEC_APB2_MUX_REG] = GBCODEC_APB2_MUX_REG_DEFAULT,
>> -};
>> -
>
> gbcodec_reg_defaults[] and the definitions of 'GBCODEC_CTL_REG_DEFAULT',
> etc. can be removed now too, can't they?

Of course, this should be removed as well. Nice catch. Thanks :)

--
vaibhav

>
> The rest looks good.
>
> Mark
> --
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 4/4] staging: greybus: audio: Ensure proper byte order

[Vaibhav Agarwal]

On Tue, Jan 17, 2017 at 11:22 PM, Mark Greer <mgr...@animalcreek.com> wrote:
> On Tue, Jan 17, 2017 at 08:19:30PM +0530, Vaibhav Agarwal wrote:
>> From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
>>
>> Proper byte order was completely disregarded for multi byte data shared
>> between AP and module (and APB1). Fix this.
>>
>> Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
>> ---
>
> Hi Vaibhav.
>
> I think you got them all except for this one in
> audio_topology.c:gbcodec_mixer_dapm_ctl_put():
>
>>>> max = info->value.integer.max;  <<<
> mask = (1 << fls(max)) - 1;
> val = ucontrol->value.integer.value[0] & mask;
> connect = !!val;

Oh Ok. I'll include this change in v3.

--
vaibhav
>
> Mark
> --
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 1/4] staging: greybus: audio: Avoid less than zero check for le32 variable

[Vaibhav Agarwal]

mixer control->info call back function checks for -ve values to rebase
min and max values. However, le32 variable is used to fetch values from
GB module FW. Thus negative value checking is not required. Fix this!!

Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_topology.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 3001a4971c06..ee2113eb899e 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -371,12 +371,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct snd_kcontrol 
*kcontrol,
uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
 
uinfo->count = data->vcount;
-   uinfo->value.integer.min = 0;
-   if (info->value.integer.min < 0 &&
-   (uinfo->type == SNDRV_CTL_ELEM_TYPE_INTEGER))
-   uinfo->value.integer.max = platform_max - platform_min;
-   else
-   uinfo->value.integer.max = platform_max;
+   uinfo->value.integer.min = platform_min;
+   uinfo->value.integer.max = platform_max;
 
return 0;
 }
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/4] staging: greybus: audio: Initialize sig_bits before configuring hwparams

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Uninitialized variable sig_bits was used while configuring stream params
for codec module. These params are used to configure PCM settings for
APBridgeA.

Usually, this is dependent on codec capability and thus populated via
codec dai_driver definition. In our case, it is fixed to 16 based on the
data format, container supported.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_codec.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index f8862c6d7102..b9d66278ff87 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -496,6 +496,11 @@ static int gbcodec_hw_params(struct snd_pcm_substream 
*substream,
 
gb_pm_runtime_put_noidle(bundle);
 
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+   sig_bits = dai->driver->playback.sig_bits;
+   else
+   sig_bits = dai->driver->capture.sig_bits;
+
params->state = GBAUDIO_CODEC_HWPARAMS;
params->format = format;
params->rate = rate;
@@ -689,6 +694,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
.rate_min = 48000,
.channels_min = 1,
.channels_max = 2,
+   .sig_bits = 16,
},
.capture = {
.stream_name = "I2S 0 Capture",
@@ -698,6 +704,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
.rate_min = 48000,
.channels_min = 1,
.channels_max = 2,
+   .sig_bits = 16,
},
.ops = _dai_ops,
},
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 4/4] staging: greybus: audio: Ensure proper byte order

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Proper byte order was completely disregarded for multi byte data shared
between AP and module (and APB1). Fix this.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_module.c   |  2 +-
 drivers/staging/greybus/audio_topology.c | 94 +---
 2 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/greybus/audio_module.c 
b/drivers/staging/greybus/audio_module.c
index 17a9948b1ba1..094c3be79b33 100644
--- a/drivers/staging/greybus/audio_module.c
+++ b/drivers/staging/greybus/audio_module.c
@@ -134,7 +134,7 @@ static int gbaudio_request_stream(struct 
gbaudio_module_info *module,
  struct gb_audio_streaming_event_request *req)
 {
dev_warn(module->dev, "Audio Event received: cport: %u, event: %u\n",
-req->data_cport, req->event);
+le16_to_cpu(req->data_cport), req->event);
 
return 0;
 }
diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index ee2113eb899e..07fac3948f3a 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -141,13 +141,14 @@ static const char **gb_generate_enum_strings(struct 
gbaudio_module_info *gb,
 {
const char **strings;
int i;
+   unsigned int items;
__u8 *data;
 
-   strings = devm_kzalloc(gb->dev, sizeof(char *) * gbenum->items,
-  GFP_KERNEL);
+   items = le32_to_cpu(gbenum->items);
+   strings = devm_kzalloc(gb->dev, sizeof(char *) * items, GFP_KERNEL);
data = gbenum->names;
 
-   for (i = 0; i < gbenum->items; i++) {
+   for (i = 0; i < items; i++) {
strings[i] = (const char *)data;
while (*data != '\0')
data++;
@@ -185,11 +186,11 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
switch (info->type) {
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
-   uinfo->value.integer.min = info->value.integer.min;
-   uinfo->value.integer.max = info->value.integer.max;
+   uinfo->value.integer.min = le32_to_cpu(info->value.integer.min);
+   uinfo->value.integer.max = le32_to_cpu(info->value.integer.max);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
-   max = info->value.enumerated.items;
+   max = le32_to_cpu(info->value.enumerated.items);
uinfo->value.enumerated.items = max;
if (uinfo->value.enumerated.item > max - 1)
uinfo->value.enumerated.item = max - 1;
@@ -249,17 +250,17 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
ucontrol->value.integer.value[0] =
-   gbvalue.value.integer_value[0];
+   le32_to_cpu(gbvalue.value.integer_value[0]);
if (data->vcount == 2)
ucontrol->value.integer.value[1] =
-   gbvalue.value.integer_value[1];
+   le32_to_cpu(gbvalue.value.integer_value[1]);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
ucontrol->value.enumerated.item[0] =
-   gbvalue.value.enumerated_item[0];
+   le32_to_cpu(gbvalue.value.enumerated_item[0]);
if (data->vcount == 2)
ucontrol->value.enumerated.item[1] =
-   gbvalue.value.enumerated_item[1];
+   le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
@@ -296,17 +297,17 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
gbvalue.value.integer_value[0] =
-   ucontrol->value.integer.value[0];
+   cpu_to_le32(ucontrol->value.integer.value[0]);
if (data->vcount == 2)
gbvalue.value.integer_value[1] =
-   ucontrol->value.integer.value[1];
+   cpu_to_le32(ucontrol->value.integer.value[1]);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
gbvalue.value.enumerated_item[0] =
-   ucontrol->value.enumerated.item[0];
+ 

[PATCH v3 0/4] Cleanup greybus audio driver

[Vaibhav Agarwal]

This patch series include following cleanup changes in GB Audio driver.
- Avoid unnecessary checks for le32 variables
- Initialize sig_bits before configuring hw_params
- Remove junk codec register mapping
- Ensure proper byte ordering

Next task is to enable build for GB codec driver. However this requires
pushing some changes in ASoC framework. Possibly in another two weeks
(based on my freetime), I'll try to submit those changes to ASoC mailing
list. And once the same are accepted there, I'll share relevant patches
for GB Audio codec driver as well.

Changes from v2:
- patch 1/4
- Update commit message - Mark Greer
- Move le32_to_cpu change to patch 4/4 - Mark Greer
- patch 3/4
- Remove junk codec register related definitions in .h file - Mark Greer
- patch 4/4
- Ensure byte order for the variable missed out in v1/v2 - Mark Greer

Changes from v1:
- Include review comments from Dan

Vaibhav Agarwal (4):
  staging: greybus: audio: Avoid less than zero check for le32 variable
  staging: greybus: audio: Initialize sig_bits before configuring
hwparams
  staging: greybus: audio: Cleanup junk codec registers
  staging: greybus: audio: Ensure proper byte order

 drivers/staging/greybus/audio_codec.c|  46 +++---
 drivers/staging/greybus/audio_codec.h|  46 --
 drivers/staging/greybus/audio_module.c   |   2 +-
 drivers/staging/greybus/audio_topology.c | 102 ---
 4 files changed, 62 insertions(+), 134 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 3/4] staging: greybus: audio: Cleanup junk codec registers

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Dummy codec register were initially added while populating dummy codec
mixer controls until module topology parser was available. Now, these
dummy registers are nowhere used and thus can be safely removed.

Since ASoC framework requires a valid callback for both read & write
register APIS, currently empty placeholders are kept to avoid panic.

Later, register mapping logic can be defined:
1. Assuming fixed number of maximum modules connected and register bits
corresponds to basic info of each module OR
2. With a logic to dynamically grow register_cache_size based on codec
modules added/removed.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_codec.c | 39 ++---
 drivers/staging/greybus/audio_codec.h | 46 ---
 2 files changed, 2 insertions(+), 83 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index b9d66278ff87..30941f9e380d 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -1026,47 +1026,16 @@ static int gbcodec_remove(struct snd_soc_codec *codec)
return 0;
 }
 
-static u8 gbcodec_reg[GBCODEC_REG_COUNT] = {
-   [GBCODEC_CTL_REG] = GBCODEC_CTL_REG_DEFAULT,
-   [GBCODEC_MUTE_REG] = GBCODEC_MUTE_REG_DEFAULT,
-   [GBCODEC_PB_LVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
-   [GBCODEC_PB_RVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
-   [GBCODEC_CAP_LVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
-   [GBCODEC_CAP_RVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
-   [GBCODEC_APB1_MUX_REG] = GBCODEC_APB1_MUX_REG_DEFAULT,
-   [GBCODEC_APB2_MUX_REG] = GBCODEC_APB2_MUX_REG_DEFAULT,
-};
-
 static int gbcodec_write(struct snd_soc_codec *codec, unsigned int reg,
 unsigned int value)
 {
-   int ret = 0;
-
-   if (reg == SND_SOC_NOPM)
-   return 0;
-
-   BUG_ON(reg >= GBCODEC_REG_COUNT);
-
-   gbcodec_reg[reg] = value;
-   dev_dbg(codec->dev, "reg[%d] = 0x%x\n", reg, value);
-
-   return ret;
+   return 0;
 }
 
 static unsigned int gbcodec_read(struct snd_soc_codec *codec,
 unsigned int reg)
 {
-   unsigned int val = 0;
-
-   if (reg == SND_SOC_NOPM)
-   return 0;
-
-   BUG_ON(reg >= GBCODEC_REG_COUNT);
-
-   val = gbcodec_reg[reg];
-   dev_dbg(codec->dev, "reg[%d] = 0x%x\n", reg, val);
-
-   return val;
+   return 0;
 }
 
 static struct snd_soc_codec_driver soc_codec_dev_gbaudio = {
@@ -1076,10 +1045,6 @@ static struct snd_soc_codec_driver soc_codec_dev_gbaudio 
= {
.read = gbcodec_read,
.write = gbcodec_write,
 
-   .reg_cache_size = GBCODEC_REG_COUNT,
-   .reg_cache_default = gbcodec_reg_defaults,
-   .reg_word_size = 1,
-
.idle_bias_off = true,
.ignore_pmdown_time = 1,
 };
diff --git a/drivers/staging/greybus/audio_codec.h 
b/drivers/staging/greybus/audio_codec.h
index 62fd93939a1f..6fb064c69a36 100644
--- a/drivers/staging/greybus/audio_codec.h
+++ b/drivers/staging/greybus/audio_codec.h
@@ -24,18 +24,6 @@ enum {
NUM_CODEC_DAIS,
 };
 
-enum gbcodec_reg_index {
-   GBCODEC_CTL_REG,
-   GBCODEC_MUTE_REG,
-   GBCODEC_PB_LVOL_REG,
-   GBCODEC_PB_RVOL_REG,
-   GBCODEC_CAP_LVOL_REG,
-   GBCODEC_CAP_RVOL_REG,
-   GBCODEC_APB1_MUX_REG,
-   GBCODEC_APB2_MUX_REG,
-   GBCODEC_REG_COUNT
-};
-
 /* device_type should be same as defined in audio.h (Android media layer) */
 enum {
GBAUDIO_DEVICE_NONE = 0x0,
@@ -51,42 +39,9 @@ enum {
GBAUDIO_DEVICE_IN_WIRED_HEADSET = GBAUDIO_DEVICE_BIT_IN | 0x10,
 };
 
-/* bit 0-SPK, 1-HP, 2-DAC,
- * 4-MIC, 5-HSMIC, 6-MIC2
- */
-#define GBCODEC_CTL_REG_DEFAULT0x00
-
-/* bit 0,1 - APB1-PB-L/R
- * bit 2,3 - APB2-PB-L/R
- * bit 4,5 - APB1-Cap-L/R
- * bit 6,7 - APB2-Cap-L/R
- */
-#defineGBCODEC_MUTE_REG_DEFAULT0x00
-
-/* 0-127 steps */
-#defineGBCODEC_PB_VOL_REG_DEFAULT  0x00
-#defineGBCODEC_CAP_VOL_REG_DEFAULT 0x00
-
-/* bit 0,1,2 - PB stereo, left, right
- * bit 8,9,10 - Cap stereo, left, right
- */
-#define GBCODEC_APB1_MUX_REG_DEFAULT   0x00
-#define GBCODEC_APB2_MUX_REG_DEFAULT   0x00
-
 #define GBCODEC_JACK_MASK  0x
 #define GBCODEC_JACK_BUTTON_MASK   0x
 
-static const u8 gbcodec_reg_defaults[GBCODEC_REG_COUNT] = {
-   GBCODEC_CTL_REG_DEFAULT,
-   GBCODEC_MUTE_REG_DEFAULT,
-   GBCODEC_PB_VOL_REG_DEFAULT,
-   GBCODEC_PB_VOL_REG_DEFAULT,
-   GBCODEC_CAP_VOL_REG_DEFAULT,
-   GBCODEC_CAP_VOL_REG_DEFAULT,
-   GBCODEC_APB1_MUX_REG_DEFAULT,
-   GBCODEC_APB2_MUX_REG_DEFAULT,
-};
-
 enum gbaudio_codec_state {
GBAUDIO_CODEC_SHUTDOWN = 0,
  

[PATCH] greybus: audio: fix uninitialized variable errors found by cppcheck

[Vaibhav Agarwal]

Currently, if info is null, the dev_err message is dereferencing an
uninitialized module pointer.  Instead, it should use codec->dev pointer
in dev_err call and better align with other err msg in this function.

Also, ret variable might be used uninitialized in a specific case.
Avoid using it this way.

Found using static analysis with cppcheck:
Checking drivers/staging/greybus/audio_topology.c...
[drivers/staging/greybus/audio_topology.c:175]: (error) Uninitialized
variable: module
[drivers/staging/greybus/audio_topology.c:495]: (error) Uninitialized
variable: ret

Reported-by: Colin Ian King <colin.k...@canonical.com>
Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_topology.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index f9f33817a092..b6251691a33d 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -172,7 +172,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
info = (struct gb_audio_ctl_elem_info *)data->info;
 
if (!info) {
-   dev_err(module->dev, "NULL info for %s\n", uinfo->id.name);
+   dev_err(codec->dev, "NULL info for %s\n", uinfo->id.name);
return -EINVAL;
}
 
@@ -489,10 +489,11 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
dev_err_ratelimited(codec->dev,
"%d:Error in %s for %s\n", ret,
__func__, kcontrol->id.name);
+   return ret;
}
}
 
-   return ret;
+   return 0;
 }
 
 #define SOC_DAPM_MIXER_GB(xname, kcount, data) \
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] drivers: staging: greybus: Fixed CHECKS for brace issues

[Vaibhav Agarwal]

On Fri, Oct 7, 2016 at 8:08 AM, Chase Metzger <chasemetzge...@gmail.com> wrote:
> Added braces to else statements where checkpatch complained.
>
> Signed-off-by: Chase Metzger <chasemetzge...@gmail.com>
> ---

Reviewed-by: Vaibhav Agarwal <vaibhav...@gmail.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] drivers: staging: greybus: audio_codec.c: Fixed CHECKS for brace issues

[Vaibhav Agarwal]

On Mon, Oct 10, 2016 at 3:56 AM, Chase Metzger <chasemetzge...@gmail.com> wrote:
> Added braces to else statement where checkpatch complained.
>
> Signed-off-by: Chase Metzger <chasemetzge...@gmail.com>
> ---
>  drivers/staging/greybus/audio_codec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/audio_codec.c 
> b/drivers/staging/greybus/audio_codec.c
> index 8a0744b..1bdf849 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -655,8 +655,10 @@ static int gbcodec_mute_stream(struct snd_soc_dai *dai, 
> int mute, int stream)
> ret = gb_audio_apbridgea_shutdown_rx(data->connection,
>  0);
> params->state = GBAUDIO_CODEC_STOP;
> -   } else
> +   } else {
> ret = -EINVAL;
> +   }
> +
> if (ret)
> dev_err_ratelimited(dai->dev,
>         "%s:Error during %s %s stream:%d\n",
> --
> 2.1.4
>
Reviewed-by: Vaibhav Agarwal <vaibhav...@gmail.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] greybus: audio: ensure module is set to avoid crash on dev_err message

[Vaibhav Agarwal]

On Fri, Sep 23, 2016 at 10:28 PM, Greg Kroah-Hartman
 wrote:
> On Fri, Sep 23, 2016 at 11:25:40AM +0100, Colin King wrote:
>> From: Colin Ian King 
>>
>> Currently, if info is null, the dev_err message is dereferencing an
>> uninitialized module pointer.  Instead, initialize module before the
>> dev_err call to fix this issue.
>>
>> Found using static analysis with cppcheck:
>> [drivers/staging/greybus/audio_topology.c:175]: (error)
>>   Uninitialized variable: module
>>
>> Signed-off-by: Colin Ian King 
>> ---
>>  drivers/staging/greybus/audio_topology.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/greybus/audio_topology.c 
>> b/drivers/staging/greybus/audio_topology.c
>> index 5eef536..c43a959 100644
>> --- a/drivers/staging/greybus/audio_topology.c
>> +++ b/drivers/staging/greybus/audio_topology.c
>> @@ -171,6 +171,9 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
>> *kcontrol,
>>   data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
>>   info = (struct gb_audio_ctl_elem_info *)data->info;
>>
>> + module = find_gb_module(gbcodec, kcontrol->id.name);
>> + if (!module)
>> + return -EINVAL;
>
> How do you know you can get a module at this point in time, you haven't
> looked at the type of info to know that?
>
> I agree that there is an issue here, but I don't think this is the
> correct fix.  Vaibhav?

Actually, fetching module is not related to info->type. However it is
only required in case of ENUMERATED element_type, thus used in
specific case. Also, I think it's better to use codec->dev in the err
message and align with other err_msg in this function.

Hi Colin, thanks for sharing this patch. Do you mind updating this
patch including my above suggestion? Otherwise, I can share a separate
patch.

--
thanks,
vaibhav
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: audio: Rename cport with intf_id

[Vaibhav Agarwal]

On Mon, Oct 17, 2016 at 9:01 PM, Johan Hovold  wrote:
> On Sun, Oct 16, 2016 at 03:29:14PM +0530, Pankaj Bharadiya wrote:
>> gb_audio_manager_module_descriptor's cport field is actually used to
>> manage and pass interface id to user space.
>>
>> Thus rename gb_audio_manager_module_descriptor's 'cport' field and
>> few other things to avoid confusion.
>
> Please be more specific about what these other things are; in this case
> sysfs attributes, uevent vars, and an odd-looking sysfs-string
> interface (more?).
>
> Wouldn't this change break any current user-space implementation?

Yes, it would require changes in user space code as well. For now, it
should be GMP-Audio HAL only.

>
> Also why aren't any of these attributes documented as they should be?

Currently, none of GB Audio specific sysfs entries are documented.
I'll share another patch for this.

--
thanks,
./va

>
> Thanks,
> Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: audio: remove redundant slot field

[Vaibhav Agarwal]

module->desc.slot,
> module->desc.vid,
> module->desc.pid,
> module->desc.intf_id,
> diff --git a/drivers/staging/greybus/audio_manager_sysfs.c 
> b/drivers/staging/greybus/audio_manager_sysfs.c
> index fc0aca6..9eae70f 100644
> --- a/drivers/staging/greybus/audio_manager_sysfs.c
> +++ b/drivers/staging/greybus/audio_manager_sysfs.c
> @@ -20,10 +20,9 @@ static ssize_t manager_sysfs_add_store(
>
> int num = sscanf(buf,
> "name=%" GB_AUDIO_MANAGER_MODULE_NAME_LEN_SSCANF "s "
> -   "slot=%d vid=%d pid=%d intf_id=%d i/p devices=0x%X"
> -   "o/p devices=0x%X",
> -   desc.name, , , ,
> -   _id, _devices, _devices);
> +   "vid=%d pid=%d intf_id=%d i/p devices=0x%X o/p 
> devices=0x%X",
> +   desc.name, , , _id,
> +   _devices, _devices);
>
> if (num != 7)
> return -EINVAL;
> diff --git a/drivers/staging/greybus/audio_module.c 
> b/drivers/staging/greybus/audio_module.c
> index 45b2519..0269a1d 100644
> --- a/drivers/staging/greybus/audio_module.c
> +++ b/drivers/staging/greybus/audio_module.c
> @@ -345,7 +345,6 @@ static int gb_audio_probe(struct gb_bundle *bundle,
> dev_dbg(dev, "Inform set_event:%d to above layer\n", 1);
> /* prepare for the audio manager */
> strlcpy(desc.name, gbmodule->name, GB_AUDIO_MANAGER_MODULE_NAME_LEN);
> -   desc.slot = 1; /* todo */
> desc.vid = 2; /* todo */
> desc.pid = 3; /* todo */
> desc.intf_id = gbmodule->dev_id;
> --
> 1.9.1
>
Reviewed-by: Vaibhav Agarwal <vaibhav...@gmail.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] drivers: staging: greybus: audio_manager_sysfs.c: Fixed CHECK for brace issue

[Vaibhav Agarwal]

On Wed, Oct 12, 2016 at 4:52 AM, Chase Metzger <chasemetzge...@gmail.com> wrote:
> Added braces to else and else if statements where checkpatch complained.
>
> Signed-off-by: Chase Metzger <chasemetzge...@gmail.com>
> ---
>  drivers/staging/greybus/audio_manager_sysfs.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_manager_sysfs.c 
> b/drivers/staging/greybus/audio_manager_sysfs.c
> index d8bf859..376e17c 100644
> --- a/drivers/staging/greybus/audio_manager_sysfs.c
> +++ b/drivers/staging/greybus/audio_manager_sysfs.c
> @@ -71,10 +71,11 @@ static ssize_t manager_sysfs_dump_store(
> num = gb_audio_manager_dump_module(id);
> if (num)
> return num;
> -   } else if (!strncmp("all", buf, 3))
> +   } else if (!strncmp("all", buf, 3)) {
> gb_audio_manager_dump_all();
> -   else
> +   } else {
>         return -EINVAL;
> +   }
>
> return count;
>  }
> --
> 2.1.4
>
Reviewed-by: Vaibhav Agarwal <vaibhav...@gmail.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] drivers: staging: greybus: audio_topology.c: Fixed CHECKS for brace issues

[Vaibhav Agarwal]

On Wed, Oct 12, 2016 at 5:19 AM, Chase Metzger  wrote:
> Added braces for else and else if statements where checkpatch complained.
>
> Signed-off-by: Chase Metzger 
> ---
>  drivers/staging/greybus/audio_topology.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_topology.c 
> b/drivers/staging/greybus/audio_topology.c
> index b625169..3a2678e 100644
> --- a/drivers/staging/greybus/audio_topology.c
> +++ b/drivers/staging/greybus/audio_topology.c
> @@ -1044,8 +1044,10 @@ static int gbaudio_tplg_create_widget(struct 
> gbaudio_module_info *module,
> control->texts = (const char * const *)
> gb_generate_enum_strings(module, gbenum);
> control->items = gbenum->items;
> -   } else
> +   } else {
> csize = sizeof(struct gb_audio_control);
> +   }
> +
> *w_size += csize;
> curr = (void *)curr + csize;
> list_add(>list, >widget_ctl_list);
> @@ -1190,8 +1192,9 @@ static int gbaudio_tplg_process_kcontrols(struct 
> gbaudio_module_info *module,
> control->texts = (const char * const *)
> gb_generate_enum_strings(module, gbenum);
> control->items = gbenum->items;
> -   } else
> +   } else {
> csize = sizeof(struct gb_audio_control);
> +   }
>
> list_add(>list, >ctl_list);
> dev_dbg(module->dev, "%d:%s created of type %d\n", curr->id,
> --
> 2.1.4
>
You have already submitted a similar patch available on staging-testing branch:
commit b7e0c9eca426 ("drivers: staging: greybus: Fixed CHECKS for brace issues")

--
thanks,
./va
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: audio: Rename cport with intf_id

[Vaibhav Agarwal]

es=0x%X\n",
> module->id,
> module->desc.name,
> module->desc.slot,
> module->desc.vid,
> module->desc.pid,
> -   module->desc.cport,
> +   module->desc.intf_id,
> module->desc.ip_devices,
> module->desc.op_devices);
>  }
> diff --git a/drivers/staging/greybus/audio_manager_sysfs.c 
> b/drivers/staging/greybus/audio_manager_sysfs.c
> index d8bf859..feb195d 100644
> --- a/drivers/staging/greybus/audio_manager_sysfs.c
> +++ b/drivers/staging/greybus/audio_manager_sysfs.c
> @@ -20,10 +20,10 @@ static ssize_t manager_sysfs_add_store(
>
> int num = sscanf(buf,
> "name=%" GB_AUDIO_MANAGER_MODULE_NAME_LEN_SSCANF "s "
> -   "slot=%d vid=%d pid=%d cport=%d i/p devices=0x%X"
> +   "slot=%d vid=%d pid=%d intf_id=%d i/p devices=0x%X"
> "o/p devices=0x%X",
> desc.name, , , ,
> -   , _devices, _devices);
> +   _id, _devices, _devices);
>
> if (num != 7)
> return -EINVAL;
> diff --git a/drivers/staging/greybus/audio_module.c 
> b/drivers/staging/greybus/audio_module.c
> index ae1c0fa..45b2519 100644
> --- a/drivers/staging/greybus/audio_module.c
> +++ b/drivers/staging/greybus/audio_module.c
> @@ -348,7 +348,7 @@ static int gb_audio_probe(struct gb_bundle *bundle,
> desc.slot = 1; /* todo */
> desc.vid = 2; /* todo */
> desc.pid = 3; /* todo */
> -   desc.cport = gbmodule->dev_id;
> +   desc.intf_id = gbmodule->dev_id;
> desc.op_devices = gbmodule->op_devices;
> desc.ip_devices = gbmodule->ip_devices;
> gbmodule->manager_id = gb_audio_manager_add();
> --
> 1.9.1
>
Reviewed-by: Vaibhav Agarwal <vaibhav...@gmail.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/4] staging: greybus: audio: Initialize sig_bits before configuring hwparams

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Uninitialized variable sig_bits was used while configuring stream params
for codec module. These params are used to configure PCM settings for
APBridgeA.

Usually, this is dependent on codec capability and thus populated via
codec dai_driver definition. In our case, it is fixed to 16 based on the
data format, container supported.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_codec.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index f8862c6d7102..b9d66278ff87 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -496,6 +496,11 @@ static int gbcodec_hw_params(struct snd_pcm_substream 
*substream,
 
gb_pm_runtime_put_noidle(bundle);
 
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+   sig_bits = dai->driver->playback.sig_bits;
+   else
+   sig_bits = dai->driver->capture.sig_bits;
+
params->state = GBAUDIO_CODEC_HWPARAMS;
params->format = format;
params->rate = rate;
@@ -689,6 +694,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
.rate_min = 48000,
.channels_min = 1,
.channels_max = 2,
+   .sig_bits = 16,
},
.capture = {
.stream_name = "I2S 0 Capture",
@@ -698,6 +704,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
.rate_min = 48000,
.channels_min = 1,
.channels_max = 2,
+   .sig_bits = 16,
},
.ops = _dai_ops,
},
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/4] Cleanup greybus audio driver

[Vaibhav Agarwal]

This patch series include following cleanup changes in GB Audio driver.
- Avoid unnecessary checks for le32 variables
- Initialize sig_bits before configuring hw_params
- Remove junk codec register mapping
- Ensure proper byte ordering

Next task is to enable build for GB codec driver. However this requires
pushing some changes in ASoC framework. Possibly in another two weeks
(based on my freetime), I'll try to submit those changes to ASoC mailing
list. And once the same are accepted there, I'll share relevant patches
for GB Audio codec driver as well.

Vaibhav Agarwal (4):
  staging: greybus: audio: Avoid less than zero check for le32 variable
  staging: greybus: audio: Initialize sig_bits before configuring
hwparams
  staging: greybus: audio: Cleanup junk codec registers
  staging: greybus: audio: Ensure proper byte order

 drivers/staging/greybus/audio_codec.c| 46 +++-
 drivers/staging/greybus/audio_module.c   |  2 +-
 drivers/staging/greybus/audio_topology.c | 94 
 3 files changed, 58 insertions(+), 84 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/4] staging: greybus: audio: Ensure proper byte order

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Proper byte order was completely disregarded for multi byte data shared
between AP and module (and APB1). Fix this.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_module.c   |  2 +-
 drivers/staging/greybus/audio_topology.c | 82 +---
 2 files changed, 45 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/greybus/audio_module.c 
b/drivers/staging/greybus/audio_module.c
index 17a9948b1ba1..094c3be79b33 100644
--- a/drivers/staging/greybus/audio_module.c
+++ b/drivers/staging/greybus/audio_module.c
@@ -134,7 +134,7 @@ static int gbaudio_request_stream(struct 
gbaudio_module_info *module,
  struct gb_audio_streaming_event_request *req)
 {
dev_warn(module->dev, "Audio Event received: cport: %u, event: %u\n",
-req->data_cport, req->event);
+le16_to_cpu(req->data_cport), req->event);
 
return 0;
 }
diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index a8b63a8b2bb0..1cff2440c6d3 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -141,13 +141,14 @@ static const char **gb_generate_enum_strings(struct 
gbaudio_module_info *gb,
 {
const char **strings;
int i;
+   unsigned int items;
__u8 *data;
 
-   strings = devm_kzalloc(gb->dev, sizeof(char *) * gbenum->items,
-  GFP_KERNEL);
+   items = le32_to_cpu(gbenum->items);
+   strings = devm_kzalloc(gb->dev, sizeof(char *) * items, GFP_KERNEL);
data = gbenum->names;
 
-   for (i = 0; i < gbenum->items; i++) {
+   for (i = 0; i < items; i++) {
strings[i] = (const char *)data;
while (*data != '\0')
data++;
@@ -185,11 +186,11 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
switch (info->type) {
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
-   uinfo->value.integer.min = info->value.integer.min;
-   uinfo->value.integer.max = info->value.integer.max;
+   uinfo->value.integer.min = le32_to_cpu(info->value.integer.min);
+   uinfo->value.integer.max = le32_to_cpu(info->value.integer.max);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
-   max = info->value.enumerated.items;
+   max = le32_to_cpu(info->value.enumerated.items);
uinfo->value.enumerated.items = max;
if (uinfo->value.enumerated.item > max - 1)
uinfo->value.enumerated.item = max - 1;
@@ -249,17 +250,17 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
ucontrol->value.integer.value[0] =
-   gbvalue.value.integer_value[0];
+   le32_to_cpu(gbvalue.value.integer_value[0]);
if (data->vcount == 2)
ucontrol->value.integer.value[1] =
-   gbvalue.value.integer_value[1];
+   le32_to_cpu(gbvalue.value.integer_value[1]);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
ucontrol->value.enumerated.item[0] =
-   gbvalue.value.enumerated_item[0];
+   le32_to_cpu(gbvalue.value.enumerated_item[0]);
if (data->vcount == 2)
ucontrol->value.enumerated.item[1] =
-   gbvalue.value.enumerated_item[1];
+   le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
@@ -296,17 +297,17 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
case GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN:
case GB_AUDIO_CTL_ELEM_TYPE_INTEGER:
gbvalue.value.integer_value[0] =
-   ucontrol->value.integer.value[0];
+   cpu_to_le32(ucontrol->value.integer.value[0]);
if (data->vcount == 2)
gbvalue.value.integer_value[1] =
-   ucontrol->value.integer.value[1];
+   cpu_to_le32(ucontrol->value.integer.value[1]);
break;
case GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED:
gbvalue.value.enumerated_item[0] =
-   ucontrol->value.enumerated.item[0];
+ 

[PATCH 3/4] staging: greybus: audio: Cleanup junk codec registers

[Vaibhav Agarwal]

From: Vaibhav Agarwal <vaibhav.agar...@linaro.org>

Dummy codec register were initially added while populating dummy codec
mixer controls until module topology parser was available. Now, these
dummy registers are nowhere used and thus can be safely removed.

Since ASoC framework requires a valid callback for both read & write
register APIS, currently empty placeholders are kept to avoid panic.

Later, register mapping logic can be defined:
1. Assuming fixed number of maximum modules connected and register bits
corresponds to basic info of each module OR
2. With a logic to dynamically grow register_cache_size based on codec
modules added/removed.

Signed-off-by: Vaibhav Agarwal <vaibhav.agar...@linaro.org>
Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_codec.c | 39 ++-
 1 file changed, 2 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index b9d66278ff87..30941f9e380d 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -1026,47 +1026,16 @@ static int gbcodec_remove(struct snd_soc_codec *codec)
return 0;
 }
 
-static u8 gbcodec_reg[GBCODEC_REG_COUNT] = {
-   [GBCODEC_CTL_REG] = GBCODEC_CTL_REG_DEFAULT,
-   [GBCODEC_MUTE_REG] = GBCODEC_MUTE_REG_DEFAULT,
-   [GBCODEC_PB_LVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
-   [GBCODEC_PB_RVOL_REG] = GBCODEC_PB_VOL_REG_DEFAULT,
-   [GBCODEC_CAP_LVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
-   [GBCODEC_CAP_RVOL_REG] = GBCODEC_CAP_VOL_REG_DEFAULT,
-   [GBCODEC_APB1_MUX_REG] = GBCODEC_APB1_MUX_REG_DEFAULT,
-   [GBCODEC_APB2_MUX_REG] = GBCODEC_APB2_MUX_REG_DEFAULT,
-};
-
 static int gbcodec_write(struct snd_soc_codec *codec, unsigned int reg,
 unsigned int value)
 {
-   int ret = 0;
-
-   if (reg == SND_SOC_NOPM)
-   return 0;
-
-   BUG_ON(reg >= GBCODEC_REG_COUNT);
-
-   gbcodec_reg[reg] = value;
-   dev_dbg(codec->dev, "reg[%d] = 0x%x\n", reg, value);
-
-   return ret;
+   return 0;
 }
 
 static unsigned int gbcodec_read(struct snd_soc_codec *codec,
 unsigned int reg)
 {
-   unsigned int val = 0;
-
-   if (reg == SND_SOC_NOPM)
-   return 0;
-
-   BUG_ON(reg >= GBCODEC_REG_COUNT);
-
-   val = gbcodec_reg[reg];
-   dev_dbg(codec->dev, "reg[%d] = 0x%x\n", reg, val);
-
-   return val;
+   return 0;
 }
 
 static struct snd_soc_codec_driver soc_codec_dev_gbaudio = {
@@ -1076,10 +1045,6 @@ static struct snd_soc_codec_driver soc_codec_dev_gbaudio 
= {
.read = gbcodec_read,
.write = gbcodec_write,
 
-   .reg_cache_size = GBCODEC_REG_COUNT,
-   .reg_cache_default = gbcodec_reg_defaults,
-   .reg_word_size = 1,
-
.idle_bias_off = true,
.ignore_pmdown_time = 1,
 };
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/4] staging: greybus: audio: Avoid less than zero check for le32 variable

[Vaibhav Agarwal]

mixer control->info call back function checks for -ve values to rebase
min and max values. However, le32 variable is used to fetch values from
GB module FW. Thus -ve value checking is not required. Fix this!!

Signed-off-by: Vaibhav Agarwal <vaibhav...@gmail.com>
---
 drivers/staging/greybus/audio_topology.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 3001a4971c06..a8b63a8b2bb0 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -361,8 +361,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct snd_kcontrol 
*kcontrol,
info = (struct gb_audio_ctl_elem_info *)data->info;
 
/* update uinfo */
-   platform_max = info->value.integer.max;
-   platform_min = info->value.integer.min;
+   platform_max = le32_to_cpu(info->value.integer.max);
+   platform_min = le32_to_cpu(info->value.integer.min);
 
if (platform_max == 1 &&
!strnstr(kcontrol->id.name, " Volume", NAME_SIZE))
@@ -371,12 +371,8 @@ static int gbcodec_mixer_dapm_ctl_info(struct snd_kcontrol 
*kcontrol,
uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
 
uinfo->count = data->vcount;
-   uinfo->value.integer.min = 0;
-   if (info->value.integer.min < 0 &&
-   (uinfo->type == SNDRV_CTL_ELEM_TYPE_INTEGER))
-   uinfo->value.integer.max = platform_max - platform_min;
-   else
-   uinfo->value.integer.max = platform_max;
+   uinfo->value.integer.min = platform_min;
+   uinfo->value.integer.max = platform_max;
 
return 0;
 }
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/11] staging: greybus: add SPDX identifiers to all greybus driver files

[Vaibhav Agarwal]

On Tue, Nov 7, 2017 at 7:28 PM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:
> It's good to have SPDX identifiers in all files to make it easier to
> audit the kernel tree for correct licenses.
>
> Update the drivers/staging/greybus files files with the correct SPDX
> license identifier based on the license text in the file itself.  The
> SPDX identifier is a legally binding shorthand, which can be used
> instead of the full boiler plate text.
>
> This work is based on a script and data from Thomas Gleixner, Philippe
> Ombredanne, and Kate Stewart.
>
> Cc: Johan Hovold <jo...@kernel.org>
> Cc: Alex Elder <el...@kernel.org>
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: Vaibhav Hiremath <hvaibhav.li...@gmail.com>
> Cc: Vaibhav Agarwal <vaibhav...@gmail.com>
> Cc: Mark Greer <mgr...@animalcreek.com>
> Cc: Viresh Kumar <vire...@kernel.org>
> Cc: Rui Miguel Silva <rmf...@gmail.com>
> Cc: David Lin <dtw...@gmail.com>
> Cc: "Bryan O'Donoghue" <pure.lo...@nexus-software.ie>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: Kate Stewart <kstew...@linuxfoundation.org>
> Cc: Philippe Ombredanne <pombreda...@nexb.com>
> Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> ---

Acked-by: Vaibhav Agarwal <vaibhav...@gmail.com>

--
vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/11] staging: greybus: Remove redundant license text

[Vaibhav Agarwal]

On Tue, Nov 7, 2017 at 7:28 PM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:
> Now that the SPDX tag is in all greybus files, that identifies the
> license in a specific and legally-defined manner.  So the extra GPL text
> wording can be removed as it is no longer needed at all.
>
> This is done on a quest to remove the 700+ different ways that files in
> the kernel describe the GPL license text.  And there's unneeded stuff
> like the address (sometimes incorrect) for the FSF which is never
> needed.
>
> No copyright headers or other non-license-description text was removed.
>
> Cc: Vaibhav Hiremath <hvaibhav.li...@gmail.com>
> Cc: Johan Hovold <jo...@kernel.org>
> Cc: Alex Elder <el...@kernel.org>
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: Vaibhav Agarwal <vaibhav...@gmail.com>
> Cc: Mark Greer <mgr...@animalcreek.com>
> Cc: Viresh Kumar <vire...@kernel.org>
> Cc: Rui Miguel Silva <rmf...@gmail.com>
> Cc: David Lin <dtw...@gmail.com>
> Cc: "Bryan O'Donoghue" <pure.lo...@nexus-software.ie>
> Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> ---

Acked-by: Vaibhav Agarwal <vaibhav...@gmail.com>

--
vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: fix spelling mistake: "Inavlid" -> "Invalid"

[Vaibhav Agarwal]

On Tue, May 22, 2018 at 2:03 PM Colin King <colin.k...@canonical.com> wrote:

> From: Colin Ian King <colin.k...@canonical.com>

> Trivial fix to spelling mistake in dev_err error message

> Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> ---
>   drivers/staging/greybus/audio_topology.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/drivers/staging/greybus/audio_topology.c
b/drivers/staging/greybus/audio_topology.c
> index de4b1b2b12f3..15e57f701630 100644
> --- a/drivers/staging/greybus/audio_topology.c
> +++ b/drivers/staging/greybus/audio_topology.c
> @@ -996,7 +996,7 @@ static int gbaudio_tplg_create_widget(struct
gbaudio_module_info *module,

>  ret = gbaudio_validate_kcontrol_count(w);
>  if (ret) {
> -   dev_err(module->dev, "Inavlid kcontrol count=%d for %s\n",
> +   dev_err(module->dev, "Invalid kcontrol count=%d for %s\n",
>  w->ncontrols, w->name);
>  return ret;
>  }
> --
> 2.17.0


Acked-by: Vaibhav Agarwal <vaibhav...@gmail.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/8] staging: greybus: audio_codec.c: Fix alignment should match open parenthesis

[Vaibhav Agarwal]

On Fri, Dec 22, 2017 at 4:50 PM, Kamal Heib <kamalhe...@gmail.com> wrote:
> Cleanup "Alignment should match open parenthesis" checkpatch.pl errors.
>
> Cc: Vaibhav Agarwal <vaibhav...@gmail.com>
> Signed-off-by: Kamal Heib <kamalhe...@gmail.com>

Hi Kamal,

Thanks for sharing cleanup patches. Kindly share the complete series
with greybus-dev mailing list as well. You can get more details on
whom to keep in the loop using get_maintainer.pl script.

Other than that, for patch v 2/8 till 8/8:
Acked-by: Vaibhav Agarwal <vaibhav...@gmail.com>

--
cheers,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: greybus: audio_codec.c: Change uint32_t to u32

[Vaibhav Agarwal]

On Wed, Jan 3, 2018 at 3:09 PM, Sumit Pundir  wrote:
> This patch fixes the following checkpatch.pl issue at multiple lines:
>
> CHECK: Prefer kernel type 'u32' over 'uint32_t'
> +   uint32_t format, rate;
>
> Signed-off-by: Sumit Pundir 
> ---

Hi Sumit,

Similar patches are already submitted by Kamal. Kindly go thru the
driverdev mailing list to get more details and share another series if
you find anything missing.


--
thanks,
./va
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] staging: greybus: Fix warning to limit chars per line

[Vaibhav Agarwal]

On Fri, Apr 6, 2018 at 4:39 PM, Gaurav Dhingra
<gauravdhingra.g...@gmail.com> wrote:
> Wrap comment to fix warning "prefer a maximum 75 chars per line"
>
> Signed-off-by: Gaurav Dhingra <gauravdhingra.g...@gmail.com>
> ---
> Changes in v2:
> - use correct format for multiline comment
> Changes in v3:
> - include commit log
> ---
>  drivers/staging/greybus/audio_codec.h | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/audio_codec.h 
> b/drivers/staging/greybus/audio_codec.h
> index a1d5440..4efd8b3 100644
> --- a/drivers/staging/greybus/audio_codec.h
> +++ b/drivers/staging/greybus/audio_codec.h
> @@ -23,7 +23,10 @@ enum {
> NUM_CODEC_DAIS,
>  };
>
> -/* device_type should be same as defined in audio.h (Android media layer) */
> +/*
> + * device_type should be same as defined in audio.h
> + * (Android media layer)
> + */

Acked-by: Vaibhav Agarwal <vaibhav...@gmail.com>

--
thanks,
./va
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] greybus: audio_manager: fix a missing check of ida_simple_get

[Vaibhav Agarwal]

On Thu, Mar 14, 2019 at 12:15 PM Kangjie Lu  wrote:
>
> ida_simple_get could fail. The fix inserts a check for its
> return value.
>
> Signed-off-by: Kangjie Lu 
> ---
>  drivers/staging/greybus/audio_manager.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/greybus/audio_manager.c 
> b/drivers/staging/greybus/audio_manager.c
> index d44b070d8862..c2a4af4c1d06 100644
> --- a/drivers/staging/greybus/audio_manager.c
> +++ b/drivers/staging/greybus/audio_manager.c
> @@ -45,6 +45,9 @@ int gb_audio_manager_add(struct 
> gb_audio_manager_module_descriptor *desc)
> int err;
>
> id = ida_simple_get(_id, 0, 0, GFP_KERNEL);
> +   if (id < 0)
> +   return id;
> +
> err = gb_audio_manager_module_create(, manager_kset,
>          id, desc);
> if (err) {

Reviewed-by: Vaibhav Agarwal 

--
./va
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: remove redundant assignment to variable is_empty

[Vaibhav Agarwal]

On Thu, Jul 4, 2019 at 7:00 PM Colin King  wrote:
>
> From: Colin Ian King 
>
> The variable is_empty is being initialized with a value that is never
> read and it is being updated later with a new value. The
> initialization is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/staging/greybus/audio_manager.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/audio_manager.c 
> b/drivers/staging/greybus/audio_manager.c
> index c2a4af4c1d06..9b19ea9d3fa1 100644
> --- a/drivers/staging/greybus/audio_manager.c
> +++ b/drivers/staging/greybus/audio_manager.c
> @@ -86,7 +86,7 @@ EXPORT_SYMBOL_GPL(gb_audio_manager_remove);
>  void gb_audio_manager_remove_all(void)
>  {
> struct gb_audio_manager_module *module, *next;
> -   int is_empty = 1;
> +   int is_empty;
>
> down_write(_rwsem);
>
> --
> 2.20.1
>
Reviewed-by: Vaibhav Agarwal 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: use after free in gb_audio_manager_remove_all()

[Vaibhav Agarwal]

On Wed, Feb 5, 2020 at 6:02 PM Dan Carpenter  wrote:
>
> When we call kobject_put() and it's the last reference to the kobject
> then it calls gb_audio_module_release() and frees module.  We dereference
> "module" on the next line which is a use after free.
>
> Fixes: c77f85bbc91a ("greybus: audio: Fix incorrect counting of 'ida'")
> Signed-off-by: Dan Carpenter 
> ---
>  drivers/staging/greybus/audio_manager.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/audio_manager.c 
> b/drivers/staging/greybus/audio_manager.c
> index 9b19ea9d3fa1..9a3f7c034ab4 100644
> --- a/drivers/staging/greybus/audio_manager.c
> +++ b/drivers/staging/greybus/audio_manager.c
> @@ -92,8 +92,8 @@ void gb_audio_manager_remove_all(void)
>
> list_for_each_entry_safe(module, next, _list, list) {
> list_del(>list);
> -   kobject_put(>kobj);
> ida_simple_remove(_id, module->id);
> +   kobject_put(>kobj);
> }
>
>     is_empty = list_empty(_list);
> --
> 2.11.0
>
Thanks Dan for sharing the fix.

Reviewed-by: Vaibhav Agarwal 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] greybus: audio: remove unused code

[Vaibhav Agarwal]

On Wed, May 13, 2020 at 11:15 PM Mark Greer  wrote:
>
> On Thu, May 07, 2020 at 11:29:11PM +0200, Alexandre Belloni wrote:
> > GREYBUS_AUDIO_MSM8994 is not an existing configuration option and as
> > reported in September 2016, it depends on an "out-of-tree qualcomm audio
> > driver". This driver never made it upstream.
> >
> > https://lore.kernel.org/lkml/20160921073905.ga31...@kroah.com/
> >
> > Moreover, there doesn't seem to be any interest in actually fixing the
> > driver as it uses ASoC APIs that have been removed from the kernel in 2018
> > as shown by its use of snd_soc_register_codec and
> > snd_soc_codec_get_drvdata, removed in commit 999f7f5af8eb ("ASoC: remove
> > Codec related code").
> >
> > Signed-off-by: Alexandre Belloni 
> > ---
Hi Alexandre,

As mentioned in the mail archive referred, the GB Codec driver that is
currently available in kernel tree is dependent on qualcomm audio
driver. And some time later I made some minor modifications to remove
the unnecessary dependencies. However, I missed to share the same with
the community :(

Thanks to you for triggering the thread. Now, I could retrieve my local
changes and I have been in the process of updating it again to make it
compatible with latest kernel.

I'm planning to share the same here in the next few days.  I'll mark you
in CC to seek your review comments as well.

In case, I'm unable to make those changes to the staging tree, I would
also recommend to drop this code. Kindly let me know your opinion.

--
thanks,
./va

>
> Everything you say is true but it is still kinda sad to see this go.
> But that is life...  If and when someone has the motivation to get this
> working again they can take a look at the git history.
>
> Thanks for this, Alexandre.
>
> Acked-by: Mark Greer 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH V1 5/6] staging: greybus: audio: Add helper APIs for dynamic audio modules

[Vaibhav Agarwal]

Greybus Codec driver allows modules to be dynamically added and removed,
which further requires updating the DAPM configurations as well.

With current snd_soc architecture, dynamic audio modules is not yet
supported. This patch provides helper APIs to update DAPM configurations
in response to modules which are dynamically added or removed. The
source is primarily based on snd_dapm.c

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/Makefile   |   2 +-
 drivers/staging/greybus/audio_codec.c  |  13 ++-
 drivers/staging/greybus/audio_helper.c | 197 +
 drivers/staging/greybus/audio_helper.h |  17 +++
 4 files changed, 224 insertions(+), 5 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h

diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 627e44f2a983..3b4b6cabff19 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_GREYBUS_VIBRATOR)+= gb-vibrator.o
 
 # Greybus Audio is a bunch of modules
 gb-audio-module-y  := audio_module.o audio_topology.o
-gb-audio-codec-y   := audio_codec.o
+gb-audio-codec-y   := audio_codec.o audio_helper.o
 gb-audio-gb-y  := audio_gb.o
 gb-audio-apbridgea-y   := audio_apbridgea.o
 gb-audio-manager-y := audio_manager.o audio_manager_module.o
diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index bbd072acda5c..866b3342515f 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -14,6 +14,7 @@
 #include "audio_codec.h"
 #include "audio_apbridgea.h"
 #include "audio_manager.h"
+#include "audio_helper.h"
 
 static struct gbaudio_codec_info *gbcodec;
 
@@ -874,7 +875,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
 
/* card already instantiated, create widgets here only */
if (component->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(component->card,
+   gbaudio_dapm_link_component_dai_widgets(component->card,
>dapm);
 #ifdef CONFIG_SND_JACK
/*
@@ -1004,19 +1005,23 @@ void gbaudio_unregister_module(struct 
gbaudio_module_info *module)
if (module->dapm_routes) {
dev_dbg(component->dev, "Removing %d routes\n",
module->num_dapm_routes);
+   /* verify routes with controls are removed */
snd_soc_dapm_del_routes(>dapm, module->dapm_routes,
module->num_dapm_routes);
}
if (module->controls) {
dev_dbg(component->dev, "Removing %d controls\n",
module->num_controls);
-   snd_soc_remove_codec_controls(component, module->controls,
- module->num_controls);
+   /* release control semaphore */
+   up_write(>controls_rwsem);
+   gbaudio_remove_component_controls(component, module->controls,
+ module->num_controls);
+   down_write(>controls_rwsem);
}
if (module->dapm_widgets) {
dev_dbg(component->dev, "Removing %d widgets\n",
module->num_dapm_widgets);
-   snd_soc_dapm_free_controls(>dapm,
+   gbaudio_dapm_free_controls(>dapm,
   module->dapm_widgets,
   module->num_dapm_widgets);
}
diff --git a/drivers/staging/greybus/audio_helper.c 
b/drivers/staging/greybus/audio_helper.c
new file mode 100644
index ..e2f113a811ee
--- /dev/null
+++ b/drivers/staging/greybus/audio_helper.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Greybus Audio Sound SoC helper APIs
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#define gbaudio_dapm_for_each_direction(dir) \
+   for ((dir) = SND_SOC_DAPM_DIR_IN; (dir) <= SND_SOC_DAPM_DIR_OUT; \
+   (dir)++)
+
+static void gbaudio_dapm_link_dai_widget(struct snd_soc_dapm_widget *dai_w,
+struct snd_soc_card *card)
+{
+   struct snd_soc_dapm_widget *w;
+   struct snd_soc_dapm_widget *src, *sink;
+   struct snd_soc_dai *dai = dai_w->priv;
+
+   /* ...find all widgets with the same stream and link them */
+   list_for_each_entry(w, >widgets, list) {
+   if (w->dapm != dai_w->dapm)
+   continue;
+
+   switch (w->id) {
+   case snd_soc_dapm_dai_in:
+   case snd_soc_dapm_dai_ou

[PATCH V1 1/6] staging: greybus: audio: Update snd_jack FW usage as per new APIs

[Vaibhav Agarwal]

snd_soc_jack APIs are modified in recent kernel versions. This patch
updates the codec driver to resolve the compilation errors related to
jack framework.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c | 59 ---
 1 file changed, 47 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 08746c85dea6..ebf8484f0ae7 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -709,17 +709,29 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
 };
 
 static int gbaudio_init_jack(struct gbaudio_module_info *module,
-struct snd_soc_codec *codec)
+struct snd_soc_card *card)
 {
int ret;
 
+   struct snd_soc_jack *jack;
+   struct snd_soc_jack_pin *headset, *button;
+
if (!module->jack_mask)
return 0;
 
snprintf(module->jack_name, NAME_SIZE, "GB %d Headset Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->jack_name, module->jack_mask,
-  >headset_jack);
+
+   headset = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
+   if (!headset)
+   return -ENOMEM;
+
+   headset->pin = module->jack_name;
+   headset->mask = module->jack_mask;
+   jack = >headset_jack;
+
+   ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
+   jack, headset, 1);
if (ret) {
dev_err(module->dev, "Failed to create new jack\n");
return ret;
@@ -730,11 +742,21 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
snprintf(module->button_name, NAME_SIZE, "GB %d Button Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->button_name, module->button_mask,
-  >button_jack);
+   button = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
+   if (!button) {
+   ret = -ENOMEM;
+   goto free_headset;
+   }
+
+   button->pin = module->button_name;
+   button->mask = module->button_mask;
+   jack = >button_jack;
+
+   ret = snd_soc_card_jack_new(card, module->button_name,
+   module->button_mask, jack, button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   return ret;
+   goto free_headset;
}
 
/*
@@ -750,7 +772,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -759,7 +781,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -768,7 +790,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -777,7 +799,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -788,6 +810,18 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
*/
 
return 0;
+
+free_button:
+   jack = >button_jack;
+   snd_device_free(card->snd_card, jack->jack);
+   list_del(>list);
+
+free_headset:
+   jack = >headset_jack;
+   snd_device_free(card->snd_card, jack->jack);
+   list_del(>list);
+
+   return ret;
 }
 
 int gbaudio_register_module(struct gbaudio_module_info *module)
@@ -815,7 +849,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EINVAL;
}
 
-   ret = gbaudio_init_jack(module, codec);
+   ret = gbaudio_init_jack(module, component->card);
if (ret) {
up_write(>controls_rwsem);

[PATCH V1 0/6] Enable Greybus Audio codec driver

[Vaibhav Agarwal]

The existing GB Audio codec driver is dependent on MSM8994 Audio driver.
During the development stage, this depdency was configured due to
various changes involved in MSM Audio driver to enable addtional codec
card and some of the changes proposed in mainline ASoC framework.
However, these are not the real dependencies and some of them can be
easily removed.

The folowing patch series includes the changes to resolve unnecessary
depedencies and make the codec driver functional with the latest kernel.

Patch 1,2: Incudes jack framework related changes.
Patch 3,4,5: Resolves compilation error observed with the latest kernel and
also provides helper APIs required to allow dynamic addition/removal of
modules.
Patch 6: Finally provides config options and related Makefile changes to
enable GB Codec driver.

Thanks to Alexandre for raising the headsup [1] and motivating me to provide
the necessary changes.

[1] 
https://lore.kernel.org/lkml/20200507212912.599433-1-alexandre.bell...@bootlin.com/

Vaibhav Agarwal (6):
  staging: greybus: audio: Update snd_jack FW usage as per new APIs
  staging: greybus: audio: Maintain jack list within GB Audio module
  staging: greybus: audio: Resolve compilation errors for GB codec
module
  staging: greybus: audio: Resolve compilation error in topology parser
  staging: greybus: audio: Add helper APIs for dynamic audio modules
  staging: greybus: audio: Enable GB codec, audio module compilation.

 drivers/staging/greybus/Kconfig  |  14 ++-
 drivers/staging/greybus/Makefile |   6 +-
 drivers/staging/greybus/audio_codec.c| 187 ++---
 drivers/staging/greybus/audio_codec.h|  12 +-
 drivers/staging/greybus/audio_helper.c   | 197 +++
 drivers/staging/greybus/audio_helper.h   |  17 +++
 drivers/staging/greybus/audio_module.c   |  20 ++--
 drivers/staging/greybus/audio_topology.c | 130 ++--
 8 files changed, 427 insertions(+), 156 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH V1 4/6] staging: greybus: audio: Resolve compilation error in topology parser

[Vaibhav Agarwal]

Fix compilation errors for GB Audio topology parser code with recent
kernel versions.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_topology.c | 130 +++
 1 file changed, 61 insertions(+), 69 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 4ac30accf226..7d5e87341a5c 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -5,8 +5,8 @@
  * Copyright 2015-2016 Linaro Ltd.
  */
 
+#include 
 #include "audio_codec.h"
-#include "greybus_protocols.h"
 
 #define GBAUDIO_INVALID_ID 0xFF
 
@@ -165,15 +165,15 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_info *info;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gbcodec = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
info = (struct gb_audio_ctl_elem_info *)data->info;
 
if (!info) {
-   dev_err(codec->dev, "NULL info for %s\n", uinfo->id.name);
+   dev_err(comp->dev, "NULL info for %s\n", uinfo->id.name);
return -EINVAL;
}
 
@@ -193,7 +193,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
uinfo->value.enumerated.items = max;
if (uinfo->value.enumerated.item > max - 1)
uinfo->value.enumerated.item = max - 1;
-   module = find_gb_module(gbcodec, kcontrol->id.name);
+   module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
name = gbaudio_map_controlid(module, data->ctl_id,
@@ -201,7 +201,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
strlcpy(uinfo->value.enumerated.name, name, NAME_SIZE);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
break;
}
@@ -216,11 +216,11 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
struct gb_bundle *bundle;
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
@@ -239,7 +239,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
gb_pm_runtime_put_autosuspend(bundle);
 
if (ret) {
-   dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret,
+   dev_err_ratelimited(comp->dev, "%d:Error in %s for %s\n", ret,
__func__, kcontrol->id.name);
return ret;
}
@@ -262,7 +262,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
ret = -EINVAL;
break;
@@ -278,11 +278,11 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kc

[PATCH V1 2/6] staging: greybus: audio: Maintain jack list within GB Audio module

[Vaibhav Agarwal]

As per the current implementation for GB codec driver, a jack list is
maintained for each module. And it expects the list to be populated by
the snd_soc_jack structure which would require modifications in
mainstream code.

However, this is not a necessary requirement and the list can be easily
maintained within gbaudio_module_info as well. This patch provides the
relevant changes for the same.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c  | 76 ++
 drivers/staging/greybus/audio_codec.h  | 10 -
 drivers/staging/greybus/audio_module.c | 20 +
 3 files changed, 60 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index ebf8484f0ae7..a2ee587e5a79 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -712,7 +712,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 struct snd_soc_card *card)
 {
int ret;
-
+   struct gbaudio_jack *gba_jack, *n;
struct snd_soc_jack *jack;
struct snd_soc_jack_pin *headset, *button;
 
@@ -728,7 +728,8 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
headset->pin = module->jack_name;
headset->mask = module->jack_mask;
-   jack = >headset_jack;
+   gba_jack = >headset;
+   jack = _jack->jack;
 
ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
jack, headset, 1);
@@ -737,6 +738,9 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
return ret;
}
 
+   /* Add to module's jack list */
+   list_add(_jack->list, >jack_list);
+
if (!module->button_mask)
return 0;
 
@@ -745,20 +749,24 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
button = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
if (!button) {
ret = -ENOMEM;
-   goto free_headset;
+   goto free_jack;
}
 
button->pin = module->button_name;
button->mask = module->button_mask;
-   jack = >button_jack;
+   gba_jack = >button;
+   jack = _jack->jack;
 
ret = snd_soc_card_jack_new(card, module->button_name,
module->button_mask, jack, button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   goto free_headset;
+   goto free_jack;
}
 
+   /* Add to module's jack list */
+   list_add(_jack->list, >jack_list);
+
/*
 * Currently, max 4 buttons are supported with following key mapping
 * BTN_0 = KEY_MEDIA
@@ -768,58 +776,55 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 */
 
if (module->button_mask & SND_JACK_BTN_0) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_0,
+   ret = snd_jack_set_key(jack->jack, SND_JACK_BTN_0,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   goto free_button;
+   goto free_jack;
}
}
 
if (module->button_mask & SND_JACK_BTN_1) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_1,
+   ret = snd_jack_set_key(jack->jack, SND_JACK_BTN_1,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   goto free_button;
+   goto free_jack;
}
}
 
if (module->button_mask & SND_JACK_BTN_2) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_2,
+   ret = snd_jack_set_key(jack->jack, SND_JACK_BTN_2,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   goto free_button;
+   goto free_jack;
}
}
 
if (module->button_mask & SND_JACK_BTN_3) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_3,
+   ret = snd_jack_set_key(jack->jack, SND_JACK_BTN_3,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   goto free_button;
+   goto free_jack;
}
}
 
/* 

[PATCH V1 6/6] staging: greybus: audio: Enable GB codec, audio module compilation.

[Vaibhav Agarwal]

Currently, GB codec and audio module is conditionally compiled based on
GREYBUS_AUDIO_MSM8994. However, audio module is not dependent on MSM8994
platform and can be used generically with any platform that follows
GB Audio class specification.

Also, GB codec driver corresponds to dummy codec represented by I2S port
available on Toshiba AP Bridge. Added config option for the same in
kconfig file and accordingly updated Makefile.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/Kconfig  | 14 +-
 drivers/staging/greybus/Makefile |  4 ++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
index d4777f5a8b90..cbcfcbba239b 100644
--- a/drivers/staging/greybus/Kconfig
+++ b/drivers/staging/greybus/Kconfig
@@ -3,7 +3,7 @@ if GREYBUS
 
 config GREYBUS_AUDIO
tristate "Greybus Audio Class driver"
-   depends on SOUND
+   depends on SOUND && SND_SOC
---help---
  Select this option if you have a device that follows the
  Greybus Audio Class specification.
@@ -11,6 +11,18 @@ config GREYBUS_AUDIO
  To compile this code as a module, chose M here: the module
  will be called gb-audio.ko
 
+config GREYBUS_AUDIO_APB_CODEC
+   tristate "Greybus APBridge Audio codec driver"
+   depends on SND_SOC && GREYBUS_AUDIO
+   help
+ Select this option if you have a Toshiba APB device that has I2S
+  ports and acts as a Greybus "Dummy codec". This device is a
+  bridge from an APB-I2S port to a Unipro network.
+
+ To compile this code as a module, chose M here: the module
+ will be called gb-audio-codec.ko
+
+
 config GREYBUS_BOOTROM
tristate "Greybus Bootrom Class driver"
---help---
diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 3b4b6cabff19..7c5e89622334 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -40,8 +40,8 @@ gb-audio-manager-y:= audio_manager.o 
audio_manager_module.o
 #ccflags-y += -DGB_AUDIO_MANAGER_SYSFS
 #endif
 
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-codec.o
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-module.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-codec.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-module.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-gb.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-apbridgea.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-manager.o
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH V1 3/6] staging: greybus: audio: Resolve compilation errors for GB codec module

[Vaibhav Agarwal]

Due to dependencies on ASoC framework changes, GB dummy codec module
compilation is currently disabled. This patch updates codec driver as
per the latest ASoC APIs.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c | 87 +--
 drivers/staging/greybus/audio_codec.h |  2 +-
 2 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index a2ee587e5a79..bbd072acda5c 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -832,7 +832,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 int gbaudio_register_module(struct gbaudio_module_info *module)
 {
int ret;
-   struct snd_soc_codec *codec;
+   struct snd_soc_component *component;
struct snd_card *card;
struct gbaudio_jack *gba_jack = NULL;
struct snd_soc_jack *jack = NULL;
@@ -842,8 +842,8 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EAGAIN;
}
 
-   codec = gbcodec->codec;
-   card = codec->card->snd_card;
+   component = gbcodec->component;
+   card = component->card->snd_card;
 
down_write(>controls_rwsem);
 
@@ -862,19 +862,20 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
}
 
if (module->dapm_widgets)
-   snd_soc_dapm_new_controls(>dapm, module->dapm_widgets,
+   snd_soc_dapm_new_controls(>dapm,
+ module->dapm_widgets,
  module->num_dapm_widgets);
if (module->controls)
-   snd_soc_add_codec_controls(codec, module->controls,
-  module->num_controls);
+   snd_soc_add_component_controls(component, module->controls,
+  module->num_controls);
if (module->dapm_routes)
-   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
+   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
module->num_dapm_routes);
 
/* card already instantiated, create widgets here only */
-   if (codec->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(codec->card,
-   >dapm);
+   if (component->card->instantiated) {
+   snd_soc_dapm_link_component_dai_widgets(component->card,
+   >dapm);
 #ifdef CONFIG_SND_JACK
/*
 * register jack devices for this module
@@ -882,7 +883,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
 */
list_for_each_entry(gba_jack, >jack_list, list) {
jack = _jack->jack;
-   snd_device_register(codec->card->snd_card,
+   snd_device_register(component->card->snd_card,
jack->jack);
}
 #endif
@@ -892,9 +893,9 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
list_add(>list, >module_list);
mutex_unlock(>lock);
 
-   if (codec->card->instantiated)
-   ret = snd_soc_dapm_new_widgets(>dapm);
-   dev_dbg(codec->dev, "Registered %s module\n", module->name);
+   if (component->card->instantiated)
+   ret = snd_soc_dapm_new_widgets(component->card);
+   dev_dbg(component->dev, "Registered %s module\n", module->name);
 
up_write(>controls_rwsem);
return ret;
@@ -965,19 +966,19 @@ static void gbaudio_codec_cleanup(struct 
gbaudio_module_info *module)
 
 void gbaudio_unregister_module(struct gbaudio_module_info *module)
 {
-   struct snd_soc_codec *codec = gbcodec->codec;
-   struct snd_card *card = codec->card->snd_card;
+   struct snd_soc_component *component = gbcodec->component;
+   struct snd_card *card = component->card->snd_card;
struct gbaudio_jack *gba_jack, *n;
struct snd_soc_jack *jack;
int mask;
 
-   dev_dbg(codec->dev, "Unregister %s module\n", module->name);
+   dev_dbg(component->dev, "Unregister %s module\n", module->name);
 
down_write(>controls_rwsem);
mutex_lock(>lock);
gbaudio_codec_cleanup(module);
list_del(>list);
-   dev_dbg(codec->dev, "Process Unregister %s module\n", module->name);
+   dev_dbg(component->dev, "Process Unregister %s module\n", module->name);
mutex_unlock(>lock);
 
 #ifdef CONFIG_

Re: [PATCH V1 5/6] staging: greybus: audio: Add helper APIs for dynamic audio modules

[Vaibhav Agarwal]

On Sun, May 17, 2020 at 07:37:06PM +0200, Alexandre Belloni wrote:
> Hi,
> 
> On 17/05/2020 22:47:20+0530, Vaibhav Agarwal wrote:
> > Greybus Codec driver allows modules to be dynamically added and removed,
> > which further requires updating the DAPM configurations as well.
> > 
> > With current snd_soc architecture, dynamic audio modules is not yet
> > supported. This patch provides helper APIs to update DAPM configurations
> > in response to modules which are dynamically added or removed. The
> > source is primarily based on snd_dapm.c
> > 
> 
> I really think you should send this patch series to the ASoC
> maintainers, especially this patch. The main goal shouldn't be to simply
> fix compilation issues but to try to get the driver out of staging else,
> the current situation will happen again.

Agree Alexandre. I'll share this with ASoC maintainers as well.

Also, I'm seeking opinion regarding the scope of pushing GB Audio out of 
staging tree. I'm keen to make the relevant changes and work actively 
for the same. However, I don't have a real device to test the latest 
code and thus not sure if the changes can be pushed to sound soc tree.
GB maintainers, kindly share your opinion.

--
Regards,
Vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/3] staging: greybus: fix warnings about endianness detected by sparse

[Vaibhav Agarwal]

On Sat, Oct 3, 2020 at 5:01 AM Coiby Xu  wrote:
>
> This patch fix the following warnings from sparse,
>
> $ make C=2 drivers/staging/greybus/
> drivers/staging/greybus/audio_module.c:222:25: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_module.c:222:25:expected restricted __le16 
> [usertype] data_cport
> drivers/staging/greybus/audio_module.c:222:25:got unsigned short 
> [usertype] intf_cport_id
> drivers/staging/greybus/audio_topology.c:460:40: warning: restricted __le32 
> degrades to integer
> drivers/staging/greybus/audio_topology.c:691:41: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:691:41:expected unsigned int 
> access
> drivers/staging/greybus/audio_topology.c:691:41:got restricted __le32 
> [usertype] access
> drivers/staging/greybus/audio_topology.c:746:44: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:746:44:expected unsigned int
> drivers/staging/greybus/audio_topology.c:746:44:got restricted __le32
> drivers/staging/greybus/audio_topology.c:748:52: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:748:52:expected unsigned int
> drivers/staging/greybus/audio_topology.c:748:52:got restricted __le32
> drivers/staging/greybus/audio_topology.c:802:42: warning: restricted __le32 
> degrades to integer
> drivers/staging/greybus/audio_topology.c:805:50: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:805:50:expected restricted __le32
> drivers/staging/greybus/audio_topology.c:805:50:got unsigned int
> drivers/staging/greybus/audio_topology.c:814:50: warning: restricted __le32 
> degrades to integer
> drivers/staging/greybus/audio_topology.c:817:58: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:817:58:expected restricted __le32
> drivers/staging/greybus/audio_topology.c:817:58:got unsigned int
> drivers/staging/greybus/audio_topology.c:889:25: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:889:25:expected unsigned int 
> access
> drivers/staging/greybus/audio_topology.c:889:25:got restricted __le32 
> [usertype] access
>
> Suggested-by: Dan Carpenter 
> Reviewed-by: Dan Carpenter 
> Reviewed-by: Alex Elder 
> Signed-off-by: Coiby Xu 
> ---
Hi Coiby,

Thanks for sharing the patch. Sorry, I could not reply to the v1 series.
Now, I have gone through the patches. Looks good (all 3 patches).

Reviewed-by: Vaibhav Agarwal 

--
Thanks,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RESEND PATCH v1 1/6] staging: greybus: audio: Update snd_jack FW usage as per new APIs

[Vaibhav Agarwal]

snd_soc_jack APIs are modified in recent kernel versions. This patch
updates the codec driver to resolve the compilation errors related to
jack framework.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c | 59 +--
 1 file changed, 47 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 08746c85dea6..ebf8484f0ae7 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -709,17 +709,29 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
 };
 
 static int gbaudio_init_jack(struct gbaudio_module_info *module,
-struct snd_soc_codec *codec)
+struct snd_soc_card *card)
 {
int ret;
 
+   struct snd_soc_jack *jack;
+   struct snd_soc_jack_pin *headset, *button;
+
if (!module->jack_mask)
return 0;
 
snprintf(module->jack_name, NAME_SIZE, "GB %d Headset Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->jack_name, module->jack_mask,
-  >headset_jack);
+
+   headset = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
+   if (!headset)
+   return -ENOMEM;
+
+   headset->pin = module->jack_name;
+   headset->mask = module->jack_mask;
+   jack = >headset_jack;
+
+   ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
+   jack, headset, 1);
if (ret) {
dev_err(module->dev, "Failed to create new jack\n");
return ret;
@@ -730,11 +742,21 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
snprintf(module->button_name, NAME_SIZE, "GB %d Button Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->button_name, module->button_mask,
-  >button_jack);
+   button = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
+   if (!button) {
+   ret = -ENOMEM;
+   goto free_headset;
+   }
+
+   button->pin = module->button_name;
+   button->mask = module->button_mask;
+   jack = >button_jack;
+
+   ret = snd_soc_card_jack_new(card, module->button_name,
+   module->button_mask, jack, button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   return ret;
+   goto free_headset;
}
 
/*
@@ -750,7 +772,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -759,7 +781,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -768,7 +790,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -777,7 +799,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -788,6 +810,18 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
*/
 
return 0;
+
+free_button:
+   jack = >button_jack;
+   snd_device_free(card->snd_card, jack->jack);
+   list_del(>list);
+
+free_headset:
+   jack = >headset_jack;
+   snd_device_free(card->snd_card, jack->jack);
+   list_del(>list);
+
+   return ret;
 }
 
 int gbaudio_register_module(struct gbaudio_module_info *module)
@@ -815,7 +849,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EINVAL;
}
 
-   ret = gbaudio_init_jack(module, codec);
+   ret = gbaudio_init_jack(module, component->card);
if (ret) {
up_write(>controls_rwsem);

[RESEND PATCH v1 3/6] staging: greybus: audio: Resolve compilation errors for GB codec module

[Vaibhav Agarwal]

Due to dependencies on ASoC framework changes, GB dummy codec module
compilation is currently disabled. This patch updates codec driver as
per the latest ASoC APIs.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c | 87 +--
 drivers/staging/greybus/audio_codec.h |  2 +-
 2 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index a2ee587e5a79..bbd072acda5c 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -832,7 +832,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 int gbaudio_register_module(struct gbaudio_module_info *module)
 {
int ret;
-   struct snd_soc_codec *codec;
+   struct snd_soc_component *component;
struct snd_card *card;
struct gbaudio_jack *gba_jack = NULL;
struct snd_soc_jack *jack = NULL;
@@ -842,8 +842,8 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EAGAIN;
}
 
-   codec = gbcodec->codec;
-   card = codec->card->snd_card;
+   component = gbcodec->component;
+   card = component->card->snd_card;
 
down_write(>controls_rwsem);
 
@@ -862,19 +862,20 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
}
 
if (module->dapm_widgets)
-   snd_soc_dapm_new_controls(>dapm, module->dapm_widgets,
+   snd_soc_dapm_new_controls(>dapm,
+ module->dapm_widgets,
  module->num_dapm_widgets);
if (module->controls)
-   snd_soc_add_codec_controls(codec, module->controls,
-  module->num_controls);
+   snd_soc_add_component_controls(component, module->controls,
+  module->num_controls);
if (module->dapm_routes)
-   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
+   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
module->num_dapm_routes);
 
/* card already instantiated, create widgets here only */
-   if (codec->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(codec->card,
-   >dapm);
+   if (component->card->instantiated) {
+   snd_soc_dapm_link_component_dai_widgets(component->card,
+   >dapm);
 #ifdef CONFIG_SND_JACK
/*
 * register jack devices for this module
@@ -882,7 +883,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
 */
list_for_each_entry(gba_jack, >jack_list, list) {
jack = _jack->jack;
-   snd_device_register(codec->card->snd_card,
+   snd_device_register(component->card->snd_card,
jack->jack);
}
 #endif
@@ -892,9 +893,9 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
list_add(>list, >module_list);
mutex_unlock(>lock);
 
-   if (codec->card->instantiated)
-   ret = snd_soc_dapm_new_widgets(>dapm);
-   dev_dbg(codec->dev, "Registered %s module\n", module->name);
+   if (component->card->instantiated)
+   ret = snd_soc_dapm_new_widgets(component->card);
+   dev_dbg(component->dev, "Registered %s module\n", module->name);
 
up_write(>controls_rwsem);
return ret;
@@ -965,19 +966,19 @@ static void gbaudio_codec_cleanup(struct 
gbaudio_module_info *module)
 
 void gbaudio_unregister_module(struct gbaudio_module_info *module)
 {
-   struct snd_soc_codec *codec = gbcodec->codec;
-   struct snd_card *card = codec->card->snd_card;
+   struct snd_soc_component *component = gbcodec->component;
+   struct snd_card *card = component->card->snd_card;
struct gbaudio_jack *gba_jack, *n;
struct snd_soc_jack *jack;
int mask;
 
-   dev_dbg(codec->dev, "Unregister %s module\n", module->name);
+   dev_dbg(component->dev, "Unregister %s module\n", module->name);
 
down_write(>controls_rwsem);
mutex_lock(>lock);
gbaudio_codec_cleanup(module);
list_del(>list);
-   dev_dbg(codec->dev, "Process Unregister %s module\n", module->name);
+   dev_dbg(component->dev, "Process Unregister %s module\n", module->name);
mutex_unlock(>lock);
 
 #ifdef CONFIG_

[RESEND PATCH v1 6/6] staging: greybus: audio: Enable GB codec, audio module compilation.

[Vaibhav Agarwal]

Currently, GB codec and audio module is conditionally compiled based on
GREYBUS_AUDIO_MSM8994. However, audio module is not dependent on MSM8994
platform and can be used generically with any platform that follows
GB Audio class specification.

Also, GB codec driver corresponds to dummy codec represented by I2S port
available on Toshiba AP Bridge. Added config option for the same in
kconfig file and accordingly updated Makefile.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/Kconfig  | 14 +-
 drivers/staging/greybus/Makefile |  4 ++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
index d4777f5a8b90..cbcfcbba239b 100644
--- a/drivers/staging/greybus/Kconfig
+++ b/drivers/staging/greybus/Kconfig
@@ -3,7 +3,7 @@ if GREYBUS
 
 config GREYBUS_AUDIO
tristate "Greybus Audio Class driver"
-   depends on SOUND
+   depends on SOUND && SND_SOC
---help---
  Select this option if you have a device that follows the
  Greybus Audio Class specification.
@@ -11,6 +11,18 @@ config GREYBUS_AUDIO
  To compile this code as a module, chose M here: the module
  will be called gb-audio.ko
 
+config GREYBUS_AUDIO_APB_CODEC
+   tristate "Greybus APBridge Audio codec driver"
+   depends on SND_SOC && GREYBUS_AUDIO
+   help
+ Select this option if you have a Toshiba APB device that has I2S
+  ports and acts as a Greybus "Dummy codec". This device is a
+  bridge from an APB-I2S port to a Unipro network.
+
+ To compile this code as a module, chose M here: the module
+ will be called gb-audio-codec.ko
+
+
 config GREYBUS_BOOTROM
tristate "Greybus Bootrom Class driver"
---help---
diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 3b4b6cabff19..7c5e89622334 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -40,8 +40,8 @@ gb-audio-manager-y:= audio_manager.o 
audio_manager_module.o
 #ccflags-y += -DGB_AUDIO_MANAGER_SYSFS
 #endif
 
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-codec.o
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-module.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-codec.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-module.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-gb.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-apbridgea.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-manager.o
-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RESEND PATCH v1 4/6] staging: greybus: audio: Resolve compilation error in topology parser

[Vaibhav Agarwal]

Fix compilation errors for GB Audio topology parser code with recent
kernel versions.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_topology.c | 130 +++
 1 file changed, 61 insertions(+), 69 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 4ac30accf226..7d5e87341a5c 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -5,8 +5,8 @@
  * Copyright 2015-2016 Linaro Ltd.
  */
 
+#include 
 #include "audio_codec.h"
-#include "greybus_protocols.h"
 
 #define GBAUDIO_INVALID_ID 0xFF
 
@@ -165,15 +165,15 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_info *info;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gbcodec = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
info = (struct gb_audio_ctl_elem_info *)data->info;
 
if (!info) {
-   dev_err(codec->dev, "NULL info for %s\n", uinfo->id.name);
+   dev_err(comp->dev, "NULL info for %s\n", uinfo->id.name);
return -EINVAL;
}
 
@@ -193,7 +193,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
uinfo->value.enumerated.items = max;
if (uinfo->value.enumerated.item > max - 1)
uinfo->value.enumerated.item = max - 1;
-   module = find_gb_module(gbcodec, kcontrol->id.name);
+   module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
name = gbaudio_map_controlid(module, data->ctl_id,
@@ -201,7 +201,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
strlcpy(uinfo->value.enumerated.name, name, NAME_SIZE);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
break;
}
@@ -216,11 +216,11 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
struct gb_bundle *bundle;
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
@@ -239,7 +239,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
gb_pm_runtime_put_autosuspend(bundle);
 
if (ret) {
-   dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret,
+   dev_err_ratelimited(comp->dev, "%d:Error in %s for %s\n", ret,
__func__, kcontrol->id.name);
return ret;
}
@@ -262,7 +262,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
ret = -EINVAL;
break;
@@ -278,11 +278,11 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+ 

[RESEND PATCH v1 0/6] Enable Greybus Audio codec driver

[Vaibhav Agarwal]

[REQUEST]

This patch series intends to "Enable Greybus Audio codec driver"
existing in the staging tree. I have shared the original patch series with
Greybus-Dev mailing list and as per the suggestion from Alexandre, I'm
also interested to push Greybus Audio to sound soc tree. Thus, now I'm
resending it to ASoc maintainers for review.

Following is the top level SW design for GB Codec driver and GB Audio
modules.

+--+
+-->+GBA Module 1  |
|   +--+
+---+
||  |
|| Greybus  |   +--+
| SND SOC| Audio+-->+GBA Module 2  |
| Framework  | Codec|   +--+
|| Driver   |
||  |
+---+   +--+
+-->+GBA Module N  |
+--+

Patch 5 contains the changes to provide helper APIs to link DAPM DAI widgets
for the module added and remove/free component controls for the module removed
dynamically. Eventually, I propose to extend snd_soc_xxx APIs for this
purpose.

Kindly share your inputs.

[COVER LETTER]

The existing GB Audio codec driver is dependent on MSM8994 Audio driver.
During the development stage, this depdency was configured due to
various changes involved in MSM Audio driver to enable addtional codec
card and some of the changes proposed in mainline ASoC framework.
However, these are not the real dependencies and some of them can be
easily removed.

The folowing patch series includes the changes to resolve unnecessary
depedencies and make the codec driver functional with the latest kernel.

Patch 1,2: Incudes jack framework related changes.
Patch 3,4,5: Resolves compilation error observed with the latest kernel and
also provides helper APIs required to allow dynamic addition/removal of
modules.
Patch 6: Finally provides config options and related Makefile changes to
enable GB Codec driver.

Thanks to Alexandre for raising the headsup [1] and motivating me to provide
the necessary changes.

[1] 
https://lore.kernel.org/lkml/20200507212912.599433-1-alexandre.bell...@bootlin.com/


Vaibhav Agarwal (6):
  staging: greybus: audio: Update snd_jack FW usage as per new APIs
  staging: greybus: audio: Maintain jack list within GB Audio module
  staging: greybus: audio: Resolve compilation errors for GB codec
module
  staging: greybus: audio: Resolve compilation error in topology parser
  staging: greybus: audio: Add helper APIs for dynamic audio modules
  staging: greybus: audio: Enable GB codec, audio module compilation.

 drivers/staging/greybus/Kconfig  |  14 +-
 drivers/staging/greybus/Makefile |   6 +-
 drivers/staging/greybus/audio_codec.c| 187 +
 drivers/staging/greybus/audio_codec.h|  12 +-
 drivers/staging/greybus/audio_helper.c   | 197 +++
 drivers/staging/greybus/audio_helper.h   |  17 ++
 drivers/staging/greybus/audio_module.c   |  20 +--
 drivers/staging/greybus/audio_topology.c | 130 +++
 8 files changed, 427 insertions(+), 156 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h


base-commit: ae73e7784871ebe2c43da619b4a1e2c9ff81508d
-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RESEND PATCH v1 2/6] staging: greybus: audio: Maintain jack list within GB Audio module

[Vaibhav Agarwal]

As per the current implementation for GB codec driver, a jack list is
maintained for each module. And it expects the list to be populated by
the snd_soc_jack structure which would require modifications in
mainstream code.

However, this is not a necessary requirement and the list can be easily
maintained within gbaudio_module_info as well. This patch provides the
relevant changes for the same.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c  | 76 ++
 drivers/staging/greybus/audio_codec.h  | 10 +++-
 drivers/staging/greybus/audio_module.c | 20 ---
 3 files changed, 60 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index ebf8484f0ae7..a2ee587e5a79 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -712,7 +712,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 struct snd_soc_card *card)
 {
int ret;
-
+   struct gbaudio_jack *gba_jack, *n;
struct snd_soc_jack *jack;
struct snd_soc_jack_pin *headset, *button;
 
@@ -728,7 +728,8 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
headset->pin = module->jack_name;
headset->mask = module->jack_mask;
-   jack = >headset_jack;
+   gba_jack = >headset;
+   jack = _jack->jack;
 
ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
jack, headset, 1);
@@ -737,6 +738,9 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
return ret;
}
 
+   /* Add to module's jack list */
+   list_add(_jack->list, >jack_list);
+
if (!module->button_mask)
return 0;
 
@@ -745,20 +749,24 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
button = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
if (!button) {
ret = -ENOMEM;
-   goto free_headset;
+   goto free_jack;
}
 
button->pin = module->button_name;
button->mask = module->button_mask;
-   jack = >button_jack;
+   gba_jack = >button;
+   jack = _jack->jack;
 
ret = snd_soc_card_jack_new(card, module->button_name,
module->button_mask, jack, button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   goto free_headset;
+   goto free_jack;
}
 
+   /* Add to module's jack list */
+   list_add(_jack->list, >jack_list);
+
/*
 * Currently, max 4 buttons are supported with following key mapping
 * BTN_0 = KEY_MEDIA
@@ -768,58 +776,55 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 */
 
if (module->button_mask & SND_JACK_BTN_0) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_0,
+   ret = snd_jack_set_key(jack->jack, SND_JACK_BTN_0,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   goto free_button;
+   goto free_jack;
}
}
 
if (module->button_mask & SND_JACK_BTN_1) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_1,
+   ret = snd_jack_set_key(jack->jack, SND_JACK_BTN_1,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   goto free_button;
+   goto free_jack;
}
}
 
if (module->button_mask & SND_JACK_BTN_2) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_2,
+   ret = snd_jack_set_key(jack->jack, SND_JACK_BTN_2,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   goto free_button;
+   goto free_jack;
}
}
 
if (module->button_mask & SND_JACK_BTN_3) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_3,
+   ret = snd_jack_set_key(jack->jack, SND_JACK_BTN_3,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   goto free_button;
+   goto free_jack;
}
}
 
/* FIXME

[RESEND PATCH v1 5/6] staging: greybus: audio: Add helper APIs for dynamic audio modules

[Vaibhav Agarwal]

Greybus Codec driver allows modules to be dynamically added and removed,
which further requires updating the DAPM configurations as well.

With current snd_soc architecture, dynamic audio modules is not yet
supported. This patch provides helper APIs to update DAPM configurations
in response to modules which are dynamically added or removed. The
source is primarily based on snd_dapm.c

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/Makefile   |   2 +-
 drivers/staging/greybus/audio_codec.c  |  13 +-
 drivers/staging/greybus/audio_helper.c | 197 +
 drivers/staging/greybus/audio_helper.h |  17 +++
 4 files changed, 224 insertions(+), 5 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h

diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 627e44f2a983..3b4b6cabff19 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_GREYBUS_VIBRATOR)+= gb-vibrator.o
 
 # Greybus Audio is a bunch of modules
 gb-audio-module-y  := audio_module.o audio_topology.o
-gb-audio-codec-y   := audio_codec.o
+gb-audio-codec-y   := audio_codec.o audio_helper.o
 gb-audio-gb-y  := audio_gb.o
 gb-audio-apbridgea-y   := audio_apbridgea.o
 gb-audio-manager-y := audio_manager.o audio_manager_module.o
diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index bbd072acda5c..866b3342515f 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -14,6 +14,7 @@
 #include "audio_codec.h"
 #include "audio_apbridgea.h"
 #include "audio_manager.h"
+#include "audio_helper.h"
 
 static struct gbaudio_codec_info *gbcodec;
 
@@ -874,7 +875,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
 
/* card already instantiated, create widgets here only */
if (component->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(component->card,
+   gbaudio_dapm_link_component_dai_widgets(component->card,
>dapm);
 #ifdef CONFIG_SND_JACK
/*
@@ -1004,19 +1005,23 @@ void gbaudio_unregister_module(struct 
gbaudio_module_info *module)
if (module->dapm_routes) {
dev_dbg(component->dev, "Removing %d routes\n",
module->num_dapm_routes);
+   /* verify routes with controls are removed */
snd_soc_dapm_del_routes(>dapm, module->dapm_routes,
module->num_dapm_routes);
}
if (module->controls) {
dev_dbg(component->dev, "Removing %d controls\n",
module->num_controls);
-   snd_soc_remove_codec_controls(component, module->controls,
- module->num_controls);
+   /* release control semaphore */
+   up_write(>controls_rwsem);
+   gbaudio_remove_component_controls(component, module->controls,
+ module->num_controls);
+   down_write(>controls_rwsem);
}
if (module->dapm_widgets) {
dev_dbg(component->dev, "Removing %d widgets\n",
module->num_dapm_widgets);
-   snd_soc_dapm_free_controls(>dapm,
+   gbaudio_dapm_free_controls(>dapm,
   module->dapm_widgets,
   module->num_dapm_widgets);
}
diff --git a/drivers/staging/greybus/audio_helper.c 
b/drivers/staging/greybus/audio_helper.c
new file mode 100644
index ..e2f113a811ee
--- /dev/null
+++ b/drivers/staging/greybus/audio_helper.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Greybus Audio Sound SoC helper APIs
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#define gbaudio_dapm_for_each_direction(dir) \
+   for ((dir) = SND_SOC_DAPM_DIR_IN; (dir) <= SND_SOC_DAPM_DIR_OUT; \
+   (dir)++)
+
+static void gbaudio_dapm_link_dai_widget(struct snd_soc_dapm_widget *dai_w,
+struct snd_soc_card *card)
+{
+   struct snd_soc_dapm_widget *w;
+   struct snd_soc_dapm_widget *src, *sink;
+   struct snd_soc_dai *dai = dai_w->priv;
+
+   /* ...find all widgets with the same stream and link them */
+   list_for_each_entry(w, >widgets, list) {
+   if (w->dapm != dai_w->dapm)
+   continue;
+
+   switch (w->id) {
+   case snd_soc_dapm_dai_in:
+   case snd_soc_dapm_dai_out:
+   

[PATCH v1] staging: greybus: audio: fix uninitialized value issue

[Vaibhav Agarwal]

The current implementation for gbcodec_mixer_dapm_ctl_put() uses
uninitialized gbvalue for comparison with updated value. This was found
using static analysis with coverity.

Uninitialized scalar variable (UNINIT)
11. uninit_use: Using uninitialized value
gbvalue.value.integer_value[0].
460if (gbvalue.value.integer_value[0] != val) {

This patch fixes the issue with fetching the gbvalue before using it for
comparision.

Fixes: 6339d2322c47 ("greybus: audio: Add topology parser for GB codec")
Reported-by: Colin Ian King 
Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_topology.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 2f9fdbdcd547..4b914d0edef2 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -456,6 +456,13 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
val = ucontrol->value.integer.value[0] & mask;
connect = !!val;
 
+   ret = gb_pm_runtime_get_sync(bundle);
+   if (ret)
+   return ret;
+
+   ret = gb_audio_gb_get_control(module->mgmt_connection, data->ctl_id,
+ GB_AUDIO_INVALID_INDEX, );
+
/* update ucontrol */
if (gbvalue.value.integer_value[0] != val) {
for (wi = 0; wi < wlist->num_widgets; wi++) {
@@ -466,16 +473,10 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
gbvalue.value.integer_value[0] =
cpu_to_le32(ucontrol->value.integer.value[0]);
 
-   ret = gb_pm_runtime_get_sync(bundle);
-   if (ret)
-   return ret;
-
ret = gb_audio_gb_set_control(module->mgmt_connection,
  data->ctl_id,
  GB_AUDIO_INVALID_INDEX, );
 
-   gb_pm_runtime_put_autosuspend(bundle);
-
if (ret) {
dev_err_ratelimited(codec_dev,
"%d:Error in %s for %s\n", ret,
@@ -483,6 +484,7 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
return ret;
}
}
+   gb_pm_runtime_put_autosuspend(bundle);
 
return 0;
 }

base-commit: 5bbd90550da8f7bdac769b5825597e67183c9411
prerequisite-patch-id: 2b8901339222ff7b94f10cf2341734c0fb82591c
prerequisite-patch-id: 38dad8879a2b73bce6e89481973c7c5b82bd7145
prerequisite-patch-id: 5f0042ccedae292395ec617789be6bf465463c1c
prerequisite-patch-id: 35d001c366dfa4b567e59abbb37bd691a18f5e14
prerequisite-patch-id: f13ce918ebc3796cd3c81716a7b2adf4519e7387
prerequisite-patch-id: 0fcc6d38699a9b72ca94280d7a4dc18f0823b6f7
prerequisite-patch-id: 8074e935bdc3dd7b114245b0648552d0ff6871c9
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: issue with uninitialized value used in a comparison in gbcodec_mixer_dapm_ctl_put

[Vaibhav Agarwal]

On Wed, Aug 05, 2020 at 08:35:15AM -0500, Alex Elder wrote:



> 
> I think the fix is to add a call to this:
> 
> ret = gb_audio_gb_get_control(module->mgmt_connection, data->ctl_id,
>   GB_AUDIO_INVALID_INDEX, );
> 
> before the field within gbvalue is used.
> 
> Looking at gbcodec_mixer_dapm_ctl_get() defined just above that, it
> seems that the call to gb_audio_gb_get_control() should be preceded
> by a call to gb_pm_runtime_get_sync().  And given that duplication,
> I suggest this call and the PM runtime wrapper functions should be
> placed in a new helper function.
> 
> I know that Vaibhav said he would be fixing this, so I guess my
> comments are directed at him.  Thanks for sending the patch Colin.
> 
>   -Alex

Thanks Alex. I'll share a patch with the proposed fix.

--
vaibhav

> 
> 
> > Colin
> > 
> > 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: audio: Uninitialized variable in gbaudio_remove_controls()

[Vaibhav Agarwal]

On Tue, Aug 4, 2020 at 3:46 PM Dan Carpenter  wrote:
>
> The "err" variable is not meaningful so there is no need to print it.
> It's uninitialized on the first iteration through the loop.
>
> Fixes: 510e340efe0c ("staging: greybus: audio: Add helper APIs for dynamic 
> audio modules")
> Signed-off-by: Dan Carpenter 

Thanks Dan for sharing this patch.

Reviewed-by: Vaibhav Agarwal 

> ---
>  drivers/staging/greybus/audio_helper.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_helper.c 
> b/drivers/staging/greybus/audio_helper.c
> index 8b100a71f02e..237531ba60f3 100644
> --- a/drivers/staging/greybus/audio_helper.c
> +++ b/drivers/staging/greybus/audio_helper.c
> @@ -173,8 +173,7 @@ static int gbaudio_remove_controls(struct snd_card *card, 
> struct device *dev,
> id.index = control->index;
> kctl = snd_ctl_find_id(card, );
> if (!kctl) {
> -   dev_err(dev, "%d: Failed to find %s\n", err,
> -   control->name);
> +   dev_err(dev, "Failed to find %s\n", control->name);
> continue;
> }
> err = snd_ctl_remove(card, kctl);
> --
> 2.27.0
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [greybus-dev] [PATCH v4 1/7] staging: greybus: audio: Update snd_jack FW usage as per new APIs

[Vaibhav Agarwal]

On Wed, Aug 5, 2020 at 6:35 PM Alex Elder  wrote:
>
> On 7/9/20 5:27 AM, Vaibhav Agarwal wrote:
> > snd_soc_jack APIs are modified in recent kernel versions. This patch
> > updates the codec driver to resolve the compilation errors related to
> > jack framework.
>
> Greg has already accepted this series so I won't review this now.  But
> I still wanted to provide this comment.
>
> It would be helpful in the future to provide a little more information
> about the nature of the changes to these APIs.  As a reviewer I had to
> go track them down to get a little more context about what you are doing
> here.  So you could say something like:
>
>   Audio jacks are now registered at the card level rather than being
>   associated with a CODEC.  The new card-based API allows a jack's pins
>   to be supplied when the jack is first registered.  See: 970939964c26
>   ("ASoC: Allow to register jacks at the card level")
>
> In other words, don't just say "the APIs changed," say "here is how
> the APIs have changed."  This kind of introduction can be very helpful
> and time saving for your reviewers.
>

Thanks for the feedback Alex. I'll take care of the commit message while
sharing similar patches.

--
vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1] staging: greybus: audio: fix uninitialized value issue

[Vaibhav Agarwal]

On Mon, Aug 10, 2020 at 8:34 PM Dan Carpenter  wrote:
>
> On Thu, Aug 06, 2020 at 09:51:57PM +0530, Vaibhav Agarwal wrote:
> > diff --git a/drivers/staging/greybus/audio_topology.c 
> > b/drivers/staging/greybus/audio_topology.c
> > index 2f9fdbdcd547..4b914d0edef2 100644
> > --- a/drivers/staging/greybus/audio_topology.c
> > +++ b/drivers/staging/greybus/audio_topology.c
> > @@ -456,6 +456,13 @@ static int gbcodec_mixer_dapm_ctl_put(struct 
> > snd_kcontrol *kcontrol,
> >   val = ucontrol->value.integer.value[0] & mask;
> >   connect = !!val;
> >
> > + ret = gb_pm_runtime_get_sync(bundle);
> > + if (ret)
> > + return ret;
> > +
> > + ret = gb_audio_gb_get_control(module->mgmt_connection, data->ctl_id,
> > +   GB_AUDIO_INVALID_INDEX, );
>
>
> We need to check "ret" after this.

Oops, my bad. Thanks Dan for reporting this. I'll share an updated patch soon.

--
regards,
vaibhav
>
> > +
> >   /* update ucontrol */
> >   if (gbvalue.value.integer_value[0] != val) {
> >   for (wi = 0; wi < wlist->num_widgets; wi++) {
> > @@ -466,16 +473,10 @@ static int gbcodec_mixer_dapm_ctl_put(struct 
> > snd_kcontrol *kcontrol,
>
> regards,
> dan carpenter
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 2/7] staging: greybus: audio: Maintain jack list within GB Audio module

[Vaibhav Agarwal]

As per the current implementation for GB codec driver, a jack list is
maintained for each module. And it expects the list to be populated by
the snd_soc_jack structure which would require modifications in
mainstream code.

However, this is not a necessary requirement and the list can be easily
maintained within gbaudio_module_info as well. This patch provides the
relevant changes for the same.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/audio_codec.c  | 74 +-
 drivers/staging/greybus/audio_codec.h  | 10 +++-
 drivers/staging/greybus/audio_module.c | 15 +++---
 3 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 5d3a5e6a8fe6..6dc4ee2bfb37 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -712,6 +712,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 struct snd_soc_card *card)
 {
int ret;
+   struct gbaudio_jack *jack, *n;
struct snd_soc_jack_pin *headset, *button;
 
if (!module->jack_mask)
@@ -726,14 +727,16 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
headset->pin = module->jack_name;
headset->mask = module->jack_mask;
-
ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
-   >headset_jack, headset, 1);
+   >headset.jack, headset, 1);
if (ret) {
dev_err(module->dev, "Failed to create new jack\n");
return ret;
}
 
+   /* Add to module's jack list */
+   list_add(>headset.list, >jack_list);
+
if (!module->button_mask)
return 0;
 
@@ -742,20 +745,22 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
button = devm_kzalloc(module->dev, sizeof(*button), GFP_KERNEL);
if (!button) {
ret = -ENOMEM;
-   goto free_headset;
+   goto free_jacks;
}
 
button->pin = module->button_name;
button->mask = module->button_mask;
-
ret = snd_soc_card_jack_new(card, module->button_name,
-   module->button_mask, >button_jack,
+   module->button_mask, >button.jack,
button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   goto free_headset;
+   goto free_jacks;
}
 
+   /* Add to module's jack list */
+   list_add(>button.list, >jack_list);
+
/*
 * Currently, max 4 buttons are supported with following key mapping
 * BTN_0 = KEY_MEDIA
@@ -765,56 +770,54 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 */
 
if (module->button_mask & SND_JACK_BTN_0) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_0,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_0,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_1) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_1,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_1,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_2) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_2,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_2,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_3) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_3,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_3,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   

[PATCH v4 4/7] staging: greybus: audio: Resolve compilation error in topology parser

[Vaibhav Agarwal]

Fix compilation errors for GB Audio topology parser code with recent
kernel versions.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/audio_topology.c | 123 +++
 1 file changed, 57 insertions(+), 66 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 4ac30accf226..ad88d3127a60 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -5,8 +5,8 @@
  * Copyright 2015-2016 Linaro Ltd.
  */
 
+#include 
 #include "audio_codec.h"
-#include "greybus_protocols.h"
 
 #define GBAUDIO_INVALID_ID 0xFF
 
@@ -165,15 +165,15 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_info *info;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gbcodec = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gbcodec = 
snd_soc_component_get_drvdata(comp);
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
info = (struct gb_audio_ctl_elem_info *)data->info;
 
if (!info) {
-   dev_err(codec->dev, "NULL info for %s\n", uinfo->id.name);
+   dev_err(comp->dev, "NULL info for %s\n", uinfo->id.name);
return -EINVAL;
}
 
@@ -201,7 +201,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
strlcpy(uinfo->value.enumerated.name, name, NAME_SIZE);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
break;
}
@@ -216,11 +216,11 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
struct gb_bundle *bundle;
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
@@ -239,7 +239,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
gb_pm_runtime_put_autosuspend(bundle);
 
if (ret) {
-   dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret,
+   dev_err_ratelimited(comp->dev, "%d:Error in %s for %s\n", ret,
__func__, kcontrol->id.name);
return ret;
}
@@ -262,7 +262,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
ret = -EINVAL;
break;
@@ -278,11 +278,11 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
struct gb_bundle *bundle;
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
@@ -309,7 +309,7 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
 

[PATCH v4 7/7] drivers: staging: audio: Fix the missing header file for helper file

[Vaibhav Agarwal]

This patch fixes the warning reported for missing prototypes due to
missing header file. Also, it includes changes to remove
unused_but_set_variables.

Reported-by: kernel test robot 
Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_helper.c   | 1 +
 drivers/staging/greybus/audio_topology.c | 8 
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/greybus/audio_helper.c 
b/drivers/staging/greybus/audio_helper.c
index faaa39708118..8b100a71f02e 100644
--- a/drivers/staging/greybus/audio_helper.c
+++ b/drivers/staging/greybus/audio_helper.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include "audio_helper.h"
 
 #define gbaudio_dapm_for_each_direction(dir) \
for ((dir) = SND_SOC_DAPM_DIR_IN; (dir) <= SND_SOC_DAPM_DIR_OUT; \
diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index ad88d3127a60..2f9fdbdcd547 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -28,14 +28,16 @@ static struct gbaudio_module_info *find_gb_module(
struct gbaudio_codec_info *codec,
char const *name)
 {
-   int dev_id, ret;
+   int dev_id;
char begin[NAME_SIZE];
struct gbaudio_module_info *module;
 
if (!name)
return NULL;
 
-   ret = sscanf(name, "%s %d", begin, _id);
+   if (sscanf(name, "%s %d", begin, _id) != 2)
+   return NULL;
+
dev_dbg(codec->dev, "%s:Find module#%d\n", __func__, dev_id);
 
mutex_lock(>lock);
@@ -377,7 +379,6 @@ static int gbcodec_mixer_dapm_ctl_get(struct snd_kcontrol 
*kcontrol,
  struct snd_ctl_elem_value *ucontrol)
 {
int ret;
-   struct gb_audio_ctl_elem_info *info;
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
@@ -393,7 +394,6 @@ static int gbcodec_mixer_dapm_ctl_get(struct snd_kcontrol 
*kcontrol,
return -EINVAL;
 
data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
-   info = (struct gb_audio_ctl_elem_info *)data->info;
bundle = to_gb_bundle(module->dev);
 
if (data->vcount == 2)
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 1/7] staging: greybus: audio: Update snd_jack FW usage as per new APIs

[Vaibhav Agarwal]

snd_soc_jack APIs are modified in recent kernel versions. This patch
updates the codec driver to resolve the compilation errors related to
jack framework.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/audio_codec.c | 54 +--
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 08746c85dea6..5d3a5e6a8fe6 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -709,17 +709,26 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
 };
 
 static int gbaudio_init_jack(struct gbaudio_module_info *module,
-struct snd_soc_codec *codec)
+struct snd_soc_card *card)
 {
int ret;
+   struct snd_soc_jack_pin *headset, *button;
 
if (!module->jack_mask)
return 0;
 
snprintf(module->jack_name, NAME_SIZE, "GB %d Headset Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->jack_name, module->jack_mask,
-  >headset_jack);
+
+   headset = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
+   if (!headset)
+   return -ENOMEM;
+
+   headset->pin = module->jack_name;
+   headset->mask = module->jack_mask;
+
+   ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
+   >headset_jack, headset, 1);
if (ret) {
dev_err(module->dev, "Failed to create new jack\n");
return ret;
@@ -730,11 +739,21 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
snprintf(module->button_name, NAME_SIZE, "GB %d Button Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->button_name, module->button_mask,
-  >button_jack);
+   button = devm_kzalloc(module->dev, sizeof(*button), GFP_KERNEL);
+   if (!button) {
+   ret = -ENOMEM;
+   goto free_headset;
+   }
+
+   button->pin = module->button_name;
+   button->mask = module->button_mask;
+
+   ret = snd_soc_card_jack_new(card, module->button_name,
+   module->button_mask, >button_jack,
+   button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   return ret;
+   goto free_headset;
}
 
/*
@@ -750,7 +769,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -759,7 +778,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -768,7 +787,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -777,7 +796,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -788,6 +807,16 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
*/
 
return 0;
+
+free_button:
+   snd_device_free(card->snd_card, module->button_jack.jack);
+   list_del(>button_jack.list);
+
+free_headset:
+   snd_device_free(card->snd_card, module->headset_jack.jack);
+   list_del(>headset_jack.list);
+
+   return ret;
 }
 
 int gbaudio_register_module(struct gbaudio_module_info *module)
@@ -815,7 +844,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EINVAL;
}
 
-   ret = gbaudio_init_jack(module, codec);
+   ret = gbaudio_init_jack(module, component->card);
if (ret) {
up_write(>controls_rwsem);
return ret;
@@ -942,7 +971,8 @@ void g

[PATCH v4 3/7] staging: greybus: audio: Resolve compilation errors for GB codec module

[Vaibhav Agarwal]

Due to dependencies on ASoC framework changes, GB dummy codec module
compilation is currently disabled. This patch updates codec driver as
per the latest ASoC APIs.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/audio_codec.c | 88 +--
 drivers/staging/greybus/audio_codec.h |  2 +-
 2 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 6dc4ee2bfb37..0ecdba27086b 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -825,7 +825,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 int gbaudio_register_module(struct gbaudio_module_info *module)
 {
int ret;
-   struct snd_soc_codec *codec;
+   struct snd_soc_component *comp;
struct snd_card *card;
struct gbaudio_jack *jack = NULL;
 
@@ -834,8 +834,8 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EAGAIN;
}
 
-   codec = gbcodec->codec;
-   card = codec->card->snd_card;
+   comp = gbcodec->component;
+   card = comp->card->snd_card;
 
down_write(>controls_rwsem);
 
@@ -847,33 +847,33 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EINVAL;
}
 
-   ret = gbaudio_init_jack(module, component->card);
+   ret = gbaudio_init_jack(module, comp->card);
if (ret) {
up_write(>controls_rwsem);
return ret;
}
 
if (module->dapm_widgets)
-   snd_soc_dapm_new_controls(>dapm, module->dapm_widgets,
+   snd_soc_dapm_new_controls(>dapm, module->dapm_widgets,
  module->num_dapm_widgets);
if (module->controls)
-   snd_soc_add_codec_controls(codec, module->controls,
-  module->num_controls);
+   snd_soc_add_component_controls(comp, module->controls,
+  module->num_controls);
if (module->dapm_routes)
-   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
+   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
module->num_dapm_routes);
 
/* card already instantiated, create widgets here only */
-   if (codec->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(codec->card,
-   >dapm);
+   if (comp->card->instantiated) {
+   snd_soc_dapm_link_component_dai_widgets(comp->card,
+   >dapm);
 #ifdef CONFIG_SND_JACK
/*
 * register jack devices for this module
 * from codec->jack_list
 */
list_for_each_entry(jack, >jack_list, list) {
-   snd_device_register(codec->card->snd_card,
+   snd_device_register(comp->card->snd_card,
jack->jack.jack);
}
 #endif
@@ -883,9 +883,9 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
list_add(>list, >module_list);
mutex_unlock(>lock);
 
-   if (codec->card->instantiated)
-   ret = snd_soc_dapm_new_widgets(>dapm);
-   dev_dbg(codec->dev, "Registered %s module\n", module->name);
+   if (comp->card->instantiated)
+   ret = snd_soc_dapm_new_widgets(comp->card);
+   dev_dbg(comp->dev, "Registered %s module\n", module->name);
 
up_write(>controls_rwsem);
return ret;
@@ -956,18 +956,18 @@ static void gbaudio_codec_cleanup(struct 
gbaudio_module_info *module)
 
 void gbaudio_unregister_module(struct gbaudio_module_info *module)
 {
-   struct snd_soc_codec *codec = gbcodec->codec;
-   struct snd_card *card = codec->card->snd_card;
+   struct snd_soc_component *comp = gbcodec->component;
+   struct snd_card *card = comp->card->snd_card;
struct gbaudio_jack *jack, *n;
int mask;
 
-   dev_dbg(codec->dev, "Unregister %s module\n", module->name);
+   dev_dbg(comp->dev, "Unregister %s module\n", module->name);
 
down_write(>controls_rwsem);
mutex_lock(>lock);
gbaudio_codec_cleanup(module);
list_del(>list);
-   dev_dbg(codec->dev, "Process Unregister %s module\n", module->name);
+   dev_dbg(comp->dev, "Process Unregister %s module\n", module->name);
mutex_unlock(>lock);
 
 #ifdef CONF

[PATCH v4 6/7] staging: greybus: audio: Enable GB codec, audio module compilation.

[Vaibhav Agarwal]

Currently you can't enable the Gey Bus Audio Codec because there is no
entry for it in the Kconfig file. Originally the config name was going
to be AUDIO_MSM8994 but that's not correct because other types of
hardware are supported now. I have chosen the name AUDIO_APB_CODEC
instead.  Also I had to update the dependencies for GREYBUS_AUDIO to
make the compile work.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/Kconfig  | 14 +-
 drivers/staging/greybus/Makefile |  4 ++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
index 9389e7a922fa..927cfa4bc989 100644
--- a/drivers/staging/greybus/Kconfig
+++ b/drivers/staging/greybus/Kconfig
@@ -3,7 +3,7 @@ if GREYBUS
 
 config GREYBUS_AUDIO
tristate "Greybus Audio Class driver"
-   depends on SOUND
+   depends on SOUND && SND_SOC
help
  Select this option if you have a device that follows the
  Greybus Audio Class specification.
@@ -11,6 +11,18 @@ config GREYBUS_AUDIO
  To compile this code as a module, chose M here: the module
  will be called gb-audio.ko
 
+config GREYBUS_AUDIO_APB_CODEC
+   tristate "Greybus APBridge Audio codec driver"
+   depends on SND_SOC && GREYBUS_AUDIO
+   help
+ Select this option if you have a Toshiba APB device that has I2S
+  ports and acts as a Greybus "Dummy codec". This device is a
+  bridge from an APB-I2S port to a Unipro network.
+
+ To compile this code as a module, chose M here: the module
+ will be called gb-audio-codec.ko
+
+
 config GREYBUS_BOOTROM
tristate "Greybus Bootrom Class driver"
help
diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 3b4b6cabff19..7c5e89622334 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -40,8 +40,8 @@ gb-audio-manager-y:= audio_manager.o 
audio_manager_module.o
 #ccflags-y += -DGB_AUDIO_MANAGER_SYSFS
 #endif
 
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-codec.o
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-module.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-codec.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-module.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-gb.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-apbridgea.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-manager.o
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 5/7] staging: greybus: audio: Add helper APIs for dynamic audio modules

[Vaibhav Agarwal]

Greybus Codec driver allows modules to be dynamically added and removed,
which further requires updating the DAPM configurations as well.

With current snd_soc architecture, dynamic audio modules is not yet
supported. This patch provides helper APIs to update DAPM configurations
in response to modules which are dynamically added or removed. The
source is primarily based on snd_dapm.c

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/Makefile   |   2 +-
 drivers/staging/greybus/audio_codec.c  |  12 +-
 drivers/staging/greybus/audio_helper.c | 197 +
 drivers/staging/greybus/audio_helper.h |  17 +++
 4 files changed, 223 insertions(+), 5 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h

diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 627e44f2a983..3b4b6cabff19 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_GREYBUS_VIBRATOR)+= gb-vibrator.o
 
 # Greybus Audio is a bunch of modules
 gb-audio-module-y  := audio_module.o audio_topology.o
-gb-audio-codec-y   := audio_codec.o
+gb-audio-codec-y   := audio_codec.o audio_helper.o
 gb-audio-gb-y  := audio_gb.o
 gb-audio-apbridgea-y   := audio_apbridgea.o
 gb-audio-manager-y := audio_manager.o audio_manager_module.o
diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 0ecdba27086b..74538f8c5fa4 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -14,6 +14,7 @@
 #include "audio_codec.h"
 #include "audio_apbridgea.h"
 #include "audio_manager.h"
+#include "audio_helper.h"
 
 static struct gbaudio_codec_info *gbcodec;
 
@@ -865,7 +866,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
 
/* card already instantiated, create widgets here only */
if (comp->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(comp->card,
+   gbaudio_dapm_link_component_dai_widgets(comp->card,
>dapm);
 #ifdef CONFIG_SND_JACK
/*
@@ -999,13 +1000,16 @@ void gbaudio_unregister_module(struct 
gbaudio_module_info *module)
if (module->controls) {
dev_dbg(comp->dev, "Removing %d controls\n",
module->num_controls);
-   snd_soc_remove_codec_controls(comp, module->controls,
- module->num_controls);
+   /* release control semaphore */
+   up_write(>controls_rwsem);
+   gbaudio_remove_component_controls(comp, module->controls,
+ module->num_controls);
+   down_write(>controls_rwsem);
}
if (module->dapm_widgets) {
dev_dbg(comp->dev, "Removing %d widgets\n",
module->num_dapm_widgets);
-   snd_soc_dapm_free_controls(>dapm, module->dapm_widgets,
+   gbaudio_dapm_free_controls(>dapm, module->dapm_widgets,
   module->num_dapm_widgets);
}
 
diff --git a/drivers/staging/greybus/audio_helper.c 
b/drivers/staging/greybus/audio_helper.c
new file mode 100644
index ..faaa39708118
--- /dev/null
+++ b/drivers/staging/greybus/audio_helper.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Greybus Audio Sound SoC helper APIs
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#define gbaudio_dapm_for_each_direction(dir) \
+   for ((dir) = SND_SOC_DAPM_DIR_IN; (dir) <= SND_SOC_DAPM_DIR_OUT; \
+   (dir)++)
+
+static void gbaudio_dapm_link_dai_widget(struct snd_soc_dapm_widget *dai_w,
+struct snd_soc_card *card)
+{
+   struct snd_soc_dapm_widget *w;
+   struct snd_soc_dapm_widget *src, *sink;
+   struct snd_soc_dai *dai = dai_w->priv;
+
+   /* ...find all widgets with the same stream and link them */
+   list_for_each_entry(w, >widgets, list) {
+   if (w->dapm != dai_w->dapm)
+   continue;
+
+   switch (w->id) {
+   case snd_soc_dapm_dai_in:
+   case snd_soc_dapm_dai_out:
+   continue;
+   default:
+   break;
+   }
+
+   if (!w->sname || !strstr(w->sname, dai_w->sname))
+   continue;
+
+   /*
+* check if widget is already linked,
+* if (w->linked)
+*  return;
+*/
+
+   if (dai_w-&

[PATCH v4 0/7] Enable Greybus Audio codec driver

[Vaibhav Agarwal]

The existing GB Audio codec driver is dependent on MSM8994 Audio driver.
During the development stage, this dependency was configured due to
various changes involved in MSM Audio driver to enable addtional codec
card and some of the changes proposed in mainline ASoC framework.
However, these are not the real dependencies and some of them can be
easily removed.

The folowing patch series includes the changes to resolve unnecessary
depedencies and make the codec driver functional with the latest kernel.

Patch 1,2: Incudes jack framework related changes.
Patch 3,4,5: Resolves compilation error observed with the latest kernel and
also provides helper APIs required to allow dynamic addition/removal of
modules.
Patch 6: Finally provides config options and related Makefile changes to
enable GB Codec driver.

Thanks to Alexandre for raising the headsup [1] and motivating me to provide
the necessary changes.

This patchset is intended to resolve the componentization issue only.
And as per the suggestion [2] from Mark, I'll share a separate patch series
aligned to ASoC tree. Once the relevant changes are accepted in snd-soc
framework, I'll share relevant patches to pull GB Audio out of the
staging tree.

v1:
- Include the changes for the review comments suggested by Dan
- Rebase to latest staging-next

v2:
- Avoid defining unused 'update' pointer
- Fix the missing connect bool value required during mixer_update_power
- Added Reviewed-by tag from Dan
- Rebase to latest staging-next

v3:
- Fix the warnings reported by lkp[3]
- Rebase to latest staging-next

[1] 
https://lore.kernel.org/lkml/20200507212912.599433-1-alexandre.bell...@bootlin.com/
[2] https://lore.kernel.org/alsa-devel/20200612160620.gk5...@sirena.org.uk/
[3] https://lore.kernel.org/lkml/202006200150.mnvj1duq%25...@intel.com/

Vaibhav Agarwal (7):
  staging: greybus: audio: Update snd_jack FW usage as per new APIs
  staging: greybus: audio: Maintain jack list within GB Audio module
  staging: greybus: audio: Resolve compilation errors for GB codec
module
  staging: greybus: audio: Resolve compilation error in topology parser
  staging: greybus: audio: Add helper APIs for dynamic audio modules
  staging: greybus: audio: Enable GB codec, audio module compilation.
  drivers: staging: audio: Fix the missing header file for helper file

 drivers/staging/greybus/Kconfig  |  14 +-
 drivers/staging/greybus/Makefile |   6 +-
 drivers/staging/greybus/audio_codec.c| 178 +++-
 drivers/staging/greybus/audio_codec.h|  12 +-
 drivers/staging/greybus/audio_helper.c   | 198 +++
 drivers/staging/greybus/audio_helper.h   |  17 ++
 drivers/staging/greybus/audio_module.c   |  15 +-
 drivers/staging/greybus/audio_topology.c | 131 +++
 8 files changed, 414 insertions(+), 157 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h


base-commit: 14442181d20490945f341644bb8257e334b01447
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 6/6] staging: greybus: audio: Enable GB codec, audio module compilation.

[Vaibhav Agarwal]

On Wed, Jul 01, 2020 at 03:36:55PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jun 19, 2020 at 04:50:26PM +0530, Vaibhav Agarwal wrote:
> > Currently you can't enable the Gey Bus Audio Codec because there is no
> > entry for it in the Kconfig file. Originally the config name was going
> > to be AUDIO_MSM8994 but that's not correct because other types of
> > hardware are supported now. I have chosen the name AUDIO_APB_CODEC
> > instead.  Also I had to update the dependencies for GREYBUS_AUDIO to
> > make the compile work.
> >
> > Signed-off-by: Vaibhav Agarwal 
> > Reviewed-by: Dan Carpenter 
> > ---
> >  drivers/staging/greybus/Kconfig  | 14 +-
> >  drivers/staging/greybus/Makefile |  4 ++--
> >  2 files changed, 15 insertions(+), 3 deletions(-)
>
> Can you fix the build issues found by the bot and resend?

Sure Greg, I'll share the updated patch set with fixes for the issues
reported.

--
thanks,
vaibhav

>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 6/6] staging: greybus: audio: Enable GB codec, audio module compilation.

[Vaibhav Agarwal]

Currently you can't enable the Gey Bus Audio Codec because there is no
entry for it in the Kconfig file. Originally the config name was going
to be AUDIO_MSM8994 but that's not correct because other types of
hardware are supported now. I have chosen the name AUDIO_APB_CODEC
instead.  Also I had to update the dependencies for GREYBUS_AUDIO to
make the compile work.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/Kconfig  | 14 +-
 drivers/staging/greybus/Makefile |  4 ++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
index 9389e7a922fa..927cfa4bc989 100644
--- a/drivers/staging/greybus/Kconfig
+++ b/drivers/staging/greybus/Kconfig
@@ -3,7 +3,7 @@ if GREYBUS
 
 config GREYBUS_AUDIO
tristate "Greybus Audio Class driver"
-   depends on SOUND
+   depends on SOUND && SND_SOC
help
  Select this option if you have a device that follows the
  Greybus Audio Class specification.
@@ -11,6 +11,18 @@ config GREYBUS_AUDIO
  To compile this code as a module, chose M here: the module
  will be called gb-audio.ko
 
+config GREYBUS_AUDIO_APB_CODEC
+   tristate "Greybus APBridge Audio codec driver"
+   depends on SND_SOC && GREYBUS_AUDIO
+   help
+ Select this option if you have a Toshiba APB device that has I2S
+  ports and acts as a Greybus "Dummy codec". This device is a
+  bridge from an APB-I2S port to a Unipro network.
+
+ To compile this code as a module, chose M here: the module
+ will be called gb-audio-codec.ko
+
+
 config GREYBUS_BOOTROM
tristate "Greybus Bootrom Class driver"
help
diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 3b4b6cabff19..7c5e89622334 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -40,8 +40,8 @@ gb-audio-manager-y:= audio_manager.o 
audio_manager_module.o
 #ccflags-y += -DGB_AUDIO_MANAGER_SYSFS
 #endif
 
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-codec.o
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-module.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-codec.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-module.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-gb.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-apbridgea.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-manager.o
-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 5/6] staging: greybus: audio: Add helper APIs for dynamic audio modules

[Vaibhav Agarwal]

Greybus Codec driver allows modules to be dynamically added and removed,
which further requires updating the DAPM configurations as well.

With current snd_soc architecture, dynamic audio modules is not yet
supported. This patch provides helper APIs to update DAPM configurations
in response to modules which are dynamically added or removed. The
source is primarily based on snd_dapm.c

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/Makefile   |   2 +-
 drivers/staging/greybus/audio_codec.c  |  12 +-
 drivers/staging/greybus/audio_helper.c | 197 +
 drivers/staging/greybus/audio_helper.h |  17 +++
 4 files changed, 223 insertions(+), 5 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h

diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 627e44f2a983..3b4b6cabff19 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_GREYBUS_VIBRATOR)+= gb-vibrator.o
 
 # Greybus Audio is a bunch of modules
 gb-audio-module-y  := audio_module.o audio_topology.o
-gb-audio-codec-y   := audio_codec.o
+gb-audio-codec-y   := audio_codec.o audio_helper.o
 gb-audio-gb-y  := audio_gb.o
 gb-audio-apbridgea-y   := audio_apbridgea.o
 gb-audio-manager-y := audio_manager.o audio_manager_module.o
diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 0ecdba27086b..74538f8c5fa4 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -14,6 +14,7 @@
 #include "audio_codec.h"
 #include "audio_apbridgea.h"
 #include "audio_manager.h"
+#include "audio_helper.h"
 
 static struct gbaudio_codec_info *gbcodec;
 
@@ -865,7 +866,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
 
/* card already instantiated, create widgets here only */
if (comp->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(comp->card,
+   gbaudio_dapm_link_component_dai_widgets(comp->card,
>dapm);
 #ifdef CONFIG_SND_JACK
/*
@@ -999,13 +1000,16 @@ void gbaudio_unregister_module(struct 
gbaudio_module_info *module)
if (module->controls) {
dev_dbg(comp->dev, "Removing %d controls\n",
module->num_controls);
-   snd_soc_remove_codec_controls(comp, module->controls,
- module->num_controls);
+   /* release control semaphore */
+   up_write(>controls_rwsem);
+   gbaudio_remove_component_controls(comp, module->controls,
+ module->num_controls);
+   down_write(>controls_rwsem);
}
if (module->dapm_widgets) {
dev_dbg(comp->dev, "Removing %d widgets\n",
module->num_dapm_widgets);
-   snd_soc_dapm_free_controls(>dapm, module->dapm_widgets,
+   gbaudio_dapm_free_controls(>dapm, module->dapm_widgets,
   module->num_dapm_widgets);
}
 
diff --git a/drivers/staging/greybus/audio_helper.c 
b/drivers/staging/greybus/audio_helper.c
new file mode 100644
index ..faaa39708118
--- /dev/null
+++ b/drivers/staging/greybus/audio_helper.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Greybus Audio Sound SoC helper APIs
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#define gbaudio_dapm_for_each_direction(dir) \
+   for ((dir) = SND_SOC_DAPM_DIR_IN; (dir) <= SND_SOC_DAPM_DIR_OUT; \
+   (dir)++)
+
+static void gbaudio_dapm_link_dai_widget(struct snd_soc_dapm_widget *dai_w,
+struct snd_soc_card *card)
+{
+   struct snd_soc_dapm_widget *w;
+   struct snd_soc_dapm_widget *src, *sink;
+   struct snd_soc_dai *dai = dai_w->priv;
+
+   /* ...find all widgets with the same stream and link them */
+   list_for_each_entry(w, >widgets, list) {
+   if (w->dapm != dai_w->dapm)
+   continue;
+
+   switch (w->id) {
+   case snd_soc_dapm_dai_in:
+   case snd_soc_dapm_dai_out:
+   continue;
+   default:
+   break;
+   }
+
+   if (!w->sname || !strstr(w->sname, dai_w->sname))
+   continue;
+
+   /*
+* check if widget is already linked,
+* if (w->linked)
+*  return;
+*/
+
+   if (dai_w-&

[PATCH v3 2/6] staging: greybus: audio: Maintain jack list within GB Audio module

[Vaibhav Agarwal]

As per the current implementation for GB codec driver, a jack list is
maintained for each module. And it expects the list to be populated by
the snd_soc_jack structure which would require modifications in
mainstream code.

However, this is not a necessary requirement and the list can be easily
maintained within gbaudio_module_info as well. This patch provides the
relevant changes for the same.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/audio_codec.c  | 74 +-
 drivers/staging/greybus/audio_codec.h  | 10 +++-
 drivers/staging/greybus/audio_module.c | 15 +++---
 3 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 5d3a5e6a8fe6..6dc4ee2bfb37 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -712,6 +712,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 struct snd_soc_card *card)
 {
int ret;
+   struct gbaudio_jack *jack, *n;
struct snd_soc_jack_pin *headset, *button;
 
if (!module->jack_mask)
@@ -726,14 +727,16 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
headset->pin = module->jack_name;
headset->mask = module->jack_mask;
-
ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
-   >headset_jack, headset, 1);
+   >headset.jack, headset, 1);
if (ret) {
dev_err(module->dev, "Failed to create new jack\n");
return ret;
}
 
+   /* Add to module's jack list */
+   list_add(>headset.list, >jack_list);
+
if (!module->button_mask)
return 0;
 
@@ -742,20 +745,22 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
button = devm_kzalloc(module->dev, sizeof(*button), GFP_KERNEL);
if (!button) {
ret = -ENOMEM;
-   goto free_headset;
+   goto free_jacks;
}
 
button->pin = module->button_name;
button->mask = module->button_mask;
-
ret = snd_soc_card_jack_new(card, module->button_name,
-   module->button_mask, >button_jack,
+   module->button_mask, >button.jack,
button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   goto free_headset;
+   goto free_jacks;
}
 
+   /* Add to module's jack list */
+   list_add(>button.list, >jack_list);
+
/*
 * Currently, max 4 buttons are supported with following key mapping
 * BTN_0 = KEY_MEDIA
@@ -765,56 +770,54 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 */
 
if (module->button_mask & SND_JACK_BTN_0) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_0,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_0,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_1) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_1,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_1,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_2) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_2,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_2,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_3) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_3,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_3,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   

[PATCH v3 3/6] staging: greybus: audio: Resolve compilation errors for GB codec module

[Vaibhav Agarwal]

Due to dependencies on ASoC framework changes, GB dummy codec module
compilation is currently disabled. This patch updates codec driver as
per the latest ASoC APIs.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/audio_codec.c | 88 +--
 drivers/staging/greybus/audio_codec.h |  2 +-
 2 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 6dc4ee2bfb37..0ecdba27086b 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -825,7 +825,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 int gbaudio_register_module(struct gbaudio_module_info *module)
 {
int ret;
-   struct snd_soc_codec *codec;
+   struct snd_soc_component *comp;
struct snd_card *card;
struct gbaudio_jack *jack = NULL;
 
@@ -834,8 +834,8 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EAGAIN;
}
 
-   codec = gbcodec->codec;
-   card = codec->card->snd_card;
+   comp = gbcodec->component;
+   card = comp->card->snd_card;
 
down_write(>controls_rwsem);
 
@@ -847,33 +847,33 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EINVAL;
}
 
-   ret = gbaudio_init_jack(module, component->card);
+   ret = gbaudio_init_jack(module, comp->card);
if (ret) {
up_write(>controls_rwsem);
return ret;
}
 
if (module->dapm_widgets)
-   snd_soc_dapm_new_controls(>dapm, module->dapm_widgets,
+   snd_soc_dapm_new_controls(>dapm, module->dapm_widgets,
  module->num_dapm_widgets);
if (module->controls)
-   snd_soc_add_codec_controls(codec, module->controls,
-  module->num_controls);
+   snd_soc_add_component_controls(comp, module->controls,
+  module->num_controls);
if (module->dapm_routes)
-   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
+   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
module->num_dapm_routes);
 
/* card already instantiated, create widgets here only */
-   if (codec->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(codec->card,
-   >dapm);
+   if (comp->card->instantiated) {
+   snd_soc_dapm_link_component_dai_widgets(comp->card,
+   >dapm);
 #ifdef CONFIG_SND_JACK
/*
 * register jack devices for this module
 * from codec->jack_list
 */
list_for_each_entry(jack, >jack_list, list) {
-   snd_device_register(codec->card->snd_card,
+   snd_device_register(comp->card->snd_card,
jack->jack.jack);
}
 #endif
@@ -883,9 +883,9 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
list_add(>list, >module_list);
mutex_unlock(>lock);
 
-   if (codec->card->instantiated)
-   ret = snd_soc_dapm_new_widgets(>dapm);
-   dev_dbg(codec->dev, "Registered %s module\n", module->name);
+   if (comp->card->instantiated)
+   ret = snd_soc_dapm_new_widgets(comp->card);
+   dev_dbg(comp->dev, "Registered %s module\n", module->name);
 
up_write(>controls_rwsem);
return ret;
@@ -956,18 +956,18 @@ static void gbaudio_codec_cleanup(struct 
gbaudio_module_info *module)
 
 void gbaudio_unregister_module(struct gbaudio_module_info *module)
 {
-   struct snd_soc_codec *codec = gbcodec->codec;
-   struct snd_card *card = codec->card->snd_card;
+   struct snd_soc_component *comp = gbcodec->component;
+   struct snd_card *card = comp->card->snd_card;
struct gbaudio_jack *jack, *n;
int mask;
 
-   dev_dbg(codec->dev, "Unregister %s module\n", module->name);
+   dev_dbg(comp->dev, "Unregister %s module\n", module->name);
 
down_write(>controls_rwsem);
mutex_lock(>lock);
gbaudio_codec_cleanup(module);
list_del(>list);
-   dev_dbg(codec->dev, "Process Unregister %s module\n", module->name);
+   dev_dbg(comp->dev, "Process Unregister %s module\n", module->name);
mutex_unlock(>lock);
 
 #ifdef CONF

[PATCH v3 1/6] staging: greybus: audio: Update snd_jack FW usage as per new APIs

[Vaibhav Agarwal]

snd_soc_jack APIs are modified in recent kernel versions. This patch
updates the codec driver to resolve the compilation errors related to
jack framework.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/audio_codec.c | 54 +--
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 08746c85dea6..5d3a5e6a8fe6 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -709,17 +709,26 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
 };
 
 static int gbaudio_init_jack(struct gbaudio_module_info *module,
-struct snd_soc_codec *codec)
+struct snd_soc_card *card)
 {
int ret;
+   struct snd_soc_jack_pin *headset, *button;
 
if (!module->jack_mask)
return 0;
 
snprintf(module->jack_name, NAME_SIZE, "GB %d Headset Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->jack_name, module->jack_mask,
-  >headset_jack);
+
+   headset = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
+   if (!headset)
+   return -ENOMEM;
+
+   headset->pin = module->jack_name;
+   headset->mask = module->jack_mask;
+
+   ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
+   >headset_jack, headset, 1);
if (ret) {
dev_err(module->dev, "Failed to create new jack\n");
return ret;
@@ -730,11 +739,21 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
snprintf(module->button_name, NAME_SIZE, "GB %d Button Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->button_name, module->button_mask,
-  >button_jack);
+   button = devm_kzalloc(module->dev, sizeof(*button), GFP_KERNEL);
+   if (!button) {
+   ret = -ENOMEM;
+   goto free_headset;
+   }
+
+   button->pin = module->button_name;
+   button->mask = module->button_mask;
+
+   ret = snd_soc_card_jack_new(card, module->button_name,
+   module->button_mask, >button_jack,
+   button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   return ret;
+   goto free_headset;
}
 
/*
@@ -750,7 +769,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -759,7 +778,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -768,7 +787,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -777,7 +796,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -788,6 +807,16 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
*/
 
return 0;
+
+free_button:
+   snd_device_free(card->snd_card, module->button_jack.jack);
+   list_del(>button_jack.list);
+
+free_headset:
+   snd_device_free(card->snd_card, module->headset_jack.jack);
+   list_del(>headset_jack.list);
+
+   return ret;
 }
 
 int gbaudio_register_module(struct gbaudio_module_info *module)
@@ -815,7 +844,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EINVAL;
}
 
-   ret = gbaudio_init_jack(module, codec);
+   ret = gbaudio_init_jack(module, component->card);
if (ret) {
up_write(>controls_rwsem);
return ret;
@@ -942,7 +971,8 @@ void g

[PATCH v3 4/6] staging: greybus: audio: Resolve compilation error in topology parser

[Vaibhav Agarwal]

Fix compilation errors for GB Audio topology parser code with recent
kernel versions.

Signed-off-by: Vaibhav Agarwal 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/greybus/audio_topology.c | 123 +++
 1 file changed, 57 insertions(+), 66 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 4ac30accf226..ad88d3127a60 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -5,8 +5,8 @@
  * Copyright 2015-2016 Linaro Ltd.
  */
 
+#include 
 #include "audio_codec.h"
-#include "greybus_protocols.h"
 
 #define GBAUDIO_INVALID_ID 0xFF
 
@@ -165,15 +165,15 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_info *info;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gbcodec = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gbcodec = 
snd_soc_component_get_drvdata(comp);
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
info = (struct gb_audio_ctl_elem_info *)data->info;
 
if (!info) {
-   dev_err(codec->dev, "NULL info for %s\n", uinfo->id.name);
+   dev_err(comp->dev, "NULL info for %s\n", uinfo->id.name);
return -EINVAL;
}
 
@@ -201,7 +201,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
strlcpy(uinfo->value.enumerated.name, name, NAME_SIZE);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
break;
}
@@ -216,11 +216,11 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
struct gb_bundle *bundle;
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
@@ -239,7 +239,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
gb_pm_runtime_put_autosuspend(bundle);
 
if (ret) {
-   dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret,
+   dev_err_ratelimited(comp->dev, "%d:Error in %s for %s\n", ret,
__func__, kcontrol->id.name);
return ret;
}
@@ -262,7 +262,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
ret = -EINVAL;
break;
@@ -278,11 +278,11 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
struct gb_bundle *bundle;
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
@@ -309,7 +309,7 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
 

[PATCH v3 0/6] Enable Greybus Audio codec driver

[Vaibhav Agarwal]

The existing GB Audio codec driver is dependent on MSM8994 Audio driver.
During the development stage, this dependency was configured due to
various changes involved in MSM Audio driver to enable addtional codec
card and some of the changes proposed in mainline ASoC framework.
However, these are not the real dependencies and some of them can be
easily removed.

The folowing patch series includes the changes to resolve unnecessary
depedencies and make the codec driver functional with the latest kernel.

Patch 1,2: Incudes jack framework related changes.
Patch 3,4,5: Resolves compilation error observed with the latest kernel and
also provides helper APIs required to allow dynamic addition/removal of
modules.
Patch 6: Finally provides config options and related Makefile changes to
enable GB Codec driver.

Thanks to Alexandre for raising the headsup [1] and motivating me to provide
the necessary changes.

This patchset is intended to resolve the componentization issue only.
And as per the suggestion [2] from Mark, I'll share a separate patch series
aligned to ASoC tree. Once the relevant changes are accepted in snd-soc
framework, I'll share relevant patches to pull GB Audio out of the
staging tree.

[1] 
https://lore.kernel.org/lkml/20200507212912.599433-1-alexandre.bell...@bootlin.com/
[2] https://lore.kernel.org/alsa-devel/20200612160620.gk5...@sirena.org.uk/

v1:
- Include the changes for the review comments suggested by Dan
- Rebase to latest staging-next

v2:
- Avoid defining unused 'update' pointer
- Fix the missing connect bool value required during mixer_update_power
- Added Reviewed-by tag from Dan
- Rebase to latest staging-next

Vaibhav Agarwal (6):
  staging: greybus: audio: Update snd_jack FW usage as per new APIs
  staging: greybus: audio: Maintain jack list within GB Audio module
  staging: greybus: audio: Resolve compilation errors for GB codec
module
  staging: greybus: audio: Resolve compilation error in topology parser
  staging: greybus: audio: Add helper APIs for dynamic audio modules
  staging: greybus: audio: Enable GB codec, audio module compilation.

 drivers/staging/greybus/Kconfig  |  14 +-
 drivers/staging/greybus/Makefile |   6 +-
 drivers/staging/greybus/audio_codec.c| 178 +++-
 drivers/staging/greybus/audio_codec.h|  12 +-
 drivers/staging/greybus/audio_helper.c   | 197 +++
 drivers/staging/greybus/audio_helper.h   |  17 ++
 drivers/staging/greybus/audio_module.c   |  15 +-
 drivers/staging/greybus/audio_topology.c | 123 +++---
 8 files changed, 409 insertions(+), 153 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h


base-commit: 98fe05e21a6e0ca242e974650ed58b64813cb2dc
-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/6] staging: greybus: audio: Maintain jack list within GB Audio module

[Vaibhav Agarwal]

As per the current implementation for GB codec driver, a jack list is
maintained for each module. And it expects the list to be populated by
the snd_soc_jack structure which would require modifications in
mainstream code.

However, this is not a necessary requirement and the list can be easily
maintained within gbaudio_module_info as well. This patch provides the
relevant changes for the same.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c  | 74 +-
 drivers/staging/greybus/audio_codec.h  | 10 +++-
 drivers/staging/greybus/audio_module.c | 15 +++---
 3 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 5d3a5e6a8fe6..6dc4ee2bfb37 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -712,6 +712,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 struct snd_soc_card *card)
 {
int ret;
+   struct gbaudio_jack *jack, *n;
struct snd_soc_jack_pin *headset, *button;
 
if (!module->jack_mask)
@@ -726,14 +727,16 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
headset->pin = module->jack_name;
headset->mask = module->jack_mask;
-
ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
-   >headset_jack, headset, 1);
+   >headset.jack, headset, 1);
if (ret) {
dev_err(module->dev, "Failed to create new jack\n");
return ret;
}
 
+   /* Add to module's jack list */
+   list_add(>headset.list, >jack_list);
+
if (!module->button_mask)
return 0;
 
@@ -742,20 +745,22 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
button = devm_kzalloc(module->dev, sizeof(*button), GFP_KERNEL);
if (!button) {
ret = -ENOMEM;
-   goto free_headset;
+   goto free_jacks;
}
 
button->pin = module->button_name;
button->mask = module->button_mask;
-
ret = snd_soc_card_jack_new(card, module->button_name,
-   module->button_mask, >button_jack,
+   module->button_mask, >button.jack,
button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   goto free_headset;
+   goto free_jacks;
}
 
+   /* Add to module's jack list */
+   list_add(>button.list, >jack_list);
+
/*
 * Currently, max 4 buttons are supported with following key mapping
 * BTN_0 = KEY_MEDIA
@@ -765,56 +770,54 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 */
 
if (module->button_mask & SND_JACK_BTN_0) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_0,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_0,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_1) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_1,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_1,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_2) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_2,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_2,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   goto free_button;
+   goto free_jacks;
}
}
 
if (module->button_mask & SND_JACK_BTN_3) {
-   ret = snd_jack_set_key(module->button_jack.jack, SND_JACK_BTN_3,
+   ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_3,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   goto free_button

[PATCH v2 0/6] Enable Greybus Audio codec driver

[Vaibhav Agarwal]

The existing GB Audio codec driver is dependent on MSM8994 Audio driver.
During the development stage, this dependency was configured due to
various changes involved in MSM Audio driver to enable addtional codec
card and some of the changes proposed in mainline ASoC framework.
However, these are not the real dependencies and some of them can be
easily removed.

The folowing patch series includes the changes to resolve unnecessary
depedencies and make the codec driver functional with the latest kernel.

Patch 1,2: Incudes jack framework related changes.
Patch 3,4,5: Resolves compilation error observed with the latest kernel and
also provides helper APIs required to allow dynamic addition/removal of
modules.
Patch 6: Finally provides config options and related Makefile changes to
enable GB Codec driver.

Thanks to Alexandre for raising the headsup [1] and motivating me to provide
the necessary changes.

[1] 
https://lore.kernel.org/lkml/20200507212912.599433-1-alexandre.bell...@bootlin.com/

Changes from v1
- Include the changes for the review comments suggested by Dan
- Rebase to latest staging-next

Vaibhav Agarwal (6):
  staging: greybus: audio: Update snd_jack FW usage as per new APIs
  staging: greybus: audio: Maintain jack list within GB Audio module
  staging: greybus: audio: Resolve compilation errors for GB codec
module
  staging: greybus: audio: Resolve compilation error in topology parser
  staging: greybus: audio: Add helper APIs for dynamic audio modules
  staging: greybus: audio: Enable GB codec, audio module compilation.

 drivers/staging/greybus/Kconfig  |  14 +-
 drivers/staging/greybus/Makefile |   6 +-
 drivers/staging/greybus/audio_codec.c| 178 +++-
 drivers/staging/greybus/audio_codec.h|  12 +-
 drivers/staging/greybus/audio_helper.c   | 197 +++
 drivers/staging/greybus/audio_helper.h   |  17 ++
 drivers/staging/greybus/audio_module.c   |  15 +-
 drivers/staging/greybus/audio_topology.c | 128 +++
 8 files changed, 412 insertions(+), 155 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h


base-commit: af7b4801030c07637840191c69eb666917e4135d
-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

[Vaibhav Agarwal]

On Wed, Jun 10, 2020 at 06:37:11PM +0100, Mark Brown wrote:
> On Wed, Jun 10, 2020 at 10:58:24PM +0530, Vaibhav Agarwal wrote:
> > The existing GB Audio codec driver is dependent on MSM8994 Audio driver.
> > During the development stage, this dependency was configured due to
> > various changes involved in MSM Audio driver to enable addtional codec
> > card and some of the changes proposed in mainline ASoC framework.
> 
> I'm not sure why you're copying me on a staging driver?  I don't recall
> the base driver having been submitted properly yet.

Hi Mark,

With patch#6 in this series, I'm proposing some of the (dummy) helper 
APIs required to link DAPM DAI widgets for the GB Audio modules 
added/removed dynamically.

Eventually, I would like to propose relevant changes in snd-soc APIs to 
enable dynamic linking of DAI widgets for the modules added and 
remove/free component controls for the module removed.

I'm seeking your opinion on the proposed changes. And as per the 
recommendation I'm sharing the changes with ASoC mailing list as well.

Kindly suggest me the preferred way to follow on this thread. 

--
Regards,
Vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 4/6] staging: greybus: audio: Resolve compilation error in topology parser

[Vaibhav Agarwal]

On Wed, Jun 10, 2020 at 08:45:35PM +0300, Dan Carpenter wrote:
> On Wed, Jun 10, 2020 at 10:58:28PM +0530, Vaibhav Agarwal wrote:
> > @@ -437,11 +433,12 @@ static int gbcodec_mixer_dapm_ctl_put(struct 
> > snd_kcontrol *kcontrol,
> > struct gbaudio_module_info *module;
> > struct snd_soc_dapm_widget_list *wlist = snd_kcontrol_chip(kcontrol);
> > struct snd_soc_dapm_widget *widget = wlist->widgets[0];
> > -   struct snd_soc_codec *codec = widget->codec;
> > -   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
> > +   struct device *codec_dev = widget->dapm->dev;
> > +   struct gbaudio_codec_info *gb = dev_get_drvdata(codec_dev);
> > +   struct snd_soc_dapm_update *update = NULL;
> ^
> 
> > struct gb_bundle *bundle;
> >  
> > -   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
> > +   dev_dbg(codec_dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
> > module = find_gb_module(gb, kcontrol->id.name);
> > if (!module)
> > return -EINVAL;
> > @@ -458,17 +455,13 @@ static int gbcodec_mixer_dapm_ctl_put(struct 
> > snd_kcontrol *kcontrol,
> > max = le32_to_cpu(info->value.integer.max);
> > mask = (1 << fls(max)) - 1;
> > val = ucontrol->value.integer.value[0] & mask;
> > -   connect = !!val;
> >  
> > /* update ucontrol */
> > if (gbvalue.value.integer_value[0] != val) {
> > for (wi = 0; wi < wlist->num_widgets; wi++) {
> > widget = wlist->widgets[wi];
> > -
> > -   widget->value = val;
> > -   widget->dapm->update = NULL;
> > -   snd_soc_dapm_mixer_update_power(widget, kcontrol,
> > -   connect);
> > +   snd_soc_dapm_mixer_update_power(widget->dapm, kcontrol,
> > +   val, update);
>  ^^
> Always NULL.  Just delete the update variable.

Aah, my bad! Thanks Dan for sharing your comments. I'll fix this while 
sharing next patchset.

--
Regards,
Vaibhav

> 
> regards,
> dan carpenter
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RESEND PATCH v1 6/6] staging: greybus: audio: Enable GB codec, audio module compilation.

[Vaibhav Agarwal]

On Tue, Jun 02, 2020 at 03:57:15PM +0300, Dan Carpenter wrote:
> On Tue, Jun 02, 2020 at 10:51:15AM +0530, Vaibhav Agarwal wrote:
> > Currently, GB codec and audio module is conditionally compiled based on
> > GREYBUS_AUDIO_MSM8994. However, audio module is not dependent on MSM8994
> > platform and can be used generically with any platform that follows
> > GB Audio class specification.
> > 
> > Also, GB codec driver corresponds to dummy codec represented by I2S port
> > available on Toshiba AP Bridge. Added config option for the same in
> > kconfig file and accordingly updated Makefile.
> > 
> 
> This commit message was a bit confusing to me.  Just say:
> 
> "Currently you can't enable the Grey Bus Audio Codec because there is no
> entry for it in the Kconfig file.  Originally the config name was going
> to be AUDIO_MSM8994 but that's not correct because other types of
> hardware are supported now.  I have chosen the name AUDIO_APB_CODEC
> instead.  Also I had to update the dependencies for GREYBUS_AUDIO to
> make the compile work."
> 
> Otherwise this looks fine.

Thanks Dan for sharing your valuable feedback. I'll make the suggested 
changes for the complete series in v2 patchset.

regards,
vaibhav

> 
> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/6] staging: greybus: audio: Update snd_jack FW usage as per new APIs

[Vaibhav Agarwal]

snd_soc_jack APIs are modified in recent kernel versions. This patch
updates the codec driver to resolve the compilation errors related to
jack framework.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c | 54 +--
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 08746c85dea6..5d3a5e6a8fe6 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -709,17 +709,26 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
 };
 
 static int gbaudio_init_jack(struct gbaudio_module_info *module,
-struct snd_soc_codec *codec)
+struct snd_soc_card *card)
 {
int ret;
+   struct snd_soc_jack_pin *headset, *button;
 
if (!module->jack_mask)
return 0;
 
snprintf(module->jack_name, NAME_SIZE, "GB %d Headset Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->jack_name, module->jack_mask,
-  >headset_jack);
+
+   headset = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
+   if (!headset)
+   return -ENOMEM;
+
+   headset->pin = module->jack_name;
+   headset->mask = module->jack_mask;
+
+   ret = snd_soc_card_jack_new(card, module->jack_name, module->jack_mask,
+   >headset_jack, headset, 1);
if (ret) {
dev_err(module->dev, "Failed to create new jack\n");
return ret;
@@ -730,11 +739,21 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 
snprintf(module->button_name, NAME_SIZE, "GB %d Button Jack",
 module->dev_id);
-   ret = snd_soc_jack_new(codec, module->button_name, module->button_mask,
-  >button_jack);
+   button = devm_kzalloc(module->dev, sizeof(*button), GFP_KERNEL);
+   if (!button) {
+   ret = -ENOMEM;
+   goto free_headset;
+   }
+
+   button->pin = module->button_name;
+   button->mask = module->button_mask;
+
+   ret = snd_soc_card_jack_new(card, module->button_name,
+   module->button_mask, >button_jack,
+   button, 1);
if (ret) {
dev_err(module->dev, "Failed to create button jack\n");
-   return ret;
+   goto free_headset;
}
 
/*
@@ -750,7 +769,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_MEDIA);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -759,7 +778,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOICECOMMAND);
if (ret) {
dev_err(module->dev, "Failed to set BTN_1\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -768,7 +787,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEUP);
if (ret) {
dev_err(module->dev, "Failed to set BTN_2\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -777,7 +796,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
   KEY_VOLUMEDOWN);
if (ret) {
dev_err(module->dev, "Failed to set BTN_0\n");
-   return ret;
+   goto free_button;
}
}
 
@@ -788,6 +807,16 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
*/
 
return 0;
+
+free_button:
+   snd_device_free(card->snd_card, module->button_jack.jack);
+   list_del(>button_jack.list);
+
+free_headset:
+   snd_device_free(card->snd_card, module->headset_jack.jack);
+   list_del(>headset_jack.list);
+
+   return ret;
 }
 
 int gbaudio_register_module(struct gbaudio_module_info *module)
@@ -815,7 +844,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EINVAL;
}
 
-   ret = gbaudio_init_jack(module, codec);
+   ret = gbaudio_init_jack(module, component->card);
if (ret) {
up_write(>controls_rwsem);
return ret;
@@ -942,7 +971,8 @@ void gbaudio_unregister_

[PATCH v2 4/6] staging: greybus: audio: Resolve compilation error in topology parser

[Vaibhav Agarwal]

Fix compilation errors for GB Audio topology parser code with recent
kernel versions.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_topology.c | 128 +++
 1 file changed, 60 insertions(+), 68 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 4ac30accf226..257370dc29fa 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -5,8 +5,8 @@
  * Copyright 2015-2016 Linaro Ltd.
  */
 
+#include 
 #include "audio_codec.h"
-#include "greybus_protocols.h"
 
 #define GBAUDIO_INVALID_ID 0xFF
 
@@ -165,15 +165,15 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_info *info;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gbcodec = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gbcodec = 
snd_soc_component_get_drvdata(comp);
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
info = (struct gb_audio_ctl_elem_info *)data->info;
 
if (!info) {
-   dev_err(codec->dev, "NULL info for %s\n", uinfo->id.name);
+   dev_err(comp->dev, "NULL info for %s\n", uinfo->id.name);
return -EINVAL;
}
 
@@ -201,7 +201,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
*kcontrol,
strlcpy(uinfo->value.enumerated.name, name, NAME_SIZE);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
break;
}
@@ -216,11 +216,11 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
struct gb_bundle *bundle;
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
@@ -239,7 +239,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
gb_pm_runtime_put_autosuspend(bundle);
 
if (ret) {
-   dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret,
+   dev_err_ratelimited(comp->dev, "%d:Error in %s for %s\n", ret,
__func__, kcontrol->id.name);
return ret;
}
@@ -262,7 +262,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol 
*kcontrol,
le32_to_cpu(gbvalue.value.enumerated_item[1]);
break;
default:
-   dev_err(codec->dev, "Invalid type: %d for %s:kcontrol\n",
+   dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
info->type, kcontrol->id.name);
ret = -EINVAL;
break;
@@ -278,11 +278,11 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
struct gbaudio_ctl_pvt *data;
struct gb_audio_ctl_elem_value gbvalue;
struct gbaudio_module_info *module;
-   struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
-   struct gbaudio_codec_info *gb = snd_soc_codec_get_drvdata(codec);
+   struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+   struct gbaudio_codec_info *gb = snd_soc_component_get_drvdata(comp);
struct gb_bundle *bundle;
 
-   dev_dbg(codec->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
+   dev_dbg(comp->dev, "Entered %s:%s\n", __func__, kcontrol->id.name);
module = find_gb_module(gb, kcontrol->id.name);
if (!module)
return -EINVAL;
@@ -309,7 +309,7 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol 
*kcontrol,
cpu_to_le32(uc

[PATCH v2 3/6] staging: greybus: audio: Resolve compilation errors for GB codec module

[Vaibhav Agarwal]

Due to dependencies on ASoC framework changes, GB dummy codec module
compilation is currently disabled. This patch updates codec driver as
per the latest ASoC APIs.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/audio_codec.c | 88 +--
 drivers/staging/greybus/audio_codec.h |  2 +-
 2 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 6dc4ee2bfb37..0ecdba27086b 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -825,7 +825,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info 
*module,
 int gbaudio_register_module(struct gbaudio_module_info *module)
 {
int ret;
-   struct snd_soc_codec *codec;
+   struct snd_soc_component *comp;
struct snd_card *card;
struct gbaudio_jack *jack = NULL;
 
@@ -834,8 +834,8 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EAGAIN;
}
 
-   codec = gbcodec->codec;
-   card = codec->card->snd_card;
+   comp = gbcodec->component;
+   card = comp->card->snd_card;
 
down_write(>controls_rwsem);
 
@@ -847,33 +847,33 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
return -EINVAL;
}
 
-   ret = gbaudio_init_jack(module, component->card);
+   ret = gbaudio_init_jack(module, comp->card);
if (ret) {
up_write(>controls_rwsem);
return ret;
}
 
if (module->dapm_widgets)
-   snd_soc_dapm_new_controls(>dapm, module->dapm_widgets,
+   snd_soc_dapm_new_controls(>dapm, module->dapm_widgets,
  module->num_dapm_widgets);
if (module->controls)
-   snd_soc_add_codec_controls(codec, module->controls,
-  module->num_controls);
+   snd_soc_add_component_controls(comp, module->controls,
+  module->num_controls);
if (module->dapm_routes)
-   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
+   snd_soc_dapm_add_routes(>dapm, module->dapm_routes,
module->num_dapm_routes);
 
/* card already instantiated, create widgets here only */
-   if (codec->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(codec->card,
-   >dapm);
+   if (comp->card->instantiated) {
+   snd_soc_dapm_link_component_dai_widgets(comp->card,
+   >dapm);
 #ifdef CONFIG_SND_JACK
/*
 * register jack devices for this module
 * from codec->jack_list
 */
list_for_each_entry(jack, >jack_list, list) {
-   snd_device_register(codec->card->snd_card,
+   snd_device_register(comp->card->snd_card,
jack->jack.jack);
}
 #endif
@@ -883,9 +883,9 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
list_add(>list, >module_list);
mutex_unlock(>lock);
 
-   if (codec->card->instantiated)
-   ret = snd_soc_dapm_new_widgets(>dapm);
-   dev_dbg(codec->dev, "Registered %s module\n", module->name);
+   if (comp->card->instantiated)
+   ret = snd_soc_dapm_new_widgets(comp->card);
+   dev_dbg(comp->dev, "Registered %s module\n", module->name);
 
up_write(>controls_rwsem);
return ret;
@@ -956,18 +956,18 @@ static void gbaudio_codec_cleanup(struct 
gbaudio_module_info *module)
 
 void gbaudio_unregister_module(struct gbaudio_module_info *module)
 {
-   struct snd_soc_codec *codec = gbcodec->codec;
-   struct snd_card *card = codec->card->snd_card;
+   struct snd_soc_component *comp = gbcodec->component;
+   struct snd_card *card = comp->card->snd_card;
struct gbaudio_jack *jack, *n;
int mask;
 
-   dev_dbg(codec->dev, "Unregister %s module\n", module->name);
+   dev_dbg(comp->dev, "Unregister %s module\n", module->name);
 
down_write(>controls_rwsem);
mutex_lock(>lock);
gbaudio_codec_cleanup(module);
list_del(>list);
-   dev_dbg(codec->dev, "Process Unregister %s module\n", module->name);
+   dev_dbg(comp->dev, "Process Unregister %s module\n", module->name);
mutex_unlock(>lock);
 
 #ifdef CONFIG_SND_JACK
@@ -983,99 +983,97 @@ void gba

[PATCH v2 6/6] staging: greybus: audio: Enable GB codec, audio module compilation.

[Vaibhav Agarwal]

Currently you can't enable the Gey Bus Audio Codec because there is no
entry for it in the Kconfig file. Originally the config name was going
to be AUDIO_MSM8994 but that's not correct because other types of
hardware are supported now. I have chosen the name AUDIO_APB_CODEC
instead.  Also I had to update the dependencies for GREYBUS_AUDIO to
make the compile work.

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/Kconfig  | 14 +-
 drivers/staging/greybus/Makefile |  4 ++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
index d4777f5a8b90..cbcfcbba239b 100644
--- a/drivers/staging/greybus/Kconfig
+++ b/drivers/staging/greybus/Kconfig
@@ -3,7 +3,7 @@ if GREYBUS
 
 config GREYBUS_AUDIO
tristate "Greybus Audio Class driver"
-   depends on SOUND
+   depends on SOUND && SND_SOC
---help---
  Select this option if you have a device that follows the
  Greybus Audio Class specification.
@@ -11,6 +11,18 @@ config GREYBUS_AUDIO
  To compile this code as a module, chose M here: the module
  will be called gb-audio.ko
 
+config GREYBUS_AUDIO_APB_CODEC
+   tristate "Greybus APBridge Audio codec driver"
+   depends on SND_SOC && GREYBUS_AUDIO
+   help
+ Select this option if you have a Toshiba APB device that has I2S
+  ports and acts as a Greybus "Dummy codec". This device is a
+  bridge from an APB-I2S port to a Unipro network.
+
+ To compile this code as a module, chose M here: the module
+ will be called gb-audio-codec.ko
+
+
 config GREYBUS_BOOTROM
tristate "Greybus Bootrom Class driver"
---help---
diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 3b4b6cabff19..7c5e89622334 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -40,8 +40,8 @@ gb-audio-manager-y:= audio_manager.o 
audio_manager_module.o
 #ccflags-y += -DGB_AUDIO_MANAGER_SYSFS
 #endif
 
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-codec.o
-obj-$(CONFIG_GREYBUS_AUDIO_MSM8994)+= gb-audio-module.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-codec.o
+obj-$(CONFIG_GREYBUS_AUDIO_APB_CODEC)  += gb-audio-module.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-gb.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-apbridgea.o
 obj-$(CONFIG_GREYBUS_AUDIO)+= gb-audio-manager.o
-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 5/6] staging: greybus: audio: Add helper APIs for dynamic audio modules

[Vaibhav Agarwal]

Greybus Codec driver allows modules to be dynamically added and removed,
which further requires updating the DAPM configurations as well.

With current snd_soc architecture, dynamic audio modules is not yet
supported. This patch provides helper APIs to update DAPM configurations
in response to modules which are dynamically added or removed. The
source is primarily based on snd_dapm.c

Signed-off-by: Vaibhav Agarwal 
---
 drivers/staging/greybus/Makefile   |   2 +-
 drivers/staging/greybus/audio_codec.c  |  12 +-
 drivers/staging/greybus/audio_helper.c | 197 +
 drivers/staging/greybus/audio_helper.h |  17 +++
 4 files changed, 223 insertions(+), 5 deletions(-)
 create mode 100644 drivers/staging/greybus/audio_helper.c
 create mode 100644 drivers/staging/greybus/audio_helper.h

diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 627e44f2a983..3b4b6cabff19 100644
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_GREYBUS_VIBRATOR)+= gb-vibrator.o
 
 # Greybus Audio is a bunch of modules
 gb-audio-module-y  := audio_module.o audio_topology.o
-gb-audio-codec-y   := audio_codec.o
+gb-audio-codec-y   := audio_codec.o audio_helper.o
 gb-audio-gb-y  := audio_gb.o
 gb-audio-apbridgea-y   := audio_apbridgea.o
 gb-audio-manager-y := audio_manager.o audio_manager_module.o
diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 0ecdba27086b..74538f8c5fa4 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -14,6 +14,7 @@
 #include "audio_codec.h"
 #include "audio_apbridgea.h"
 #include "audio_manager.h"
+#include "audio_helper.h"
 
 static struct gbaudio_codec_info *gbcodec;
 
@@ -865,7 +866,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
*module)
 
/* card already instantiated, create widgets here only */
if (comp->card->instantiated) {
-   snd_soc_dapm_link_component_dai_widgets(comp->card,
+   gbaudio_dapm_link_component_dai_widgets(comp->card,
>dapm);
 #ifdef CONFIG_SND_JACK
/*
@@ -999,13 +1000,16 @@ void gbaudio_unregister_module(struct 
gbaudio_module_info *module)
if (module->controls) {
dev_dbg(comp->dev, "Removing %d controls\n",
module->num_controls);
-   snd_soc_remove_codec_controls(comp, module->controls,
- module->num_controls);
+   /* release control semaphore */
+   up_write(>controls_rwsem);
+   gbaudio_remove_component_controls(comp, module->controls,
+ module->num_controls);
+   down_write(>controls_rwsem);
}
if (module->dapm_widgets) {
dev_dbg(comp->dev, "Removing %d widgets\n",
module->num_dapm_widgets);
-   snd_soc_dapm_free_controls(>dapm, module->dapm_widgets,
+   gbaudio_dapm_free_controls(>dapm, module->dapm_widgets,
   module->num_dapm_widgets);
}
 
diff --git a/drivers/staging/greybus/audio_helper.c 
b/drivers/staging/greybus/audio_helper.c
new file mode 100644
index ..faaa39708118
--- /dev/null
+++ b/drivers/staging/greybus/audio_helper.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Greybus Audio Sound SoC helper APIs
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#define gbaudio_dapm_for_each_direction(dir) \
+   for ((dir) = SND_SOC_DAPM_DIR_IN; (dir) <= SND_SOC_DAPM_DIR_OUT; \
+   (dir)++)
+
+static void gbaudio_dapm_link_dai_widget(struct snd_soc_dapm_widget *dai_w,
+struct snd_soc_card *card)
+{
+   struct snd_soc_dapm_widget *w;
+   struct snd_soc_dapm_widget *src, *sink;
+   struct snd_soc_dai *dai = dai_w->priv;
+
+   /* ...find all widgets with the same stream and link them */
+   list_for_each_entry(w, >widgets, list) {
+   if (w->dapm != dai_w->dapm)
+   continue;
+
+   switch (w->id) {
+   case snd_soc_dapm_dai_in:
+   case snd_soc_dapm_dai_out:
+   continue;
+   default:
+   break;
+   }
+
+   if (!w->sname || !strstr(w->sname, dai_w->sname))
+   continue;
+
+   /*
+* check if widget is already linked,
+* if (w->linked)
+*  return;
+*/
+
+   if (dai_w->id == snd_soc_dapm_dai_in) 

Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

[Vaibhav Agarwal]

On Thu, Jun 11, 2020 at 09:26:16AM +0100, Mark Brown wrote:
> On Wed, Jun 10, 2020 at 11:53:24PM +0530, Vaibhav Agarwal wrote:
> 
> > With patch#6 in this series, I'm proposing some of the (dummy) helper 
> > APIs required to link DAPM DAI widgets for the GB Audio modules 
> > added/removed dynamically.
> 
> > Eventually, I would like to propose relevant changes in snd-soc APIs to 
> > enable dynamic linking of DAI widgets for the modules added and 
> > remove/free component controls for the module removed.
> 
> > I'm seeking your opinion on the proposed changes. And as per the 
> > recommendation I'm sharing the changes with ASoC mailing list as well.
> 
> These are proposed incremental changes to an out of tree driver that has
> never been submitted.  I don't know what the current code looks like,
> what it's supposed to be doing or anything like that so I've no idea
> what's going on or why.
> 
> > Kindly suggest me the preferred way to follow on this thread. 
> 
> This is effectively out of tree code, until someone submits it properly
> I'm not sure it's useful to submit incremental patches upstream.

Thanks for the suggestion Mark. I'll create a separate patchset aligned 
to the ASoC tree in next ~2 weeks and share them separately.

Hi Greg,

Do you think the current patchset can be considered for the purpose of 
resolving componentization issue raised by Alexandre?

--
Regards,
Vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: issue with uninitialized value used in a comparison in gbcodec_mixer_dapm_ctl_put

[Vaibhav Agarwal]

On Thu, Jul 30, 2020 at 05:02:22PM +0100, Colin Ian King wrote:
> Hi,
> 
> Static analysis with Coverity has detected an uninitialized value being
> used in a comparison.  The error was detected on a recent change to
> drivers/staging/greybus/audio_topology.c however the issue actually
> dates back to the original commit:

Thanks Colin for reporting the issue. I'll fix the same and share a 
patch soon.

--
Regards,
Vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: greybus: audio: fix uninitialized value issue

[Vaibhav Agarwal]

The current implementation for gbcodec_mixer_dapm_ctl_put() uses
uninitialized gbvalue for comparison with updated value. This was found
using static analysis with coverity.

Uninitialized scalar variable (UNINIT)
11. uninit_use: Using uninitialized value
gbvalue.value.integer_value[0].
460if (gbvalue.value.integer_value[0] != val) {

This patch fixes the issue with fetching the gbvalue before using it for
comparision.

Fixes: 6339d2322c47 ("greybus: audio: Add topology parser for GB codec")
Reported-by: Colin Ian King 
Signed-off-by: Vaibhav Agarwal 
---
Changelog:
v2: Fix the missing check for return value after get_control.
---
 drivers/staging/greybus/audio_topology.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 2f9fdbdcd547..9f98691b2f5f 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -456,6 +456,18 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
val = ucontrol->value.integer.value[0] & mask;
connect = !!val;
 
+   ret = gb_pm_runtime_get_sync(bundle);
+   if (ret)
+   return ret;
+
+   ret = gb_audio_gb_get_control(module->mgmt_connection, data->ctl_id,
+ GB_AUDIO_INVALID_INDEX, );
+   if (ret) {
+   dev_err_ratelimited(codec_dev, "%d:Error in %s for %s\n", ret,
+   __func__, kcontrol->id.name);
+   return ret;
+   }
+
/* update ucontrol */
if (gbvalue.value.integer_value[0] != val) {
for (wi = 0; wi < wlist->num_widgets; wi++) {
@@ -466,16 +478,10 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
gbvalue.value.integer_value[0] =
cpu_to_le32(ucontrol->value.integer.value[0]);
 
-   ret = gb_pm_runtime_get_sync(bundle);
-   if (ret)
-   return ret;
-
ret = gb_audio_gb_set_control(module->mgmt_connection,
  data->ctl_id,
  GB_AUDIO_INVALID_INDEX, );
 
-   gb_pm_runtime_put_autosuspend(bundle);
-
if (ret) {
dev_err_ratelimited(codec_dev,
"%d:Error in %s for %s\n", ret,
@@ -483,6 +489,7 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
return ret;
}
}
+   gb_pm_runtime_put_autosuspend(bundle);
 
return 0;
 }

base-commit: fc80c51fd4b23ec007e88d4c688f2cac1b8648e7
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: greybus: audio: fix uninitialized value issue

[Vaibhav Agarwal]

The current implementation for gbcodec_mixer_dapm_ctl_put() uses
uninitialized gbvalue for comparison with updated value. This was found
using static analysis with coverity.

Uninitialized scalar variable (UNINIT)
11. uninit_use: Using uninitialized value
gbvalue.value.integer_value[0].
460if (gbvalue.value.integer_value[0] != val) {

This patch fixes the issue with fetching the gbvalue before using it for
comparision.

Fixes: 6339d2322c47 ("greybus: audio: Add topology parser for GB codec")
Reported-by: Colin Ian King 
Signed-off-by: Vaibhav Agarwal 
---
Changelog:
v2: Fix the missing check for return value after get_control.
v3: Use single exit path to avoid missing autosuspend sequence.
---
 drivers/staging/greybus/audio_topology.c | 29 
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c 
b/drivers/staging/greybus/audio_topology.c
index 2f9fdbdcd547..83b38ae8908c 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -456,6 +456,15 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
val = ucontrol->value.integer.value[0] & mask;
connect = !!val;

+   ret = gb_pm_runtime_get_sync(bundle);
+   if (ret)
+   return ret;
+
+   ret = gb_audio_gb_get_control(module->mgmt_connection, data->ctl_id,
+ GB_AUDIO_INVALID_INDEX, );
+   if (ret)
+   goto exit;
+
/* update ucontrol */
if (gbvalue.value.integer_value[0] != val) {
for (wi = 0; wi < wlist->num_widgets; wi++) {
@@ -466,25 +475,17 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol 
*kcontrol,
gbvalue.value.integer_value[0] =
cpu_to_le32(ucontrol->value.integer.value[0]);

-   ret = gb_pm_runtime_get_sync(bundle);
-   if (ret)
-   return ret;
-
ret = gb_audio_gb_set_control(module->mgmt_connection,
  data->ctl_id,
  GB_AUDIO_INVALID_INDEX, );
-
-   gb_pm_runtime_put_autosuspend(bundle);
-
-   if (ret) {
-   dev_err_ratelimited(codec_dev,
-   "%d:Error in %s for %s\n", ret,
-   __func__, kcontrol->id.name);
-   return ret;
-   }
}

-   return 0;
+exit:
+   gb_pm_runtime_put_autosuspend(bundle);
+   if (ret)
+   dev_err_ratelimited(codec_dev, "%d:Error in %s for %s\n", ret,
+   __func__, kcontrol->id.name);
+   return ret;
 }

 #define SOC_DAPM_MIXER_GB(xname, kcount, data) \

base-commit: fc80c51fd4b23ec007e88d4c688f2cac1b8648e7
--
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: audio: Add missing unlock in gbaudio_dapm_free_controls()

[Vaibhav Agarwal]

On Fri, Dec 4, 2020 at 2:10 PM Johan Hovold  wrote:
>
> On Fri, Dec 04, 2020 at 10:13:50AM +0800, Wang Hai wrote:
> > Add the missing unlock before return from function
> > gbaudio_dapm_free_controls() in the error handling case.
> >
> > Fixes: 510e340efe0c ("staging: greybus: audio: Add helper APIs for dynamic 
> > audio module")
> > Reported-by: Hulk Robot 
> > Signed-off-by: Wang Hai 
> > ---
> >  drivers/staging/greybus/audio_helper.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/greybus/audio_helper.c 
> > b/drivers/staging/greybus/audio_helper.c
> > index 237531ba60f3..293675dbea10 100644
> > --- a/drivers/staging/greybus/audio_helper.c
> > +++ b/drivers/staging/greybus/audio_helper.c
> > @@ -135,6 +135,7 @@ int gbaudio_dapm_free_controls(struct 
> > snd_soc_dapm_context *dapm,
> >   if (!w) {
> >   dev_err(dapm->dev, "%s: widget not found\n",
> >   widget->name);
> > + mutex_unlock(>card->dapm_mutex);
> >   return -EINVAL;
> >   }
> >   widget++;
>
> This superficially looks correct, but there seems to be another bug in
> this function. It can be used free an array of widgets, but if one of
> them isn't found we just leak the rest. Perhaps that return should
> rather be "widget++; continue;".
>
> Vaibhav?

Thanks Wang for sharing the patch. As already pointed by Johan, this
function indeed has another bug as well.
Pls feel free to share the patch as suggested above.

--
vaibhav

>
> Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: audio: Fix possible leak free widgets in gbaudio_dapm_free_controls

[Vaibhav Agarwal]

On Sat, Dec 5, 2020 at 4:02 PM Wang Hai  wrote:
>
> In gbaudio_dapm_free_controls(), if one of the widgets is not found, an error
> will be returned directly, which will cause the rest to be unable to be freed,
> resulting in leak.
>
> This patch fixes the bug. If if one of them is not found, just skip and free 
> the others.
>

nit, typo error "If if one".

> Fixes: 510e340efe0c ("staging: greybus: audio: Add helper APIs for dynamic 
> audio module")
> Reported-by: Hulk Robot 
> Signed-off-by: Wang Hai 
> ---

Reviewed-by: Vaibhav Agarwal 

--
thanks,
vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel