Skip to content
This repository was archived by the owner on Jan 17, 2019. It is now read-only.

Conversation

@bkokoszx
Copy link
Contributor

I've added two logger flags for firmware version verification:
-e - enables checking fw version with default file
-v ver_file - enables checking fw version with ver_file file

Signed-off-by: Bartosz Kokoszko bartoszx.kokoszko@linux.intel.com

-c Set timestamp clock in MHz
-e Enable checking firmware version with default verification file
"/sys/kernel/debug/sof/fw_version"
-v ver_file Enable checking firmware version with ver_file file,
Copy link
Member

Choose a reason for hiding this comment

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

Version check be done by default, this option should optionally disable it. Btw, why do we need a file here ? I thought version would be added to ldc file and read from sysfs ?

Copy link
Member

@akloniex akloniex Oct 29, 2018

Choose a reason for hiding this comment

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

As discussed in #111 we decided to introduce this feature in 2 stages, to allow for parallel implementation from FW and kernel sides and to ensure that behaviour doesn't break anything in case of misalignment.
After verification of proper behaviour we'll change it to enable verification by default.
The version file is there to allow custom path to file dumped by driver, as our diagnostic driver on Windows uses different path than kernel one.

@bkokoszx bkokoszx force-pushed the logger-fw-verification branch from 03616d0 to 8c63e0f Compare October 29, 2018 13:35
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Also need to address the header file, this may be a day or so before submodules working so it may be worth while also adding the component names into logger e.g. VOLUME1.1, VOLUME1.0 etc.
This means we should be able to catch all updates with one PR rather than several.

@@ -0,0 +1,1020 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

Why are we duplicating this header ? @cujomalainey will be creating some git sub modules so we should have a local version of this header in the next day or so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will update including headers, when the submodules are ready

@bkokoszx bkokoszx force-pushed the logger-fw-verification branch 4 times, most recently from 84f03e1 to 5d29941 Compare November 2, 2018 08:33
I've added two logger flags for firmware version verification:
-e		- enables checking fw version with default file
-v ver_file	- enables checking fw version with ver_file file

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>
@bkokoszx bkokoszx force-pushed the logger-fw-verification branch from 5d29941 to c6546c3 Compare November 5, 2018 10:53
@bkokoszx
Copy link
Contributor Author

bkokoszx commented Nov 5, 2018

@lgirdwood @akloniex @xiulipan
I've added also logging topology number capability due to thesofproject/sof#530.
I'm waiting for moving soft to sof in order to remove dependency between logger and fw.

@bkokoszx bkokoszx force-pushed the logger-fw-verification branch 2 times, most recently from adc634c to a5d628a Compare November 5, 2018 15:19
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Minor change flagged by github. Btw any chance you paste some example output in the commit message to show improvement.

} __attribute__((__packed__));

#endif //#ifndef __INCLUDE_LOGGING__
#endif //#ifndef __INCLUDE_LOGGING__
Copy link
Member

Choose a reason for hiding this comment

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

Need newline (github flags this automatically)

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Minor change flagged by github. Btw any chance you paste some example output in the commit message to show improvement.

@bkokoszx bkokoszx force-pushed the logger-fw-verification branch 2 times, most recently from f8c8eea to 3ced9d3 Compare November 5, 2018 16:11
@bkokoszx
Copy link
Contributor Author

bkokoszx commented Nov 5, 2018

@lgirdwood I've updated github issue and commit message.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Ok, is there any PRs for this driver or can both this and the FW patch now be merged ?

@bkokoszx
Copy link
Contributor Author

bkokoszx commented Nov 6, 2018

@lgirdwood @xiulipan @akloniex
FW part: thesofproject/sof#530
Linux part: thesofproject/linux#222
Since checking fw version in logger is optional in my opinion there isn't necessity to wait
for kernel part.

@bkokoszx bkokoszx force-pushed the logger-fw-verification branch from 3ced9d3 to 1ecd8ca Compare November 6, 2018 10:06
@bkokoszx
Copy link
Contributor Author

bkokoszx commented Nov 6, 2018

@lgirdwood
I've added also macro
#define uintptr_t uint32_t
to enforce using uintptr_t as 4 bytes value (as in fw).

@lgirdwood
Copy link
Member

@lgirdwood
I've added also macro
#define uintptr_t uint32_t
to enforce using uintptr_t as 4 bytes value (as in fw).

@bkokoszx we can't redefine types as it will silently break other things.

Once fixed, can you submit FW PR to topic/abi branch (we will do all ABI updates in a single merge)

@bkokoszx
Copy link
Contributor Author

bkokoszx commented Nov 7, 2018

@lgirdwood
To enforce building logger for 32 bit I should use gcc with -m32 flag, but
it requires gcc-multilib. Do you think we should use it or find another way?

@bkokoszx bkokoszx force-pushed the logger-fw-verification branch from 1ecd8ca to c7002ae Compare November 7, 2018 13:09
@lgirdwood
Copy link
Member

@bkokoszx We cant do this 32bit build either, host build trace should just revert to printf directly (so pointers are not needed to lookup anyway). i.e. host will not use the trace mechanism. You can use an #ifdef in trace.h to separate DSP trace from host trace if needed.

I've added printing component topology number
(i.e. pipeline_id.component_id). If id's are not
defined logger will not print any values.
e.g.:
CORE  LEVEL      COMP_ID                TIMESTAMP ...
   0      2         HOST            487048.229167 ...  (not defined id's)
   0      2         PIPE 1.3        591235.052083 ...  (defined id's)

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>
@bkokoszx
Copy link
Contributor Author

bkokoszx commented Nov 8, 2018

@lgirdwood
I'm working on updating uapi here: thesofproject/sof#548

@lgirdwood
Copy link
Member

@bkokoszx will be merging ABI kernel and FW updates today. Will do it all in one big update to minimize disruption.

@bkokoszx
Copy link
Contributor Author

bkokoszx commented Nov 8, 2018

@lgirdwood
Uapi is updated due to thesofproject/sof#548

@bkokoszx
Copy link
Contributor Author

bkokoszx commented Nov 9, 2018

@lgirdwood
Do you have any additional remarks?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants