2022-11-24 05:15:56

by Owen Yang

[permalink] [raw]
Subject: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

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

Signed-off-by: Owen Yang <[email protected]>
---

Changes in v4:
- Dropping the redundant 'DT binding for' as requested by Krzysztof. v4.
- Adding an empty line here before "/dts-v1/;" in "sc7280-herobrine-zombie-lte.dts", "sc7280-herobrine-zombie.dts" as requested by Matthias. v4.
- Deleteing "/dts-v1/;" in "sc7280-herobrine-zombie.dtsi" as requested by Matthias. v4.
- Droping changing file path in description. v3. as requested by Matthias. v3.
- Changing Patch order, binding patch first and dt file second, as requested by Douglas. v2.
- Adding "arm64: dts: qcom: sc7280:" in dt patch ${SUBJECT}, as requested by Douglas. v2.
- Adding "dt-bindings: arm: qcom:" in bind patch ${SUBJECT}, as requested by Douglas. v2.
- Adding '#include "sc7280-herobrine-wifi-sku.dtsi"' in sc7280-herobrine-zombie.dts, as requested by Douglas. v2.
- Adding "(newest rev)" for zombie entry description in qcom.yaml, as requested by Douglas. v2.
- Adding "post-power-on-delay-ms = <100>;" for trackpad in "sc7280-herobrine-zombie.dtsi". v2
- Changing "vcc-supply" to "vdd-supply" for trackpad in "sc7280-herobrine-zombie.dtsi", as requested by Douglas. v2.

Documentation/devicetree/bindings/arm/qcom.yaml | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 463509f0f23a..7ec6240311db 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -655,6 +655,16 @@ properties:
- const: google,villager-sku512
- const: qcom,sc7280

+ - description: Google Zombie (newest rev)
+ items:
+ - const: google,zombie
+ - const: qcom,sc7280
+
+ - description: Google Zombie with LTE (newest rev)
+ items:
+ - const: google,zombie-sku512
+ - const: qcom,sc7280
+
- items:
- enum:
- lenovo,flex-5g
--
2.17.1


2022-11-24 10:01:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

On 24/11/2022 05:30, 楊宗翰 wrote:
> Hi Reviewers,
>
> I really appreciate your kind guide for this task, Matthias is right, I am
> newer to upstream kernel development.
> There are a lot of thing I need to learn, so I just try my best do not ask
> stupid questions. (please forgive me)
>
> 1.
> Because I missed V2, V3 changed log, so I supply in V4 commit, and using V#
> to note which version's change.
>
> 2.
> I notice Kryzysztof say he didn't in cc mail loop at beggin, and below is
> my updated mail list:
> ---
> Series-to: LKML <[email protected]>
> Series-cc: Douglas Anderson <[email protected]>
> Series-cc: Bob Moragues <[email protected]>
> Series-cc: Harvey <[email protected]>
> Series-cc: Stephen Boyd <[email protected]>
> Series-cc: Matthias Kaehlcke <[email protected]>
> Series-cc: Krzysztof Kozlowski <[email protected]>
> ---
> Is there anyone I missed?

These are not correct addresses and not complete list of them. Don't
invent the entries, don't add there some weird addresses.

Use get_maintainers.pl. That's it. Nothing more, nothing less.

Best regards,
Krzysztof

2022-11-24 11:54:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

On 24/11/2022 12:20, 楊宗翰 wrote:
> Hi Krzysztof, Matthias,
>
> How to use "get_maintainers.pl"?
>
> I find this script in path "<MyCodebase>/kernel/v5.15/script", and output

This looks like v5.15 kernel which is heavily outdated. Please never
work on such kernels when interacting with upstream. The rule is you
work on either last mainline kernel (v6.1-rc6), maintainers for-next
branch (you should know who is the maintainer of subsystem you submit
to, get_maintainers.pl gives this information) or on moderately recent
linux-next. For bigger patchsets there might be exceptions for these
rules, but it's not the case here.

Best regards,
Krzysztof

2022-11-28 16:14:26

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

Hi,

