Skip to content

Conversation

@xiulipan
Copy link

Add probe support from thesofproject/sof#2338
Add debug ABI version from thesofproject/sof#2398

bump ABI to 3.14.0
Need by #1890

Add new probe support extend date to show the probe support info.

Signed-off-by: Pan Xiuli <xiuli.pan@linux.intel.com>
Add new debug ABI version to be increased when changing user space debug
interfaces while the the main ABI is not affected.

Signed-off-by: Pan Xiuli <xiuli.pan@linux.intel.com>
@xiulipan xiulipan added the ABI involves ABI changes, need extra attention label Mar 24, 2020
@xiulipan
Copy link
Author

@plbossart @lgirdwood I will add printk that use this ABI update later.

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Some changes to commit message are needed, but otherwise good. Thanks @xiulipan

} __packed;

/* extended data: user abi version(s) */
struct sof_ipc_user_abi_version {
Copy link
Collaborator

Choose a reason for hiding this comment

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

"user abi" is a bit weird name, but alas this was already merged to FW, so it's what it is.

SOF_IPC_EXT_UNUSED = 0,
SOF_IPC_EXT_WINDOW = 1,
SOF_IPC_EXT_CC_INFO = 2,
SOF_IPC_EXT_PROBE_INFO = 3,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lgirdwood @jajanusz It really would be better to do the kernel PRs at the same time and have the person doing the change write a git commit message that explains the change. This "late-bound" sync of changes to kernel is not fun. Thanks for @xiulipan doing the patch, but I again have to ask for some improvements to the commit message:

  • "add support for probe extended data"
  • add explanation to commit body about the feature.. you should be able to copy this from the FW commit

Copy link
Member

Choose a reason for hiding this comment

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

agree with @kv2019i , this is a complete fail of the ABI change process.

Copy link
Member

Choose a reason for hiding this comment

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

The kernel and FW PRs must be done together and mention each other. It is up to maintainers to label as ABI and then to align both PRs using the classifier. So this is several steps taken by at least 3 people and is not being fully processed by everybody.

I am now wondering whether we should now be moving to a ABI minor bump only for every FW release (when the regular FW releases start soon). This would allow the kernel team to plan each merge window in advance to a specific FW version. @lbetlej fyi - regular FW releases would help here.
There is also complexity about bumping ABI in relation to kernel releases and FW releases. i.e. when do we close the ABI minor merge window for new kernel and FW releases (and use the next minor).

@xiulipan xiulipan changed the title align mismatch ABI in info.h [RFC]align mismatch ABI in info.h Mar 25, 2020
@xiulipan
Copy link
Author

@plbossart @lgirdwood @kv2019i To make ABI version bump in order, I will close this PR and merge the ABI patch into #1890

@xiulipan xiulipan closed this Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ABI involves ABI changes, need extra attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants