Hi,
On Mon, 2 Mar 2020, Kuninori Morimoto wrote:
> I guess you are thinking more big scale tracking/solution (?).
> Indeed it is needed, but my indicated one is not for it.
> It is just for "we want to use soc_pcm_close() as soc_pcm_open() error handling".
yes, I'm thinking it would be better to do proper tracking and not do an
intermediate solution with IDs.
> > > int soc_pcm_open(...)
> > > {
> > > static u8 id;
> > >
> > > /* update ID */
> > > id++;
> > > if (id == 0)
> > > id++;
>
> Maybe the naming of "ID" makes you confused ?
> It is just "mark" for this "soc_pcm_open()".
> If error happen during open, "error handling soc_pcm_close()" cares only this mark.
> It is just for avoiding mismatch close.
Yes, the main issues I see:
- the name (maybe "open_tag" would be better than "open_id"),
- declaration of the id with "static u8 id" -- if multiple unrelated
streams are opened concurrently, the id management needs to be handled
in a thread safe way,
- the "component->open_id" field only refers to the last substream
open -- i.e. field is only relevant in contact of error rollbacks
The "new id" really just refers to the substream being opened, so
you could create it from the substream pointer as well. For error
rollback, you want to close all components of the substream being
opened. But this is still a bit unelegant as you'd end up storing
the last substream opened to component struct (3rd bullet above).
I was thinking more in the lines of:
/* add list of opened substreams to snd_soc_component */
struct snd_soc_component {
...
struct list_head substream_list;
int snd_soc_component_open(struct snd_soc_component *component,
» » » struct snd_pcm_substream *substream)
{
int res = 0;
if (component->driver->open)
res = component->driver->open(component, substream);
/* on success, add proxy of substream to component->substream_list */
...
int snd_soc_component_close(struct snd_soc_component *component,
» » » struct snd_pcm_substream *substream)
{
/*
* lookup substream from "component->substream_List",
* only call driver->close() if found
*/
...
... this is arguably more code, but makes the state created in
snd_soc_component_open() explicit. Upon error in middle of
components_open(), one can just call soc_pcm_components_close() which will
close all components for that substream (that had been succesfully
opened).
Br, Kai
