ASoC: rt715-sdca: First version of rt715 sdw sdca codec driver by jack-cy-yu · Pull Request #2308 · thesofproject/linux

jack-cy-yu · 2020-07-22T11:06:50Z

Update driver according to #2303 request.

Signed-off-by: Jack Yu jack.yu@realtek.com

lyakh

Thanks for the update! A couple more comments.

lyakh

Thanks for the update! Usually we update patches by force-pushing a new version, but a few weeks ago I indeed raised a question and proposed to do exactly what you have done - add a fixup patch on top of the existing commits. And your PR is a rather good example of why this is easier to review. Instead of comparing the two versions of your patch reviewing this small fixup is certainly easier. This difference is even much greater in multi-commit PRs. But I don't know what the maintainers will say, whether they'll agree to take these two commits and merge them into one themselves. What would probably help them though if you added a keyword "fixup!" in the beginning of your patch subject. You can just change the subject here to add it. You can also look through other PRs for examples.

RanderWang

LGTM

Update driver according to thesofproject#2308 request. Signed-off-by: Jack Yu <jack.yu@realtek.com>

bardliao

Thanks @jack-cy-yu We are very closed. Please add rt714 in the id_table. Use dev_err instead of pr_err if possible and merge the two commits at last.

Update driver according to thesofproject#2308 request. Signed-off-by: Jack Yu <jack.yu@realtek.com>

marc-hb · 2020-10-12T16:32:08Z

This needs (at least) to be rebased.

plbossart · 2020-10-15T21:54:49Z

@jack-cy-yu can you rebase to have a newer branch, and address all Travis CI/checkpatch issues (except for alignment and newline after {), thanks!

marc-hb · 2020-10-15T23:20:07Z

This has already been rebased, "age of git base" is zero in https://travis-ci.org/github/thesofproject/linux/jobs/735662250

bardliao

@jack-cy-yu It looks overall good. Some minor comments and checkpatch issues should be addressed. You can run "./scripts/checkpatch.pl --strict <patch name" before updating the patch. Thanks.

marc-hb · 2020-10-16T04:15:06Z

@jack-cy-yu I'm curious why you keep closing and re-opening?

jack-cy-yu · 2020-10-16T06:32:31Z

@marc-hb I guess that's because I update my remote branch first and then send PR to thesofproject. When my remote branch got updated, this PR will also be closed. and when I push the latest PR to my remote branch, this PR can be reopened again.

kv2019i

Looks good to me. The many checkpatch issues on alignment seem like something easy to fix, but some comments indicate these can be left, so maybe this has been covered already (tried to search the comments, but could not find where this was discussed).

plbossart

@jack-cy-yu I think we are missing a devm_ initialization, see below.

plbossart · 2020-10-19T15:58:29Z

+	slave->ops = &rt715_sdca_slave_ops;
+
+	/* Regmap Initialization */
+	mbq_regmap = regmap_init_sdw_mbq(slave, &rt715_sdca_mbq_regmap);


can we use devm_regmap_init_sdw_mbq now?

@plbossart Already fix it on v16. Thanks.

bardliao · 2020-10-20T06:10:27Z

@jack-cy-yu Can you remove the "v16" tag on the commit title?

First version of rt715 sdw sdca codec driver. Signed-off-by: Jack Yu <jack.yu@realtek.com>

jack-cy-yu · 2020-10-20T06:34:56Z

@bardliao I've updated both title and latest PR comment. Thanks.

bardliao

Thanks @jack-cy-yu LGTM

plbossart

Thanks @jack-cy-yu I will merge but please look at the comments below for improvements that can be done with a fixup! patch

plbossart · 2020-10-20T14:56:05Z

+enum {
+	RT715_AIF1,
+	RT715_AIF2,
+	RT715_AIFS,


you are not using this AIFS, what was it intended for?
I also see if in rt715.h.

@plbossart I'll send a fixup patch to remove RT715_AIFS for rt715.h.

you are not using this AIFS, what was it intended for?
I also see if in rt715.h.
The _AIFS indicates total number of audio interfaces. It is a common usage in Realtek codec drivers. For example, rt5682.h

marc-hb · 2020-10-20T17:23:07Z

@marc-hb I guess that's because I update my remote branch first and then send PR to thesofproject.

I'm not sure what you mean by "send PR to thesofproject". When you push to a remote branch already submitted in a PR then the PR gets updated automatically, there is nothing else to do besides git push.

When my remote branch got updated, this PR will also be closed. and when I push the latest PR to my remote branch, this PR can be reopened again.

bardliao · 2020-10-21T02:33:52Z

@marc-hb I guess that's because I update my remote branch first and then send PR to thesofproject.
I'm not sure what you mean by "send PR to thesofproject". When you push to a remote branch already submitted in a PR then the PR gets updated automatically, there is nothing else to do besides git push.
When my remote branch got updated, this PR will also be closed. and when I push the latest PR to my remote branch, this PR can be reopened again.

@marc-hb I discussed it with @jack-cy-yu. Now I think I can explain it. @jack-cy-yu pulled the latest code from thesofproject/topic/sof-dev to his own branch, and pushed it without adding his commits to his remote branch. i.e, jack-cy-yu/topic/sof-dev and thesofproject/topic/sof-dev are identical at this point. And github is smart enough to close the PR since there is nothing to merge. @jack-cy-yu pushed his remote branch with his commits then, and the PR re-opened.

jack-cy-yu · 2020-10-21T06:41:16Z

@plbossart I've created another PR #2517 for fixup.

marc-hb · 2020-10-21T17:08:06Z

Thanks @bardliao for solving this github mystery!

And github is smart enough to close the PR since there is nothing to merge.

I think you're being a bit too generous calling this "smart", especially so considering github confusingly does not say what happened. But I understand its logic :-)

jack-cy-yu requested review from RanderWang, bardliao, dbaluta, kv2019i, lgirdwood, oder-chiou, plbossart, ranj063 and shumingfan as code owners July 22, 2020 11:06

lyakh requested changes Jul 23, 2020
View reviewed changes

lyakh previously approved these changes Jul 24, 2020
View reviewed changes

RanderWang previously approved these changes Jul 24, 2020
View reviewed changes

bardliao pushed a commit to bardliao/linux that referenced this pull request Aug 3, 2020
[v5] ASoC: rt715-sdca: First version of rt715 sdw sdca codec driver
68b348a
Update driver according to thesofproject#2308 request. Signed-off-by: Jack Yu <jack.yu@realtek.com>

bardliao pushed a commit to bardliao/linux that referenced this pull request Aug 3, 2020
[v5] ASoC: rt715-sdca: First version of rt715 sdw sdca codec driver
288bc53
Update driver according to thesofproject#2308 request. Signed-off-by: Jack Yu <jack.yu@realtek.com>

bardliao requested changes Aug 3, 2020
View reviewed changes

Comment threadsound/soc/codecs/rt715-sdca-sdw.c
Comment threadsound/soc/codecs/rt715-sdca.c Outdated
Comment threadsound/soc/codecs/rt715-sdca.c Outdated

jack-cy-yu closed this Aug 4, 2020

jack-cy-yu reopened this Aug 4, 2020

jack-cy-yu dismissed stale reviews from RanderWang and lyakh via fb41e75August 4, 2020 05:55

bardliao pushed a commit to bardliao/linux that referenced this pull request Aug 4, 2020
[v5] ASoC: rt715-sdca: First version of rt715 sdw sdca codec driver
4e5e3d3
Update driver according to thesofproject#2308 request. Signed-off-by: Jack Yu <jack.yu@realtek.com>

bardliao reviewed Aug 4, 2020
View reviewed changes

Comment threadsound/soc/codecs/rt715-sdca.h

lyakh previously approved these changes Aug 4, 2020
View reviewed changes

bardliao previously approved these changes Aug 4, 2020
View reviewed changes

bardliao pushed a commit to bardliao/linux that referenced this pull request Aug 4, 2020
[v5] ASoC: rt715-sdca: First version of rt715 sdw sdca codec driver
798e260
Update driver according to thesofproject#2308 request. Signed-off-by: Jack Yu <jack.yu@realtek.com>

bardliao pushed a commit to bardliao/linux that referenced this pull request Aug 11, 2020
[v5] ASoC: rt715-sdca: First version of rt715 sdw sdca codec driver
8707062
Update driver according to thesofproject#2308 request. Signed-off-by: Jack Yu <jack.yu@realtek.com>

jack-cy-yu closed this Aug 11, 2020

jack-cy-yu reopened this Aug 11, 2020

jack-cy-yu dismissed stale reviews from bardliao and lyakh via 957798aAugust 11, 2020 09:07

jack-cy-yu closed this Aug 12, 2020

jack-cy-yu reopened this Sep 29, 2020

jack-cy-yu closed this Oct 13, 2020

jack-cy-yu reopened this Oct 13, 2020

jack-cy-yu closed this Oct 14, 2020

jack-cy-yu reopened this Oct 14, 2020

bardliao reviewed Oct 16, 2020
View reviewed changes

jack-cy-yu closed this Oct 16, 2020

jack-cy-yu reopened this Oct 16, 2020

kv2019i reviewed Oct 19, 2020
View reviewed changes

plbossart requested changes Oct 19, 2020
View reviewed changes

ASoC: rt715-sdca: First version of rt715 sdw sdca codec driver
ba56593
First version of rt715 sdw sdca codec driver. Signed-off-by: Jack Yu <jack.yu@realtek.com>

jack-cy-yu changed the title ~~[v4] ASoC: rt715-sdca: First version of rt715 sdw sdca codec driver~~ASoC: rt715-sdca: First version of rt715 sdw sdca codec driverOct 20, 2020

bardliao approved these changes Oct 20, 2020
View reviewed changes

bardliao requested review from RanderWang, kv2019i, lyakh and plbossart October 20, 2020 07:40

plbossart approved these changes Oct 20, 2020
View reviewed changes

plbossart merged commit 2b6d699 into thesofproject:topic/sof-devOct 20, 2020

Uh oh!

Conversation

jack-cy-yu commented Jul 22, 2020

Uh oh!

lyakh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lyakh left a comment

Choose a reason for hiding this comment

Uh oh!

RanderWang left a comment

Choose a reason for hiding this comment

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marc-hb commented Oct 12, 2020

Uh oh!

plbossart commented Oct 15, 2020

Uh oh!

marc-hb commented Oct 15, 2020

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marc-hb commented Oct 16, 2020

Uh oh!

jack-cy-yu commented Oct 16, 2020

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossartOct 19, 2020

Choose a reason for hiding this comment

Uh oh!

jack-cy-yuOct 20, 2020

Choose a reason for hiding this comment

Uh oh!

bardliao commented Oct 20, 2020

Uh oh!

jack-cy-yu commented Oct 20, 2020

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossartOct 20, 2020

Choose a reason for hiding this comment

marc-hb commented Oct 21, 2020•
edited
Loading