On Thu, Nov 24, 2022 at 1:29 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> > 2.
> > I notice Kryzysztof say he didn't in cc mail loop at beggin, and below is
> > my updated mail list:
> > ---
> > Series-to: LKML <[email protected]>
> > Series-cc: Douglas Anderson <[email protected]>
> > Series-cc: Bob Moragues <[email protected]>
> > Series-cc: Harvey <[email protected]>
> > Series-cc: Stephen Boyd <[email protected]>
> > Series-cc: Matthias Kaehlcke <[email protected]>
> > Series-cc: Krzysztof Kozlowski <[email protected]>
> > ---
> > Is there anyone I missed?
>
> These are not correct addresses and not complete list of them. Don't
> invent the entries, don't add there some weird addresses.
>
> Use get_maintainers.pl. That's it. Nothing more, nothing less.

Just to give context here, I think Owen is using `patman` [0] to send
patches. Yes, it's part of the u-boot tree but it's designed for
sending Linux patches too.

By default, that means that get_maintainer is automatically called on
all patches and those entries are CCed. The extra "Series-cc" just
lets you add extra people. It's fine to add extra people to patches if
you think that those people are interested in getting it.

[0] https://source.denx.de/u-boot/u-boot/-/blob/master/tools/patman/patman.rst

2022-11-28 17:10:43

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

Hi,

On Thu, Nov 24, 2022 at 3:27 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 24/11/2022 12:20, 楊宗翰 wrote:
> > Hi Krzysztof, Matthias,
> >
> > How to use "get_maintainers.pl"?
> >
> > I find this script in path "<MyCodebase>/kernel/v5.15/script", and output
>
> This looks like v5.15 kernel which is heavily outdated. Please never
> work on such kernels when interacting with upstream. The rule is you
> work on either last mainline kernel (v6.1-rc6), maintainers for-next
> branch (you should know who is the maintainer of subsystem you submit
> to, get_maintainers.pl gives this information) or on moderately recent
> linux-next. For bigger patchsets there might be exceptions for these
> rules, but it's not the case here.

