Re: [PATCH v5 2/2] arm64: dts: qcom: sc7180: Add sku_id and board id for lazor/limozeen

[Doug Anderson]

Hi,

On Tue, Aug 22, 2023 at 3:04 AM Sheng-Liang Pan
<sheng-liang.pan@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---

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@xxxxxxxxxxxx>