Skip to content

cc_version: use fixed length for CC_DESC #2522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgirdwood merged 2 commits into from
Apr 1, 2020
Merged

cc_version: use fixed length for CC_DESC #2522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a546444
ipc: cc_version: change type char to uint8_t
xiulipan Mar 26, 2020
988fcf6
ipc: cc_version: use fixed length for CC_DESC
xiulipan Mar 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/include/ipc/info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,9 @@ struct sof_ipc_cc_version {
/* reserved for future use */
uint32_t reserved[4];

char name[16]; /* null terminated compiler name */
char optim[4]; /* null terminated compiler -O flag value */
char desc[]; /* null terminated compiler description */
uint8_t name[16]; /* null terminated compiler name */
uint8_t optim[4]; /* null terminated compiler -O flag value */
uint8_t desc[32]; /* null terminated compiler description */
} __attribute__((packed));

/* extended data: Probe setup */
Expand Down
2 changes: 1 addition & 1 deletion src/include/kernel/abi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

/** \brief SOF ABI version major, minor and patch numbers */
#define SOF_ABI_MAJOR 3
#define SOF_ABI_MINOR 14
#define SOF_ABI_MINOR 15
Copy link
Member

@lgirdwood lgirdwood Mar 27, 2020

Choose a reason for hiding this comment

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

@xiulipan This does not align with the classifier or the commit message. Can you double check this.

Copy link
Collaborator

@kv2019i kv2019i Mar 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@lgirdwood @xiulipan D'oh, you are right. The kernel patch (not yet merged) puts cc_version as ABI16:
thesofproject/linux@3552e9c
So definitely commit message and patch are not in sync here.

This probably happened because FW PRs for ABI15 have been merged without bumping the version. E.g.
e6940da
... was the last PR for ABI14, but it was merged with abi.h of ABI14.

I think we can probably still go ahead with this. The abi.h version needs to be changed to 16 when the last PR for ABI16 is merged. As @lyakh commented on the kernel side, if this is the norm to have multiple changes done in a single bump, we need to separate the commits that add functionality from commits that bump the version. I.e. we open ABIxx for development, have interface changes approved for this ABI level, and then we have a closing commit that freezes ABIxx. No binary FW releases should be done from master during this time.

Copy link
Member

@lgirdwood lgirdwood Mar 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@kv2019i I see why I'm not finding the kernel PR's - @xiulipan can you split your kernel PR into 3, one PR per ABI bump. That way we can align the with the FW on the classifier and follow. Once this is done and ABIs bumps aligned we can merge all at one go.

Copy link
Member

@lgirdwood lgirdwood Mar 30, 2020

Choose a reason for hiding this comment

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

@xiulipan ping ?

Copy link
Contributor Author

@xiulipan xiulipan Mar 31, 2020

Choose a reason for hiding this comment

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

@lgirdwood The three kernel pr is already there
thesofproject/linux#1947
thesofproject/linux#1946
thesofproject/linux#1890

#define SOF_ABI_PATCH 0

/** \brief SOF ABI version number. Format within 32bit word is MMmmmppp */
Expand Down
10 changes: 7 additions & 3 deletions src/ipc/cc_version.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,22 @@
field = CC_OPTIMIZE_FLAGS, \
field[ARRAY_SIZE(((struct sof_ipc_cc_version *)(0))->optim) - 1] = 0

/* copy CC_DESC to arrays during compilation time */
#define CC_DESC_COPY(field) \
field = CC_DESC, \
field[ARRAY_SIZE(((struct sof_ipc_cc_version *)(0))->desc) - 1] = 0

const struct sof_ipc_cc_version cc_version
__section(".fw_ready_metadata") = {
.ext_hdr = {
.hdr.cmd = SOF_IPC_FW_READY,
.hdr.size = ALIGN_UP(sizeof(struct sof_ipc_cc_version)
+ sizeof(CC_DESC), 4),
.hdr.size = sizeof(struct sof_ipc_cc_version),
.type = SOF_IPC_EXT_CC_INFO,
},
.micro = CC_MICRO,
.minor = CC_MINOR,
.major = CC_MAJOR,
CC_NAME_COPY(.name),
CC_OPTIM_COPY(.optim),
.desc = CC_DESC,
CC_DESC_COPY(.desc),
};