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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Judy,

On Fri, Sep 30, 2022 at 8:36 AM Judy Hsiao <judyhsiao@xxxxxxxxxxxx> 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@xxxxxxxxxxxx
[2] https://lore.kernel.org/r/20220930152613.2018360-1-judyhsiao@xxxxxxxxxxxx
[3] https://lore.kernel.org/r/20220930153643.2018907-1-judyhsiao@xxxxxxxxxxxx
[4] https://lore.kernel.org/r/20210907094628.RESEND.1.If29cd838efbcee4450a62b8d84a99b23c86e0a3f@changeid/



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux