Skip to content

Conversation

@mmaka1
Copy link

@mmaka1 mmaka1 commented Feb 19, 2020

Back pointer must be initialized before any instance
level logging is performed by the new() operation since
the instance identification requires access to the uuid
that will be implemented on the component driver level.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is confusing here. Since cdev->drv is already set, I think it is better to remove this assignment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@dbaluta
Copy link
Collaborator

dbaluta commented Feb 19, 2020

@mmaka1 PR looks good to me. See one comment inline. It's a pity that we need to change the API but I don't see other solution.

@mmaka1 mmaka1 force-pushed the init-comp-drv-early branch from c78cf8f to 2c7627b Compare February 19, 2020 15:55
Marcin Maka added 3 commits February 19, 2020 17:30
Missed drv pointer as the first parameter

Signed-off-by: Marcin Maka <marcin.maka@linux.intel.com>
Missed drv pointer as the first parameter

Signed-off-by: Marcin Maka <marcin.maka@linux.intel.com>
Back pointer must be initialized before any instance
level logging is performed by the new() operation since
the instance identification requires access to the uuid
that will be implemented on the component driver level.

Signed-off-by: Marcin Maka <marcin.maka@linux.intel.com>
@mmaka1 mmaka1 force-pushed the init-comp-drv-early branch from 2c7627b to 8c35b95 Compare February 19, 2020 16:32
@mmaka1 mmaka1 mentioned this pull request Feb 19, 2020
@paulstelian97
Copy link
Collaborator

I don't like the terminology "back pointer", at least for me it is more confusing than revealing. Just say "pointer to driver" or something similar.

Other than that, LGTM.

@lgirdwood lgirdwood merged commit 247af13 into thesofproject:master Feb 19, 2020
@mmaka1 mmaka1 deleted the init-comp-drv-early branch February 20, 2020 10:17
@mmaka1
Copy link
Author

mmaka1 commented Feb 20, 2020

I don't like the terminology "back pointer", at least for me it is more confusing than revealing. Just say "pointer to driver" or something similar.

Why confusing? Just look at the back pointer definition plus assume that the aggregation (and its direction) exists in the model even if not explicitly reflected in the source code, then this term should be obvious.

@paulstelian97
Copy link
Collaborator

Yeah, I guess. Still I won't use such terminology myself because it's way too far from being intuitive for me to easily understand it.

@dbaluta
Copy link
Collaborator

dbaluta commented Feb 20, 2020

@paulstelian97 @mmaka1 please have this discussion over a beer or something. :)

@plbossart
Copy link
Member

I don't like the terminology "back pointer", at least for me it is more confusing than revealing. Just say "pointer to driver" or something similar.

Why confusing? Just look at the back pointer definition plus assume that the aggregation (and its direction) exists in the model even if not explicitly reflected in the source code, then this term should be obvious.

code is like jokes, if you need explanations it's not funny.
I also have never seen the wording 'back pointer'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants