On Fri 30 Apr 2021 at 19:09, Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote: > On Fri 30 Apr 2021 at 16:55, Pavel Hofman <pavel.hofman@xxxxxxxxxxx> wrote: > >> Dne 30. 04. 21 v 16:26 Jerome Brunet napsal(a): >>> From: Ruslan Bilovol <ruslan.bilovol@xxxxxxxxx> >>> >>> As per USB and UAC2 specs, asynchronous audio sink endpoint >>> requires explicit synchronization mechanism (Isochronous >>> Feedback Endpoint) >>> >>> Implement feedback companion endpoint for ISO OUT endpoint >>> >>> This patch adds all required infrastructure and USB requests >>> handling for feedback endpoint. Syncrhonization itself is >>> still dummy (feedback ep always reports 'nomimal frequency' >>> e.g. no adjustement is needed). This satisfies hosts that >>> require feedback endpoint (like Win10) and poll it periodically >>> >>> Actual synchronization mechanism should be implemented >>> separately >>> >>> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@xxxxxxxxx> >>> Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx> >> >> Hi, >> >> The HS calculation of Q16.16 feedback value >> overflows at some 524kHz, disallowing use of larger samplerates (e.g. >> 768kHz or higher). >> >> I tested the formula used in alsa USB driver >> https://github.com/torvalds/linux/blob/d99676af540c2dc829999928fb81c58c80a1dce4/sound/usb/endpoint.c#L80 >> which uses only 10bit shift. The feedback control in UAC2 gadget now >> works up to 4M samplerate with 1Hz precision (tested on RPi4 with >> bInterval = 1, checked in stream0 proc file on linux host). >> >> --- a/drivers/usb/gadget/function/u_audio.c >> +++ b/drivers/usb/gadget/function/u_audio.c >> @@ -118,7 +119,8 @@ static void u_audio_set_fback_frequency(enum >> usb_device_speed speed, >> * Prevent integer overflow by calculating in Q12.13 >> format and >> * then shifting to Q16.16 >> */ >> - ff = DIV_ROUND_UP((freq << 13), (8*1000)) << 3; >> + ff = ((freq << 10) + 62) / 125; > > Pavel, The code posted is a little different from snip here. > While I understand the "<< 10" and "/ 125", the "+ 62" would welcome a > comment. OOhhh I got it now ... I think using ROUND_UP() is lot more readable (and maintainable) > > Also in the final patch, the calculation is a bit different and moved to > "long long" ... but I'm sure the same type of improvement could be done. > >> } >> *(__le32 *)buf = cpu_to_le32(ff); >> } >> >> >> Best regards, >> >> Pavel.
