Re: [PATCH 2/3] arm64: dts: qcom: msm8916: Drop underscore in node name

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Stephan Gerhold (2021-09-21 12:45:43)
> On Tue, Sep 21, 2021 at 11:20:18AM -0700, Stephen Boyd wrote:
> > Quoting Stephan Gerhold (2021-09-21 08:21:19)
> > > Using underscores in device tree nodes is not very common.
> > > Additionally, the _region suffix in "smem_region@..." is not really
> > > useful since it's obvious that it describes a reserved memory region.
> > >
> > > Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
> > > ---
> > >  arch/arm64/boot/dts/qcom/msm8916.dtsi | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > > index 5551dba2d5fd..95dea20cde75 100644
> > > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > > @@ -41,7 +41,7 @@ tz-apps@86000000 {
> > >                         no-map;
> > >                 };
> > >
> > > -               smem_mem: smem_region@86300000 {
> > > +               smem_mem: smem@86300000 {
> >
> > Shouldn't that be smem_mem: memory@86300000? Node names should be
> > generic.
> >
>
> The way I read it, the DT schema [1] and device tree specification [2]
> interprets the generic name recommendation a bit different here:
>
> > Following the generic-names recommended practice, node names should
> > reflect the purpose of the node (ie. "framebuffer" or "dma-pool").
>
> "framebuffer" or "dma-pool" would also be "memory", yet they suggest
> a name reflecting the purpose instead. The purpose of the node is
> "smem", it's not just arbitrary "memory".

I don't think most people know what 'smem' means. Maybe if the node name
was 'shared-memory' it would be OK.

>
> However, my main problem with using memory@ here is that it actually
> causes new dtbs_check errors:
>
> apq8016-sbc.dt.yaml: memory@86000000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@86300000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@86400000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@86500000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@86680000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@86700000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@867e0000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@86800000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@89300000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@89900000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
> apq8016-sbc.dt.yaml: memory@8ea00000: 'device_type' is a required property (From schema: dtschema/schemas/memory.yaml)
>
> Looks like it thinks this is a definition of physical memory now.
> I would rather not add more errors. :)

Got it. Doesn't seem right that the schema is checking for memory node
names anywhere except for in the root of the tree. Rob? I also see that
the reserved memory binding could do with some YAML conversion.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux