diff mbox series

[v3,13/14] arm64: dts: qcom: sc7280: Add the CPU compatible to the soc@0 node

Message ID 20220202132301.v3.13.I7924ce4592e3e75b2293804d8a3f8a4dae44646e@changeid (mailing list archive)
State Changes Requested
Headers show
Series arm64: dts: qcom: sc7x80: A smattering of misc dts cleanups + herobrine-rev1 | expand

Commit Message

Doug Anderson Feb. 2, 2022, 9:23 p.m. UTC
We'd like to start including the CPU name as the compatible under the
"soc" node so that we can get rid of it from the top-level compatible
string.

Suggested-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Probably needs a .yaml file somewhere?

Changes in v3:
- ("sc7280: Add the CPU compatible to the soc@0 node") new for v3.

 arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stephen Boyd Feb. 3, 2022, 9:44 p.m. UTC | #1
Quoting Douglas Anderson (2022-02-02 13:23:47)
> We'd like to start including the CPU name as the compatible under the
> "soc" node so that we can get rid of it from the top-level compatible
> string.
>
> Suggested-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Bjorn Andersson Feb. 4, 2022, 9:51 p.m. UTC | #2
On Wed 02 Feb 15:23 CST 2022, Douglas Anderson wrote:

> We'd like to start including the CPU name as the compatible under the
> "soc" node so that we can get rid of it from the top-level compatible
> string.
> 
> Suggested-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Probably needs a .yaml file somewhere?
> 
> Changes in v3:
> - ("sc7280: Add the CPU compatible to the soc@0 node") new for v3.
> 
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 618ae0407cd6..2bfc919d4018 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -573,7 +573,7 @@ soc: soc@0 {
>  		#size-cells = <2>;
>  		ranges = <0 0 0 0 0x10 0>;
>  		dma-ranges = <0 0 0 0 0x10 0>;
> -		compatible = "simple-bus";
> +		compatible = "qcom,sc7280", "simple-bus";

To me this implies that /soc represents the sc7280, but as noted earlier
I don't think that's accurate. E.g. if this node represents the sc7280,
why are the cpus described outside this node?

Further more, if we look at the reg nodes on this bus it's clear that
this is some mmio bus, which per the ranges has 36 bit address width.
But not all buses in the sc7280 has 36 bit address width, so it's not
inconceivable that one would actually have to split /soc into more than
one entity with different dma-ranges. Perhaps not today, but I don't
like the precedence it sets.

Regards,
Bjorn

>  
>  		gcc: clock-controller@100000 {
>  			compatible = "qcom,gcc-sc7280";
> -- 
> 2.35.0.rc2.247.g8bbb082509-goog
>
Stephen Boyd Feb. 5, 2022, 2:59 a.m. UTC | #3
Quoting Bjorn Andersson (2022-02-04 13:51:44)
> On Wed 02 Feb 15:23 CST 2022, Douglas Anderson wrote:
>
> > We'd like to start including the CPU name as the compatible under the
> > "soc" node so that we can get rid of it from the top-level compatible
> > string.
> >
> > Suggested-by: Stephen Boyd <swboyd@chromium.org>
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > Probably needs a .yaml file somewhere?
> >
> > Changes in v3:
> > - ("sc7280: Add the CPU compatible to the soc@0 node") new for v3.
> >
> >  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index 618ae0407cd6..2bfc919d4018 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -573,7 +573,7 @@ soc: soc@0 {
> >               #size-cells = <2>;
> >               ranges = <0 0 0 0 0x10 0>;
> >               dma-ranges = <0 0 0 0 0x10 0>;
> > -             compatible = "simple-bus";
> > +             compatible = "qcom,sc7280", "simple-bus";
>
> To me this implies that /soc represents the sc7280, but as noted earlier
> I don't think that's accurate. E.g. if this node represents the sc7280,
> why are the cpus described outside this node?

They're outside the soc node because cpus have historically been
described at the root of the DT. The concept of an 'soc' node came after
the cpu nodes.

>
> Further more, if we look at the reg nodes on this bus it's clear that
> this is some mmio bus, which per the ranges has 36 bit address width.
> But not all buses in the sc7280 has 36 bit address width, so it's not
> inconceivable that one would actually have to split /soc into more than
> one entity with different dma-ranges. Perhaps not today, but I don't
> like the precedence it sets.
>

That should be fine. We can separate the nodes inside the soc node if we
need to by having different bus nodes underneath the soc node and then
change the dma-ranges within those bus nodes accordingly. We wouldn't
introduce another soc node to solve this problem.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 618ae0407cd6..2bfc919d4018 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -573,7 +573,7 @@  soc: soc@0 {
 		#size-cells = <2>;
 		ranges = <0 0 0 0 0x10 0>;
 		dma-ranges = <0 0 0 0 0x10 0>;
-		compatible = "simple-bus";
+		compatible = "qcom,sc7280", "simple-bus";
 
 		gcc: clock-controller@100000 {
 			compatible = "qcom,gcc-sc7280";