linux-kernel - Re: [PATCH v5 2/2] arm64: dts: qcom: sc7180: Add sku

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 Aug 2023 07:13:23 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Sheng-Liang Pan <sheng-liang.pan@...nta.corp-partner.google.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        cros-qcom-dts-watchers@...omium.org, devicetree@...r.kernel.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v5 2/2] arm64: dts: qcom: sc7180: Add sku_id and board id
 for lazor/limozeen

Hi,

On Tue, Aug 22, 2023 at 3:04 AM Sheng-Liang Pan
<sheng-liang.pan@...nta.corp-partner.google.com> wrote:
>
> SKU ID 10: Lazor LTE+Wifi, no-esim (Strapped 0 X 0)
> SKU ID 15: Limozeen LTE+Wifi, TS, no esim (Strapped 1 X 0)
> SKU ID 18: Limozeen LTE+Wifi, no TS, no esim (Strapped X 0 0)
>
> Even though the "no esim" boards are strapped differently than
> ones that have an esim, the esim isn't represented in the
> device tree so the same device tree can be used for LTE w/ esim
> and LTE w/out esim.
>
> add BRD_ID(0, Z, 0) = 10 for new board with ALC5682i-VS
>
> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@...nta.corp-partner.google.com>
> ---

You should add a note here ("after the cut", in other words after the
"---" above but before your changelog) explaining that your patch
absolutely requires my patch [1] in order to compile. Please provide a
link to my patch (AKA include link [1]), too. I _think_ that maybe
you're using "patman" to format your patch? If so then this would be
done using "Commit-notes:". Maintainers have a lot of patches to apply
and we need to make it really easy for them to figure out what order
they need to apply patches in and which patches depend on others.

[1] https://lore.kernel.org/r/20230816112143.1.I7227efd47e0dc42b6ff243bd22aa1a3e01923220@changeid/


> Changes in v5:
> - include rt5682s node for new board version 10

This isn't quite what you did in v5. I would say:
- rebased on patch moving rt5682s to a fragment



> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
> index 1c4f0773a242..cabe99c23a7c 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
> @@ -14,8 +14,8 @@
>  #include "sc7180-lite.dtsi"
>
>  / {
> -       model = "Google Lazor (rev9+) with KB Backlight";
> -       compatible = "google,lazor-sku2", "qcom,sc7180";
> +       model = "Google Lazor (rev9) with KB Backlight";
> +       compatible = "google,lazor-rev9-sku2", "qcom,sc7180";;

nit: the above line has two ";". Remove one.

IMO this is something you should spin a quick v6 for. Once that's fixed:

Reviewed-by: Douglas Anderson <dianders@...omium.org>