[tlauda]

Adds check if comp_ops are available.
Without this change there is a possibility of
exception.

Signed-off-by: Tomasz Lauda tomasz.lauda@linux.intel.com

[ranj063]

@tlauda we ran into something similar in the driver as well where we had to alter the return value from ops.

Basically, some of the ops are OK to return 0 if not set, say for ex: params but something like trigger should return an error isnt it? So we should identify mandatory and optional ops and fix the return value accordingly.

[tlauda]

@ranj063 Hard to decide which one should be mandatory and which one shouldn't. It really depends on the internal component implementation and purpose.

[ranj063]

@tlauda i dont know about the others but comp_cmd should not return a 0. it will be misleading to the driver. I think we should return an error in that case

[lgirdwood]

@ranj063 the general intention is that a non implemented operation will always return 0. i.e. the component does not care hence it has not implemented the operation.

[ranj063]

@lgirdwood OK where I was coming from was that if we add a kcontrol to a component, ideally the comp should implement a cmd callback to process kcontrol IO data.

[mmaka1]

comp ops, as an application level extension point, might be the first one and most popular area of the development based on SOF. That is why there should be strong verification of the API requirements enforced I believe. The documentation above says that ops.free() is mandatory which seems right since it might be better to enforce empty handler rather than allow for memory leaks. The proposed change quietly allows to skip the implementation which is inconsistent with the documented requirements. This one (as well as other handlers marked as mandatory like params()) seems to be a good candidate for assert(not null). And I would opt for assert() that compiles in non-debug version to panic (it might be difficult to deliver extra debug configuration on some platforms and force the use) to make sure that no mandatory part is missed during the component development process.

[lgirdwood]

@mmaka1 Ack, if it's mandatory we shout and let the user know. If optional we perform the check.

[tlauda]

@mmaka1 @lgirdwood I've added simple assert macro, which will cause panic. Params, prepare, cmd and reset ops shouldn't be mandatory, so I've changed the documentation.

[lgirdwood]

@zrombel can this be requeued on CI as it should be back up now after down time. Would like to merge today.

[ranj063]

@tlauda @lgirdwood i have an issue with comp_cmd() returning 0 if not implemented.
It is not a mandatory op but when the driver issues a get() command it is expecting data back from the comp and does it make sense to return a 0 even if we dont really handle the request?

[lgirdwood]

@ranj063 It looks like this would only be a problem if topology incorrectly added kcontrols for a component that does not have any, so your right we should have -EINVAL for this case. @tlauda small update needed.

[tlauda]

@lgirdwood @ranj063 Updated.

[ranj063]

looks good. Thanks @tlauda

[tlauda]

@lgirdwood The only question remains if the ABI change is needed. Kernel change: thesofproject/linux#762.