Skip to content

Comments

ASoC: SOF: fixup the name of the new structs for IPC generic functionality by prefixing them with sof_#3415

Merged
ranj063 merged 26 commits intothesofproject:topic/sof-devfrom
ujfalusi:peter/sof/pr/fixup_ipc_ops_names_01
Feb 10, 2022
Merged

ASoC: SOF: fixup the name of the new structs for IPC generic functionality by prefixing them with sof_#3415
ranj063 merged 26 commits intothesofproject:topic/sof-devfrom
ujfalusi:peter/sof/pr/fixup_ipc_ops_names_01

Conversation

@ujfalusi
Copy link
Collaborator

@ujfalusi ujfalusi commented Feb 4, 2022

Hi,

It slipped through the review that we have missed to prefix the ops structs with a proper sof_ prefix.
We should have it right for upstream and we need a reminder which patches needs to be corrected.

Probably this is not the best way, but I went through all the patches and checked where the ops struct is used and added a fixup
specific to that patch. In Between the patches we will have non compiling code, which I don't like much, but when preparing for upstream and moving the fixes in position we will see how they broke and we are going to be forced to fix them up.

The other option would be to do separate patches for each struct, but it can not be a fixup anymore and the change needs to be done by hand when preparing things for sending out.
But, it will compile all the way.

I can swap to that if it is preferred..

plbossart
plbossart previously approved these changes Feb 4, 2022
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

the changes look alright @ujfalusi

Another suggestion would be a set of reverts and redo, don't know if that would be easier. The main issue I have with fixups is that the bots will tell us the same issue for a while.

@ujfalusi
Copy link
Collaborator Author

ujfalusi commented Feb 4, 2022

@plbossart, true, I'm not going attempt it today, but that is not going to be easier either. I'll sleep on it over the weekend.

I'm tempted to do a name-by-name change and put a postit to the monitor to not forget this ;)

@ujfalusi
Copy link
Collaborator Author

ujfalusi commented Feb 9, 2022

@plbossart, @ranj063, any preference on how this should be done?

  1. this PR
  2. one patch per struct name
  3. one monster patch to change all

@ranj063
Copy link
Collaborator

ranj063 commented Feb 9, 2022

@plbossart, @ranj063, any preference on how this should be done?

  1. this PR
  2. one patch per struct name
  3. one monster patch to change all

I prefer this PR

ranj063
ranj063 previously approved these changes Feb 9, 2022
@plbossart
Copy link
Member

conflicts @ujfalusi ?

Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
…arrays

Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
…ipelines

Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
…-control

Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add sof_ prefix for the ops struct names.

Note: breaks compilation and it will conflict when moved!!!

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
@ujfalusi ujfalusi dismissed stale reviews from ranj063 and plbossart via 482e28f February 10, 2022 09:48
@ujfalusi ujfalusi force-pushed the peter/sof/pr/fixup_ipc_ops_names_01 branch from f70e1a2 to 482e28f Compare February 10, 2022 09:48
@ujfalusi
Copy link
Collaborator Author

Changes since v1:

  • rebased on sof-dev
    • new fixup for the "ASoC: SOF: Add a new op to set up volume table" which caused the conflict.

@ranj063 ranj063 merged commit f1e2b3b into thesofproject:topic/sof-dev Feb 10, 2022
@ujfalusi ujfalusi deleted the peter/sof/pr/fixup_ipc_ops_names_01 branch November 18, 2022 07:36
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.

3 participants