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

[Sheng-Liang Pan]

Hi

> 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/
Patch v6 add dependent patch link.
> > 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
done.
> > 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>
done.

Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>

Sheng-Liang