[<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
Powered by blists - more mailing lists