[PATCH v5 0/3] Add dtsi for sc7280 boards that using rt5682

Judy Hsiao posted 3 patches 1 year, 6 months ago
.../qcom/sc7280-herobrine-audio-rt5682.dtsi   | 122 ++++++++++++++++++
.../qcom/sc7280-herobrine-herobrine-r1.dts    |   1 +
.../dts/qcom/sc7280-herobrine-villager-r0.dts |   1 +
.../arm64/boot/dts/qcom/sc7280-herobrine.dtsi |  30 +++++
4 files changed, 154 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-audio-rt5682.dtsi
[PATCH v5 0/3] Add dtsi for sc7280 boards that using rt5682
Posted by Judy Hsiao 1 year, 6 months ago
Put sound node and lpass_cpu node settings for boards that use rt5682
codec in the sc7280-herobrine-audio-rt5682.dtsi as there are different
choices of headset codec for herobrine projects. Common audio setting
for the internal speaker is in sc7280-herobrine.dtsi.

Change Since V4
- Rebase and include sc7280-herobrine-villager-r0.dts change.

Changes Since V3:
- Remove Change-Id in the commit message.
- Add dependency in cover letter.

Changes Since V2:
- Fix sc7280-herobrine-audio-rt5682.dtsi syntax.

Changes Since V1:
- Not to include the herobrine-villager-r0.dts changes in this patch
  series to avoid conflict.

Judy Hsiao (3):
  arm64: dts: qcom: sc7280: herobrine: Add pinconf settings for mi2s1
  arm64: dts: qcom: sc7280: Add sc7280-herobrine-audio-rt5682.dtsi
  arm64: dts: qcom: sc7280: Include sc7280-herobrine-audio-rt5682.dtsi
    in herobrine-r1 and villager-r0

 .../qcom/sc7280-herobrine-audio-rt5682.dtsi   | 122 ++++++++++++++++++
 .../qcom/sc7280-herobrine-herobrine-r1.dts    |   1 +
 .../dts/qcom/sc7280-herobrine-villager-r0.dts |   1 +
 .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi |  30 +++++
 4 files changed, 154 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-audio-rt5682.dtsi

-- 
2.37.3.998.g577e59143f-goog
Re: [PATCH v5 0/3] Add dtsi for sc7280 boards that using rt5682
Posted by Doug Anderson 1 year, 6 months ago
Judy,

On Fri, Sep 30, 2022 at 8:36 AM Judy Hsiao <judyhsiao@chromium.org> wrote:
>
> Put sound node and lpass_cpu node settings for boards that use rt5682
> codec in the sc7280-herobrine-audio-rt5682.dtsi as there are different
> choices of headset codec for herobrine projects. Common audio setting
> for the internal speaker is in sc7280-herobrine.dtsi.
>
> Change Since V4
> - Rebase and include sc7280-herobrine-villager-r0.dts change.
>
> Changes Since V3:
> - Remove Change-Id in the commit message.
> - Add dependency in cover letter.
>
> Changes Since V2:
> - Fix sc7280-herobrine-audio-rt5682.dtsi syntax.
>
> Changes Since V1:
> - Not to include the herobrine-villager-r0.dts changes in this patch
>   series to avoid conflict.
>
> Judy Hsiao (3):
>   arm64: dts: qcom: sc7280: herobrine: Add pinconf settings for mi2s1
>   arm64: dts: qcom: sc7280: Add sc7280-herobrine-audio-rt5682.dtsi
>   arm64: dts: qcom: sc7280: Include sc7280-herobrine-audio-rt5682.dtsi
>     in herobrine-r1 and villager-r0
>
>  .../qcom/sc7280-herobrine-audio-rt5682.dtsi   | 122 ++++++++++++++++++
>  .../qcom/sc7280-herobrine-herobrine-r1.dts    |   1 +
>  .../dts/qcom/sc7280-herobrine-villager-r0.dts |   1 +
>  .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi |  30 +++++
>  4 files changed, 154 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-audio-rt5682.dtsi

Your two posts today were a bit confusing. You really need something
in the explaining what's going on. Specifically:

1. You posted a "v5" upstream a week ago [1].

2. Then you posted another "v5" [2] today.

3. Finally, you posted this "v5" [3] today.

They are all marked "v5" and there's nothing to explain why there are
3 versions of v5.

--

Better would have been:

a) Use some type of "prefix" in the subject line to help people
understand that the contents are the same as the previous "v5" patch
and this is just a new posting. Usually people will use "REPOST" or
"RESEND". So, for instance, the subject line for your cover letter
could have been "[RESEND PATCH v5 0/3] Add dtsi for sc7280 boards that
using rt5682". All of the individual patches would have also had the
"RESEND" in their subject.

b) You should explain somewhere _why_ you're re-sending the patches.
If you've got a cover letter (like you do), the cover letter is the
perfect place to explain why you're resending the patch. If you didn't
have a cover letter, you can explain "after the cut".

Brian's patch [4] is a great example of this. He has both the hint in
the subject line and an explanation: "Resending, because I missed the
mailing lists on the first version."

--

In your case, I assume you were trying to get Bjorn's email address
correct as I requested a week ago. Thus, I would have expected your
first patch from today [2] to say something like "I'm resending patch
v5 to get Bjorn's email address correct. Other than that this is
exactly the same as the previous v5". Then, I would have expected your
second patch from today [3] to say something like "Oops, I still
didn't get Bjorn's email address right in the earlier patch today.
Trying yet again. Contents of all of the v5 patches are identical"

--

In any case, I'm not expecting you to send yet-another v5, but hopefully:
* This explains to Bjorn what's going on this time.
* You'll know better for next time.


[1] https://lore.kernel.org/r/20220923140918.2825043-1-judyhsiao@chromium.org
[2] https://lore.kernel.org/r/20220930152613.2018360-1-judyhsiao@chromium.org
[3] https://lore.kernel.org/r/20220930153643.2018907-1-judyhsiao@chromium.org
[4] https://lore.kernel.org/r/20210907094628.RESEND.1.If29cd838efbcee4450a62b8d84a99b23c86e0a3f@changeid/