* [PATCH] usb: gadget: u_audio: fix calculations for small bInterval
@ 2022-01-04 18:32 John Keeping
2022-01-05 10:36 ` Pavel Hofman
0 siblings, 1 reply; 2+ messages in thread
From: John Keeping @ 2022-01-04 18:32 UTC (permalink / raw)
To: linux-usb
Cc: John Keeping, Felipe Balbi, Greg Kroah-Hartman, Pavel Hofman,
linux-kernel
If bInterval is 1, then p_interval is 8000 and p_interval_mil is 8E9,
which is too big for a 32-bit value. While the storage is indeed
64-bit, this value is used as the divisor in do_div() which will
truncate it into a uint32_t leading to incorrect calculated values.
Switch back to keeping the base value in struct snd_uac_chip which fits
easily into an int, meaning that the division can be done in two steps
with the divisor fitting safely into a uint32_t on both steps.
Fixes: 6fec018a7e70 ("usb: gadget: u_audio.c: Adding Playback Pitch ctl for sync playback")
Signed-off-by: John Keeping <john@metanate.com>
---
drivers/usb/gadget/function/u_audio.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index c46400be5464..4fb05f9576a6 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -76,8 +76,8 @@ struct snd_uac_chip {
struct snd_pcm *pcm;
/* pre-calculated values for playback iso completion */
- unsigned long long p_interval_mil;
unsigned long long p_residue_mil;
+ unsigned int p_interval;
unsigned int p_framesize;
};
@@ -194,21 +194,24 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
* If there is a residue from this division, add it to the
* residue accumulator.
*/
+ unsigned long long p_interval_mil = uac->p_interval * 1000000ULL;
+
pitched_rate_mil = (unsigned long long)
params->p_srate * prm->pitch;
div_result = pitched_rate_mil;
- do_div(div_result, uac->p_interval_mil);
+ do_div(div_result, uac->p_interval);
+ do_div(div_result, 1000000);
frames = (unsigned int) div_result;
pr_debug("p_srate %d, pitch %d, interval_mil %llu, frames %d\n",
- params->p_srate, prm->pitch, uac->p_interval_mil, frames);
+ params->p_srate, prm->pitch, p_interval_mil, frames);
p_pktsize = min_t(unsigned int,
uac->p_framesize * frames,
ep->maxpacket);
if (p_pktsize < ep->maxpacket) {
- residue_frames_mil = pitched_rate_mil - frames * uac->p_interval_mil;
+ residue_frames_mil = pitched_rate_mil - frames * p_interval_mil;
p_pktsize_residue_mil = uac->p_framesize * residue_frames_mil;
} else
p_pktsize_residue_mil = 0;
@@ -222,11 +225,11 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
* size and decrease the accumulator.
*/
div_result = uac->p_residue_mil;
- do_div(div_result, uac->p_interval_mil);
+ do_div(div_result, uac->p_interval);
+ do_div(div_result, 1000000);
if ((unsigned int) div_result >= uac->p_framesize) {
req->length += uac->p_framesize;
- uac->p_residue_mil -= uac->p_framesize *
- uac->p_interval_mil;
+ uac->p_residue_mil -= uac->p_framesize * p_interval_mil;
pr_debug("increased req length to %d\n", req->length);
}
pr_debug("remains uac->p_residue_mil %llu\n", uac->p_residue_mil);
@@ -591,7 +594,7 @@ int u_audio_start_playback(struct g_audio *audio_dev)
unsigned int factor;
const struct usb_endpoint_descriptor *ep_desc;
int req_len, i;
- unsigned int p_interval, p_pktsize;
+ unsigned int p_pktsize;
ep = audio_dev->in_ep;
prm = &uac->p_prm;
@@ -612,11 +615,10 @@ int u_audio_start_playback(struct g_audio *audio_dev)
/* pre-compute some values for iso_complete() */
uac->p_framesize = params->p_ssize *
num_channels(params->p_chmask);
- p_interval = factor / (1 << (ep_desc->bInterval - 1));
- uac->p_interval_mil = (unsigned long long) p_interval * 1000000;
+ uac->p_interval = factor / (1 << (ep_desc->bInterval - 1));
p_pktsize = min_t(unsigned int,
uac->p_framesize *
- (params->p_srate / p_interval),
+ (params->p_srate / uac->p_interval),
ep->maxpacket);
req_len = p_pktsize;
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: fix calculations for small bInterval
2022-01-04 18:32 [PATCH] usb: gadget: u_audio: fix calculations for small bInterval John Keeping
@ 2022-01-05 10:36 ` Pavel Hofman
0 siblings, 0 replies; 2+ messages in thread
From: Pavel Hofman @ 2022-01-05 10:36 UTC (permalink / raw)
To: John Keeping, linux-usb; +Cc: Felipe Balbi, Greg Kroah-Hartman, linux-kernel
Dne 04. 01. 22 v 19:32 John Keeping napsal(a):
> If bInterval is 1, then p_interval is 8000 and p_interval_mil is 8E9,
> which is too big for a 32-bit value. While the storage is indeed
> 64-bit, this value is used as the divisor in do_div() which will
> truncate it into a uint32_t leading to incorrect calculated values.
>
> Switch back to keeping the base value in struct snd_uac_chip which fits
> easily into an int, meaning that the division can be done in two steps
> with the divisor fitting safely into a uint32_t on both steps.
>
> Fixes: 6fec018a7e70 ("usb: gadget: u_audio.c: Adding Playback Pitch ctl for sync playback")
> Signed-off-by: John Keeping <john@metanate.com>
Tested-by: Pavel Hofman <pavel.hofman@ivitera.com>
> ---
> drivers/usb/gadget/function/u_audio.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index c46400be5464..4fb05f9576a6 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -76,8 +76,8 @@ struct snd_uac_chip {
> struct snd_pcm *pcm;
>
> /* pre-calculated values for playback iso completion */
> - unsigned long long p_interval_mil;
> unsigned long long p_residue_mil;
> + unsigned int p_interval;
> unsigned int p_framesize;
> };
>
> @@ -194,21 +194,24 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
> * If there is a residue from this division, add it to the
> * residue accumulator.
> */
> + unsigned long long p_interval_mil = uac->p_interval * 1000000ULL;
> +
> pitched_rate_mil = (unsigned long long)
> params->p_srate * prm->pitch;
> div_result = pitched_rate_mil;
> - do_div(div_result, uac->p_interval_mil);
> + do_div(div_result, uac->p_interval);
> + do_div(div_result, 1000000);
> frames = (unsigned int) div_result;
>
> pr_debug("p_srate %d, pitch %d, interval_mil %llu, frames %d\n",
> - params->p_srate, prm->pitch, uac->p_interval_mil, frames);
> + params->p_srate, prm->pitch, p_interval_mil, frames);
>
> p_pktsize = min_t(unsigned int,
> uac->p_framesize * frames,
> ep->maxpacket);
>
> if (p_pktsize < ep->maxpacket) {
> - residue_frames_mil = pitched_rate_mil - frames * uac->p_interval_mil;
> + residue_frames_mil = pitched_rate_mil - frames * p_interval_mil;
> p_pktsize_residue_mil = uac->p_framesize * residue_frames_mil;
> } else
> p_pktsize_residue_mil = 0;
> @@ -222,11 +225,11 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
> * size and decrease the accumulator.
> */
> div_result = uac->p_residue_mil;
> - do_div(div_result, uac->p_interval_mil);
> + do_div(div_result, uac->p_interval);
> + do_div(div_result, 1000000);
> if ((unsigned int) div_result >= uac->p_framesize) {
> req->length += uac->p_framesize;
> - uac->p_residue_mil -= uac->p_framesize *
> - uac->p_interval_mil;
> + uac->p_residue_mil -= uac->p_framesize * p_interval_mil;
> pr_debug("increased req length to %d\n", req->length);
> }
> pr_debug("remains uac->p_residue_mil %llu\n", uac->p_residue_mil);
> @@ -591,7 +594,7 @@ int u_audio_start_playback(struct g_audio *audio_dev)
> unsigned int factor;
> const struct usb_endpoint_descriptor *ep_desc;
> int req_len, i;
> - unsigned int p_interval, p_pktsize;
> + unsigned int p_pktsize;
>
> ep = audio_dev->in_ep;
> prm = &uac->p_prm;
> @@ -612,11 +615,10 @@ int u_audio_start_playback(struct g_audio *audio_dev)
> /* pre-compute some values for iso_complete() */
> uac->p_framesize = params->p_ssize *
> num_channels(params->p_chmask);
> - p_interval = factor / (1 << (ep_desc->bInterval - 1));
> - uac->p_interval_mil = (unsigned long long) p_interval * 1000000;
> + uac->p_interval = factor / (1 << (ep_desc->bInterval - 1));
> p_pktsize = min_t(unsigned int,
> uac->p_framesize *
> - (params->p_srate / p_interval),
> + (params->p_srate / uac->p_interval),
> ep->maxpacket);
>
> req_len = p_pktsize;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-05 10:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04 18:32 [PATCH] usb: gadget: u_audio: fix calculations for small bInterval John Keeping
2022-01-05 10:36 ` Pavel Hofman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.