On Wed, Sep 17, 2008 at 11:06:54AM -0400, Frank Mandarino wrote:
> Mark Brown wrote:
> >> +
> >> + switch (div_id) {
> >> + case AT91SSC_CMR_DIV:
> >> + /*
> >> + * The same master clock divider is used for both
> >> + * transmit and receive, so if a value has already
> >> + * been set, it must match this value.
> >> + */
> >> + if (ssc_p->cmr_div == 0)
> >> + ssc_p->cmr_div = div;
> >> + else
> >> + if (div != ssc_p->cmr_div)
> >> + return -EBUSY;
> >> + break;
> > What happens if the user wants to change the master clock divider at
> > runtime - for example, when changing sample rates?
> This is code from at91-ssc.c. I really didn't consider the case of
> changing the sample rate on an open substream. This logic could be
> updated to allow the new divider value if there is only one substream open.
Ah, right - on further inspection I see that cmr_div is reset when the
stream is shut down. That's fine since it means that the clocks can be
reconfigured when reopening the device which is the case I was worried
about.
Changing the dividers on an active stream is unlikely to work well so
it's perfectly reasonable to not support it.
> >> + start_event = channels == 1
> >> + ? 4
> >> + : 7;
> > This would be much clearer if it were expanded into multiple statements.
>
> This was a little clearer in at91-ssc.c:
> start_event = channels == 1
> ? AT91_SSC_START_FALLING_RF
> : AT91_SSC_START_EDGE_RF;
> Perhaps these constant definitions are no longer available it the latest
> kernel. Are there updated definitions to use instead of magic numbers?
> Also, I'm fine with using multiple statements if that helps readability.
I wasn't so worried about the magic numbers as the combination of
assignment and an equality test without even any brackets. I can see
what's going on but it's certainly not the most transparent way of
writing it.
> > These may as well be removed - if someone implements suspend/resume
> > support they can add them then then.
> Is there a reason that suspend/resume was removed? It is really
> important for embedded systems.
Yeah, I did wonder, though there are plenty of embedded systems that
aren't particular power sensitive for one reason or another.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
