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

[Dan Carpenter]

On Tue, Jun 02, 2020 at 10:51:10AM +0530, Vaibhav Agarwal wrote:
> 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;
>  

No blank line please.

> +	struct snd_soc_jack *jack;

This code would be nicer without the "jack" pointer.  Just use
"module->headset_jack" directly so that it's easier to use grep on the
code.

> +	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,
> -			       &module->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 = &module->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,
> -			       &module->button_jack);
> +	button = devm_kzalloc(module->dev, sizeof(*headset), GFP_KERNEL);
                                           ^^^^^^^^^^^^^^^^
Use "sizeof(*button)".  It's the same size so it doesn't affect runtime.


> +	if (!button) {
> +		ret = -ENOMEM;
> +		goto free_headset;
> +	}

regards,
dan carpenter