Re: [PATCH v3 5/8] arm64: dts: qcom: Add msm8939 SoC

[Bjorn Andersson <andersson@kernel.org>]

On Tue, Jan 17, 2023 at 10:48:37PM +0000, Bryan O'Donoghue wrote:
> On 17/01/2023 20:58, Bjorn Andersson wrote:
> > On Tue, Jan 17, 2023 at 02:48:43AM +0000, Bryan O'Donoghue wrote:
> > > Add msm8939 a derivative SoC of msm8916. This SoC contains a number of key
> > > differences to msm8916.
> > > 
> > > - big.LITTLE Octa Core - quad 1.5GHz + quad 1.0GHz
> > > - DRAM 1x800 LPDDR3
> > > - Camera 4+4 lane CSI
> > > - Venus @ 1080p60 HEVC
> > > - DSI x 2
> > > - Adreno A405
> > > - WiFi wcn3660/wcn3680b 802.11ac
> > > 
> > > Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > Co-developed-by: Jun Nie <jun.nie@linaro.org>
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > Co-developed-by: Benjamin Li <benl@squareup.com>
> > > Signed-off-by: Benjamin Li <benl@squareup.com>
> > > Co-developed-by: James Willcox <jwillcox@squareup.com>
> > > Signed-off-by: James Willcox <jwillcox@squareup.com>
> > > Co-developed-by: Leo Yan <leo.yan@linaro.org>
> > > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > > Co-developed-by: Joseph Gates <jgates@squareup.com>
> > > Signed-off-by: Joseph Gates <jgates@squareup.com>
> > > Co-developed-by: Max Chen <mchen@squareup.com>
> > > Signed-off-by: Max Chen <mchen@squareup.com>
> > > Co-developed-by: Zac Crosby <zac@squareup.com>
> > > Signed-off-by: Zac Crosby <zac@squareup.com>
> > > Co-developed-by: Vincent Knecht <vincent.knecht@mailoo.org>
> > > Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
> > > Co-developed-by: Stephan Gerhold <stephan@gerhold.net>
> > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > 
> > Just to make sure when I get the question, you all co-developed this
> > patch, right?
> 
> A long list but a fair one.
> 
> > > ---
> > >   arch/arm64/boot/dts/qcom/msm8939.dtsi | 2393 +++++++++++++++++++++++++
> > >   1 file changed, 2393 insertions(+)
> > >   create mode 100644 arch/arm64/boot/dts/qcom/msm8939.dtsi
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/msm8939.dtsi b/arch/arm64/boot/dts/qcom/msm8939.dtsi
> > > new file mode 100644
> > > index 0000000000000..8cd358a9fe623
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/qcom/msm8939.dtsi
> > > @@ -0,0 +1,2393 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
> > > + * Copyright (c) 2020-2023, Linaro Limited
> > > + */
> > > +
> > > +#include <dt-bindings/clock/qcom,gcc-msm8939.h>
> > > +#include <dt-bindings/clock/qcom,rpmcc.h>
> > > +#include <dt-bindings/interconnect/qcom,msm8939.h>
> > > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +#include <dt-bindings/power/qcom-rpmpd.h>
> > > +#include <dt-bindings/reset/qcom,gcc-msm8939.h>
> > > +#include <dt-bindings/thermal/thermal.h>
> > > +
> > > +/ {
> > > +	interrupt-parent = <&intc>;
> > > +
> > > +	#address-cells = <2>;
> > > +	#size-cells = <2>;
> > 
> > Why do you use a default of 2? In particular since you reduce it to 1 in
> > /soc...
> 
> You asked that before, and I took a note of the answer but, then because I
> was away from the main machine when I sent V2, I didn't have the log.
> 
> Here's what I wrote down.
> 
> "  - address-cells/size-cells = 1 in /soc - Bjorn
>     I experimentally changed address/cell sizes to 2
>     I'm finding that lk chokes "
> 
> So AFAIR LK was unhappy about changing the top level address/size cells to
> <1> <1> and converting the /soc address/size cells to <2> <2> caused a
> number of breakages during boot.
> 
> To be honest, this pattern is copied from the msm8916.dtsi original.
> msm8953.dtsi has the same thing. msm8994 too, and 8998.
> 
> If you think it needs changing, then I'll have to see what can be done with
> soc@{} entries.
> 

Sounds like problems not worth pursuing further. How about leaving a
comment for the next person here about LK's expectation of these being
2?

Thanks,
Bjorn