Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support
On Mon, 30 Jul 2018 18:05:47 +0200,
Jorge wrote:
>
>
>
> On 30/07/18 14:03, Takashi Iwai wrote:
> > On Mon, 30 Jul 2018 11:23:33 +0200,
> > Jorge Sanjuan wrote:
> >>
> >> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
> >> index 334bfa6dfb47..786e5939d831 100644
> >> --- a/include/linux/usb/audio-v3.h
> >> +++ b/include/linux/usb/audio-v3.h
> >> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
> >> /* BADD sample rate is always fixed to 48kHz */
> >> #define UAC3_BADD_SAMPLING_RATE 48000
> >>
> >> +/* BADD power domains recovery times */
> >> +#define UAC3_BADD_PD_RECOVER_D1D0 0x0258
> >> +#define UAC3_BADD_PD_RECOVER_D2D0 0x1770
> >
> > Please put the unit for these values. I guess they don't need to be
> > hex?
> >
>
> Hi!
>
> The BADD spec defines these inferred values as hex (see section 6.2.2.13
> of the spec). Should we keep them as per spec to avoid confusion? I'll
> update comment there with the units (50 us increments).
Well, if they were defined in hex, it makes sense to keep the raw
values as is. But it'd be also helpful to give a comment showing the
actual usecs.
thanks,
Takashi
Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support
On Mon, 30 Jul 2018 18:05:47 +0200,
Jorge wrote:
>
>
>
> On 30/07/18 14:03, Takashi Iwai wrote:
> > On Mon, 30 Jul 2018 11:23:33 +0200,
> > Jorge Sanjuan wrote:
> >>
> >> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
> >> index 334bfa6dfb47..786e5939d831 100644
> >> --- a/include/linux/usb/audio-v3.h
> >> +++ b/include/linux/usb/audio-v3.h
> >> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
> >> /* BADD sample rate is always fixed to 48kHz */
> >> #define UAC3_BADD_SAMPLING_RATE 48000
> >>
> >> +/* BADD power domains recovery times */
> >> +#define UAC3_BADD_PD_RECOVER_D1D0 0x0258
> >> +#define UAC3_BADD_PD_RECOVER_D2D0 0x1770
> >
> > Please put the unit for these values. I guess they don't need to be
> > hex?
> >
>
> Hi!
>
> The BADD spec defines these inferred values as hex (see section 6.2.2.13
> of the spec). Should we keep them as per spec to avoid confusion? I'll
> update comment there with the units (50 us increments).
Well, if they were defined in hex, it makes sense to keep the raw
values as is. But it'd be also helpful to give a comment showing the
actual usecs.
thanks,
Takashi
Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support
On 30/07/18 14:03, Takashi Iwai wrote:
> On Mon, 30 Jul 2018 11:23:33 +0200,
> Jorge Sanjuan wrote:
>>
>> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
>> index 334bfa6dfb47..786e5939d831 100644
>> --- a/include/linux/usb/audio-v3.h
>> +++ b/include/linux/usb/audio-v3.h
>> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
>> /* BADD sample rate is always fixed to 48kHz */
>> #define UAC3_BADD_SAMPLING_RATE 48000
>>
>> +/* BADD power domains recovery times */
>> +#define UAC3_BADD_PD_RECOVER_D1D0 0x0258
>> +#define UAC3_BADD_PD_RECOVER_D2D0 0x1770
>
> Please put the unit for these values. I guess they don't need to be
> hex?
>
Hi!
The BADD spec defines these inferred values as hex (see section 6.2.2.13
of the spec). Should we keep them as per spec to avoid confusion? I'll
update comment there with the units (50 us increments).
Thanks for the reviews today. I'll update this series soon with the
suggested changes.
Jorge
>
> Takashi
>
Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support
On 30/07/18 14:03, Takashi Iwai wrote:
> On Mon, 30 Jul 2018 11:23:33 +0200,
> Jorge Sanjuan wrote:
>>
>> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
>> index 334bfa6dfb47..786e5939d831 100644
>> --- a/include/linux/usb/audio-v3.h
>> +++ b/include/linux/usb/audio-v3.h
>> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
>> /* BADD sample rate is always fixed to 48kHz */
>> #define UAC3_BADD_SAMPLING_RATE 48000
>>
>> +/* BADD power domains recovery times */
>> +#define UAC3_BADD_PD_RECOVER_D1D0 0x0258
>> +#define UAC3_BADD_PD_RECOVER_D2D0 0x1770
>
> Please put the unit for these values. I guess they don't need to be
> hex?
>
Hi!
The BADD spec defines these inferred values as hex (see section 6.2.2.13
of the spec). Should we keep them as per spec to avoid confusion? I'll
update comment there with the units (50 us increments).
Thanks for the reviews today. I'll update this series soon with the
suggested changes.
Jorge
>
> Takashi
>
Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support
On Mon, 30 Jul 2018 11:23:33 +0200,
Jorge Sanjuan wrote:
>
> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
> index 334bfa6dfb47..786e5939d831 100644
> --- a/include/linux/usb/audio-v3.h
> +++ b/include/linux/usb/audio-v3.h
> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
> /* BADD sample rate is always fixed to 48kHz */
> #define UAC3_BADD_SAMPLING_RATE 48000
>
> +/* BADD power domains recovery times */
> +#define UAC3_BADD_PD_RECOVER_D1D00x0258
> +#define UAC3_BADD_PD_RECOVER_D2D00x1770
Please put the unit for these values. I guess they don't need to be
hex?
Takashi
Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support
On Mon, 30 Jul 2018 11:23:33 +0200,
Jorge Sanjuan wrote:
>
> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
> index 334bfa6dfb47..786e5939d831 100644
> --- a/include/linux/usb/audio-v3.h
> +++ b/include/linux/usb/audio-v3.h
> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
> /* BADD sample rate is always fixed to 48kHz */
> #define UAC3_BADD_SAMPLING_RATE 48000
>
> +/* BADD power domains recovery times */
> +#define UAC3_BADD_PD_RECOVER_D1D00x0258
> +#define UAC3_BADD_PD_RECOVER_D2D00x1770
Please put the unit for these values. I guess they don't need to be
hex?
Takashi
Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support
On Mon, 30 Jul 2018 11:23:33 +0200, Jorge Sanjuan wrote: > > diff --git a/sound/usb/power.c b/sound/usb/power.c > new file mode 100644 > index ..ce3fea2bd030 > --- /dev/null > +++ b/sound/usb/power.c > @@ -0,0 +1,118 @@ > +/* > + * UAC3 Power Domain state management functions > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + * > + */ For a new code, let's use the simpler SPDX form. thanks, Takashi
Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support
On Mon, 30 Jul 2018 11:23:33 +0200, Jorge Sanjuan wrote: > > diff --git a/sound/usb/power.c b/sound/usb/power.c > new file mode 100644 > index ..ce3fea2bd030 > --- /dev/null > +++ b/sound/usb/power.c > @@ -0,0 +1,118 @@ > +/* > + * UAC3 Power Domain state management functions > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + * > + */ For a new code, let's use the simpler SPDX form. thanks, Takashi
