Re: [PATCH 2/2] Adding DT binding for zombie

[Doug Anderson]

Hi,

On Wed, Nov 16, 2022 at 5:44 PM Owen Yang <ecs.taipeikernel@xxxxxxxxx> wrote:
>
>     creating first device tree binding for zombie case.
>
>     Documentation/devicetree/bindings/arm/qcom.yaml
>
>     Series-to: LKML <linux-kernel@xxxxxxxxxxxxxxx>
>     Series-cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
>     Series-cc: Bob Moragues <moragues@xxxxxxxxxxxx>
>     Series-cc: Harvey <hunge@xxxxxxxxxx>
>
> Signed-off-by: Owen Yang <ecs.taipeikernel@xxxxxxxxx>
> ---
>
>  Documentation/devicetree/bindings/arm/qcom.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)

There are a few problems with this patch.

1. The patch does not apply to the top of the upstream Qualcomm tree
(it gets merge conflicts). You should be sending your patches against
the upstream Qualcomm dts tree, which is where they will land. For
instance, since you're sending patches with patman you could make sure
you're targeting the right tree with something like this:

git remote add linux_qcom
git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
get fetch --no-tags linux_qcom
git checkout -b 221117-send-zombie linux_qcom/for-next
git cherry-pick ${PATCH1_TO_SEND}
git cherry-pick ${PATCH2_TO_SEND}
...
git cherry-pick ${PATCHN_TO_SEND}
patman

2. Because you were based on the wrong tree, you got Bjorn's email
address wrong. You need his @kernel.org address. If you were targeting
the correct tree then it would have been auto-fixed up for you by the
.mailmap.

3. A minor detail, but "bindings" should be patch #1 and the device
tree should be patch #2.

4. If I were picking the right place to add Zombie in the bindings, I
would put it right after the "Villager" entries. Then all the Google
sc7280 devices are next to each other and sorted by name.

5. Somehow your patch description contains a bunch of "patman"
commands directly. I think maybe this is because you indented them and
thus patman didn't process the commands? Please try sending again
after getting rid of the indentation. Also the
"Documentation/devicetree/bindings/arm/qcom.yaml" line doesn't belong
in the description.

6. The ${SUBJECT} of your patch (which comes from the first line of
the patch description) needs tags. It probably should be:

dt-bindings: arm: qcom: Add initial sc7280-zombie entries

7. The patch description should ideally be a proper sentence (with the
first word capitalized, for instance), like:

Add an entry in the device tree binding for sc7280-zombie.

8. Just to be consistent with other Chromebooks and to handle the case
when we later add extra revs, please add "(newest rev)" for all of
your descriptions in the yaml.