Fixed suggestions about getting ES8336 quirk default. #477

nnseva · 2023-09-25T08:51:52Z

The documentation was wrong about getting the quirk default. I've fixed these details.

lgirdwood

@plbossart @ujfalusi pls review.

cujomalainey

commit message is empty and unhelpful

also probably missing sign-off

deb-intel · 2023-09-25T17:42:13Z

commit message is empty and unhelpful

also probably missing sign-off

@cujomalainey, to whom are you addressing?

cujomalainey · 2023-09-25T18:05:50Z

commit message is empty and unhelpful

also probably missing sign-off

@cujomalainey, to whom are you addressing?

The author, you cannot comment on commit messages directly hence the un-anchored review comment

marc-hb

Do not add warnings.

getting_started/intel_debug/suggestions.rst

nnseva · 2023-09-28T08:57:15Z

commit message is empty and unhelpful

also probably missing sign-off

Tried to explain the change. Added my sign-off.

marc-hb · 2023-09-28T18:45:18Z

The commit message body in d555d7a7646 is still empty. Not a problem for me because the commit message title looks very good to me now and enough for this small fix but there are some people and some policies who insist on non-empty commit messages :-)

btian1 · 2023-10-09T08:38:59Z

fixed --> fix?

cujomalainey · 2023-10-12T21:48:50Z

some people and some policies who insist on non-empty commit messages :-)

Yes but I won't argue to block it if everyone else is fine

deb-intel

Please incorporate the minor grammatical edits at your convenience.

deb-intel · 2023-09-25T14:02:10Z

getting_started/intel_debug/suggestions.rst

@@ -294,10 +294,31 @@ driver:
   #define SOC_ES8336_HEADSET_MIC1		BIT(8)


-The default quirk value for the platform can be read from
+The default and actual quirk values for the platform can be got


Suggested change

The default and actual quirk values for the platform can be got

The default and actual quirk values for the platform can be obtained

deb-intel · 2023-09-25T14:02:52Z

getting_started/intel_debug/suggestions.rst

@@ -294,10 +294,31 @@ driver:
   #define SOC_ES8336_HEADSET_MIC1		BIT(8)


-The default quirk value for the platform can be read from
+The default and actual quirk values for the platform can be got
+from the dmesg log file (/var/log/dmesg) in the line like:


Suggested change

from the dmesg log file (/var/log/dmesg) in the line like:

from the dmesg log file (/var/log/dmesg) in the line as follows:

deb-intel · 2023-09-25T14:03:32Z

getting_started/intel_debug/suggestions.rst

+
+for the quirk overriding.
+
+The overriden quirk value can also be got from the


Suggested change

The overriden quirk value can also be got from the

The overridden quirk value can also be obtained from

deb-intel · 2023-09-25T14:05:32Z

getting_started/intel_debug/suggestions.rst

 /sys/module/snd_soc_sof_es8336/parameters/quirk (the value is reported
-as plain integer, not hexadecimal). Changes to the default can be
-added with the following option in
+as plain integer, not hexadecimal). If the quirk is not overriden,


Suggested change

as plain integer, not hexadecimal). If the quirk is not overriden,

as a plain integer, not hexadecimal). If the quirk is not overridden,

deb-intel · 2023-09-25T14:06:14Z

getting_started/intel_debug/suggestions.rst

+as plain integer, not hexadecimal). If the quirk is not overriden,
+the `-1` value is returned.
+
+Changes to the default can be added with the following option in
 e.g. /etc/modprobe.d/alsa-base.conf. Only the bits listed above can be
 modified, others need to be kept as is.


Suggested change

modified, others need to be kept as is.

modified; others need to remain as is.

marc-hb

Please git squash @deb-intel 's fixes and force push a single commit.

We don't use GitHub "the GitHub way" in this project, we use GitHub in the older, more traditional git way where each commit in a PR stands alone / is its own thing.

For more context:
zephyrproject-rtos/zephyr#39194
https://gitlab.com/gitlab-org/gitlab/-/issues/24096

PS: yes the maintainer can "squash and merge" at the end but we usually don't do that, see above why.

@deb-intel

Suggestions fixed corresponding to the reality. Also some fixes mostly regarding syntax provided by request from @deb-intel. Signed-off-by: Vsevolod Novikov <[email protected]>

nnseva · 2023-11-17T08:19:34Z

@marc-hb

Please git squash @deb-intel 's fixes and force push a single commit.

done

deb-intel

Approved with integrated change requests.

lgirdwood · 2023-11-24T17:05:16Z

All reviewer comments now addressed.

nnseva requested review from lgirdwood, deb-intel, intelkevinputnam, mmaka1 and mwasko as code owners September 25, 2023 08:51

nnseva mentioned this pull request Sep 25, 2023

the sys virtual fs does not report a default quirk for snd_soc_sof_es8336, but only overriden thesofproject/linux#4422

Open

lgirdwood reviewed Sep 25, 2023

View reviewed changes

cujomalainey suggested changes Sep 25, 2023

View reviewed changes

marc-hb requested changes Sep 25, 2023

View reviewed changes

getting_started/intel_debug/suggestions.rst Outdated Show resolved Hide resolved

marc-hb reviewed Sep 25, 2023

View reviewed changes

getting_started/intel_debug/suggestions.rst Outdated Show resolved Hide resolved

marc-hb previously approved these changes Sep 28, 2023

View reviewed changes

marc-hb changed the title ~~ES8336 quirk default~~ Fixed suggestions about getting ES8336 quirk default. Sep 28, 2023

lgirdwood approved these changes Sep 29, 2023

View reviewed changes

lgirdwood previously approved these changes Oct 14, 2023

View reviewed changes

deb-intel requested changes Oct 19, 2023

View reviewed changes

btian1 previously approved these changes Oct 24, 2023

View reviewed changes

nnseva dismissed stale reviews from btian1, lgirdwood, and marc-hb via 865706f November 8, 2023 09:42

nnseva requested review from deb-intel and cujomalainey November 8, 2023 09:43

marc-hb reviewed Nov 8, 2023

View reviewed changes

intel_debug: Fixed suggestions about getting ES8336 quirk default.

da405c1

Suggestions fixed corresponding to the reality. Also some fixes mostly regarding syntax provided by request from @deb-intel. Signed-off-by: Vsevolod Novikov <[email protected]>

deb-intel approved these changes Nov 17, 2023

View reviewed changes

lgirdwood approved these changes Nov 24, 2023

View reviewed changes

lgirdwood merged commit cd9ed32 into thesofproject:master Nov 24, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed suggestions about getting ES8336 quirk default. #477

Fixed suggestions about getting ES8336 quirk default. #477

nnseva commented Sep 25, 2023

lgirdwood left a comment

cujomalainey left a comment

deb-intel commented Sep 25, 2023

cujomalainey commented Sep 25, 2023

marc-hb left a comment

nnseva commented Sep 28, 2023

marc-hb commented Sep 28, 2023

btian1 commented Oct 9, 2023

cujomalainey commented Oct 12, 2023

deb-intel left a comment

deb-intel Sep 25, 2023

nnseva Nov 8, 2023

deb-intel Sep 25, 2023

nnseva Nov 8, 2023

deb-intel Sep 25, 2023

nnseva Nov 8, 2023

deb-intel Sep 25, 2023

nnseva Nov 8, 2023

deb-intel Sep 25, 2023

nnseva Nov 8, 2023

marc-hb left a comment

nnseva commented Nov 17, 2023 •

edited

Loading

deb-intel left a comment

lgirdwood commented Nov 24, 2023

	The default and actual quirk values for the platform can be got
	The default and actual quirk values for the platform can be obtained

	from the dmesg log file (/var/log/dmesg) in the line like:
	from the dmesg log file (/var/log/dmesg) in the line as follows:


		for the quirk overriding.

		The overriden quirk value can also be got from the

	The overriden quirk value can also be got from the
	The overridden quirk value can also be obtained from

	as plain integer, not hexadecimal). If the quirk is not overriden,
	as a plain integer, not hexadecimal). If the quirk is not overridden,

	modified, others need to be kept as is.
	modified; others need to remain as is.

Fixed suggestions about getting ES8336 quirk default. #477

Fixed suggestions about getting ES8336 quirk default. #477

Conversation

nnseva commented Sep 25, 2023

lgirdwood left a comment

Choose a reason for hiding this comment

cujomalainey left a comment

Choose a reason for hiding this comment

deb-intel commented Sep 25, 2023

cujomalainey commented Sep 25, 2023

marc-hb left a comment

Choose a reason for hiding this comment

nnseva commented Sep 28, 2023

marc-hb commented Sep 28, 2023

btian1 commented Oct 9, 2023

cujomalainey commented Oct 12, 2023

deb-intel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb left a comment

Choose a reason for hiding this comment

nnseva commented Nov 17, 2023 • edited Loading

deb-intel left a comment

Choose a reason for hiding this comment

lgirdwood commented Nov 24, 2023

nnseva commented Nov 17, 2023 •

edited

Loading