All of lore.kernel.org
 help / color / mirror / Atom feed
* UCM modifier names
@ 2011-06-02 19:56 Stephen Warren
  2011-06-03 10:11 ` Liam Girdwood
  0 siblings, 1 reply; 2+ messages in thread
From: Stephen Warren @ 2011-06-02 19:56 UTC (permalink / raw)
  To: Liam Girdwood (lrg@ti.com)
  Cc: alsa-devel@alsa-project.org,
	Mark Brown (broonie@opensource.wolfsonmicro.com)

Liam,

UCM syntax is e.g.:

SectionModifier."Capture Voice".0 {

This maps to a modifier->name of "Capture Voice.0".

However, when find_modifier searches for modifiers, it strips off the
index (".0") from modifier->name, and just compares base name. As such,
the index doesn't seem useful.

I propose:

a) In parse_modifier, fail if index!=0.

b) In parse_modifier, store just the name in modifier->name.

c) In find_modifier, remove all the special handling of "." in the name,
   and just strcmp(modifier->name, modifier_name).

Perhaps the index should be completely removed from the file syntax too?
If an index was actually useful in any case, the author of the file could
simply add it within the name string, i.e.:

typical: SectionModifier."Capture Voice" {

unusual: SectionModifier."Capture Voice 0" {

Of course, that wouldn't be backwards-compatible file syntax. Perhaps we
could make the index optional, but always validate it was 0 if present,
as I said above.

I actually wonder if the index on the SectionDevice makes any sense
either; functions like is_modifier_supported would be a lot simpler if
devices simply never had an index, such that is_modifier_supported was
a simple strcmp too. The worst fallout from that change might be a
requirement to list more entries in SupportedDevice/ConflictingDevice.


Second, when find_modifier is called from get_value, it'll fail with a
valid modifier name that happens not to be supported on any currently
active devices. Is that intended? I'd expect to be able to retrieve
values from any/all devices and modifiers irrespective of whether those
devices or modifiers could actually be activated given the currently
active configuration.

I propose: Adding a parameter to find_modifier to indicate whether to
call is_modifier_supported or not, this new parameter being false for
get_value and true in other cases.

This issue will also be relevant to find_device when I modify that to
call new function is_device_supported.

For find_device, we may also have to add a parameter for a device name
that gets ignore when checking SupportedDevice/ConflictingDevice, which
can be passed when switching devices.

Thanks for your thoughts.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: UCM modifier names
  2011-06-02 19:56 UCM modifier names Stephen Warren
@ 2011-06-03 10:11 ` Liam Girdwood
  0 siblings, 0 replies; 2+ messages in thread
From: Liam Girdwood @ 2011-06-03 10:11 UTC (permalink / raw)
  To: Stephen Warren
  Cc: alsa-devel@alsa-project.org,
	Mark Brown (broonie@opensource.wolfsonmicro.com)

Hi Stephen,

On 02/06/11 20:56, Stephen Warren wrote:
> Liam,
> 
> UCM syntax is e.g.:
> 
> SectionModifier."Capture Voice".0 {
> 
> This maps to a modifier->name of "Capture Voice.0".
> 
> However, when find_modifier searches for modifiers, it strips off the
> index (".0") from modifier->name, and just compares base name. As such,
> the index doesn't seem useful.
> 

Your right, the index is probably not very useful for modifiers atm. 

> I propose:
> 
> a) In parse_modifier, fail if index!=0.
> 
> b) In parse_modifier, store just the name in modifier->name.
> 
> c) In find_modifier, remove all the special handling of "." in the name,
>    and just strcmp(modifier->name, modifier_name).
> 
> Perhaps the index should be completely removed from the file syntax too?

Yeah, I would go with that for modifier.

> If an index was actually useful in any case, the author of the file could
> simply add it within the name string, i.e.:
> 
> typical: SectionModifier."Capture Voice" {
> 
> unusual: SectionModifier."Capture Voice 0" {
> 
> Of course, that wouldn't be backwards-compatible file syntax. Perhaps we
> could make the index optional, but always validate it was 0 if present,
> as I said above.
> 

we do have some users using the modifier.0 syntax so maybe best to deprecate it.

> I actually wonder if the index on the SectionDevice makes any sense
> either; functions like is_modifier_supported would be a lot simpler if
> devices simply never had an index, such that is_modifier_supported was
> a simple strcmp too. The worst fallout from that change might be a
> requirement to list more entries in SupportedDevice/ConflictingDevice.
> 

ok.

> 
> Second, when find_modifier is called from get_value, it'll fail with a
> valid modifier name that happens not to be supported on any currently
> active devices. Is that intended? I'd expect to be able to retrieve
> values from any/all devices and modifiers irrespective of whether those
> devices or modifiers could actually be activated given the currently
> active configuration.
> 
> I propose: Adding a parameter to find_modifier to indicate whether to
> call is_modifier_supported or not, this new parameter being false for
> get_value and true in other cases.
> 
> This issue will also be relevant to find_device when I modify that to
> call new function is_device_supported.
> 
> For find_device, we may also have to add a parameter for a device name
> that gets ignore when checking SupportedDevice/ConflictingDevice, which
> can be passed when switching devices.
> 
> Thanks for your thoughts.
> 

Sounds reasonable to me.

Liam

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-06-03 10:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-02 19:56 UCM modifier names Stephen Warren
2011-06-03 10:11 ` Liam Girdwood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.