linux-kernel - Re: [PATCH] ANDROID: sound: usb: Add vendor's hooking interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d37660f8-3bb6-5f3a-8ac1-c0d6d5a08468@linux.intel.com>
Date:   Thu, 18 Jun 2020 07:27:07 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     JaeHun Jung <jh0801.jung@...sung.com>, tiwai@...e.de
Cc:     alsa-devel@...a-project.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ANDROID: sound: usb: Add vendor's hooking interface



On 6/16/20 9:18 PM, JaeHun Jung wrote:
> In mobile, a co-processor is used when using USB audio
> to improve power consumption.
> hooking is required for sync-up when operating
> the co-processor. So register call-back function.
> The main operation of the call-back function is as follows:
> - Initialize the co-processor by transmitting data
>    when initializing.
> - Change the co-processor setting value through
>    the interface function.
> - Configure sampling rate
> - pcm open/close

Thank you for this patch. With the removal of the 3.5mm jack on a number 
of platforms, and improvements to reduce power consumption on a variety 
of hosts there's indeed a need to enhance sound/usb in the kernel.

> 
> Bug: 156315379
> 
> Change-Id: I32e1dd408e64aaef68ee06c480c4b4d4c95546dc
> Signed-off-by: JaeHun Jung <jh0801.jung@...sung.com>

You probably want to remove bug references and Change-Id if they are not 
publicly visible

> @@ -891,6 +897,15 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
>   	struct usb_interface *iface;
>   	int ret;
>   
> +	if (usb_audio_ops && usb_audio_ops->vendor_set_pcmbuf) {
> +		ret = usb_audio_ops->vendor_set_pcmbuf(subs->dev);
> +
> +		if (ret < 0) {
> +			dev_err(&subs->dev->dev, "pcm buf transfer failed\n");
> +			return ret;
> +		}
> +	}
> +
>   	if (! subs->cur_audiofmt) {
>   		dev_err(&subs->dev->dev, "no format is specified!\n");
>   		return -ENXIO;
> @@ -924,6 +939,15 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
>   		if (ret < 0)
>   			goto unlock;
>   
> +		if (usb_audio_ops && usb_audio_ops->vendor_set_rate) {
> +			subs->need_setup_ep = false;
> +			usb_audio_ops->vendor_set_rate(
> +					subs->cur_audiofmt->iface,
> +					subs->cur_rate,
> +					subs->cur_audiofmt->altsetting);
> +			goto unlock;
> +		}
> +
>   		ret = configure_endpoint(subs);
>   		if (ret < 0)
>   			goto unlock;
> @@ -1333,6 +1357,9 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
>   	struct snd_usb_substream *subs = &as->substream[direction];
>   	int ret;
>   
> +	if (usb_audio_ops && usb_audio_ops->vendor_pcm_con)
> +		usb_audio_ops->vendor_pcm_con(true, direction);
> +
>   	subs->interface = -1;
>   	subs->altset_idx = 0;
>   	runtime->hw = snd_usb_hardware;
> @@ -1361,12 +1388,18 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
>   	struct snd_usb_substream *subs = &as->substream[direction];
>   	int ret;
>   
> +	if (usb_audio_ops && usb_audio_ops->vendor_pcm_con)
> +		usb_audio_ops->vendor_pcm_con(false, direction);
> +

can you elaborate on the PCM streaming parts? if you allocate a buffer 
for the co-processor to deal with PCM data, will you end-up copying data 
from the existing ring buffers allocated with vmalloc() into your 
coprocessor?

I was instead thinking of having a different 'mode' where the PCM 
buffers are not allocated by sound/usb, but instead allocated by the 
driver taking care of the low-power USB path, so that apps can directly 
write into ring buffers that are handled by the coprocessor/offload unit.

Thanks
-Pierre