lkml.org 
[lkml]   [2022]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v5 0/3] Add dtsi for sc7280 boards that using rt5682
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/

\
 
 \ /
  Last update: 2022-09-30 20:21    [W:0.041 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site