thesofproject / sof-docs

Documentation for SOF
Other
16 stars 72 forks source link

sof-logger-doc: how to disable dsp suspension on imx8mp #466

Closed eurofun closed 1 year ago

eurofun commented 1 year ago

plus some general restructuring

paulstelian97 commented 1 year ago

Hello. The changes themselves are good. However, there's a few structural things that prevent us from using your work as-is:

  1. The commits aren't properly separated. Each commit should only do one thing. I still expect 3 commits, but one of them only does the info of disabling automatic suspend on imx8mp, one only adds (or rewrites) the information about trace verbosity flags, and one does the various smaller cleanups (such as shorter lines, fixing the underlines on titles etc).
  2. The commit titles and descriptions are... lacking. I did see some potentially good stuff (e.g. the "logger:" prefix makes sense for all 3 commits). A commit should have a title and a description. The title is a summary of the changes, and the description is a more detailed explanation. For example, on the commit that does the cleanup you can put in the description the individual cleanups (underlines + line wrapping etc).
  3. None of your commits has the COA. This is a problem because without that we cannot use your work at all. The COA implicitly states that the changes were authored by you, and that you allow us to use them. A proper COA looks something like:
Signed-off-by: Full Name <mail.address@mail.domain>

You can look at past commits for examples. If there are multiple authors which all had a significant contribution to the commits you can add multiple such lines, one for each of the authors.

eurofun commented 1 year ago

Thanks for the critique, I updated the commits accordingly. I even have a typo in the branch name, but I'll leave that as-is...

dbaluta commented 1 year ago

@eurofun this looks good to me! Thanks for the patches.

@deb-intel @lgirdwood Good for you?