2013-09-05 14:50:25

by Dave Jones

[permalink] [raw]
Subject: Re: ALSA: hdspm - AES32: Enable TCO/Sync-In in snd_hdspm_put_sync_ref()

On Thu, Sep 05, 2013 at 01:38:37AM +0000, Linux Kernel wrote:
> Gitweb: http://git.kernel.org/linus/;a=commit;h=2d60fc7f7d3d79e5646646bb34811961f19d111a
> Commit: 2d60fc7f7d3d79e5646646bb34811961f19d111a
> Parent: dbae4a0c8d8794df1a6bd7e644ed94b915f46f7e
> Author: Adrian Knoth <[email protected]>
> AuthorDate: Fri Jul 5 11:28:15 2013 +0200
> Committer: Takashi Iwai <[email protected]>
> CommitDate: Fri Jul 5 14:52:42 2013 +0200
>
> ALSA: hdspm - AES32: Enable TCO/Sync-In in snd_hdspm_put_sync_ref()

> static int hdspm_autosync_ref(struct hdspm *hdspm)
> {
...
> + unsigned int syncref = (status >> HDSPM_AES32_syncref_bit) & 0xF;
> + if ((syncref >= HDSPM_AES32_AUTOSYNC_FROM_WORD) &&
> + (syncref <= HDSPM_AES32_AUTOSYNC_FROM_SYNC_IN)) {
> return syncref;
> + }

Because syncref is unsigned, the first part of that if always evaluates true.
(it will always be >0)

Dave


2013-09-05 14:59:09

by Takashi Iwai

[permalink] [raw]
Subject: Re: ALSA: hdspm - AES32: Enable TCO/Sync-In in snd_hdspm_put_sync_ref()

At Thu, 5 Sep 2013 10:50:13 -0400,
Dave Jones wrote:
>
> On Thu, Sep 05, 2013 at 01:38:37AM +0000, Linux Kernel wrote:
> > Gitweb: http://git.kernel.org/linus/;a=commit;h=2d60fc7f7d3d79e5646646bb34811961f19d111a
> > Commit: 2d60fc7f7d3d79e5646646bb34811961f19d111a
> > Parent: dbae4a0c8d8794df1a6bd7e644ed94b915f46f7e
> > Author: Adrian Knoth <[email protected]>
> > AuthorDate: Fri Jul 5 11:28:15 2013 +0200
> > Committer: Takashi Iwai <[email protected]>
> > CommitDate: Fri Jul 5 14:52:42 2013 +0200
> >
> > ALSA: hdspm - AES32: Enable TCO/Sync-In in snd_hdspm_put_sync_ref()
>
> > static int hdspm_autosync_ref(struct hdspm *hdspm)
> > {
> ...
> > + unsigned int syncref = (status >> HDSPM_AES32_syncref_bit) & 0xF;
> > + if ((syncref >= HDSPM_AES32_AUTOSYNC_FROM_WORD) &&
> > + (syncref <= HDSPM_AES32_AUTOSYNC_FROM_SYNC_IN)) {
> > return syncref;
> > + }
>
> Because syncref is unsigned, the first part of that if always evaluates true.
> (it will always be >0)

True. But from the coding POV, it's not so bad to show both "from"
and "to" for clearly indicating a range, IMO. (And the compiler
should be cleverer than the programmer and will optimize it out in
anyway :)


thanks,

Takashi

2013-09-05 15:21:31

by Dave Jones

[permalink] [raw]
Subject: Re: ALSA: hdspm - AES32: Enable TCO/Sync-In in snd_hdspm_put_sync_ref()

On Thu, Sep 05, 2013 at 05:01:20PM +0200, Takashi Iwai wrote:
> At Thu, 5 Sep 2013 10:50:13 -0400,
> Dave Jones wrote:
> >
> > On Thu, Sep 05, 2013 at 01:38:37AM +0000, Linux Kernel wrote:
> > > Gitweb: http://git.kernel.org/linus/;a=commit;h=2d60fc7f7d3d79e5646646bb34811961f19d111a
> > > Commit: 2d60fc7f7d3d79e5646646bb34811961f19d111a
> > > Parent: dbae4a0c8d8794df1a6bd7e644ed94b915f46f7e
> > > Author: Adrian Knoth <[email protected]>
> > > AuthorDate: Fri Jul 5 11:28:15 2013 +0200
> > > Committer: Takashi Iwai <[email protected]>
> > > CommitDate: Fri Jul 5 14:52:42 2013 +0200
> > >
> > > ALSA: hdspm - AES32: Enable TCO/Sync-In in snd_hdspm_put_sync_ref()
> >
> > > static int hdspm_autosync_ref(struct hdspm *hdspm)
> > > {
> > ...
> > > + unsigned int syncref = (status >> HDSPM_AES32_syncref_bit) & 0xF;
> > > + if ((syncref >= HDSPM_AES32_AUTOSYNC_FROM_WORD) &&
> > > + (syncref <= HDSPM_AES32_AUTOSYNC_FROM_SYNC_IN)) {
> > > return syncref;
> > > + }
> >
> > Because syncref is unsigned, the first part of that if always evaluates true.
> > (it will always be >0)
>
> True. But from the coding POV, it's not so bad to show both "from"
> and "to" for clearly indicating a range, IMO. (And the compiler
> should be cleverer than the programmer and will optimize it out in
> anyway :)

Good point.

hmm, I thought we had a within_range function (maybe it was driver specific)

Dave