linux-kernel - Re: [PATCH 1/4] dt-bindings: soc: actions: Add Actions Semi Owl socinfo binding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210329222133.GA1792209@BV030612LT>
Date:   Tue, 30 Mar 2021 01:21:33 +0300
From:   Cristian Ciocaltea <cristian.ciocaltea@...il.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Andreas Färber <afaerber@...e.de>,
        Manivannan Sadhasivam <mani@...nel.org>,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-actions@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] dt-bindings: soc: actions: Add Actions Semi Owl
 socinfo binding

On Sat, Mar 27, 2021 at 10:30:06AM -0600, Rob Herring wrote:
> On Fri, Mar 19, 2021 at 08:27:59PM +0200, Cristian Ciocaltea wrote:
> > Add devicetree binding for the Actions Semi Owl SoCs info module.
> > 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...il.com>
> > ---
> >  .../bindings/soc/actions/owl-socinfo.yaml     | 71 +++++++++++++++++++

[...]

> > +
> > +patternProperties:
> > +  "^soc(@[0-9a-f]+)?$":
> 
> Make this a $nodename property.
> 
> > +    type: object
> > +    properties:
> 
> And move this up to top-level.
> 
> You need a custom 'select' entry to exclude 'simple-bus'.

Indeed, I missed it..

> > +      compatible:
> > +        items:
> > +          - enum:
> > +              - actions,s500-soc
> > +              - actions,s700-soc
> > +              - actions,s900-soc
> > +          - const: simple-bus
> > +
> > +      "#address-cells":
> > +        enum: [1, 2]
> > +
> > +      "#size-cells":
> > +        enum: [1, 2]
> > +
> > +      ranges: true
> > +
> > +      actions,serial-number-addrs:
> > +        description: |
> > +          Contains the physical addresses in DDR memory where the two parts
> > +          of the serial number (low & high) can be read from.
> > +          This is currently supported only on the S500 SoC variant.
> > +        $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        minItems: 2
> > +        maxItems: 2
> 
> Humm, it doesn't really seem you have an actual device or bus here, but 
> are abusing DT to create your socinfo device.
> 
> As the only property is data in main memory, you should do a compatible 
> for that memory region and put it under reserved-memory. You need that 
> anyway to prevent the kernel from using the memory, right?

Right, this region should be exposed as reserved-memory. Will handle
it in the next revision.

> Rob

Thanks for the review,
Cristi