Just to add context here, it's a fairly standard workflow for ChromeOS
kernel engineers to work in a "versioned" kernel directory but still
checkout and work with the upstream kernel. I'm sure it's confusing to
anyone not used to working with the ChromeOS source tree and build
system. Sorry! :( So the fact that Owen is in a directory called
"v5.15" doesn't mean that he's actually working with the v5.15 kernel.
The fact that Bjorn's address is correct in his CC list implies to me
that he's actually got a proper upstream kernel.

I had previously [0] instructed Owen to send against Bjorn's tree, so
hopefully it's correct.

[0] https://lore.kernel.org/r/CAD=FV=Vd4UFabWeEsd7cDhhpnFkjTuYhc38zwAbfyxq2AHnhYA@mail.gmail.com/

-Doug

2022-11-28 18:17:33

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

On 28/11/2022 16:56, Doug Anderson wrote:
> Hi,
>
> On Thu, Nov 24, 2022 at 3:27 AM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 24/11/2022 12:20, 楊宗翰 wrote:
>>> Hi Krzysztof, Matthias,
>>>
>>> How to use "get_maintainers.pl"?
>>>
>>> I find this script in path "<MyCodebase>/kernel/v5.15/script", and output
>>
>> This looks like v5.15 kernel which is heavily outdated. Please never
>> work on such kernels when interacting with upstream. The rule is you
>> work on either last mainline kernel (v6.1-rc6), maintainers for-next
>> branch (you should know who is the maintainer of subsystem you submit
>> to, get_maintainers.pl gives this information) or on moderately recent
>> linux-next. For bigger patchsets there might be exceptions for these
>> rules, but it's not the case here.
>
> Just to add context here, it's a fairly standard workflow for ChromeOS
> kernel engineers to work in a "versioned" kernel directory but still
> checkout and work with the upstream kernel. I'm sure it's confusing to
> anyone not used to working with the ChromeOS source tree and build
> system. Sorry! :( So the fact that Owen is in a directory called
> "v5.15" doesn't mean that he's actually working with the v5.15 kernel.
> The fact that Bjorn's address is correct in his CC list implies to me
> that he's actually got a proper upstream kernel.
>
> I had previously [0] instructed Owen to send against Bjorn's tree, so
> hopefully it's correct.

If it was on Bjorn's tree, get_maintainers.pl would not produce such result:

---
Series-to: LKML <[email protected]>
Series-cc: Douglas Anderson <[email protected]>
Series-cc: Bob Moragues <[email protected]>
Series-cc: Harvey <[email protected]>
Series-cc: Stephen Boyd <[email protected]>
Series-cc: Matthias Kaehlcke <[email protected]>
Series-cc: Krzysztof Kozlowski <[email protected]>
---

or this:

---
owen@buildsvr-HP-ProDesk-600-G4-MT:~/chromebook_zombie_os/src/third_party/kernel/v5.15$
perl scripts/get_maintainer.pl -f MAINTAINERS --email
[email protected] (open list)
---

as Owen indicated earlier. They are either incomplete or not correct.

Of course I don't know whether the base tree is the problem or usage of
get_maintainers.pl...

Best regards,
Krzysztof

2022-11-28 18:54:17

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

On 28/11/2022 16:51, Doug Anderson wrote:
> Hi,
>
> On Thu, Nov 24, 2022 at 1:29 AM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>>> 2.
>>> I notice Kryzysztof say he didn't in cc mail loop at beggin, and below is
>>> my updated mail list:
>>> ---
>>> Series-to: LKML <[email protected]>
>>> Series-cc: Douglas Anderson <[email protected]>
>>> Series-cc: Bob Moragues <[email protected]>
>>> Series-cc: Harvey <[email protected]>
>>> Series-cc: Stephen Boyd <[email protected]>
>>> Series-cc: Matthias Kaehlcke <[email protected]>
>>> Series-cc: Krzysztof Kozlowski <[email protected]>
>>> ---
>>> Is there anyone I missed?
>>
>> These are not correct addresses and not complete list of them. Don't
>> invent the entries, don't add there some weird addresses.
>>
>> Use get_maintainers.pl. That's it. Nothing more, nothing less.
>
> Just to give context here, I think Owen is using `patman` [0] to send
> patches. Yes, it's part of the u-boot tree but it's designed for
> sending Linux patches too.
>
> By default, that means that get_maintainer is automatically called on
> all patches and those entries are CCed. The extra "Series-cc" just
> lets you add extra people. It's fine to add extra people to patches if
> you think that those people are interested in getting it.

The tool is just the tool, if it uses get_maintainers.pl, then result
would be correct. The problem was that CC list on v1 and v2:

https://lore.kernel.org/linux-devicetree/20221117094251.2.Ibfc4751e4ba044d1caa1f88a16015e7c45c7db65@changeid/

https://lore.kernel.org/linux-devicetree/20221122203635.v2.1.Ie05fd439d0b271b927acb25c2a6e41af7a927e90@changeid/

As you can notice there easily:
1. Bjorn's address is wrong
2. My and Konrad's are missing

So if you say that get_maintainers.pl were used and tree is not v5.15,
then what else?


Best regards,
Krzysztof

2022-11-28 19:08:00

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

On Mon, Nov 28, 2022 at 06:22:39PM +0100, Krzysztof Kozlowski wrote:
> On 28/11/2022 16:56, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Nov 24, 2022 at 3:27 AM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 24/11/2022 12:20, 楊宗翰 wrote:
> >>> Hi Krzysztof, Matthias,
> >>>
> >>> How to use "get_maintainers.pl"?
> >>>
> >>> I find this script in path "<MyCodebase>/kernel/v5.15/script", and output
> >>
> >> This looks like v5.15 kernel which is heavily outdated. Please never
> >> work on such kernels when interacting with upstream. The rule is you
> >> work on either last mainline kernel (v6.1-rc6), maintainers for-next
> >> branch (you should know who is the maintainer of subsystem you submit
> >> to, get_maintainers.pl gives this information) or on moderately recent
> >> linux-next. For bigger patchsets there might be exceptions for these
> >> rules, but it's not the case here.
> >
> > Just to add context here, it's a fairly standard workflow for ChromeOS
> > kernel engineers to work in a "versioned" kernel directory but still
> > checkout and work with the upstream kernel. I'm sure it's confusing to
> > anyone not used to working with the ChromeOS source tree and build
> > system. Sorry! :( So the fact that Owen is in a directory called
> > "v5.15" doesn't mean that he's actually working with the v5.15 kernel.
> > The fact that Bjorn's address is correct in his CC list implies to me
> > that he's actually got a proper upstream kernel.
> >
> > I had previously [0] instructed Owen to send against Bjorn's tree, so
> > hopefully it's correct.
>
> If it was on Bjorn's tree, get_maintainers.pl would not produce such result:
>
> ---
> Series-to: LKML <[email protected]>
> Series-cc: Douglas Anderson <[email protected]>
> Series-cc: Bob Moragues <[email protected]>
> Series-cc: Harvey <[email protected]>
> Series-cc: Stephen Boyd <[email protected]>
> Series-cc: Matthias Kaehlcke <[email protected]>
> Series-cc: Krzysztof Kozlowski <[email protected]>

These look like manual entries for patman

> or this:
>
> ---
> owen@buildsvr-HP-ProDesk-600-G4-MT:~/chromebook_zombie_os/src/third_party/kernel/v5.15$
> perl scripts/get_maintainer.pl -f MAINTAINERS --email
> [email protected] (open list)
> ---
>
> as Owen indicated earlier. They are either incomplete or not correct.
>
> Of course I don't know whether the base tree is the problem or usage of
> get_maintainers.pl...

That looks like an operator error, the above command produces the same result with
an upstream tree.

Issue one is the use of '-f' which seems to expect a file with a list of e-mail
addresses, which MAINTAINERS is not. The second issue is that no patch file or
directory is specified.

2022-11-28 19:51:16

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

Hi,

On Mon, Nov 28, 2022 at 9:22 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 28/11/2022 16:56, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Nov 24, 2022 at 3:27 AM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 24/11/2022 12:20, 楊宗翰 wrote:
> >>> Hi Krzysztof, Matthias,
> >>>
> >>> How to use "get_maintainers.pl"?
> >>>
> >>> I find this script in path "<MyCodebase>/kernel/v5.15/script", and output
> >>
> >> This looks like v5.15 kernel which is heavily outdated. Please never
> >> work on such kernels when interacting with upstream. The rule is you
> >> work on either last mainline kernel (v6.1-rc6), maintainers for-next
> >> branch (you should know who is the maintainer of subsystem you submit
> >> to, get_maintainers.pl gives this information) or on moderately recent
> >> linux-next. For bigger patchsets there might be exceptions for these
> >> rules, but it's not the case here.
> >
> > Just to add context here, it's a fairly standard workflow for ChromeOS
> > kernel engineers to work in a "versioned" kernel directory but still
> > checkout and work with the upstream kernel. I'm sure it's confusing to
> > anyone not used to working with the ChromeOS source tree and build
> > system. Sorry! :( So the fact that Owen is in a directory called
> > "v5.15" doesn't mean that he's actually working with the v5.15 kernel.
> > The fact that Bjorn's address is correct in his CC list implies to me
> > that he's actually got a proper upstream kernel.
> >
> > I had previously [0] instructed Owen to send against Bjorn's tree, so
> > hopefully it's correct.
>
> If it was on Bjorn's tree, get_maintainers.pl would not produce such result:
>
> ---
> Series-to: LKML <[email protected]>
> Series-cc: Douglas Anderson <[email protected]>
> Series-cc: Bob Moragues <[email protected]>
> Series-cc: Harvey <[email protected]>
> Series-cc: Stephen Boyd <[email protected]>
> Series-cc: Matthias Kaehlcke <[email protected]>
> Series-cc: Krzysztof Kozlowski <[email protected]>
> ---

So the above is the _manual_ set of names to add atop get_maintainers.
Patman starts with the list you've manually added (via Series-to and
Series-cc) and then automatically CCs the results of
get_maintainers.pl


> or this:
>
> ---
> owen@buildsvr-HP-ProDesk-600-G4-MT:~/chromebook_zombie_os/src/third_party/kernel/v5.15$
> perl scripts/get_maintainer.pl -f MAINTAINERS --email
> [email protected] (open list)

Wow, really? Maybe I don't have Bjorn's tree correctly checked out
either. ...or you can tell me what I'm doing wrong.

$ git checkout linux_qcom/for-next
HEAD is now at 4d2b529bce12 Merge branches 'arm64-defconfig-for-6.2',
'arm64-for-6.2', 'clk-for-6.2', 'defconfig-for-6.2',
'drivers-for-6.2', 'dts-for-6.2' and 'arm64-fixes-for-6.1' into
for-next

$ perl scripts/get_maintainer.pl -f MAINTAINERS --email
[email protected] (open list)


> as Owen indicated earlier. They are either incomplete or not correct.
>
> Of course I don't know whether the base tree is the problem or usage of
> get_maintainers.pl...

I suspect it's because the only "maintainer" of the file MAINTAINERS is LKML.

2022-11-28 19:54:32

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: arm: qcom: Add zombie

Hi,

On Mon, Nov 28, 2022 at 9:30 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 28/11/2022 16:51, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Nov 24, 2022 at 1:29 AM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >>> 2.
> >>> I notice Kryzysztof say he didn't in cc mail loop at beggin, and below is
> >>> my updated mail list:
> >>> ---
> >>> Series-to: LKML <[email protected]>
> >>> Series-cc: Douglas Anderson <[email protected]>
> >>> Series-cc: Bob Moragues <[email protected]>
> >>> Series-cc: Harvey <[email protected]>
> >>> Series-cc: Stephen Boyd <[email protected]>
> >>> Series-cc: Matthias Kaehlcke <[email protected]>
> >>> Series-cc: Krzysztof Kozlowski <[email protected]>
> >>> ---
> >>> Is there anyone I missed?
> >>
> >> These are not correct addresses and not complete list of them. Don't
> >> invent the entries, don't add there some weird addresses.
> >>
> >> Use get_maintainers.pl. That's it. Nothing more, nothing less.
> >
> > Just to give context here, I think Owen is using `patman` [0] to send
> > patches. Yes, it's part of the u-boot tree but it's designed for
> > sending Linux patches too.
> >
> > By default, that means that get_maintainer is automatically called on
> > all patches and those entries are CCed. The extra "Series-cc" just
> > lets you add extra people. It's fine to add extra people to patches if
> > you think that those people are interested in getting it.
>
> The tool is just the tool, if it uses get_maintainers.pl, then result
> would be correct. The problem was that CC list on v1 and v2:
>
> https://lore.kernel.org/linux-devicetree/20221117094251.2.Ibfc4751e4ba044d1caa1f88a16015e7c45c7db65@changeid/
>
> https://lore.kernel.org/linux-devicetree/20221122203635.v2.1.Ie05fd439d0b271b927acb25c2a6e41af7a927e90@changeid/
>
> As you can notice there easily:
> 1. Bjorn's address is wrong
> 2. My and Konrad's are missing
>
> So if you say that get_maintainers.pl were used and tree is not v5.15,
> then what else?

Certainly on v1 and v2 he was targeting v5.15, not upstream. When I
replied to v1 I told him this. Apparently he messed up still on v2.
Matthias again pointed it out on v2. After v2, it was corrected. ...so
right, you didn't get v1 and v2 and those of us on the email thread
pointed that out and it got corrected. I'm not sure what happened to
v3, but that seems like yet another mistake and I believe Matthias
also commented on this. Here we're on v4 which is correctly tagged as
v4 and sent (as far as I can tell) mostly correctly. It makes sense
that you're surprised that v4 came without you seeing earlier
versions, but given that the error had already been identified and
corrected (which is why you got v4 at all) then I don't think we need
to keep debugging it, right?


-Doug