linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* unnecessary test?
@ 2018-06-06 12:39 Julia Lawall
  2018-06-06 12:43 ` Julia Lawall
  2018-06-06 13:04 ` Takashi Iwai
  0 siblings, 2 replies; 5+ messages in thread
From: Julia Lawall @ 2018-06-06 12:39 UTC (permalink / raw)
  To: perex, tiwai; +Cc: alsa-devel, linux-kernel

In the file sound/pci/ctxfi/cthw20k1.c, the function daio_mgr_dao_init
contains:

set_field(&ctl->spoctl, SPOCTL_OS << (idx*8),
          ((conf >> 3) & 0x1) ? 2 : 2); /* Raw */

Could the second argument just be 2?  It's true that the preceeding call
contains conf >> ..., but in a more useful way, so perhaps it could be
useful for uniformity?

thanks,
julia

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: unnecessary test?
  2018-06-06 12:39 unnecessary test? Julia Lawall
@ 2018-06-06 12:43 ` Julia Lawall
  2018-06-06 13:04 ` Takashi Iwai
  1 sibling, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2018-06-06 12:43 UTC (permalink / raw)
  To: perex; +Cc: tiwai, alsa-devel, linux-kernel



On Wed, 6 Jun 2018, Julia Lawall wrote:

> In the file sound/pci/ctxfi/cthw20k1.c, the function daio_mgr_dao_init
> contains:
>
> set_field(&ctl->spoctl, SPOCTL_OS << (idx*8),
>           ((conf >> 3) & 0x1) ? 2 : 2); /* Raw */
>
> Could the second argument just be 2?  It's true that the preceeding call
> contains conf >> ..., but in a more useful way, so perhaps it could be
> useful for uniformity?

There is similar code in daio_mgr_dao_init in sound/pci/ctxfi/cthw20k2.c:

        	set_field(&ctl->txctl[idx], ATXCTL_RAW,
                         ((conf >> 3) & 0x1) ? 0 : 0);


julia

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: unnecessary test?
  2018-06-06 12:39 unnecessary test? Julia Lawall
  2018-06-06 12:43 ` Julia Lawall
@ 2018-06-06 13:04 ` Takashi Iwai
  2018-06-06 13:06   ` Julia Lawall
  1 sibling, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2018-06-06 13:04 UTC (permalink / raw)
  To: Julia Lawall; +Cc: perex, alsa-devel, linux-kernel

On Wed, 06 Jun 2018 14:39:05 +0200,
Julia Lawall wrote:
> 
> In the file sound/pci/ctxfi/cthw20k1.c, the function daio_mgr_dao_init
> contains:
> 
> set_field(&ctl->spoctl, SPOCTL_OS << (idx*8),
>           ((conf >> 3) & 0x1) ? 2 : 2); /* Raw */
> 
> Could the second argument just be 2?  It's true that the preceeding call
> contains conf >> ..., but in a more useful way, so perhaps it could be
> useful for uniformity?

I guess this is a typo of "2 : 0".  The code seems toggling the
control bit depending on the S/PDIF passthru mode.  It might be
reversed, but I bet 1 for non-audio from a common sense.

Ditto for cthw20k1.c.  This one is likely 1, not 2, though.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: unnecessary test?
  2018-06-06 13:04 ` Takashi Iwai
@ 2018-06-06 13:06   ` Julia Lawall
  2018-06-06 13:09     ` Takashi Iwai
  0 siblings, 1 reply; 5+ messages in thread
From: Julia Lawall @ 2018-06-06 13:06 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: perex, alsa-devel, linux-kernel



On Wed, 6 Jun 2018, Takashi Iwai wrote:

> On Wed, 06 Jun 2018 14:39:05 +0200,
> Julia Lawall wrote:
> >
> > In the file sound/pci/ctxfi/cthw20k1.c, the function daio_mgr_dao_init
> > contains:
> >
> > set_field(&ctl->spoctl, SPOCTL_OS << (idx*8),
> >           ((conf >> 3) & 0x1) ? 2 : 2); /* Raw */
> >
> > Could the second argument just be 2?  It's true that the preceeding call
> > contains conf >> ..., but in a more useful way, so perhaps it could be
> > useful for uniformity?
>
> I guess this is a typo of "2 : 0".  The code seems toggling the
> control bit depending on the S/PDIF passthru mode.  It might be
> reversed, but I bet 1 for non-audio from a common sense.
>
> Ditto for cthw20k1.c.  This one is likely 1, not 2, though.

OK, should I send a patch?  I have no way to test it.

julia

>
>
> thanks,
>
> Takashi
>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: unnecessary test?
  2018-06-06 13:06   ` Julia Lawall
@ 2018-06-06 13:09     ` Takashi Iwai
  0 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2018-06-06 13:09 UTC (permalink / raw)
  To: Julia Lawall; +Cc: perex, alsa-devel, linux-kernel

On Wed, 06 Jun 2018 15:06:28 +0200,
Julia Lawall wrote:
> 
> 
> 
> On Wed, 6 Jun 2018, Takashi Iwai wrote:
> 
> > On Wed, 06 Jun 2018 14:39:05 +0200,
> > Julia Lawall wrote:
> > >
> > > In the file sound/pci/ctxfi/cthw20k1.c, the function daio_mgr_dao_init
> > > contains:
> > >
> > > set_field(&ctl->spoctl, SPOCTL_OS << (idx*8),
> > >           ((conf >> 3) & 0x1) ? 2 : 2); /* Raw */
> > >
> > > Could the second argument just be 2?  It's true that the preceeding call
> > > contains conf >> ..., but in a more useful way, so perhaps it could be
> > > useful for uniformity?
> >
> > I guess this is a typo of "2 : 0".  The code seems toggling the
> > control bit depending on the S/PDIF passthru mode.  It might be
> > reversed, but I bet 1 for non-audio from a common sense.
> >
> > Ditto for cthw20k1.c.  This one is likely 1, not 2, though.
> 
> OK, should I send a patch?  I have no way to test it.

Me neither :)

Feel free to submit the patch.  The SPDIF output seems very rarely
used on these devices (otherwise people must have noticed it), so the
chance to break anything must be small.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-06 13:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-06 12:39 unnecessary test? Julia Lawall
2018-06-06 12:43 ` Julia Lawall
2018-06-06 13:04 ` Takashi Iwai
2018-06-06 13:06   ` Julia Lawall
2018-06-06 13:09     ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).